From 79bb20b31f56d0b731ccf7cd11fc592e58cd67d2 Mon Sep 17 00:00:00 2001 From: Arthur Baars Date: Mon, 22 Feb 2021 12:31:11 +0100 Subject: [PATCH] AST: add MethodCall as a subclass of Call --- ql/src/codeql_ruby/ast/Call.qll | 66 +++++++++++-------- ql/src/codeql_ruby/ast/internal/Call.qll | 36 +++++----- ql/src/codeql_ruby/controlflow/CfgNodes.qll | 6 +- .../codeql_ruby/dataflow/internal/SsaImpl.qll | 4 +- .../queries/variables/UninitializedLocal.ql | 2 +- .../library-tests/ast/calls/calls.expected | 10 +-- ql/test/library-tests/ast/calls/calls.ql | 12 ++-- .../ast/literals/literals.expected | 8 +-- .../ast/modules/classes.expected | 2 +- .../ast/modules/modules.expected | 10 +-- .../ast/modules/singleton_classes.expected | 2 +- .../controlflow/graph/Cfg.expected | 4 +- 12 files changed, 89 insertions(+), 73 deletions(-) diff --git a/ql/src/codeql_ruby/ast/Call.qll b/ql/src/codeql_ruby/ast/Call.qll index c360677c065..522b19aa8e9 100644 --- a/ql/src/codeql_ruby/ast/Call.qll +++ b/ql/src/codeql_ruby/ast/Call.qll @@ -9,32 +9,6 @@ class Call extends Expr { override string getAPrimaryQlClass() { result = "Call" } - /** - * Gets the receiver of this call, if any. For example: - * - * ```rb - * foo.bar - * Baz::qux - * corge() - * ``` - * - * The result for the call to `bar` is the `Expr` for `foo`; the result for - * the call to `qux` is the `Expr` for `Baz`; for the call to `corge` there - * is no result. - */ - final Expr getReceiver() { result = range.getReceiver() } - - /** - * Gets the name of the method being called. For example, in: - * - * ```rb - * foo.bar x, y - * ``` - * - * the result is `"bar"`. - */ - final string getMethodName() { result = range.getMethodName() } - /** * Gets the `n`th argument of this method call. In the following example, the * result for n=0 is the `IntegerLiteral` 0, while for n=1 the result is a @@ -44,6 +18,7 @@ class Call extends Expr { * `getKeywordArgument(string keyword)` predicate. * ```rb * foo(0, bar: 1) + * yield 0, bar: 1 * ``` */ final Expr getArgument(int n) { result = range.getArgument(n) } @@ -73,6 +48,41 @@ class Call extends Expr { * Gets the number of arguments of this method call. */ final int getNumberOfArguments() { result = count(this.getAnArgument()) } +} + +/** + * A method call. + */ +class MethodCall extends Call { + override MethodCall::Range range; + + override string getAPrimaryQlClass() { result = "MethodCall" } + + /** + * Gets the receiver of this call, if any. For example: + * + * ```rb + * foo.bar + * Baz::qux + * corge() + * ``` + * + * The result for the call to `bar` is the `Expr` for `foo`; the result for + * the call to `qux` is the `Expr` for `Baz`; for the call to `corge` there + * is no result. + */ + final Expr getReceiver() { result = range.getReceiver() } + + /** + * Gets the name of the method being called. For example, in: + * + * ```rb + * foo.bar x, y + * ``` + * + * the result is `"bar"`. + */ + final string getMethodName() { result = range.getMethodName() } /** * Gets the block of this method call, if any. @@ -89,7 +99,7 @@ class Call extends Expr { * a[0] * ``` */ -class ElementReference extends Call, @element_reference { +class ElementReference extends MethodCall, @element_reference { final override ElementReference::Range range; final override string getAPrimaryQlClass() { result = "ElementReference" } @@ -117,7 +127,7 @@ class YieldCall extends Call, @yield { * end * ``` */ -class SuperCall extends Call { +class SuperCall extends MethodCall { final override SuperCall::Range range; final override string getAPrimaryQlClass() { result = "SuperCall" } diff --git a/ql/src/codeql_ruby/ast/internal/Call.qll b/ql/src/codeql_ruby/ast/internal/Call.qll index 595d25f5944..dc95bdaeea9 100644 --- a/ql/src/codeql_ruby/ast/internal/Call.qll +++ b/ql/src/codeql_ruby/ast/internal/Call.qll @@ -6,26 +6,34 @@ private import codeql_ruby.ast.internal.Variable module Call { abstract class Range extends Expr::Range { + abstract Expr getArgument(int n); + + override predicate child(string label, AstNode::Range child) { + label = "getArgument" and child = getArgument(_) + } + } +} + +module MethodCall { + abstract class Range extends Call::Range { + abstract Block getBlock(); + abstract Expr getReceiver(); abstract string getMethodName(); - abstract Expr getArgument(int n); - - abstract Block getBlock(); - override string toString() { result = "call to " + this.getMethodName() } final override predicate child(string label, AstNode::Range child) { - label = "getReceiver" and child = getReceiver() + super.child(label, child) or - label = "getArgument" and child = getArgument(_) + label = "getReceiver" and child = getReceiver() or label = "getBlock" and child = getBlock() } } - private class IdentifierCallRange extends Call::Range, @token_identifier { + private class IdentifierCallRange extends MethodCall::Range, @token_identifier { final override Generated::Identifier generated; IdentifierCallRange() { vcall(this) and not access(this, _) } @@ -39,7 +47,7 @@ module Call { final override Block getBlock() { none() } } - private class ScopeResolutionIdentifierCallRange extends Call::Range, @scope_resolution { + private class ScopeResolutionIdentifierCallRange extends MethodCall::Range, @scope_resolution { final override Generated::ScopeResolution generated; Generated::Identifier identifier; @@ -57,7 +65,7 @@ module Call { final override Block getBlock() { none() } } - private class RegularCallRange extends Call::Range, @call { + private class RegularCallRange extends MethodCall::Range, @call { final override Generated::Call generated; final override Expr getReceiver() { @@ -88,7 +96,7 @@ module Call { } module ElementReference { - class Range extends Call::Range, @element_reference { + class Range extends MethodCall::Range, @element_reference { final override Generated::ElementReference generated; final override Expr getReceiver() { result = generated.getObject() } @@ -107,18 +115,14 @@ module YieldCall { class Range extends Call::Range, @yield { final override Generated::Yield generated; - final override Expr getReceiver() { none() } - - final override string getMethodName() { result = "yield" } - final override Expr getArgument(int n) { result = generated.getChild().getChild(n) } - final override Block getBlock() { none() } + final override string toString() { result = "yield ..." } } } module SuperCall { - abstract class Range extends Call::Range { } + abstract class Range extends MethodCall::Range { } private class SuperTokenCallRange extends SuperCall::Range, @token_super { final override Generated::Super generated; diff --git a/ql/src/codeql_ruby/controlflow/CfgNodes.qll b/ql/src/codeql_ruby/controlflow/CfgNodes.qll index 2258f02c28b..7f310b86bb1 100644 --- a/ql/src/codeql_ruby/controlflow/CfgNodes.qll +++ b/ql/src/codeql_ruby/controlflow/CfgNodes.qll @@ -247,7 +247,9 @@ module ExprNodes { } private class CallExprChildMapping extends ExprChildMapping, Call { - override predicate relevantChild(Expr e) { e = [this.getAnArgument(), this.getReceiver()] } + override predicate relevantChild(Expr e) { + e = [this.getAnArgument(), this.(MethodCall).getReceiver()] + } } /** A control-flow node that wraps a `Call` AST expression. */ @@ -260,7 +262,7 @@ module ExprNodes { final ExprCfgNode getArgument(int n) { e.hasCfgChild(e.getArgument(n), this, result) } /** Gets the receiver of this call. */ - final ExprCfgNode getReceiver() { e.hasCfgChild(e.getReceiver(), this, result) } + final ExprCfgNode getReceiver() { e.hasCfgChild(e.(MethodCall).getReceiver(), this, result) } } private class CaseExprChildMapping extends ExprChildMapping, CaseExpr { diff --git a/ql/src/codeql_ruby/dataflow/internal/SsaImpl.qll b/ql/src/codeql_ruby/dataflow/internal/SsaImpl.qll index 3f7d066ad83..f8d4671e563 100644 --- a/ql/src/codeql_ruby/dataflow/internal/SsaImpl.qll +++ b/ql/src/codeql_ruby/dataflow/internal/SsaImpl.qll @@ -89,7 +89,7 @@ private predicate capturedCallRead(BasicBlock bb, int i, LocalVariable v) { or // If the read happens inside a block, we restrict to the call that // contains the block - scope = any(Call c | bb.getNode(i) = c.getAControlFlowNode()).getBlock() + scope = any(MethodCall c | bb.getNode(i) = c.getAControlFlowNode()).getBlock() ) } @@ -146,7 +146,7 @@ private module Cached { or // If the write happens inside a block, we restrict to the call that // contains the block - scope = any(Call c | bb.getNode(i) = c.getAControlFlowNode()).getBlock() + scope = any(MethodCall c | bb.getNode(i) = c.getAControlFlowNode()).getBlock() ) } diff --git a/ql/src/queries/variables/UninitializedLocal.ql b/ql/src/queries/variables/UninitializedLocal.ql index 081ab771b7b..2b4f4511a10 100644 --- a/ql/src/queries/variables/UninitializedLocal.ql +++ b/ql/src/queries/variables/UninitializedLocal.ql @@ -15,7 +15,7 @@ import codeql_ruby.dataflow.SSA class RelevantLocalVariableReadAccess extends LocalVariableReadAccess { RelevantLocalVariableReadAccess() { - not exists(Call c | + not exists(MethodCall c | c.getReceiver() = this and c.getMethodName() = "nil?" ) diff --git a/ql/test/library-tests/ast/calls/calls.expected b/ql/test/library-tests/ast/calls/calls.expected index 6ba250f77dc..48d1a4389cd 100644 --- a/ql/test/library-tests/ast/calls/calls.expected +++ b/ql/test/library-tests/ast/calls/calls.expected @@ -1,7 +1,7 @@ callsWithNoReceiverArgumentsOrBlock | calls.rb:2:1:2:5 | call to foo | foo | | calls.rb:8:1:8:7 | call to bar | bar | -| calls.rb:31:3:31:7 | call to yield | yield | +| calls.rb:31:3:31:7 | yield ... | (none) | | calls.rb:46:1:46:3 | call to foo | foo | | calls.rb:50:2:50:4 | call to foo | foo | | calls.rb:54:11:54:13 | call to foo | foo | @@ -82,8 +82,8 @@ callsWithArguments | calls.rb:14:1:14:11 | call to foo | foo | 1 | calls.rb:14:8:14:8 | 1 | | calls.rb:14:1:14:11 | call to foo | foo | 2 | calls.rb:14:11:14:11 | 2 | | calls.rb:25:1:27:3 | call to bar | bar | 0 | calls.rb:25:9:25:13 | "foo" | -| calls.rb:36:3:36:16 | call to yield | yield | 0 | calls.rb:36:9:36:11 | 100 | -| calls.rb:36:3:36:16 | call to yield | yield | 1 | calls.rb:36:14:36:16 | 200 | +| calls.rb:36:3:36:16 | yield ... | (none) | 0 | calls.rb:36:9:36:11 | 100 | +| calls.rb:36:3:36:16 | yield ... | (none) | 1 | calls.rb:36:14:36:16 | 200 | | calls.rb:54:1:54:14 | call to some_func | some_func | 0 | calls.rb:54:11:54:13 | call to foo | | calls.rb:55:1:55:17 | call to some_func | some_func | 0 | calls.rb:55:11:55:16 | call to foo | | calls.rb:234:1:234:8 | ...[...] | [] | 0 | calls.rb:234:5:234:7 | call to bar | @@ -195,8 +195,8 @@ callsWithBlock | calls.rb:292:5:292:30 | call to super | calls.rb:292:16:292:30 | { ... } | | calls.rb:293:5:293:33 | call to super | calls.rb:293:16:293:33 | do ... end | yieldCalls -| calls.rb:31:3:31:7 | call to yield | -| calls.rb:36:3:36:16 | call to yield | +| calls.rb:31:3:31:7 | yield ... | +| calls.rb:36:3:36:16 | yield ... | superCalls | calls.rb:286:5:286:9 | call to super | | calls.rb:287:5:287:11 | call to super | diff --git a/ql/test/library-tests/ast/calls/calls.ql b/ql/test/library-tests/ast/calls/calls.ql index f49874f8ed0..fa771eb3e32 100644 --- a/ql/test/library-tests/ast/calls/calls.ql +++ b/ql/test/library-tests/ast/calls/calls.ql @@ -2,16 +2,16 @@ import ruby import codeql_ruby.ast.internal.TreeSitter private string getMethodName(Call c) { - result = c.getMethodName() + result = c.(MethodCall).getMethodName() or - not exists(c.getMethodName()) and result = "(none)" + not c instanceof MethodCall and result = "(none)" } query predicate callsWithNoReceiverArgumentsOrBlock(Call c, string name) { name = getMethodName(c) and - not exists(c.getReceiver()) and + not exists(c.(MethodCall).getReceiver()) and not exists(c.getAnArgument()) and - not exists(c.getBlock()) + not exists(c.(MethodCall).getBlock()) } query predicate callsWithArguments(Call c, string name, int n, Expr argN) { @@ -19,9 +19,9 @@ query predicate callsWithArguments(Call c, string name, int n, Expr argN) { argN = c.getArgument(n) } -query predicate callsWithReceiver(Call c, Expr rcv) { rcv = c.getReceiver() } +query predicate callsWithReceiver(MethodCall c, Expr rcv) { rcv = c.getReceiver() } -query predicate callsWithBlock(Call c, Block b) { b = c.getBlock() } +query predicate callsWithBlock(MethodCall c, Block b) { b = c.getBlock() } query predicate yieldCalls(YieldCall c) { any() } diff --git a/ql/test/library-tests/ast/literals/literals.expected b/ql/test/library-tests/ast/literals/literals.expected index 522bdb32ab5..94252726bef 100644 --- a/ql/test/library-tests/ast/literals/literals.expected +++ b/ql/test/library-tests/ast/literals/literals.expected @@ -534,7 +534,7 @@ stringInterpolations | literals.rb:64:11:64:20 | #{...} | 0 | literals.rb:64:14:64:18 | ... * ... | MulExpr | | literals.rb:65:6:65:30 | #{...} | 0 | literals.rb:65:9:65:28 | "bar #{...} baz" | StringLiteral | | literals.rb:65:14:65:23 | #{...} | 0 | literals.rb:65:17:65:21 | ... + ... | AddExpr | -| literals.rb:66:6:66:21 | #{...} | 0 | literals.rb:66:9:66:14 | call to blah | Call | +| literals.rb:66:6:66:21 | #{...} | 0 | literals.rb:66:9:66:14 | call to blah | MethodCall | | literals.rb:66:6:66:21 | #{...} | 1 | literals.rb:66:17:66:19 | ... + ... | AddExpr | | literals.rb:87:7:87:15 | #{...} | 0 | literals.rb:87:10:87:14 | ... + ... | AddExpr | | literals.rb:101:11:101:16 | #{...} | 0 | literals.rb:101:13:101:15 | ... + ... | AddExpr | @@ -543,9 +543,9 @@ stringInterpolations | literals.rb:129:10:129:19 | #{...} | 0 | literals.rb:129:13:129:17 | ... - ... | SubExpr | | literals.rb:136:5:136:14 | #{...} | 0 | literals.rb:136:8:136:12 | ... + ... | AddExpr | | literals.rb:142:7:142:16 | #{...} | 0 | literals.rb:142:10:142:14 | ... + ... | AddExpr | -| literals.rb:154:14:154:22 | #{...} | 0 | literals.rb:154:17:154:20 | call to name | Call | -| literals.rb:168:12:168:29 | #{...} | 0 | literals.rb:168:15:168:27 | call to interpolation | Call | -| literals.rb:173:15:173:32 | #{...} | 0 | literals.rb:173:18:173:30 | call to interpolation | Call | +| literals.rb:154:14:154:22 | #{...} | 0 | literals.rb:154:17:154:20 | call to name | MethodCall | +| literals.rb:168:12:168:29 | #{...} | 0 | literals.rb:168:15:168:27 | call to interpolation | MethodCall | +| literals.rb:173:15:173:32 | #{...} | 0 | literals.rb:173:18:173:30 | call to interpolation | MethodCall | concatenatedStrings | literals.rb:62:1:62:17 | "..." "..." | foobarbaz | 0 | literals.rb:62:1:62:5 | "foo" | | literals.rb:62:1:62:17 | "..." "..." | foobarbaz | 1 | literals.rb:62:7:62:11 | "bar" | diff --git a/ql/test/library-tests/ast/modules/classes.expected b/ql/test/library-tests/ast/modules/classes.expected index 49afd7ee415..9eb7604f0df 100644 --- a/ql/test/library-tests/ast/modules/classes.expected +++ b/ql/test/library-tests/ast/modules/classes.expected @@ -17,7 +17,7 @@ classesWithGlobalNameScopeExprs exprsInClasses | classes.rb:20:1:37:3 | Wibble | 0 | classes.rb:21:3:23:5 | method_a | Method | | classes.rb:20:1:37:3 | Wibble | 1 | classes.rb:25:3:27:5 | method_b | Method | -| classes.rb:20:1:37:3 | Wibble | 2 | classes.rb:29:3:29:20 | call to some_method_call | Call | +| classes.rb:20:1:37:3 | Wibble | 2 | classes.rb:29:3:29:20 | call to some_method_call | MethodCall | | classes.rb:20:1:37:3 | Wibble | 3 | classes.rb:30:3:30:19 | ... = ... | AssignExpr | | classes.rb:20:1:37:3 | Wibble | 4 | classes.rb:32:3:33:5 | ClassInWibble | Class | | classes.rb:20:1:37:3 | Wibble | 5 | classes.rb:35:3:36:5 | ModuleInWibble | Module | diff --git a/ql/test/library-tests/ast/modules/modules.expected b/ql/test/library-tests/ast/modules/modules.expected index 14569cf827c..b21ae05c813 100644 --- a/ql/test/library-tests/ast/modules/modules.expected +++ b/ql/test/library-tests/ast/modules/modules.expected @@ -16,23 +16,23 @@ exprsInModules | modules.rb:4:1:24:3 | Foo | 0 | modules.rb:5:3:14:5 | Bar | Module | | modules.rb:4:1:24:3 | Foo | 1 | modules.rb:16:3:17:5 | method_in_foo | Method | | modules.rb:4:1:24:3 | Foo | 2 | modules.rb:19:3:20:5 | ClassInFoo | Class | -| modules.rb:4:1:24:3 | Foo | 3 | modules.rb:22:3:22:19 | call to puts | Call | +| modules.rb:4:1:24:3 | Foo | 3 | modules.rb:22:3:22:19 | call to puts | MethodCall | | modules.rb:4:1:24:3 | Foo | 4 | modules.rb:23:3:23:17 | ... = ... | AssignExpr | | modules.rb:5:3:14:5 | Bar | 0 | modules.rb:6:5:7:7 | ClassInFooBar | Class | | modules.rb:5:3:14:5 | Bar | 1 | modules.rb:9:5:10:7 | method_in_foo_bar | Method | -| modules.rb:5:3:14:5 | Bar | 2 | modules.rb:12:5:12:26 | call to puts | Call | +| modules.rb:5:3:14:5 | Bar | 2 | modules.rb:12:5:12:26 | call to puts | MethodCall | | modules.rb:5:3:14:5 | Bar | 3 | modules.rb:13:5:13:19 | ... = ... | AssignExpr | | modules.rb:26:1:35:3 | Foo | 0 | modules.rb:27:3:28:5 | method_in_another_definition_of_foo | Method | | modules.rb:26:1:35:3 | Foo | 1 | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo | Class | -| modules.rb:26:1:35:3 | Foo | 2 | modules.rb:33:3:33:25 | call to puts | Call | +| modules.rb:26:1:35:3 | Foo | 2 | modules.rb:33:3:33:25 | call to puts | MethodCall | | modules.rb:26:1:35:3 | Foo | 3 | modules.rb:34:3:34:17 | ... = ... | AssignExpr | | modules.rb:37:1:46:3 | Bar | 0 | modules.rb:38:3:39:5 | method_a | Method | | modules.rb:37:1:46:3 | Bar | 1 | modules.rb:41:3:42:5 | method_b | Method | -| modules.rb:37:1:46:3 | Bar | 2 | modules.rb:44:3:44:19 | call to puts | Call | +| modules.rb:37:1:46:3 | Bar | 2 | modules.rb:44:3:44:19 | call to puts | MethodCall | | modules.rb:37:1:46:3 | Bar | 3 | modules.rb:45:3:45:17 | ... = ... | AssignExpr | | modules.rb:48:1:57:3 | Bar | 0 | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar | Class | | modules.rb:48:1:57:3 | Bar | 1 | modules.rb:52:3:53:5 | method_in_another_definition_of_foo_bar | Method | -| modules.rb:48:1:57:3 | Bar | 2 | modules.rb:55:3:55:30 | call to puts | Call | +| modules.rb:48:1:57:3 | Bar | 2 | modules.rb:55:3:55:30 | call to puts | MethodCall | | modules.rb:48:1:57:3 | Bar | 3 | modules.rb:56:3:56:17 | ... = ... | AssignExpr | methodsInModules | modules.rb:4:1:24:3 | Foo | modules.rb:16:3:17:5 | method_in_foo | method_in_foo | diff --git a/ql/test/library-tests/ast/modules/singleton_classes.expected b/ql/test/library-tests/ast/modules/singleton_classes.expected index b7b9ab29966..845afe551e7 100644 --- a/ql/test/library-tests/ast/modules/singleton_classes.expected +++ b/ql/test/library-tests/ast/modules/singleton_classes.expected @@ -3,7 +3,7 @@ singletonClasses exprsInSingletonClasses | classes.rb:41:1:52:3 | class << ... | 0 | classes.rb:42:3:44:5 | length | Method | | classes.rb:41:1:52:3 | class << ... | 1 | classes.rb:46:3:48:5 | wibble | Method | -| classes.rb:41:1:52:3 | class << ... | 2 | classes.rb:50:3:50:21 | call to another_method_call | Call | +| classes.rb:41:1:52:3 | class << ... | 2 | classes.rb:50:3:50:21 | call to another_method_call | MethodCall | | classes.rb:41:1:52:3 | class << ... | 3 | classes.rb:51:3:51:20 | ... = ... | AssignExpr | methodsInSingletonClasses | classes.rb:41:1:52:3 | class << ... | classes.rb:42:3:44:5 | length | diff --git a/ql/test/library-tests/controlflow/graph/Cfg.expected b/ql/test/library-tests/controlflow/graph/Cfg.expected index 8e3167bc71d..2857ce80056 100644 --- a/ql/test/library-tests/controlflow/graph/Cfg.expected +++ b/ql/test/library-tests/controlflow/graph/Cfg.expected @@ -59,7 +59,7 @@ cfg.rb: #-----| -> a # 187| enter run_block -#-----| -> call to yield +#-----| -> yield ... # 191| enter { ... } #-----| -> x @@ -2030,7 +2030,7 @@ cfg.rb: # 187| run_block #-----| -> run_block -# 188| call to yield +# 188| yield ... #-----| -> 42 # 188| 42