From abcabeef066683ef275cc7b68364086da4dd6ca8 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Fri, 21 May 2021 16:02:39 +0200 Subject: [PATCH] Remove `*Real` predicates and enable recursive desugaring --- ql/src/codeql_ruby/ast/Call.qll | 12 +- ql/src/codeql_ruby/ast/Literal.qll | 24 +++- ql/src/codeql_ruby/ast/Operation.qll | 10 +- ql/src/codeql_ruby/ast/Variable.qll | 10 +- ql/src/codeql_ruby/ast/internal/AST.qll | 14 +- ql/src/codeql_ruby/ast/internal/Call.qll | 123 +++++++++++------- ql/src/codeql_ruby/ast/internal/Operation.qll | 24 +++- ql/src/codeql_ruby/ast/internal/Synthesis.qll | 44 ++++--- ql/src/codeql_ruby/ast/internal/Variable.qll | 56 ++++---- ql/test/library-tests/ast/AstDesugar.expected | 5 + 10 files changed, 192 insertions(+), 130 deletions(-) diff --git a/ql/src/codeql_ruby/ast/Call.qll b/ql/src/codeql_ruby/ast/Call.qll index ecf26f9aeec..389a166cd0d 100644 --- a/ql/src/codeql_ruby/ast/Call.qll +++ b/ql/src/codeql_ruby/ast/Call.qll @@ -21,7 +21,7 @@ class Call extends Expr, TCall { * yield 0, bar: 1 * ``` */ - Expr getArgument(int n) { none() } + final Expr getArgument(int n) { result = this.(CallImpl).getArgumentImpl(n) } /** * Gets an argument of this method call. @@ -47,7 +47,7 @@ class Call extends Expr, TCall { /** * Gets the number of arguments of this method call. */ - final int getNumberOfArguments() { result = count(this.getAnArgument()) } + final int getNumberOfArguments() { result = this.(CallImpl).getNumberOfArgumentsImpl() } override AstNode getAChild(string pred) { pred = "getArgument" and result = this.getArgument(_) } } @@ -71,7 +71,7 @@ class MethodCall extends Call, TMethodCall { * the call to `qux` is the `Expr` for `Baz`; for the call to `corge` there * is no result. */ - Expr getReceiver() { none() } + final Expr getReceiver() { result = this.(MethodCallImpl).getReceiverImpl() } /** * Gets the name of the method being called. For example, in: @@ -94,7 +94,7 @@ class MethodCall extends Call, TMethodCall { override string toString() { result = "call to " + this.getMethodName() } - final override AstNode getAChild(string pred) { + override AstNode getAChild(string pred) { result = super.getAChild(pred) or pred = "getReceiver" and result = this.getReceiver() @@ -143,14 +143,12 @@ class ElementReference extends MethodCall, TElementReference { * ``` */ class YieldCall extends Call, TYieldCall { - private Generated::Yield g; + Generated::Yield g; YieldCall() { this = TYieldCall(g) } final override string getAPrimaryQlClass() { result = "YieldCall" } - final override Expr getArgument(int n) { toGenerated(result) = g.getChild().getChild(n) } - final override string toString() { result = "yield ..." } } diff --git a/ql/src/codeql_ruby/ast/Literal.qll b/ql/src/codeql_ruby/ast/Literal.qll index 6a62ad084d3..edbb0aacac2 100644 --- a/ql/src/codeql_ruby/ast/Literal.qll +++ b/ql/src/codeql_ruby/ast/Literal.qll @@ -42,14 +42,22 @@ class NumericLiteral extends Literal, TNumericLiteral { } * ``` */ class IntegerLiteral extends NumericLiteral, TIntegerLiteral { + /** Gets the numerical value of this integer literal. */ + int getValue() { none() } + + final override string toString() { result = this.getValueText() } + + final override string getAPrimaryQlClass() { result = "IntegerLiteral" } +} + +private class IntegerLiteralReal extends IntegerLiteral, TIntegerLiteralReal { private Generated::Integer g; - IntegerLiteral() { this = TIntegerLiteral(g) } + IntegerLiteralReal() { this = TIntegerLiteralReal(g) } final override string getValueText() { result = g.getValue() } - /** Gets the numerical value of this integer literal. */ - final int getValue() { + final override int getValue() { exists(string s, string values, string str | s = this.getValueText().toLowerCase() and ( @@ -83,10 +91,16 @@ class IntegerLiteral extends NumericLiteral, TIntegerLiteral { ) ) } +} - final override string toString() { result = this.getValueText() } +private class IntegerLiteralSynth extends IntegerLiteral, TIntegerLiteralSynth { + private int value; - final override string getAPrimaryQlClass() { result = "IntegerLiteral" } + IntegerLiteralSynth() { this = TIntegerLiteralSynth(_, _, value) } + + final override string getValueText() { result = value.toString() } + + final override int getValue() { result = value } } /** diff --git a/ql/src/codeql_ruby/ast/Operation.qll b/ql/src/codeql_ruby/ast/Operation.qll index 04e238a80a5..4df56be9bf0 100644 --- a/ql/src/codeql_ruby/ast/Operation.qll +++ b/ql/src/codeql_ruby/ast/Operation.qll @@ -493,10 +493,10 @@ class NoRegexMatchExpr extends BinaryOperation, TNoRegexMatchExpr { */ class Assignment extends Operation, TAssignment { /** Gets the left hand side of this assignment. */ - Pattern getLeftOperand() { none() } + final Pattern getLeftOperand() { result = this.(AssignmentImpl).getLeftOperandImpl() } /** Gets the right hand side of this assignment. */ - Expr getRightOperand() { none() } + final Expr getRightOperand() { result = this.(AssignmentImpl).getRightOperandImpl() } final override Expr getAnOperand() { result = this.getLeftOperand() or result = this.getRightOperand() @@ -529,15 +529,11 @@ class AssignExpr extends Assignment, TAssignExpr { * A binary assignment operation other than `=`. */ class AssignOperation extends Assignment, TAssignOperation { - private Generated::OperatorAssignment g; + Generated::OperatorAssignment g; AssignOperation() { g = toGenerated(this) } final override string getOperator() { result = g.getOperator() } - - final override LhsExpr getLeftOperand() { toGenerated(result) = g.getLeft() } - - final override Expr getRightOperand() { toGenerated(result) = g.getRight() } } /** diff --git a/ql/src/codeql_ruby/ast/Variable.qll b/ql/src/codeql_ruby/ast/Variable.qll index 07f0f6397a7..c7677a75a03 100644 --- a/ql/src/codeql_ruby/ast/Variable.qll +++ b/ql/src/codeql_ruby/ast/Variable.qll @@ -75,7 +75,7 @@ class ClassVariable extends VariableReal, TClassVariable { /** An access to a variable. */ class VariableAccess extends Expr, TVariableAccess { /** Gets the variable this identifier refers to. */ - Variable getVariable() { none() } + final Variable getVariable() { result = this.(VariableAccessImpl).getVariableImpl() } /** * Holds if this access is a write access belonging to the explicit @@ -126,8 +126,6 @@ class VariableReadAccess extends VariableAccess { /** An access to a local variable. */ class LocalVariableAccess extends VariableAccess, TLocalVariableAccess { - override LocalVariable getVariable() { none() } - final override string getAPrimaryQlClass() { result = "LocalVariableAccess" } /** @@ -156,8 +154,6 @@ class LocalVariableReadAccess extends LocalVariableAccess, VariableReadAccess { /** An access to a global variable. */ class GlobalVariableAccess extends VariableAccess, TGlobalVariableAccess { - override GlobalVariable getVariable() { none() } - final override string getAPrimaryQlClass() { result = "GlobalVariableAccess" } } @@ -169,14 +165,10 @@ class GlobalVariableReadAccess extends GlobalVariableAccess, VariableReadAccess /** An access to an instance variable. */ class InstanceVariableAccess extends VariableAccess, TInstanceVariableAccess { - override InstanceVariable getVariable() { none() } - final override string getAPrimaryQlClass() { result = "InstanceVariableAccess" } } /** An access to a class variable. */ class ClassVariableAccess extends VariableAccess, TClassVariableAccess { - override ClassVariable getVariable() { none() } - final override string getAPrimaryQlClass() { result = "ClassVariableAccess" } } diff --git a/ql/src/codeql_ruby/ast/internal/AST.qll b/ql/src/codeql_ruby/ast/internal/AST.qll index 3f7b747217f..77cab9b8c3f 100644 --- a/ql/src/codeql_ruby/ast/internal/AST.qll +++ b/ql/src/codeql_ruby/ast/internal/AST.qll @@ -153,7 +153,10 @@ private module Cached { TInstanceVariableAccessSynth(AST::AstNode parent, int i, AST::InstanceVariable v) { mkSynthChild(InstanceVariableAccessKind(v), parent, i) } or - TIntegerLiteral(Generated::Integer g) { not any(Generated::Rational r).getChild() = g } or + TIntegerLiteralReal(Generated::Integer g) { not any(Generated::Rational r).getChild() = g } or + TIntegerLiteralSynth(AST::AstNode parent, int i, int value) { + mkSynthChild(IntegerLiteralKind(value), parent, i) + } or TKeywordParameter(Generated::KeywordParameter g) or TLEExpr(Generated::Binary g) { g instanceof @binary_langleequal } or TLShiftExprReal(Generated::Binary g) { g instanceof @binary_langlelangle } or @@ -344,7 +347,7 @@ private module Cached { n = TIf(result) or n = TIfModifierExpr(result) or n = TInstanceVariableAccessReal(result, _) or - n = TIntegerLiteral(result) or + n = TIntegerLiteralReal(result) or n = TKeywordParameter(result) or n = TLEExpr(result) or n = TLShiftExprReal(result) or @@ -462,6 +465,11 @@ private module Cached { result = TInstanceVariableAccessSynth(parent, i, v) ) or + exists(int value | + kind = IntegerLiteralKind(value) and + result = TIntegerLiteralSynth(parent, i, value) + ) + or kind = LShiftExprKind() and result = TLShiftExprSynth(parent, i) or @@ -591,6 +599,8 @@ class TLiteral = class TNumericLiteral = TIntegerLiteral or TFloatLiteral or TRationalLiteral or TComplexLiteral; +class TIntegerLiteral = TIntegerLiteralReal or TIntegerLiteralSynth; + class TBooleanLiteral = TTrueLiteral or TFalseLiteral; class TStringComponent = diff --git a/ql/src/codeql_ruby/ast/internal/Call.qll b/ql/src/codeql_ruby/ast/internal/Call.qll index 66986f2bd8f..aa7e1a552d9 100644 --- a/ql/src/codeql_ruby/ast/internal/Call.qll +++ b/ql/src/codeql_ruby/ast/internal/Call.qll @@ -38,7 +38,34 @@ string getMethodName(MethodCall mc, string s) { if mc instanceof SetterMethodCall then result = s + "=" else result = s } -class MethodCallSynth extends MethodCall, TMethodCallSynth { +abstract class CallImpl extends Call { + abstract Expr getArgumentImpl(int n); + + /** + * It is not possible to define this predicate as + * + * ```ql + * result = count(this.getArgumentImpl(_)) + * ``` + * + * since that will result in a non-monotonicity error. + */ + abstract int getNumberOfArgumentsImpl(); +} + +abstract class MethodCallImpl extends CallImpl, MethodCall { + abstract Expr getReceiverImpl(); +} + +/** + * Gets the special integer literal used to specify the number of arguments + * in a synthesized call. + */ +private TIntegerLiteralSynth getNumberOfArgumentsSynth(MethodCall mc, int value) { + result = TIntegerLiteralSynth(mc, -2, value) +} + +class MethodCallSynth extends MethodCallImpl, TMethodCallSynth { final override string getMethodName() { exists(string name | this = TMethodCallSynth(_, _, name) and @@ -46,40 +73,35 @@ class MethodCallSynth extends MethodCall, TMethodCallSynth { ) } - final override Expr getReceiver() { synthChild(this, 0, result) } + final override Expr getReceiverImpl() { synthChild(this, 0, result) } - final override Expr getArgument(int n) { synthChild(this, n + 1, result) and n >= 0 } + final override Expr getArgumentImpl(int n) { synthChild(this, n + 1, result) and n >= 0 } + + final override int getNumberOfArgumentsImpl() { exists(getNumberOfArgumentsSynth(this, result)) } + + final override AstNode getAChild(string pred) { + result = super.getAChild(pred) + or + pred = "getNumberOfArguments" and + result = getNumberOfArgumentsSynth(this, _) + } } -abstract class MethodCallReal extends MethodCall { - abstract Expr getReceiverReal(); - - abstract Expr getArgumentReal(int n); - - abstract int getNumberOfArgumentsReal(); - - override Expr getReceiver() { result = this.getReceiverReal() } - - final override Expr getArgument(int n) { result = this.getArgumentReal(n) } -} - -class IdentifierMethodCall extends MethodCallReal, TIdentifierMethodCall { +class IdentifierMethodCall extends MethodCallImpl, TIdentifierMethodCall { private Generated::Identifier g; IdentifierMethodCall() { this = TIdentifierMethodCall(g) } final override string getMethodName() { result = getMethodName(this, g.getValue()) } - final override Self getReceiver() { result = TSelfSynth(this, 0) } + final override Self getReceiverImpl() { result = TSelfSynth(this, 0) } - final override Self getReceiverReal() { none() } + final override Expr getArgumentImpl(int n) { none() } - final override Expr getArgumentReal(int n) { none() } - - final override int getNumberOfArgumentsReal() { result = 0 } + final override int getNumberOfArgumentsImpl() { result = 0 } } -class ScopeResolutionMethodCall extends MethodCallReal, TScopeResolutionMethodCall { +class ScopeResolutionMethodCall extends MethodCallImpl, TScopeResolutionMethodCall { private Generated::ScopeResolution g; private Generated::Identifier i; @@ -87,38 +109,36 @@ class ScopeResolutionMethodCall extends MethodCallReal, TScopeResolutionMethodCa final override string getMethodName() { result = getMethodName(this, i.getValue()) } - final override Expr getReceiverReal() { toGenerated(result) = g.getScope() } + final override Expr getReceiverImpl() { toGenerated(result) = g.getScope() } - final override Expr getArgumentReal(int n) { none() } + final override Expr getArgumentImpl(int n) { none() } - final override int getNumberOfArgumentsReal() { result = 0 } + final override int getNumberOfArgumentsImpl() { result = 0 } } -class RegularMethodCall extends MethodCallReal, TRegularMethodCall { +class RegularMethodCall extends MethodCallImpl, TRegularMethodCall { private Generated::Call g; RegularMethodCall() { this = TRegularMethodCall(g) } - final override Expr getReceiver() { - result = TSelfSynth(this, 0) or result = this.getReceiverReal() - } - - final override Expr getReceiverReal() { + final override Expr getReceiverImpl() { toGenerated(result) = g.getReceiver() or not exists(g.getReceiver()) and toGenerated(result) = g.getMethod().(Generated::ScopeResolution).getScope() + or + result = TSelfSynth(this, 0) } final override string getMethodName() { result = getMethodName(this, regularMethodCallName(g)) } - final override Expr getArgumentReal(int n) { + final override Expr getArgumentImpl(int n) { toGenerated(result) = g.getArguments().getChild(n) or toGenerated(result) = g.getMethod().(Generated::ArgumentList).getChild(n) } - final override int getNumberOfArgumentsReal() { + final override int getNumberOfArgumentsImpl() { result = count(g.getArguments().getChild(_)) + count(g.getMethod().(Generated::ArgumentList).getChild(_)) @@ -127,22 +147,31 @@ class RegularMethodCall extends MethodCallReal, TRegularMethodCall { final override Block getBlock() { toGenerated(result) = g.getBlock() } } -class ElementReferenceReal extends MethodCallReal, TElementReferenceReal { +class ElementReferenceReal extends MethodCallImpl, TElementReferenceReal { private Generated::ElementReference g; ElementReferenceReal() { this = TElementReferenceReal(g) } - final override Expr getReceiverReal() { toGenerated(result) = g.getObject() } + final override Expr getReceiverImpl() { toGenerated(result) = g.getObject() } - final override Expr getArgumentReal(int n) { toGenerated(result) = g.getChild(n) } + final override Expr getArgumentImpl(int n) { toGenerated(result) = g.getChild(n) } - final override int getNumberOfArgumentsReal() { result = count(g.getChild(_)) } + final override int getNumberOfArgumentsImpl() { result = count(g.getChild(_)) } } -class ElementReferenceSynth extends MethodCall, TElementReferenceSynth { - final override Expr getReceiver() { synthChild(this, 0, result) } +class ElementReferenceSynth extends MethodCallImpl, TElementReferenceSynth { + final override Expr getReceiverImpl() { synthChild(this, 0, result) } - final override Expr getArgument(int n) { synthChild(this, n + 1, result) and n >= 0 } + final override Expr getArgumentImpl(int n) { synthChild(this, n + 1, result) and n >= 0 } + + final override int getNumberOfArgumentsImpl() { exists(getNumberOfArgumentsSynth(this, result)) } + + final override AstNode getAChild(string pred) { + result = super.getAChild(pred) + or + pred = "getNumberOfArguments" and + result = getNumberOfArgumentsSynth(this, _) + } } class TokenSuperCall extends SuperCall, TTokenSuperCall { @@ -153,7 +182,7 @@ class TokenSuperCall extends SuperCall, TTokenSuperCall { final override string getMethodName() { result = getMethodName(this, g.getValue()) } } -class RegularSuperCall extends SuperCall, MethodCallReal, TRegularSuperCall { +class RegularSuperCall extends SuperCall, MethodCallImpl, TRegularSuperCall { private Generated::Call g; RegularSuperCall() { this = TRegularSuperCall(g) } @@ -162,11 +191,17 @@ class RegularSuperCall extends SuperCall, MethodCallReal, TRegularSuperCall { result = getMethodName(this, g.getMethod().(Generated::Super).getValue()) } - final override Expr getReceiverReal() { none() } + final override Expr getReceiverImpl() { none() } - final override Expr getArgumentReal(int n) { toGenerated(result) = g.getArguments().getChild(n) } + final override Expr getArgumentImpl(int n) { toGenerated(result) = g.getArguments().getChild(n) } - final override int getNumberOfArgumentsReal() { result = count(g.getArguments().getChild(_)) } + final override int getNumberOfArgumentsImpl() { result = count(g.getArguments().getChild(_)) } final override Block getBlock() { toGenerated(result) = g.getBlock() } } + +class YieldCallImpl extends CallImpl, YieldCall { + final override Expr getArgumentImpl(int n) { toGenerated(result) = g.getChild().getChild(n) } + + final override int getNumberOfArgumentsImpl() { result = count(g.getChild().getChild(_)) } +} diff --git a/ql/src/codeql_ruby/ast/internal/Operation.qll b/ql/src/codeql_ruby/ast/internal/Operation.qll index 31cdee96aa0..8891b2a059c 100644 --- a/ql/src/codeql_ruby/ast/internal/Operation.qll +++ b/ql/src/codeql_ruby/ast/internal/Operation.qll @@ -2,18 +2,30 @@ private import codeql_ruby.AST private import AST private import TreeSitter -class AssignExprReal extends AssignExpr, TAssignExprReal { +class AssignmentImpl extends Operation, TAssignment { + abstract Pattern getLeftOperandImpl(); + + abstract Expr getRightOperandImpl(); +} + +class AssignExprReal extends AssignmentImpl, AssignExpr, TAssignExprReal { private Generated::Assignment g; AssignExprReal() { this = TAssignExprReal(g) } - final override Pattern getLeftOperand() { toGenerated(result) = g.getLeft() } + final override Pattern getLeftOperandImpl() { toGenerated(result) = g.getLeft() } - final override Expr getRightOperand() { toGenerated(result) = g.getRight() } + final override Expr getRightOperandImpl() { toGenerated(result) = g.getRight() } } -class AssignExprSynth extends AssignExpr, TAssignExprSynth { - final override Pattern getLeftOperand() { synthChild(this, 0, result) } +class AssignExprSynth extends AssignmentImpl, AssignExpr, TAssignExprSynth { + final override Pattern getLeftOperandImpl() { synthChild(this, 0, result) } - final override Expr getRightOperand() { synthChild(this, 1, result) } + final override Expr getRightOperandImpl() { synthChild(this, 1, result) } +} + +class AssignOperationImpl extends AssignmentImpl, AssignOperation { + final override LhsExpr getLeftOperandImpl() { toGenerated(result) = g.getLeft() } + + final override Expr getRightOperandImpl() { toGenerated(result) = g.getRight() } } diff --git a/ql/src/codeql_ruby/ast/internal/Synthesis.qll b/ql/src/codeql_ruby/ast/internal/Synthesis.qll index 74d1744a318..4afba00cc3f 100644 --- a/ql/src/codeql_ruby/ast/internal/Synthesis.qll +++ b/ql/src/codeql_ruby/ast/internal/Synthesis.qll @@ -21,6 +21,7 @@ newtype SynthKind = ExponentExprKind() or GlobalVariableAccessKind(GlobalVariable v) or InstanceVariableAccessKind(InstanceVariable v) or + IntegerLiteralKind(int i) { i in [0 .. 1000] } or LShiftExprKind() or LocalVariableAccessRealKind(LocalVariableReal v) or LocalVariableAccessSynthKind(TLocalVariableSynth v) or @@ -158,7 +159,7 @@ private module SetterDesugar { */ private class SetterMethodCallSynthesis extends Synthesis { final override predicate child(AstNode parent, int i, Child child, LocationOption l) { - exists(AssignExprReal ae, MethodCallReal mc | mc = ae.getLeftOperand() | + exists(AssignExpr ae, MethodCall mc | mc = ae.getLeftOperand() | parent = ae and i = -1 and child = SynthChild(getCallKind(mc)) and @@ -168,18 +169,23 @@ private module SetterDesugar { l = NoneLocation() and ( i = 0 and - child = RealChild(mc.getReceiverReal()) + child = RealChild(mc.getReceiver()) or - child = RealChild(mc.getArgumentReal(i - 1)) + child = RealChild(mc.getArgument(i - 1)) or - i = mc.getNumberOfArgumentsReal() + 1 and + i = mc.getNumberOfArguments() + 1 and child = RealChild(ae.getRightOperand()) + or + // special "number of arguments argument"; required to avoid non-monotonic recursion + i = -2 and + child = SynthChild(IntegerLiteralKind(mc.getNumberOfArguments() + 1)) and + l = NoneLocation() ) ) } final override predicate excludeFromControlFlowTree(AstNode n) { - n.(MethodCall) = any(AssignExprReal ae).getLeftOperand() + n.(MethodCall) = any(AssignExpr ae).getLeftOperand() } } } @@ -241,7 +247,7 @@ private module AssignOperationDesugar { private class VariableAssignOperationSynthesis extends Synthesis { final override predicate child(AstNode parent, int i, Child child, LocationOption l) { exists(AssignOperation ao, VariableReal v | - v = ao.getLeftOperand().(VariableAccessReal).getVariableReal() + v = ao.getLeftOperand().(VariableAccess).getVariable() | parent = ao and i = -1 and @@ -279,9 +285,7 @@ private module AssignOperationDesugar { } /** Gets an assignment operation where the LHS is method call `mc`. */ - private AssignOperation assignOperationMethodCall(MethodCallReal mc) { - result.getLeftOperand() = mc - } + private AssignOperation assignOperationMethodCall(MethodCall mc) { result.getLeftOperand() = mc } /** * ```rb @@ -300,27 +304,27 @@ private module AssignOperationDesugar { */ private class MethodCallAssignOperationSynthesis extends Synthesis { final override predicate child(AstNode parent, int i, Child child, LocationOption l) { - exists(AssignOperation ao, MethodCallReal mc | ao = assignOperationMethodCall(mc) | + exists(AssignOperation ao, MethodCall mc | ao = assignOperationMethodCall(mc) | parent = ao and i = -1 and child = SynthChild(StmtSequenceKind()) and l = NoneLocation() or exists(AstNode seq, AstNode receiver | - seq = getSynthChild(ao, -1) and receiver = mc.getReceiverReal() + seq = getSynthChild(ao, -1) and receiver = mc.getReceiver() | // `__synth__0 = foo` assign(parent, i, child, TLocalVariableSynth(ao, 0), seq, 0, receiver) and l = getSomeLocation(receiver) or // `__synth__1 = bar` - exists(Expr arg, int j | arg = mc.getArgumentReal(j - 1) | + exists(Expr arg, int j | arg = mc.getArgument(j - 1) | assign(parent, i, child, TLocalVariableSynth(ao, j), seq, j, arg) and l = getSomeLocation(arg) ) or // `__synth__2 = __synth__0.[](__synth__1) + y` - exists(int opAssignIndex | opAssignIndex = mc.getNumberOfArgumentsReal() + 1 | + exists(int opAssignIndex | opAssignIndex = mc.getNumberOfArguments() + 1 | parent = seq and i = opAssignIndex and child = SynthChild(AssignExprKind()) and @@ -351,7 +355,7 @@ private module AssignOperationDesugar { i = 0 and l = getSomeLocation(receiver) or - l = getSomeLocation(mc.getArgumentReal(i - 1)) + l = getSomeLocation(mc.getArgument(i - 1)) ) or parent = op and @@ -374,7 +378,7 @@ private module AssignOperationDesugar { i = 0 and l = getSomeLocation(receiver) or - l = getSomeLocation(mc.getArgumentReal(i - 1)) + l = getSomeLocation(mc.getArgument(i - 1)) ) or parent = setter and @@ -382,6 +386,12 @@ private module AssignOperationDesugar { child = SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(ao, opAssignIndex))) and l = SomeLocation(getAssignOperationLocation(ao)) + or + // special "number of arguments argument"; required to avoid non-monotonic recursion + parent = setter and + i = -2 and + child = SynthChild(IntegerLiteralKind(opAssignIndex + 1)) and + l = NoneLocation() ) or parent = seq and @@ -394,8 +404,8 @@ private module AssignOperationDesugar { } final override predicate localVariable(AstNode n, int i) { - exists(MethodCallReal mc | n = assignOperationMethodCall(mc) | - i in [0 .. mc.getNumberOfArgumentsReal() + 1] + exists(MethodCall mc | n = assignOperationMethodCall(mc) | + i in [0 .. mc.getNumberOfArguments() + 1] ) } diff --git a/ql/src/codeql_ruby/ast/internal/Variable.qll b/ql/src/codeql_ruby/ast/internal/Variable.qll index 21becb15cce..c61f75795d0 100644 --- a/ql/src/codeql_ruby/ast/internal/Variable.qll +++ b/ql/src/codeql_ruby/ast/internal/Variable.qll @@ -490,6 +490,10 @@ module ClassVariable { } } +abstract class VariableAccessImpl extends VariableAccess { + abstract Variable getVariableImpl(); +} + module LocalVariableAccess { predicate range(Generated::Identifier id, LocalVariable v) { access(id, v) and @@ -507,35 +511,25 @@ class TVariableAccessReal = TLocalVariableAccessReal or TGlobalVariableAccess or TInstanceVariableAccess or TClassVariableAccess; -abstract class VariableAccessReal extends VariableAccess, TVariableAccessReal { - /** - * Same as `getVariable()`, but restricted to non-synthesized variable accesses. - * - * The sole purpose of this predicate is to make AST synthesis monotonic. - */ - abstract VariableReal getVariableReal(); -} - -private class LocalVariableAccessReal extends VariableAccessReal, LocalVariableAccess, +private class LocalVariableAccessReal extends VariableAccessImpl, LocalVariableAccess, TLocalVariableAccessReal { private Generated::Identifier g; private LocalVariable v; LocalVariableAccessReal() { this = TLocalVariableAccessReal(g, v) } - final override LocalVariable getVariable() { result = v } - - final override LocalVariableReal getVariableReal() { result = v } + final override LocalVariable getVariableImpl() { result = v } final override string toString() { result = g.getValue() } } -private class LocalVariableAccessSynth extends LocalVariableAccess, TLocalVariableAccessSynth { +private class LocalVariableAccessSynth extends VariableAccessImpl, LocalVariableAccess, + TLocalVariableAccessSynth { private LocalVariable v; LocalVariableAccessSynth() { this = TLocalVariableAccessSynth(_, _, v) } - final override LocalVariable getVariable() { result = v } + final override LocalVariable getVariableImpl() { result = v } final override string toString() { result = v.getName() } } @@ -544,26 +538,25 @@ module GlobalVariableAccess { predicate range(Generated::GlobalVariable n, GlobalVariable v) { n.getValue() = v.getName() } } -private class GlobalVariableAccessReal extends VariableAccessReal, GlobalVariableAccess, +private class GlobalVariableAccessReal extends GlobalVariableAccess, VariableAccessImpl, TGlobalVariableAccessReal { private Generated::GlobalVariable g; private GlobalVariable v; GlobalVariableAccessReal() { this = TGlobalVariableAccessReal(g, v) } - final override GlobalVariable getVariable() { result = v } - - final override GlobalVariable getVariableReal() { result = v } + final override GlobalVariable getVariableImpl() { result = v } final override string toString() { result = g.getValue() } } -private class GlobalVariableAccessSynth extends GlobalVariableAccess, TGlobalVariableAccessSynth { +private class GlobalVariableAccessSynth extends GlobalVariableAccess, VariableAccessImpl, + TGlobalVariableAccessSynth { private GlobalVariable v; GlobalVariableAccessSynth() { this = TGlobalVariableAccessSynth(_, _, v) } - final override GlobalVariable getVariable() { result = v } + final override GlobalVariable getVariableImpl() { result = v } final override string toString() { result = v.getName() } } @@ -574,27 +567,25 @@ module InstanceVariableAccess { } } -private class InstanceVariableAccessReal extends VariableAccessReal, InstanceVariableAccess, +private class InstanceVariableAccessReal extends InstanceVariableAccess, VariableAccessImpl, TInstanceVariableAccessReal { private Generated::InstanceVariable g; private InstanceVariable v; InstanceVariableAccessReal() { this = TInstanceVariableAccessReal(g, v) } - final override InstanceVariable getVariable() { result = v } - - final override InstanceVariable getVariableReal() { result = v } + final override InstanceVariable getVariableImpl() { result = v } final override string toString() { result = g.getValue() } } -private class InstanceVariableAccessSynth extends InstanceVariableAccess, +private class InstanceVariableAccessSynth extends InstanceVariableAccess, VariableAccessImpl, TInstanceVariableAccessSynth { private InstanceVariable v; InstanceVariableAccessSynth() { this = TInstanceVariableAccessSynth(_, _, v) } - final override InstanceVariable getVariable() { result = v } + final override InstanceVariable getVariableImpl() { result = v } final override string toString() { result = v.getName() } } @@ -603,26 +594,25 @@ module ClassVariableAccess { predicate range(Generated::ClassVariable n, ClassVariable v) { classVariableAccess(n, v) } } -private class ClassVariableAccessReal extends VariableAccessReal, ClassVariableAccess, +private class ClassVariableAccessReal extends ClassVariableAccess, VariableAccessImpl, TClassVariableAccessReal { private Generated::ClassVariable g; private ClassVariable v; ClassVariableAccessReal() { this = TClassVariableAccessReal(g, v) } - final override ClassVariable getVariable() { result = v } - - final override ClassVariable getVariableReal() { result = v } + final override ClassVariable getVariableImpl() { result = v } final override string toString() { result = g.getValue() } } -private class ClassVariableAccessSynth extends ClassVariableAccess, TClassVariableAccessSynth { +private class ClassVariableAccessSynth extends ClassVariableAccess, VariableAccessImpl, + TClassVariableAccessSynth { private ClassVariable v; ClassVariableAccessSynth() { this = TClassVariableAccessSynth(_, _, v) } - final override ClassVariable getVariable() { result = v } + final override ClassVariable getVariableImpl() { result = v } final override string toString() { result = v.getName() } } diff --git a/ql/test/library-tests/ast/AstDesugar.expected b/ql/test/library-tests/ast/AstDesugar.expected index 7fb6947aeb7..2afd9c26998 100644 --- a/ql/test/library-tests/ast/AstDesugar.expected +++ b/ql/test/library-tests/ast/AstDesugar.expected @@ -13,14 +13,17 @@ calls/calls.rb: # 67| getReceiver: [ConstantReadAccess] X # 314| [SetterMethodCall] call to foo= # 314| getReceiver: [Self] self +# 314| getNumberOfArguments: [IntegerLiteral] 1 # 314| getArgument: [IntegerLiteral] 10 # 315| [ElementReference, SetterMethodCall] ...[...] # 315| getReceiver: [MethodCall] call to foo # 315| getReceiver: [Self] self +# 315| getNumberOfArguments: [IntegerLiteral] 2 # 315| getArgument: [IntegerLiteral] 0 # 315| getArgument: [IntegerLiteral] 10 # 318| [StmtSequence] ... # 318| getStmt: [SetterMethodCall] call to count= +# 318| getNumberOfArguments: [IntegerLiteral] 2 # 318| getReceiver: [LocalVariableAccess] __synth__0 # 318| getArgument: [LocalVariableAccess] __synth__1 # 318| getStmt: [AssignExpr] ... = ... @@ -39,6 +42,7 @@ calls/calls.rb: # 319| getReceiver: [Self] self # 319| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0 # 319| getStmt: [ElementReference, SetterMethodCall] ...[...] +# 319| getNumberOfArguments: [IntegerLiteral] 3 # 319| getReceiver: [LocalVariableAccess] __synth__0 # 319| getArgument: [LocalVariableAccess] __synth__1 # 319| getArgument: [LocalVariableAccess] __synth__2 @@ -60,6 +64,7 @@ calls/calls.rb: # 320| getReceiver: [Self] self # 320| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0 # 320| getStmt: [ElementReference, SetterMethodCall] ...[...] +# 320| getNumberOfArguments: [IntegerLiteral] 5 # 320| getReceiver: [LocalVariableAccess] __synth__0 # 320| getArgument: [LocalVariableAccess] __synth__1 # 320| getArgument: [LocalVariableAccess] __synth__2