From bde9f59e0e598d0b030c15040205e3c2ff032e22 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 1 Dec 2020 13:00:30 +0100 Subject: [PATCH] Introduce `Parameter::getAVariable()` --- ql/src/codeql_ruby/ast/Parameter.qll | 15 ++++++ ql/src/codeql_ruby/ast/Pattern.qll | 5 +- ql/src/codeql_ruby/ast/Variable.qll | 6 ++- ql/src/codeql_ruby/ast/internal/Variable.qll | 6 +-- .../library-tests/ast/params/params.expected | 2 +- ql/test/library-tests/ast/params/params.ql | 6 ++- .../variables/parameter.expected | 49 ++++++++++++++++--- ql/test/library-tests/variables/parameter.ql | 11 ++++- 8 files changed, 80 insertions(+), 20 deletions(-) diff --git a/ql/src/codeql_ruby/ast/Parameter.qll b/ql/src/codeql_ruby/ast/Parameter.qll index 7f8f79175cf..704468cb754 100644 --- a/ql/src/codeql_ruby/ast/Parameter.qll +++ b/ql/src/codeql_ruby/ast/Parameter.qll @@ -21,6 +21,15 @@ class Parameter extends AstNode { /** Gets the zero-based position of this parameter. */ final int getPosition() { result = pos } + + /** Gets a variable introduced by this parameter. */ + Variable getAVariable() { none() } + + /** Gets the variable named `name` introduced by this parameter. */ + final Variable getVariable(string name) { + result = this.getAVariable() and + result.getName() = name + } } /** @@ -33,6 +42,10 @@ class PatternParameter extends Parameter, Pattern { } /** A parameter defined using a tuple pattern. */ class TuplePatternParameter extends PatternParameter, TuplePattern { final override string describeQlClass() { result = "TuplePatternParameter" } + + final override Variable getAVariable() { + result = this.getAnElement+().(VariablePattern).getVariable() + } } /** A named parameter. */ @@ -45,6 +58,8 @@ class NamedParameter extends Parameter { /** Gets the variable introduced by this parameter. */ Variable getVariable() { none() } + final override Variable getAVariable() { result = this.getVariable() } + /** Gets an access to this parameter. */ final VariableAccess getAnAccess() { result = this.getVariable().getAnAccess() } } diff --git a/ql/src/codeql_ruby/ast/Pattern.qll b/ql/src/codeql_ruby/ast/Pattern.qll index 7790ef44b36..d8e277a0018 100644 --- a/ql/src/codeql_ruby/ast/Pattern.qll +++ b/ql/src/codeql_ruby/ast/Pattern.qll @@ -2,6 +2,7 @@ import codeql_ruby.AST private import codeql_ruby.Generated private import codeql.Locations private import internal.Pattern +private import internal.Variable private import Variable /** A pattern. */ @@ -16,8 +17,8 @@ class VariablePattern extends Pattern { /** Gets the name of the variable used in this pattern. */ final string getVariableName() { result = generated.getValue() } - /** Gets the variable used in this pattern. */ - Variable getVariable() { this = result.getAnAccess() } + /** Gets the variable used in (or introduced by) this pattern. */ + Variable getVariable() { access(this, result) } override string toString() { result = this.getVariableName() } } diff --git a/ql/src/codeql_ruby/ast/Variable.qll b/ql/src/codeql_ruby/ast/Variable.qll index d55016797b8..01983c322e4 100644 --- a/ql/src/codeql_ruby/ast/Variable.qll +++ b/ql/src/codeql_ruby/ast/Variable.qll @@ -60,7 +60,11 @@ class VariableAccess extends AstNode, @token_identifier { override Generated::Identifier generated; Variable variable; - VariableAccess() { access(this, variable) } + VariableAccess() { + access(this, variable) and + // Do not generate an access at the defining location + not variable = TLocalVariable(_, _, this) + } /** Gets the variable this identifier refers to. */ Variable getVariable() { result = variable } diff --git a/ql/src/codeql_ruby/ast/internal/Variable.qll b/ql/src/codeql_ruby/ast/internal/Variable.qll index c7bbf3d961d..58f1fd1c5bc 100644 --- a/ql/src/codeql_ruby/ast/internal/Variable.qll +++ b/ql/src/codeql_ruby/ast/internal/Variable.qll @@ -123,11 +123,7 @@ private module Cached { cached predicate access(Generated::Identifier access, Variable variable) { - exists(string name | - name = access.getValue() and - // Do not generate an access at the defining location - not variable = TLocalVariable(_, name, access) - | + exists(string name | name = access.getValue() | variable = enclosingScope(access).getVariable(name) and not strictlyBefore(access.getLocation(), variable.getLocation()) or diff --git a/ql/test/library-tests/ast/params/params.expected b/ql/test/library-tests/ast/params/params.expected index 9c8c3b511fe..f6a1d9d9fb2 100644 --- a/ql/test/library-tests/ast/params/params.expected +++ b/ql/test/library-tests/ast/params/params.expected @@ -108,7 +108,7 @@ paramsInLambdas | params.rb:70:31:70:64 | -> { ... } | 0 | params.rb:70:35:70:35 | a | SimpleParameter | | params.rb:70:31:70:64 | -> { ... } | 1 | params.rb:70:38:70:45 | b | OptionalParameter | | params.rb:70:31:70:64 | -> { ... } | 2 | params.rb:70:48:70:53 | c | OptionalParameter | -#select +params | params.rb:4:30:4:32 | foo | 0 | SimpleParameter | | params.rb:4:35:4:37 | bar | 1 | SimpleParameter | | params.rb:4:40:4:42 | baz | 2 | SimpleParameter | diff --git a/ql/test/library-tests/ast/params/params.ql b/ql/test/library-tests/ast/params/params.ql index 77394ff3a4c..0f93986fd28 100644 --- a/ql/test/library-tests/ast/params/params.ql +++ b/ql/test/library-tests/ast/params/params.ql @@ -42,5 +42,7 @@ query predicate paramsInLambdas(Lambda l, int i, Parameter p, string pClass) { //////////////////////////////////////////////////////////////////////////////// // General query selecting all parameters -from Parameter p -select p, p.getPosition(), p.describeQlClass() +query predicate params(Parameter p, int i, string pClass) { + i = p.getPosition() and + pClass = p.describeQlClass() +} diff --git a/ql/test/library-tests/variables/parameter.expected b/ql/test/library-tests/variables/parameter.expected index 0feace65501..2c461b83dc8 100644 --- a/ql/test/library-tests/variables/parameter.expected +++ b/ql/test/library-tests/variables/parameter.expected @@ -1,6 +1,8 @@ -parameter +parameterVariable | nested_scopes.rb:15:23:15:23 | a | nested_scopes.rb:15:23:15:23 | a | +| nested_scopes.rb:16:26:16:26 | x | nested_scopes.rb:16:26:16:26 | x | | nested_scopes.rb:16:29:16:29 | a | nested_scopes.rb:16:29:16:29 | a | +| nested_scopes.rb:18:26:18:26 | x | nested_scopes.rb:18:26:18:26 | x | | nested_scopes.rb:22:21:22:21 | a | nested_scopes.rb:22:21:22:21 | a | | parameters.rb:1:14:1:14 | x | parameters.rb:1:14:1:14 | x | | parameters.rb:1:18:1:18 | y | parameters.rb:1:18:1:18 | y | @@ -18,10 +20,43 @@ parameter | parameters.rb:35:11:35:21 | a | parameters.rb:35:11:35:11 | a | | parameters.rb:40:12:40:19 | d | parameters.rb:40:12:40:12 | d | | parameters.rb:45:20:45:20 | _ | parameters.rb:45:20:45:20 | _ | +| parameters.rb:49:12:49:16 | (..., ...) | parameters.rb:49:13:49:13 | a | +| parameters.rb:49:12:49:16 | (..., ...) | parameters.rb:49:15:49:15 | b | | parameters.rb:54:14:54:24 | y | parameters.rb:54:14:54:14 | y | -parameterNoAcess -| nested_scopes.rb:16:26:16:26 | x | -| nested_scopes.rb:18:26:18:26 | x | -| parameters.rb:45:22:45:22 | _ | -| scopes.rb:2:14:2:14 | x | -| scopes.rb:9:14:9:14 | x | +| scopes.rb:2:14:2:14 | x | scopes.rb:2:14:2:14 | x | +| scopes.rb:9:14:9:14 | x | scopes.rb:9:14:9:14 | x | +parameterVariableAccess +| nested_scopes.rb:15:23:15:23 | a | nested_scopes.rb:15:23:15:23 | a | nested_scopes.rb:16:13:16:13 | a | +| nested_scopes.rb:16:29:16:29 | a | nested_scopes.rb:16:29:16:29 | a | nested_scopes.rb:17:15:17:15 | a | +| nested_scopes.rb:16:29:16:29 | a | nested_scopes.rb:16:29:16:29 | a | nested_scopes.rb:18:15:18:15 | a | +| nested_scopes.rb:16:29:16:29 | a | nested_scopes.rb:16:29:16:29 | a | nested_scopes.rb:18:34:18:34 | a | +| nested_scopes.rb:22:21:22:21 | a | nested_scopes.rb:22:21:22:21 | a | nested_scopes.rb:23:16:23:16 | a | +| parameters.rb:1:14:1:14 | x | parameters.rb:1:14:1:14 | x | parameters.rb:3:9:3:9 | x | +| parameters.rb:1:18:1:18 | y | parameters.rb:1:18:1:18 | y | parameters.rb:2:4:2:4 | y | +| parameters.rb:1:18:1:18 | y | parameters.rb:1:18:1:18 | y | parameters.rb:4:9:4:9 | y | +| parameters.rb:7:17:7:22 | client | parameters.rb:7:17:7:22 | client | parameters.rb:9:25:9:30 | client | +| parameters.rb:7:17:7:22 | client | parameters.rb:7:17:7:22 | client | parameters.rb:11:41:11:46 | client | +| parameters.rb:7:25:7:31 | *pizzas | parameters.rb:7:26:7:31 | pizzas | parameters.rb:8:6:8:11 | pizzas | +| parameters.rb:7:25:7:31 | *pizzas | parameters.rb:7:26:7:31 | pizzas | parameters.rb:11:14:11:19 | pizzas | +| parameters.rb:15:15:15:19 | **map | parameters.rb:15:17:15:19 | map | parameters.rb:16:3:16:5 | map | +| parameters.rb:16:16:16:18 | key | parameters.rb:16:16:16:18 | key | parameters.rb:17:13:17:15 | key | +| parameters.rb:16:21:16:25 | value | parameters.rb:16:21:16:25 | value | parameters.rb:17:22:17:26 | value | +| parameters.rb:21:16:21:21 | &block | parameters.rb:21:17:21:21 | block | parameters.rb:22:3:22:7 | block | +| parameters.rb:25:15:25:30 | name | parameters.rb:25:15:25:18 | name | parameters.rb:25:40:25:43 | name | +| parameters.rb:25:15:25:30 | name | parameters.rb:25:15:25:18 | name | parameters.rb:26:8:26:11 | name | +| parameters.rb:25:33:25:50 | size | parameters.rb:25:33:25:36 | size | parameters.rb:27:8:27:11 | size | +| parameters.rb:30:15:30:20 | first | parameters.rb:30:15:30:19 | first | parameters.rb:31:11:31:15 | first | +| parameters.rb:30:24:30:33 | middle | parameters.rb:30:24:30:29 | middle | parameters.rb:31:20:31:25 | middle | +| parameters.rb:30:36:30:40 | last | parameters.rb:30:36:30:39 | last | parameters.rb:31:30:31:33 | last | +| parameters.rb:35:11:35:21 | a | parameters.rb:35:11:35:11 | a | parameters.rb:37:11:37:11 | a | +| parameters.rb:40:12:40:19 | d | parameters.rb:40:12:40:12 | d | parameters.rb:42:11:42:11 | d | +| parameters.rb:45:20:45:20 | _ | parameters.rb:45:20:45:20 | _ | parameters.rb:45:22:45:22 | _ | +| parameters.rb:45:20:45:20 | _ | parameters.rb:45:20:45:20 | _ | parameters.rb:46:8:46:8 | _ | +| parameters.rb:49:12:49:16 | (..., ...) | parameters.rb:49:13:49:13 | a | parameters.rb:50:11:50:11 | a | +| parameters.rb:49:12:49:16 | (..., ...) | parameters.rb:49:15:49:15 | b | parameters.rb:50:16:50:16 | b | +| parameters.rb:54:14:54:24 | y | parameters.rb:54:14:54:14 | y | parameters.rb:56:9:56:9 | y | +parameterVariableNoAcess +| nested_scopes.rb:16:26:16:26 | x | nested_scopes.rb:16:26:16:26 | x | +| nested_scopes.rb:18:26:18:26 | x | nested_scopes.rb:18:26:18:26 | x | +| scopes.rb:2:14:2:14 | x | scopes.rb:2:14:2:14 | x | +| scopes.rb:9:14:9:14 | x | scopes.rb:9:14:9:14 | x | diff --git a/ql/test/library-tests/variables/parameter.ql b/ql/test/library-tests/variables/parameter.ql index 5826fca9e29..e93e1db6c3c 100644 --- a/ql/test/library-tests/variables/parameter.ql +++ b/ql/test/library-tests/variables/parameter.ql @@ -1,5 +1,12 @@ import codeql_ruby.ast.Variable -query predicate parameter(NamedParameter p, Variable v) { p.getAnAccess().getVariable() = v } +query predicate parameterVariable(Parameter p, Variable v) { v = p.getAVariable() } -query predicate parameterNoAcess(NamedParameter p) { not exists(p.getAnAccess()) } +query predicate parameterVariableAccess(Parameter p, Variable v, VariableAccess va) { + v = p.getAVariable() and + va = v.getAnAccess() +} + +query predicate parameterVariableNoAcess(Parameter p, Variable v) { + v = p.getAVariable() and not exists(v.getAnAccess()) +}