From 77129e473a088ab5b6ae6f104f66bcac1d30325c Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 2 Dec 2020 11:27:00 +0100 Subject: [PATCH] Adhere to `::Range` pattern --- ql/src/codeql_ruby/ast/Method.qll | 22 +-- ql/src/codeql_ruby/ast/Parameter.qll | 4 +- ql/src/codeql_ruby/ast/Pattern.qll | 14 +- ql/src/codeql_ruby/ast/Variable.qll | 20 +-- ql/src/codeql_ruby/ast/internal/Method.qll | 82 +++++++----- ql/src/codeql_ruby/ast/internal/Pattern.qll | 62 +++++---- ql/src/codeql_ruby/ast/internal/Variable.qll | 134 +++++++++++-------- 7 files changed, 186 insertions(+), 152 deletions(-) diff --git a/ql/src/codeql_ruby/ast/Method.qll b/ql/src/codeql_ruby/ast/Method.qll index f674c6540ec..bf284ea29f8 100644 --- a/ql/src/codeql_ruby/ast/Method.qll +++ b/ql/src/codeql_ruby/ast/Method.qll @@ -4,9 +4,9 @@ private import internal.Method /** A callable. */ class Callable extends AstNode { - CallableRange self; + Callable::Range range; - Callable() { self = this } + Callable() { range = this } /** Gets the number of parameters of this callable. */ final int getNumberOfParameters() { result = count(this.getAParameter()) } @@ -15,12 +15,12 @@ class Callable extends AstNode { final Parameter getAParameter() { result = this.getParameter(_) } /** Gets the `n`th parameter of this callable. */ - final Parameter getParameter(int n) { result = self.getParameter(n) } + final Parameter getParameter(int n) { result = range.getParameter(n) } } /** A method. */ class Method extends Callable, @method { - final override MethodRange self; + final override Method::Range range; final override Generated::Method generated; final override string describeQlClass() { result = "Method" } @@ -28,7 +28,7 @@ class Method extends Callable, @method { final override string toString() { result = this.getName() } /** Gets the name of this method. */ - final string getName() { result = self.getName() } + final string getName() { result = range.getName() } /** * Holds if this is a setter method, as in the following example: @@ -45,14 +45,14 @@ class Method extends Callable, @method { /** A singleton method. */ class SingletonMethod extends Callable, @singleton_method { - final override SingletonMethodRange self; + final override SingletonMethod::Range range; final override string describeQlClass() { result = "SingletonMethod" } final override string toString() { result = this.getName() } /** Gets the name of this method. */ - final string getName() { result = self.getName() } + final string getName() { result = range.getName() } } /** @@ -62,7 +62,7 @@ class SingletonMethod extends Callable, @singleton_method { * ``` */ class Lambda extends Callable, @lambda { - final override LambdaRange self; + final override Lambda::Range range; final override string describeQlClass() { result = "Lambda" } @@ -71,12 +71,12 @@ class Lambda extends Callable, @lambda { /** A block. */ class Block extends AstNode, Callable { - override BlockRange self; + override Block::Range range; } /** A block enclosed within `do` and `end`. */ class DoBlock extends Block, @do_block { - final override DoBlockRange self; + final override DoBlock::Range range; final override string describeQlClass() { result = "DoBlock" } @@ -90,7 +90,7 @@ class DoBlock extends Block, @do_block { * ``` */ class BraceBlock extends Block, @block { - final override BraceBlockRange self; + final override BraceBlock::Range range; final override string describeQlClass() { result = "BraceBlock" } diff --git a/ql/src/codeql_ruby/ast/Parameter.qll b/ql/src/codeql_ruby/ast/Parameter.qll index 4435858b7bd..a2c5417f29d 100644 --- a/ql/src/codeql_ruby/ast/Parameter.qll +++ b/ql/src/codeql_ruby/ast/Parameter.qll @@ -1,6 +1,4 @@ import codeql_ruby.AST -private import Variable -private import Pattern private import internal.Pattern private import internal.TreeSitter private import internal.Variable @@ -65,7 +63,7 @@ class NamedParameter extends Parameter { /** A simple (normal) parameter. */ class SimpleParameter extends NamedParameter, PatternParameter, VariablePattern { - final override string getName() { result = this.(VariablePatternRange).getVariableName() } + final override string getName() { result = this.(VariablePattern::Range).getVariableName() } final override Variable getVariable() { result = TLocalVariable(_, _, this) } diff --git a/ql/src/codeql_ruby/ast/Pattern.qll b/ql/src/codeql_ruby/ast/Pattern.qll index 962e5ccb02b..093cb665740 100644 --- a/ql/src/codeql_ruby/ast/Pattern.qll +++ b/ql/src/codeql_ruby/ast/Pattern.qll @@ -7,23 +7,23 @@ private import Variable /** A pattern. */ class Pattern extends AstNode { - PatternRange self; + Pattern::Range range; - Pattern() { self = this } + Pattern() { range = this } /** Gets a variable used in (or introduced by) this pattern. */ - Variable getAVariable() { result = self.getAVariable() } + Variable getAVariable() { result = range.getAVariable() } } /** A simple variable pattern. */ class VariablePattern extends Pattern { - final override VariablePatternRange self; + final override VariablePattern::Range range; final override Generated::Identifier generated; /** Gets the variable used in (or introduced by) this pattern. */ Variable getVariable() { access(this, result) } - override string toString() { result = self.getVariableName() } + override string toString() { result = range.getVariableName() } } /** @@ -32,10 +32,10 @@ class VariablePattern extends Pattern { * This includes both tuple patterns in parameters and assignments. */ class TuplePattern extends Pattern { - final override TuplePatternRange self; + final override TuplePattern::Range range; /** Gets the `i`th pattern in this tuple pattern. */ - final Pattern getElement(int i) { result = self.getElement(i) } + final Pattern getElement(int i) { result = range.getElement(i) } /** Gets a sub pattern in this tuple pattern. */ final Pattern getAnElement() { result = this.getElement(_) } diff --git a/ql/src/codeql_ruby/ast/Variable.qll b/ql/src/codeql_ruby/ast/Variable.qll index 22fb6ac0358..6e56ab83c1b 100644 --- a/ql/src/codeql_ruby/ast/Variable.qll +++ b/ql/src/codeql_ruby/ast/Variable.qll @@ -7,15 +7,15 @@ private import internal.Variable /** A scope in which variables can be declared. */ class VariableScope extends TScope { - VariableScopeRange self; + VariableScope::Range range; - VariableScope() { self = this } + VariableScope() { range = this } /** Gets a textual representation of this element. */ - final string toString() { result = self.toString() } + final string toString() { result = range.toString() } /** Gets the program element this scope is associated with, if any. */ - final AstNode getScopeElement() { result = self.getScopeElement() } + final AstNode getScopeElement() { result = range.getScopeElement() } /** Gets the location of the program element this scope is associated with. */ final Location getLocation() { result = getScopeElement().getLocation() } @@ -32,21 +32,21 @@ class VariableScope extends TScope { /** A variable declared in a scope. */ class Variable extends TVariable { - VariableRange self; + Variable::Range range; - Variable() { self = this } + Variable() { range = this } /** Gets the name of this variable. */ - final string getName() { result = self.getName() } + final string getName() { result = range.getName() } /** Gets a textual representation of this variable. */ final string toString() { result = this.getName() } /** Gets the location of this variable. */ - final Location getLocation() { result = self.getLocation() } + final Location getLocation() { result = range.getLocation() } /** Gets the scope this variable is declared in. */ - final VariableScope getDeclaringScope() { result = self.getDeclaringScope() } + final VariableScope getDeclaringScope() { result = range.getDeclaringScope() } /** Gets an access to this variable. */ VariableAccess getAnAccess() { result.getVariable() = this } @@ -54,7 +54,7 @@ class Variable extends TVariable { /** A local variable. */ class LocalVariable extends Variable { - override LocalVariableRange self; + override LocalVariable::Range range; final override LocalVariableAccess getAnAccess() { result.getVariable() = this } } diff --git a/ql/src/codeql_ruby/ast/internal/Method.qll b/ql/src/codeql_ruby/ast/internal/Method.qll index dc75ac4ced1..577e1ebdcf0 100644 --- a/ql/src/codeql_ruby/ast/internal/Method.qll +++ b/ql/src/codeql_ruby/ast/internal/Method.qll @@ -1,56 +1,70 @@ import codeql_ruby.AST private import TreeSitter -abstract class CallableRange extends AstNode { - abstract Parameter getParameter(int n); -} - -class MethodRange extends CallableRange, @method { - final override Generated::Method generated; - - override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } - - string getName() { - result = generated.getName().(Generated::Token).getValue() or - // TODO: use hand-written Symbol class - result = generated.getName().(Generated::Symbol).toString() or - result = generated.getName().(Generated::Setter).getName().getValue() + "=" +module Callable { + abstract class Range extends AstNode { + abstract Parameter getParameter(int n); } } -class SingletonMethodRange extends CallableRange, @singleton_method { - final override Generated::SingletonMethod generated; +module Method { + class Range extends Callable::Range, @method { + final override Generated::Method generated; - override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } + override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } - string getName() { - result = generated.getName().(Generated::Token).getValue() or - // TODO: use hand-written Symbol class - result = generated.getName().(Generated::Symbol).toString() or - result = generated.getName().(Generated::Setter).getName().getValue() + "=" + string getName() { + result = generated.getName().(Generated::Token).getValue() or + // TODO: use hand-written Symbol class + result = generated.getName().(Generated::Symbol).toString() or + result = generated.getName().(Generated::Setter).getName().getValue() + "=" + } } } -class LambdaRange extends CallableRange, @lambda { - final override Generated::Lambda generated; +module SingletonMethod { + class Range extends Callable::Range, @singleton_method { + final override Generated::SingletonMethod generated; - final override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } + override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } + + string getName() { + result = generated.getName().(Generated::Token).getValue() or + // TODO: use hand-written Symbol class + result = generated.getName().(Generated::Symbol).toString() or + result = generated.getName().(Generated::Setter).getName().getValue() + "=" + } + } } -abstract class BlockRange extends CallableRange { - Generated::BlockParameters params; +module Lambda { + class Range extends Callable::Range, @lambda { + final override Generated::Lambda generated; - final override Parameter getParameter(int n) { result = params.getChild(n) } + final override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } + } } -class DoBlockRange extends BlockRange, @do_block { - final override Generated::DoBlock generated; +module Block { + abstract class Range extends Callable::Range { + Generated::BlockParameters params; - DoBlockRange() { params = generated.getParameters() } + final override Parameter getParameter(int n) { result = params.getChild(n) } + } } -class BraceBlockRange extends BlockRange, @block { - final override Generated::Block generated; +module DoBlock { + class Range extends Block::Range, @do_block { + final override Generated::DoBlock generated; - BraceBlockRange() { params = generated.getParameters() } + Range() { params = generated.getParameters() } + } +} + +module BraceBlock { + class Range extends Block::Range, @block { + final override Generated::Block generated; + + Range() { params = generated.getParameters() } + } } diff --git a/ql/src/codeql_ruby/ast/internal/Pattern.qll b/ql/src/codeql_ruby/ast/internal/Pattern.qll index 6210ea6bb10..5f55dd9c664 100644 --- a/ql/src/codeql_ruby/ast/internal/Pattern.qll +++ b/ql/src/codeql_ruby/ast/internal/Pattern.qll @@ -35,40 +35,46 @@ private predicate patternNode(Generated::AstNode n, boolean parameter) { */ predicate assignment(Generated::Identifier i, boolean parameter) { patternNode(i, parameter) } -abstract class PatternRange extends AstNode { - PatternRange() { patternNode(this, _) } +module Pattern { + abstract class Range extends AstNode { + Range() { patternNode(this, _) } - abstract Variable getAVariable(); + abstract Variable getAVariable(); + } } -class VariablePatternRange extends PatternRange { - override Generated::Identifier generated; +module VariablePattern { + class Range extends Pattern::Range { + override Generated::Identifier generated; - string getVariableName() { result = generated.getValue() } + string getVariableName() { result = generated.getValue() } - override Variable getAVariable() { access(this, result) } + override Variable getAVariable() { access(this, result) } + } } -abstract class TuplePatternRange extends PatternRange { - abstract PatternRange getElement(int i); +module TuplePattern { + abstract class Range extends Pattern::Range { + abstract Pattern::Range getElement(int i); - override Variable getAVariable() { result = this.getElement(_).getAVariable() } -} - -private class ParameterTuplePatternRange extends TuplePatternRange { - override Generated::DestructuredParameter generated; - - override PatternRange getElement(int i) { result = generated.getChild(i) } -} - -private class AssignmentTuplePatternRange extends TuplePatternRange { - override Generated::DestructuredLeftAssignment generated; - - override PatternRange getElement(int i) { result = generated.getChild(i) } -} - -private class AssignmentListPatternRange extends TuplePatternRange { - override Generated::LeftAssignmentList generated; - - override PatternRange getElement(int i) { result = generated.getChild(i) } + override Variable getAVariable() { result = this.getElement(_).getAVariable() } + } + + private class ParameterTuplePatternRange extends Range { + override Generated::DestructuredParameter generated; + + override Pattern::Range getElement(int i) { result = generated.getChild(i) } + } + + private class AssignmentTuplePatternRange extends Range { + override Generated::DestructuredLeftAssignment generated; + + override Pattern::Range getElement(int i) { result = generated.getChild(i) } + } + + private class AssignmentListPatternRange extends Range { + override Generated::LeftAssignmentList generated; + + override Pattern::Range getElement(int i) { result = generated.getChild(i) } + } } diff --git a/ql/src/codeql_ruby/ast/internal/Variable.qll b/ql/src/codeql_ruby/ast/internal/Variable.qll index 6dab791c9e9..a6d89adda95 100644 --- a/ql/src/codeql_ruby/ast/internal/Variable.qll +++ b/ql/src/codeql_ruby/ast/internal/Variable.qll @@ -13,7 +13,9 @@ private VariableScope enclosingScope(Generated::AstNode node) { result.getScopeElement() = parent*(node.getParent()) } -private predicate parameterAssignment(CallableScopeRange scope, string name, Generated::Identifier i) { +private predicate parameterAssignment( + CallableScope::Range scope, string name, Generated::Identifier i +) { assignment(i, true) and scope = enclosingScope(i) and name = i.getValue() @@ -21,7 +23,7 @@ private predicate parameterAssignment(CallableScopeRange scope, string name, Gen /** Holds if `scope` defines `name` in its parameter declaration at `i`. */ private predicate scopeDefinesParameterVariable( - CallableScopeRange scope, string name, Generated::Identifier i + CallableScope::Range scope, string name, Generated::Identifier i ) { parameterAssignment(scope, name, i) and // In case of overlapping parameter names (e.g. `_`), only the first @@ -142,69 +144,83 @@ private module Cached { import Cached -abstract class VariableScopeRange extends TScope { - abstract string toString(); +module VariableScope { + abstract class Range extends TScope { + abstract string toString(); - abstract AstNode getScopeElement(); -} - -class TopLevelScopeRange extends VariableScopeRange, TTopLevelScope { - override string toString() { result = "top-level scope" } - - override AstNode getScopeElement() { TTopLevelScope(result) = this } -} - -class ModuleScopeRange extends VariableScopeRange, TModuleScope { - override string toString() { result = "module scope" } - - override AstNode getScopeElement() { TModuleScope(result) = this } -} - -class ClassScopeRange extends VariableScopeRange, TClassScope { - override string toString() { result = "class scope" } - - override AstNode getScopeElement() { TClassScope(result) = this } -} - -class CallableScopeRange extends VariableScopeRange, TCallableScope { - private Callable c; - - CallableScopeRange() { this = TCallableScope(c) } - - override string toString() { - (c instanceof Method or c instanceof SingletonMethod) and - result = "method scope" - or - c instanceof Lambda and - result = "lambda scope" - or - c instanceof Block and - result = "block scope" + abstract AstNode getScopeElement(); } - - override Callable getScopeElement() { TCallableScope(result) = this } } -class VariableRange extends TVariable { - abstract string getName(); +module TopLevelScope { + class Range extends VariableScope::Range, TTopLevelScope { + override string toString() { result = "top-level scope" } - string toString() { result = this.getName() } - - abstract Location getLocation(); - - abstract VariableScope getDeclaringScope(); + override AstNode getScopeElement() { TTopLevelScope(result) = this } + } } -class LocalVariableRange extends VariableRange { - private VariableScope scope; - private string name; - private Generated::Identifier i; +module ModuleScope { + class Range extends VariableScope::Range, TModuleScope { + override string toString() { result = "module scope" } - LocalVariableRange() { this = TLocalVariable(scope, name, i) } - - final override string getName() { result = name } - - final override Location getLocation() { result = i.getLocation() } - - final override VariableScope getDeclaringScope() { result = scope } + override AstNode getScopeElement() { TModuleScope(result) = this } + } +} + +module ClassScope { + class Range extends VariableScope::Range, TClassScope { + override string toString() { result = "class scope" } + + override AstNode getScopeElement() { TClassScope(result) = this } + } +} + +module CallableScope { + class Range extends VariableScope::Range, TCallableScope { + private Callable c; + + Range() { this = TCallableScope(c) } + + override string toString() { + (c instanceof Method or c instanceof SingletonMethod) and + result = "method scope" + or + c instanceof Lambda and + result = "lambda scope" + or + c instanceof Block and + result = "block scope" + } + + override Callable getScopeElement() { TCallableScope(result) = this } + } +} + +module Variable { + class Range extends TVariable { + abstract string getName(); + + string toString() { result = this.getName() } + + abstract Location getLocation(); + + abstract VariableScope getDeclaringScope(); + } +} + +module LocalVariable { + class Range extends Variable::Range { + private VariableScope scope; + private string name; + private Generated::Identifier i; + + Range() { this = TLocalVariable(scope, name, i) } + + final override string getName() { result = name } + + final override Location getLocation() { result = i.getLocation() } + + final override VariableScope getDeclaringScope() { result = scope } + } }