From 32daf28b344e08983fe8d9317914ae784391252e Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Wed, 3 Feb 2021 10:23:44 +0100 Subject: [PATCH 1/2] Rangify `AstNode` --- ql/src/codeql_ruby/AST.qll | 12 +- ql/src/codeql_ruby/ast/Call.qll | 8 -- ql/src/codeql_ruby/ast/Control.qll | 24 ---- ql/src/codeql_ruby/ast/Expr.qll | 26 +--- ql/src/codeql_ruby/ast/Method.qll | 13 +- ql/src/codeql_ruby/ast/Module.qll | 6 - ql/src/codeql_ruby/ast/Operation.qll | 6 - ql/src/codeql_ruby/ast/Parameter.qll | 77 +++------- ql/src/codeql_ruby/ast/Pattern.qll | 11 +- ql/src/codeql_ruby/ast/Variable.qll | 2 - ql/src/codeql_ruby/ast/internal/AST.qll | 20 +++ ql/src/codeql_ruby/ast/internal/Call.qll | 8 ++ ql/src/codeql_ruby/ast/internal/Control.qll | 26 ++++ ql/src/codeql_ruby/ast/internal/Expr.qll | 29 +++- ql/src/codeql_ruby/ast/internal/Method.qll | 31 +++- ql/src/codeql_ruby/ast/internal/Module.qll | 6 + ql/src/codeql_ruby/ast/internal/Operation.qll | 6 + ql/src/codeql_ruby/ast/internal/Parameter.qll | 133 ++++++++++++++++++ ql/src/codeql_ruby/ast/internal/Pattern.qll | 40 +++--- ql/src/codeql_ruby/ast/internal/Variable.qll | 115 ++++++++------- 20 files changed, 367 insertions(+), 232 deletions(-) create mode 100644 ql/src/codeql_ruby/ast/internal/AST.qll create mode 100644 ql/src/codeql_ruby/ast/internal/Parameter.qll diff --git a/ql/src/codeql_ruby/AST.qll b/ql/src/codeql_ruby/AST.qll index 5131e6fd027..25edc0ec394 100644 --- a/ql/src/codeql_ruby/AST.qll +++ b/ql/src/codeql_ruby/AST.qll @@ -8,17 +8,16 @@ import ast.Parameter import ast.Operation import ast.Pattern import ast.Variable -private import ast.internal.TreeSitter +private import ast.internal.AST /** * A node in the abstract syntax tree. This class is the base class for all Ruby * program elements. */ -// TODO: Replace base class with an abstract range class once we have full coverage class AstNode extends @ast_node { - Generated::AstNode generated; + AstNode::Range range; - AstNode() { generated = this } + AstNode() { range = this } /** * Gets the name of a primary CodeQL class to which this node belongs. @@ -30,9 +29,8 @@ class AstNode extends @ast_node { string getAPrimaryQlClass() { result = "???" } /** Gets a textual representation of this node. */ - cached - string toString() { result = "AstNode" } + final string toString() { result = range.toString() } /** Gets the location of this node. */ - Location getLocation() { result = generated.getLocation() } + final Location getLocation() { result = range.getLocation() } } diff --git a/ql/src/codeql_ruby/ast/Call.qll b/ql/src/codeql_ruby/ast/Call.qll index 4f9248a0ecb..2f75be0b1f0 100644 --- a/ql/src/codeql_ruby/ast/Call.qll +++ b/ql/src/codeql_ruby/ast/Call.qll @@ -9,8 +9,6 @@ class Call extends Expr { override string getAPrimaryQlClass() { result = "Call" } - final override string toString() { result = "call to " + this.getMethodName() } - /** * Gets the receiver of this call, if any. For example: * ```rb @@ -135,8 +133,6 @@ class BlockArgument extends Expr, @block_argument { final override string getAPrimaryQlClass() { result = "BlockArgument" } - final override string toString() { result = "&..." } - /** * Gets the underlying expression representing the block. In the following * example, the result is the `Expr` for `bar`: @@ -158,8 +154,6 @@ class SplatArgument extends Expr, @splat_argument { final override string getAPrimaryQlClass() { result = "SplatArgument" } - final override string toString() { result = "*..." } - /** * Gets the underlying expression. In the following example, the result is * the `Expr` for `bar`: @@ -181,8 +175,6 @@ class HashSplatArgument extends Expr, @hash_splat_argument { final override string getAPrimaryQlClass() { result = "HashSplatArgument" } - final override string toString() { result = "**..." } - /** * Gets the underlying expression. In the following example, the result is * the `Expr` for `bar`: diff --git a/ql/src/codeql_ruby/ast/Control.qll b/ql/src/codeql_ruby/ast/Control.qll index f41b711a8f7..64b27212840 100644 --- a/ql/src/codeql_ruby/ast/Control.qll +++ b/ql/src/codeql_ruby/ast/Control.qll @@ -53,8 +53,6 @@ class IfExpr extends ConditionalExpr { final override string getAPrimaryQlClass() { result = "IfExpr" } - final override string toString() { if isElsif() then result = "elsif ..." else result = "if ..." } - /** Holds if this is an `elsif` expression. */ final predicate isElsif() { this instanceof @elsif } @@ -108,8 +106,6 @@ class UnlessExpr extends ConditionalExpr, @unless { final override string getAPrimaryQlClass() { result = "UnlessExpr" } - final override string toString() { result = "unless ..." } - /** * Gets the 'then' branch of this `unless` expression. In the following * example, the result is the `ExprSequence` containing `foo`. @@ -148,8 +144,6 @@ class IfModifierExpr extends ConditionalExpr, @if_modifier { final override string getAPrimaryQlClass() { result = "IfModifierExpr" } - final override string toString() { result = "... if ..." } - /** * Gets the expression that is conditionally evaluated. In the following * example, the result is the `Expr` for `foo`. @@ -171,8 +165,6 @@ class UnlessModifierExpr extends ConditionalExpr, @unless_modifier { final override string getAPrimaryQlClass() { result = "UnlessModifierExpr" } - final override string toString() { result = "... unless ..." } - /** * Gets the expression that is conditionally evaluated. In the following * example, the result is the `Expr` for `foo`. @@ -194,8 +186,6 @@ class TernaryIfExpr extends ConditionalExpr, @conditional { final override string getAPrimaryQlClass() { result = "TernaryIfExpr" } - final override string toString() { result = "... ? ... : ..." } - /** Gets the 'then' branch of this ternary if expression. */ final Expr getThen() { result = range.getThen() } @@ -208,8 +198,6 @@ class CaseExpr extends ControlExpr, @case__ { final override string getAPrimaryQlClass() { result = "CaseExpr" } - final override string toString() { result = "case ..." } - /** * Gets the expression being compared, if any. For example, `foo` in the following example. * ```rb @@ -268,8 +256,6 @@ class WhenExpr extends Expr, @when { final override string getAPrimaryQlClass() { result = "WhenExpr" } - final override string toString() { result = "when ..." } - /** Gets the body of this case-when expression. */ final ExprSequence getBody() { result = range.getBody() } @@ -333,8 +319,6 @@ class WhileExpr extends ConditionalLoop, @while { final override string getAPrimaryQlClass() { result = "WhileExpr" } - final override string toString() { result = "while ..." } - /** Gets the body of this `while` loop. */ final override ExprSequence getBody() { result = range.getBody() } } @@ -353,8 +337,6 @@ class UntilExpr extends ConditionalLoop, @until { final override string getAPrimaryQlClass() { result = "UntilExpr" } - final override string toString() { result = "until ..." } - /** Gets the body of this `until` loop. */ final override ExprSequence getBody() { result = range.getBody() } } @@ -369,8 +351,6 @@ class WhileModifierExpr extends ConditionalLoop, @while_modifier { final override WhileModifierExpr::Range range; final override string getAPrimaryQlClass() { result = "WhileModifierExpr" } - - final override string toString() { result = "... while ..." } } /** @@ -383,8 +363,6 @@ class UntilModifierExpr extends ConditionalLoop, @until_modifier { final override UntilModifierExpr::Range range; final override string getAPrimaryQlClass() { result = "UntilModifierExpr" } - - final override string toString() { result = "... until ..." } } /** @@ -400,8 +378,6 @@ class ForExpr extends Loop, @for { final override string getAPrimaryQlClass() { result = "ForExpr" } - final override string toString() { result = "for ... in ..." } - /** Gets the body of this `for` loop. */ final override Expr getBody() { result = range.getBody() } diff --git a/ql/src/codeql_ruby/ast/Expr.qll b/ql/src/codeql_ruby/ast/Expr.qll index 120a498fe94..4cfcd2ce836 100644 --- a/ql/src/codeql_ruby/ast/Expr.qll +++ b/ql/src/codeql_ruby/ast/Expr.qll @@ -10,7 +10,7 @@ private import codeql_ruby.controlflow.internal.ControlFlowGraphImpl * This is the root QL class for all expressions. */ class Expr extends AstNode { - Expr::Range range; + override Expr::Range range; Expr() { this = range } @@ -35,8 +35,6 @@ class Expr extends AstNode { class Literal extends Expr { override Literal::Range range; - override string toString() { result = range.toString() } - /** Gets the source text for this literal, if it is constant. */ final string getValueText() { result = range.getValueText() } } @@ -124,16 +122,6 @@ class ExprSequence extends Expr { override string getAPrimaryQlClass() { result = "ExprSequence" } - override string toString() { - exists(int c | c = this.getNumberOfExpressions() | - c = 0 and result = ";" - or - c = 1 and result = this.getExpr(0).toString() - or - c > 1 and result = "...; ..." - ) - } - /** Gets the `n`th expression in this sequence. */ final Expr getExpr(int n) { result = range.getExpr(n) } @@ -178,14 +166,6 @@ class ParenthesizedExpr extends ExprSequence, @parenthesized_statements { final override ParenthesizedExpr::Range range; final override string getAPrimaryQlClass() { result = "ParenthesizedExpr" } - - final override string toString() { - exists(int c | c = this.getNumberOfExpressions() | - c = 0 and result = "()" - or - c > 0 and result = "(" + ExprSequence.super.toString() + ")" - ) - } } /** @@ -200,8 +180,6 @@ class ScopeResolution extends Expr, @scope_resolution { final override string getAPrimaryQlClass() { result = "ScopeResolution" } - final override string toString() { result = "...::" + this.getName() } - /** * Gets the expression representing the scope, if any. In the following * example, the scope is the `Expr` for `Foo`: @@ -238,8 +216,6 @@ class Pair extends Expr, @pair { final override string getAPrimaryQlClass() { result = "Pair" } - final override string toString() { result = "Pair" } - /** * Gets the key expression of this pair. For example, the `SymbolLiteral` * representing the keyword `foo` in the following example: diff --git a/ql/src/codeql_ruby/ast/Method.qll b/ql/src/codeql_ruby/ast/Method.qll index 2e24be8b84c..b6390e71d06 100644 --- a/ql/src/codeql_ruby/ast/Method.qll +++ b/ql/src/codeql_ruby/ast/Method.qll @@ -1,5 +1,6 @@ private import codeql_ruby.AST private import codeql_ruby.controlflow.ControlFlowGraph +private import internal.AST private import internal.TreeSitter private import internal.Method @@ -23,8 +24,6 @@ class Method extends Callable, BodyStatement, @method { final override string getAPrimaryQlClass() { result = "Method" } - final override string toString() { result = this.getName() } - /** Gets the name of this method. */ final string getName() { result = range.getName() } @@ -47,8 +46,6 @@ class SingletonMethod extends Callable, BodyStatement, @singleton_method { final override string getAPrimaryQlClass() { result = "SingletonMethod" } - final override string toString() { result = this.getName() } - /** Gets the name of this method. */ final string getName() { result = range.getName() } } @@ -63,12 +60,10 @@ class Lambda extends Callable, @lambda { final override Lambda::Range range; final override string getAPrimaryQlClass() { result = "Lambda" } - - final override string toString() { result = "-> { ... }" } } /** A block. */ -class Block extends AstNode, Callable { +class Block extends Callable { override Block::Range range; } @@ -77,8 +72,6 @@ class DoBlock extends Block, BodyStatement, @do_block { final override DoBlock::Range range; final override string getAPrimaryQlClass() { result = "DoBlock" } - - final override string toString() { result = "do ... end" } } /** @@ -91,6 +84,4 @@ class BraceBlock extends Block, @block { final override BraceBlock::Range range; final override string getAPrimaryQlClass() { result = "BraceBlock" } - - final override string toString() { result = "{ ... }" } } diff --git a/ql/src/codeql_ruby/ast/Module.qll b/ql/src/codeql_ruby/ast/Module.qll index b2ba9d32879..1c5e404c431 100644 --- a/ql/src/codeql_ruby/ast/Module.qll +++ b/ql/src/codeql_ruby/ast/Module.qll @@ -41,8 +41,6 @@ class Class extends ModuleBase { final override string getAPrimaryQlClass() { result = "Class" } - final override string toString() { result = this.getName() } - /** * Gets the name of the class. In the following example, the result is * `"Foo"`. @@ -110,8 +108,6 @@ class SingletonClass extends ModuleBase, @singleton_class { final override string getAPrimaryQlClass() { result = "Class" } - final override string toString() { result = "class << ..." } - /** * Gets the expression resulting in the object on which the singleton class * is defined. In the following example, the result is the `Expr` for `foo`: @@ -154,8 +150,6 @@ class Module extends ModuleBase, @module { final override string getAPrimaryQlClass() { result = "Module" } - final override string toString() { result = this.getName() } - /** * Gets the name of the module. In the following example, the result is * `"Foo"`. diff --git a/ql/src/codeql_ruby/ast/Operation.qll b/ql/src/codeql_ruby/ast/Operation.qll index 9c0a3f7d778..3544a9dd3f4 100644 --- a/ql/src/codeql_ruby/ast/Operation.qll +++ b/ql/src/codeql_ruby/ast/Operation.qll @@ -22,8 +22,6 @@ class UnaryOperation extends Operation, @unary { /** Gets the operand of this unary operation. */ final Expr getOperand() { result = range.getOperand() } - - override string toString() { result = this.getOperator() + " ..." } } /** A unary logical operation. */ @@ -106,8 +104,6 @@ class DefinedExpr extends UnaryOperation, @unary_definedquestion { class BinaryOperation extends Operation, @binary { override BinaryOperation::Range range; - override string toString() { result = "... " + this.getOperator() + " ..." } - /** Gets the left operand of this binary operation. */ final Expr getLeftOperand() { result = range.getLeftOperand() } @@ -448,8 +444,6 @@ class NoRegexMatchExpr extends BinaryOperation, @binary_bangtilde { class Assignment extends Operation { override Assignment::Range range; - override string toString() { result = "... " + this.getOperator() + " ..." } - /** Gets the left hand side of this assignment. */ final Expr getLhs() { result = range.getLhs() } diff --git a/ql/src/codeql_ruby/ast/Parameter.qll b/ql/src/codeql_ruby/ast/Parameter.qll index b9ed67a144c..76cba8c717f 100644 --- a/ql/src/codeql_ruby/ast/Parameter.qll +++ b/ql/src/codeql_ruby/ast/Parameter.qll @@ -2,27 +2,20 @@ private import codeql_ruby.AST private import internal.Pattern private import internal.TreeSitter private import internal.Variable +private import internal.Parameter /** A parameter. */ class Parameter extends AstNode { - private int pos; - - Parameter() { - this = any(Generated::BlockParameters bp).getChild(pos) - or - this = any(Generated::MethodParameters mp).getChild(pos) - or - this = any(Generated::LambdaParameters lp).getChild(pos) - } + override Parameter::Range range; /** Gets the callable that this parameter belongs to. */ final Callable getCallable() { result.getAParameter() = this } /** Gets the zero-based position of this parameter. */ - final int getPosition() { result = pos } + final int getPosition() { result = range.getPosition() } /** Gets a variable introduced by this parameter. */ - LocalVariable getAVariable() { none() } + LocalVariable getAVariable() { result = range.getAVariable() } /** Gets the variable named `name` introduced by this parameter. */ final LocalVariable getVariable(string name) { @@ -37,23 +30,27 @@ class Parameter extends AstNode { * This includes both simple parameters and tuple parameters. */ class PatternParameter extends Parameter, Pattern { + override PatternParameter::Range range; + override LocalVariable getAVariable() { result = Pattern.super.getAVariable() } } /** A parameter defined using a tuple pattern. */ class TuplePatternParameter extends PatternParameter, TuplePattern { + override TuplePatternParameter::Range range; + final override string getAPrimaryQlClass() { result = "TuplePatternParameter" } } /** A named parameter. */ class NamedParameter extends Parameter { - NamedParameter() { not this instanceof TuplePattern } + override NamedParameter::Range range; /** Gets the name of this parameter. */ - string getName() { none() } + final string getName() { result = range.getName() } /** Gets the variable introduced by this parameter. */ - LocalVariable getVariable() { none() } + LocalVariable getVariable() { result = range.getVariable() } override LocalVariable getAVariable() { result = this.getVariable() } @@ -63,15 +60,13 @@ class NamedParameter extends Parameter { /** A simple (normal) parameter. */ class SimpleParameter extends NamedParameter, PatternParameter, VariablePattern { - final override string getName() { result = range.getVariableName() } + override SimpleParameter::Range range; - final override LocalVariable getVariable() { result = TLocalVariable(_, _, this) } + final override LocalVariable getVariable() { result = range.getVariable() } - final override LocalVariable getAVariable() { result = this.getVariable() } + final override LocalVariable getAVariable() { result = range.getAVariable() } final override string getAPrimaryQlClass() { result = "SimpleParameter" } - - final override string toString() { result = this.getName() } } /** @@ -83,15 +78,11 @@ class SimpleParameter extends NamedParameter, PatternParameter, VariablePattern * ``` */ class BlockParameter extends @block_parameter, NamedParameter { - final override Generated::BlockParameter generated; + final override BlockParameter::Range range; - final override LocalVariable getVariable() { result = TLocalVariable(_, _, generated.getName()) } + final override LocalVariable getVariable() { result = range.getVariable() } final override string getAPrimaryQlClass() { result = "BlockParameter" } - - final override string toString() { result = "&" + this.getName() } - - final override string getName() { result = generated.getName().getValue() } } /** @@ -104,15 +95,9 @@ class BlockParameter extends @block_parameter, NamedParameter { * ``` */ class HashSplatParameter extends @hash_splat_parameter, NamedParameter { - final override Generated::HashSplatParameter generated; - - final override LocalVariable getVariable() { result = TLocalVariable(_, _, generated.getName()) } + final override HashSplatParameter::Range range; final override string getAPrimaryQlClass() { result = "HashSplatParameter" } - - final override string toString() { result = "**" + this.getName() } - - final override string getName() { result = generated.getName().getValue() } } /** @@ -127,29 +112,23 @@ class HashSplatParameter extends @hash_splat_parameter, NamedParameter { * ``` */ class KeywordParameter extends @keyword_parameter, NamedParameter { - final override Generated::KeywordParameter generated; - - final override LocalVariable getVariable() { result = TLocalVariable(_, _, generated.getName()) } + final override KeywordParameter::Range range; final override string getAPrimaryQlClass() { result = "KeywordParameter" } - final override string getName() { result = generated.getName().getValue() } - /** * Gets the default value, i.e. the value assigned to the parameter when one * is not provided by the caller. If the parameter is mandatory and does not * have a default value, this predicate has no result. * TODO: better return type (Expr?) */ - final AstNode getDefaultValue() { result = generated.getValue() } + final AstNode getDefaultValue() { result = range.getDefaultValue() } /** * Holds if the parameter is optional. That is, there is a default value that * is used when the caller omits this parameter. */ final predicate isOptional() { exists(this.getDefaultValue()) } - - final override string toString() { result = this.getName() } } /** @@ -162,22 +141,16 @@ class KeywordParameter extends @keyword_parameter, NamedParameter { * ``` */ class OptionalParameter extends @optional_parameter, NamedParameter { - final override Generated::OptionalParameter generated; - - final override LocalVariable getVariable() { result = TLocalVariable(_, _, generated.getName()) } + final override OptionalParameter::Range range; final override string getAPrimaryQlClass() { result = "OptionalParameter" } - final override string toString() { result = this.getName() } - - final override string getName() { result = generated.getName().getValue() } - /** * Gets the default value, i.e. the value assigned to the parameter when one * is not provided by the caller. * TODO: better return type (Expr?) */ - final AstNode getDefaultValue() { result = generated.getValue() } + final AstNode getDefaultValue() { result = range.getDefaultValue() } } /** @@ -189,13 +162,7 @@ class OptionalParameter extends @optional_parameter, NamedParameter { * ``` */ class SplatParameter extends @splat_parameter, NamedParameter { - final override Generated::SplatParameter generated; - - final override LocalVariable getVariable() { result = TLocalVariable(_, _, generated.getName()) } + final override SplatParameter::Range range; final override string getAPrimaryQlClass() { result = "SplatParameter" } - - final override string toString() { result = "*" + this.getName() } - - final override string getName() { result = generated.getName().getValue() } } diff --git a/ql/src/codeql_ruby/ast/Pattern.qll b/ql/src/codeql_ruby/ast/Pattern.qll index cc90d18ef24..4f975857d56 100644 --- a/ql/src/codeql_ruby/ast/Pattern.qll +++ b/ql/src/codeql_ruby/ast/Pattern.qll @@ -6,7 +6,7 @@ private import internal.Variable /** A pattern. */ class Pattern extends AstNode { - Pattern::Range range; + override Pattern::Range range; Pattern() { range = this } @@ -16,13 +16,10 @@ class Pattern extends AstNode { /** A simple variable pattern. */ class VariablePattern extends Pattern { - final override VariablePattern::Range range; - final override Generated::Identifier generated; + override VariablePattern::Range range; /** Gets the variable used in (or introduced by) this pattern. */ Variable getVariable() { access(this, result) } - - override string toString() { result = range.getVariableName() } } /** @@ -31,13 +28,11 @@ class VariablePattern extends Pattern { * This includes both tuple patterns in parameters and assignments. */ class TuplePattern extends Pattern { - final override TuplePattern::Range range; + override TuplePattern::Range range; /** Gets the `i`th pattern in this tuple pattern. */ final Pattern getElement(int i) { result = range.getElement(i) } /** Gets a sub pattern in this tuple pattern. */ final Pattern getAnElement() { result = this.getElement(_) } - - final override string toString() { result = "(..., ...)" } } diff --git a/ql/src/codeql_ruby/ast/Variable.qll b/ql/src/codeql_ruby/ast/Variable.qll index 7ede082bb4d..0159c63b6f0 100644 --- a/ql/src/codeql_ruby/ast/Variable.qll +++ b/ql/src/codeql_ruby/ast/Variable.qll @@ -140,8 +140,6 @@ class VariableAccess extends Expr { * as is the first access to `e`. */ predicate isImplicitWrite() { implicitWriteAccess(this) } - - final override string toString() { result = this.getVariable().getName() } } /** An access to a variable where the value is updated. */ diff --git a/ql/src/codeql_ruby/ast/internal/AST.qll b/ql/src/codeql_ruby/ast/internal/AST.qll new file mode 100644 index 00000000000..0b5b63976d0 --- /dev/null +++ b/ql/src/codeql_ruby/ast/internal/AST.qll @@ -0,0 +1,20 @@ +import codeql.Locations +private import TreeSitter + +module AstNode { + abstract class Range extends @ast_node { + Generated::AstNode generated; + + Range() { this = generated } + + cached + string toString() { result = "AstNode" } + + Location getLocation() { result = generated.getLocation() } + } + + // TODO: Remove, and when removed make Range::toString() above abstract + private class RemoveWhenFullCoverage extends Range { + RemoveWhenFullCoverage() { this instanceof Generated::AstNode } + } +} diff --git a/ql/src/codeql_ruby/ast/internal/Call.qll b/ql/src/codeql_ruby/ast/internal/Call.qll index 407e6e938c2..88b68831021 100644 --- a/ql/src/codeql_ruby/ast/internal/Call.qll +++ b/ql/src/codeql_ruby/ast/internal/Call.qll @@ -14,6 +14,8 @@ module Call { abstract Expr getArgument(int n); abstract Block getBlock(); + + final override string toString() { result = "call to " + this.getMethodName() } } private class IdentifierCallRange extends Call::Range, @token_identifier { @@ -111,6 +113,8 @@ module BlockArgument { final override Generated::BlockArgument generated; final Expr getExpr() { result = generated.getChild() } + + final override string toString() { result = "&..." } } } @@ -119,6 +123,8 @@ module SplatArgument { final override Generated::SplatArgument generated; final Expr getExpr() { result = generated.getChild() } + + final override string toString() { result = "*..." } } } @@ -127,5 +133,7 @@ module HashSplatArgument { final override Generated::HashSplatArgument generated; final Expr getExpr() { result = generated.getChild() } + + final override string toString() { result = "**..." } } } diff --git a/ql/src/codeql_ruby/ast/internal/Control.qll b/ql/src/codeql_ruby/ast/internal/Control.qll index add4448581a..cfc98f5e3cb 100644 --- a/ql/src/codeql_ruby/ast/internal/Control.qll +++ b/ql/src/codeql_ruby/ast/internal/Control.qll @@ -20,6 +20,10 @@ module IfExpr { abstract ExprSequence getThen(); abstract Expr getElse(); + + final override string toString() { + if this instanceof @elsif then result = "elsif ..." else result = "if ..." + } } private class IfRange extends IfExpr::Range, @if { @@ -70,6 +74,8 @@ module UnlessExpr { or cond = true and result = getElse() } + + final override string toString() { result = "unless ..." } } } @@ -82,6 +88,8 @@ module IfModifierExpr { final Expr getExpr() { result = generated.getBody() } final override Expr getBranch(boolean cond) { cond = true and result = getExpr() } + + final override string toString() { result = "... if ..." } } } @@ -94,6 +102,8 @@ module UnlessModifierExpr { final Expr getExpr() { result = generated.getBody() } final override Expr getBranch(boolean cond) { cond = false and result = getExpr() } + + final override string toString() { result = "... unless ..." } } } @@ -112,6 +122,8 @@ module TernaryIfExpr { or cond = false and result = getElse() } + + final override string toString() { result = "... ? ... : ..." } } } @@ -122,6 +134,8 @@ module CaseExpr { final Expr getValue() { result = generated.getValue() } final Expr getBranch(int n) { result = generated.getChild(n) } + + final override string toString() { result = "case ..." } } } @@ -132,6 +146,8 @@ module WhenExpr { final ExprSequence getBody() { result = generated.getBody() } final Expr getPattern(int n) { result = generated.getPattern(n).getChild() } + + final override string toString() { result = "when ..." } } } @@ -154,6 +170,8 @@ module WhileExpr { final override ExprSequence getBody() { result = generated.getBody() } final override Expr getCondition() { result = generated.getCondition() } + + final override string toString() { result = "while ..." } } } @@ -164,6 +182,8 @@ module UntilExpr { final override ExprSequence getBody() { result = generated.getBody() } final override Expr getCondition() { result = generated.getCondition() } + + final override string toString() { result = "until ..." } } } @@ -174,6 +194,8 @@ module WhileModifierExpr { final override Expr getBody() { result = generated.getBody() } final override Expr getCondition() { result = generated.getCondition() } + + final override string toString() { result = "... while ..." } } } @@ -184,6 +206,8 @@ module UntilModifierExpr { final override Expr getBody() { result = generated.getBody() } final override Expr getCondition() { result = generated.getCondition() } + + final override string toString() { result = "... until ..." } } } @@ -196,5 +220,7 @@ module ForExpr { final Pattern getPattern() { result = generated.getPattern() } final Expr getValue() { result = generated.getValue().getChild() } + + final override string toString() { result = "for ... in ..." } } } diff --git a/ql/src/codeql_ruby/ast/internal/Expr.qll b/ql/src/codeql_ruby/ast/internal/Expr.qll index 6786f2bb9f9..27e944c8aff 100644 --- a/ql/src/codeql_ruby/ast/internal/Expr.qll +++ b/ql/src/codeql_ruby/ast/internal/Expr.qll @@ -1,14 +1,17 @@ private import codeql_ruby.AST +private import codeql_ruby.ast.internal.AST private import codeql_ruby.ast.internal.TreeSitter private import codeql_ruby.ast.internal.Variable module Expr { - abstract class Range extends AstNode { } + abstract class Range extends AstNode::Range { } } module Literal { abstract class Range extends Expr::Range { abstract string getValueText(); + + override string toString() { result = this.getValueText() } } } @@ -165,6 +168,18 @@ module SymbolLiteral { module ExprSequence { abstract class Range extends Expr::Range { abstract Expr getExpr(int n); + + int getNumberOfExpressions() { result = count(this.getExpr(_)) } + + override string toString() { + exists(int c | c = this.getNumberOfExpressions() | + c = 0 and result = ";" + or + c = 1 and result = this.getExpr(0).toString() + or + c > 1 and result = "...; ..." + ) + } } } @@ -177,6 +192,14 @@ module ParenthesizedExpr { final override Generated::ParenthesizedStatements generated; final override Expr getExpr(int n) { result = generated.getChild(n) } + + final override string toString() { + exists(int c | c = this.getNumberOfExpressions() | + c = 0 and result = "()" + or + c > 0 and result = "(" + ExprSequence::Range.super.toString() + ")" + ) + } } } @@ -211,6 +234,8 @@ module ScopeResolution { final Expr getScope() { result = generated.getScope() } final string getName() { result = generated.getName().(Generated::Token).getValue() } + + final override string toString() { result = "...::" + this.getName() } } } @@ -221,5 +246,7 @@ module Pair { final Expr getKey() { result = generated.getKey() } final Expr getValue() { result = generated.getValue() } + + final override string toString() { result = "Pair" } } } diff --git a/ql/src/codeql_ruby/ast/internal/Method.qll b/ql/src/codeql_ruby/ast/internal/Method.qll index b6c1109a841..834a959abbf 100644 --- a/ql/src/codeql_ruby/ast/internal/Method.qll +++ b/ql/src/codeql_ruby/ast/internal/Method.qll @@ -1,10 +1,11 @@ private import codeql_ruby.AST private import codeql_ruby.ast.internal.Expr +private import codeql_ruby.ast.internal.Parameter private import TreeSitter module Callable { abstract class Range extends Expr::Range { - abstract Parameter getParameter(int n); + abstract Parameter::Range getParameter(int n); } } @@ -12,7 +13,7 @@ module Method { class Range extends Callable::Range, BodyStatement::Range, @method { final override Generated::Method generated; - override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } + override Parameter::Range getParameter(int n) { result = generated.getParameters().getChild(n) } string getName() { result = generated.getName().(Generated::Token).getValue() or @@ -23,6 +24,8 @@ module Method { final predicate isSetter() { generated.getName() instanceof Generated::Setter } final override Expr getExpr(int i) { result = generated.getChild(i) } + + final override string toString() { result = this.getName() } } } @@ -30,7 +33,7 @@ module SingletonMethod { class Range extends Callable::Range, BodyStatement::Range, @singleton_method { final override Generated::SingletonMethod generated; - override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } + override Parameter::Range getParameter(int n) { result = generated.getParameters().getChild(n) } string getName() { result = generated.getName().(Generated::Token).getValue() or @@ -39,6 +42,8 @@ module SingletonMethod { } final override Expr getExpr(int i) { result = generated.getChild(i) } + + final override string toString() { result = this.getName() } } } @@ -46,7 +51,11 @@ module Lambda { class Range extends Callable::Range, @lambda { final override Generated::Lambda generated; - final override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } + final override Parameter::Range getParameter(int n) { + result = generated.getParameters().getChild(n) + } + + final override string toString() { result = "-> { ... }" } } } @@ -58,9 +67,13 @@ module DoBlock { class Range extends Block::Range, BodyStatement::Range, @do_block { final override Generated::DoBlock generated; - final override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } - final override Expr getExpr(int i) { result = generated.getChild(i) } + + final override Parameter::Range getParameter(int n) { + result = generated.getParameters().getChild(n) + } + + final override string toString() { result = "do ... end" } } } @@ -68,6 +81,10 @@ module BraceBlock { class Range extends Block::Range, @block { final override Generated::Block generated; - final override Parameter getParameter(int n) { result = generated.getParameters().getChild(n) } + final override Parameter::Range getParameter(int n) { + result = generated.getParameters().getChild(n) + } + + final override string toString() { result = "{ ... }" } } } diff --git a/ql/src/codeql_ruby/ast/internal/Module.qll b/ql/src/codeql_ruby/ast/internal/Module.qll index 60477728d4b..e0a4a3d65ab 100644 --- a/ql/src/codeql_ruby/ast/internal/Module.qll +++ b/ql/src/codeql_ruby/ast/internal/Module.qll @@ -20,6 +20,8 @@ module Class { final ScopeResolution getNameScopeResolution() { result = generated.getName() } final Expr getSuperclassExpr() { result = generated.getSuperclass().getChild() } + + final override string toString() { result = this.getName() } } } @@ -30,6 +32,8 @@ module SingletonClass { final override Expr getExpr(int i) { result = generated.getChild(i) } final Expr getValue() { result = generated.getValue() } + + final override string toString() { result = "class << ..." } } } @@ -45,5 +49,7 @@ module Module { } final ScopeResolution getNameScopeResolution() { result = generated.getName() } + + final override string toString() { result = this.getName() } } } diff --git a/ql/src/codeql_ruby/ast/internal/Operation.qll b/ql/src/codeql_ruby/ast/internal/Operation.qll index 15762d94380..124bd18e03e 100644 --- a/ql/src/codeql_ruby/ast/internal/Operation.qll +++ b/ql/src/codeql_ruby/ast/internal/Operation.qll @@ -19,6 +19,8 @@ module UnaryOperation { Expr getOperand() { result = generated.getOperand() } final override Expr getAnOperand() { result = this.getOperand() } + + override string toString() { result = this.getOperator() + " ..." } } } @@ -69,6 +71,8 @@ module BinaryOperation { final override Expr getAnOperand() { result = this.getLeftOperand() or result = this.getRightOperand() } + + override string toString() { result = "... " + this.getOperator() + " ..." } } } @@ -219,6 +223,8 @@ module Assignment { abstract Expr getRhs(); final override Expr getAnOperand() { result = this.getLhs() or result = this.getRhs() } + + override string toString() { result = "... " + this.getOperator() + " ..." } } } diff --git a/ql/src/codeql_ruby/ast/internal/Parameter.qll b/ql/src/codeql_ruby/ast/internal/Parameter.qll new file mode 100644 index 00000000000..b62adfcc59f --- /dev/null +++ b/ql/src/codeql_ruby/ast/internal/Parameter.qll @@ -0,0 +1,133 @@ +private import codeql_ruby.AST +private import TreeSitter +private import codeql_ruby.ast.internal.AST +private import codeql_ruby.ast.internal.Variable +private import codeql_ruby.ast.internal.Method +private import codeql_ruby.ast.internal.Pattern +private import codeql.Locations + +module Parameter { + class Range extends AstNode::Range { + private int pos; + + Range() { + this = any(Generated::BlockParameters bp).getChild(pos) + or + this = any(Generated::MethodParameters mp).getChild(pos) + or + this = any(Generated::LambdaParameters lp).getChild(pos) + } + + final int getPosition() { result = pos } + + LocalVariable getAVariable() { none() } + } +} + +module NamedParameter { + abstract class Range extends Parameter::Range { + abstract string getName(); + + abstract LocalVariable getVariable(); + + override LocalVariable getAVariable() { result = this.getVariable() } + } +} + +module SimpleParameter { + class Range extends NamedParameter::Range, PatternParameter::Range, VariablePattern::Range { + final override string getName() { result = this.getVariableName() } + + final override LocalVariable getVariable() { result = TLocalVariable(_, _, this) } + + final override LocalVariable getAVariable() { result = this.getVariable() } + + final override string toString() { result = this.getName() } + } +} + +module PatternParameter { + class Range extends Parameter::Range, Pattern::Range { + override LocalVariable getAVariable() { result = this.(Pattern::Range).getAVariable() } + } +} + +module TuplePatternParameter { + class Range extends PatternParameter::Range, TuplePattern::Range { + override LocalVariable getAVariable() { result = TuplePattern::Range.super.getAVariable() } + } +} + +module BlockParameter { + class Range extends NamedParameter::Range, @block_parameter { + final override Generated::BlockParameter generated; + + final override string getName() { result = generated.getName().getValue() } + + final override LocalVariable getVariable() { + result = TLocalVariable(_, _, generated.getName()) + } + + final override string toString() { result = "&" + this.getName() } + } +} + +module HashSplatParameter { + class Range extends NamedParameter::Range, @hash_splat_parameter { + final override Generated::HashSplatParameter generated; + + final override LocalVariable getVariable() { + result = TLocalVariable(_, _, generated.getName()) + } + + final override string toString() { result = "**" + this.getName() } + + final override string getName() { result = generated.getName().getValue() } + } +} + +module KeywordParameter { + class Range extends NamedParameter::Range, @keyword_parameter { + final override Generated::KeywordParameter generated; + + final override LocalVariable getVariable() { + result = TLocalVariable(_, _, generated.getName()) + } + + final Generated::AstNode getDefaultValue() { result = generated.getValue() } + + final override string toString() { result = this.getName() } + + final override string getName() { result = generated.getName().getValue() } + } +} + +module OptionalParameter { + class Range extends NamedParameter::Range, @optional_parameter { + final override Generated::OptionalParameter generated; + + final override LocalVariable getVariable() { + result = TLocalVariable(_, _, generated.getName()) + } + + final Generated::AstNode getDefaultValue() { result = generated.getValue() } + + final override string toString() { result = this.getName() } + + final override string getName() { result = generated.getName().getValue() } + } +} + +module SplatParameter { + class Range extends NamedParameter::Range, @splat_parameter { + final override Generated::SplatParameter generated; + + final override LocalVariable getVariable() { + result = TLocalVariable(_, _, generated.getName()) + } + + final override string toString() { result = "*" + this.getName() } + + final override string getName() { result = generated.getName().getValue() } + } +} diff --git a/ql/src/codeql_ruby/ast/internal/Pattern.qll b/ql/src/codeql_ruby/ast/internal/Pattern.qll index 413c1696f33..d99b96ccd1e 100644 --- a/ql/src/codeql_ruby/ast/internal/Pattern.qll +++ b/ql/src/codeql_ruby/ast/internal/Pattern.qll @@ -1,5 +1,6 @@ private import codeql_ruby.AST private import TreeSitter +private import codeql_ruby.ast.internal.AST private import codeql_ruby.ast.internal.Variable private import codeql_ruby.ast.internal.Method private import codeql.Locations @@ -39,7 +40,7 @@ predicate implicitParameterAssignmentNode(Generated::AstNode n, Callable::Range } module Pattern { - abstract class Range extends AstNode { + class Range extends AstNode::Range { cached Range() { explicitAssignmentNode(this, _) @@ -49,42 +50,37 @@ module Pattern { implicitParameterAssignmentNode(this, _) } - abstract Variable getAVariable(); + Variable getAVariable() { none() } } } module VariablePattern { - class Range extends Pattern::Range { + class Range extends Pattern::Range, @token_identifier { override Generated::Identifier generated; string getVariableName() { result = generated.getValue() } override Variable getAVariable() { access(this, result) } + + override string toString() { result = this.getVariableName() } } } module TuplePattern { - abstract class Range extends Pattern::Range { - abstract Pattern::Range getElement(int i); + private class Range_ = + @destructured_parameter or @destructured_left_assignment or @left_assignment_list; + + class Range extends Pattern::Range, Range_ { + Pattern::Range getElement(int i) { + result = this.(Generated::DestructuredParameter).getChild(i) + or + result = this.(Generated::DestructuredLeftAssignment).getChild(i) + or + result = this.(Generated::LeftAssignmentList).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) } + final override string toString() { result = "(..., ...)" } } } diff --git a/ql/src/codeql_ruby/ast/internal/Variable.qll b/ql/src/codeql_ruby/ast/internal/Variable.qll index 7177334f62e..1deed56915e 100644 --- a/ql/src/codeql_ruby/ast/internal/Variable.qll +++ b/ql/src/codeql_ruby/ast/internal/Variable.qll @@ -1,9 +1,11 @@ private import TreeSitter private import codeql.Locations private import codeql_ruby.AST +private import codeql_ruby.ast.internal.AST private import codeql_ruby.ast.internal.Expr private import codeql_ruby.ast.internal.Method private import codeql_ruby.ast.internal.Pattern +private import codeql_ruby.ast.internal.Parameter Generated::AstNode parentOf(Generated::AstNode n) { exists(Generated::AstNode parent | parent = n.getParent() | @@ -22,7 +24,7 @@ Generated::AstNode parentOf(Generated::AstNode n) { private Generated::AstNode parentOfNoScope(Generated::AstNode n) { result = parentOf(n) and - not n = any(VariableScope s).getScopeElement() + not n = any(VariableScope::Range s).getScopeElement() } private predicate instanceVariableAccess( @@ -53,11 +55,11 @@ private TCallableScope parentCallableScope(TCallableScope scope) { not c instanceof Method::Range and not c instanceof SingletonMethod::Range | - result = scope.(VariableScope).getOuterScope() + result = scope.(VariableScope::Range).getOuterScope() ) } -private VariableScope parentScope(VariableScope scope) { +private VariableScope::Range parentScope(VariableScope::Range scope) { not scope instanceof ModuleOrClassScope and result = scope.getOuterScope() } @@ -86,9 +88,9 @@ private predicate scopeDefinesParameterVariable( other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn() ) or - exists(Parameter p | - p = scope.getScopeElement().getParameter(_) and - name = p.(NamedParameter).getName() + exists(Parameter::Range p | + p = scope.getScopeElement().(Callable::Range).getParameter(_) and + name = p.(NamedParameter::Range).getName() | i = p.(Generated::BlockParameter).getName() or i = p.(Generated::HashSplatParameter).getName() or @@ -113,42 +115,11 @@ private predicate strictlyBefore(Location one, Location two) { one.getStartLine() = two.getStartLine() and one.getStartColumn() < two.getStartColumn() } -/** A scope that may capture outer local variables. */ -private class CapturingScope extends VariableScope { - CapturingScope() { - exists(Callable::Range c | c = this.getScopeElement() | - c instanceof Block::Range - or - c instanceof DoBlock::Range - or - c instanceof Lambda::Range // TODO: Check if this is actually the case - ) - } - - /** Holds if this scope inherits `name` from an outer scope `outer`. */ - predicate inherits(string name, VariableScope outer) { - not scopeDefinesParameterVariable(this, name, _) and - ( - outer = this.getOuterScope() and - ( - scopeDefinesParameterVariable(outer, name, _) - or - exists(Generated::Identifier i | - scopeAssigns(outer, name, i) and - strictlyBefore(i.getLocation(), this.getLocation()) - ) - ) - or - this.getOuterScope().(CapturingScope).inherits(name, outer) - ) - } -} - cached private module Cached { /** Gets the enclosing scope for `node`. */ cached - VariableScope enclosingScope(Generated::AstNode node) { + VariableScope::Range enclosingScope(Generated::AstNode node) { result.getScopeElement() = parentOfNoScope*(parentOf(node)) } @@ -157,7 +128,7 @@ private module Cached { TGlobalScope() or TTopLevelScope(Generated::Program node) or TModuleScope(Generated::Module node) or - TClassScope(AstNode cls) { + TClassScope(Generated::AstNode cls) { cls instanceof Generated::Class or cls instanceof Generated::SingletonClass } or TCallableScope(Callable::Range c) @@ -181,7 +152,7 @@ private module Cached { other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn() ) } or - TLocalVariable(VariableScope scope, string name, Generated::Identifier i) { + TLocalVariable(VariableScope::Range scope, string name, Generated::Identifier i) { scopeDefinesParameterVariable(scope, name, i) or i = @@ -191,7 +162,7 @@ private module Cached { other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn() ) and not scopeDefinesParameterVariable(scope, name, _) and - not scope.(CapturingScope).inherits(name, _) + not scope.inherits(name, _) } // Token types that can be vcalls @@ -340,7 +311,8 @@ private module Cached { cached predicate access(Generated::Identifier access, Variable variable) { exists(string name | name = access.getValue() | - variable = enclosingScope(access).getVariable(name) and + variable.getDeclaringScope() = enclosingScope(access) and + variable.getName() = name and not strictlyBefore(access.getLocation(), variable.getLocation()) and // In case of overlapping parameter names, later parameters should not // be considered accesses to the first parameter @@ -350,7 +322,7 @@ private module Cached { or exists(VariableScope declScope | variable = declScope.getVariable(name) and - enclosingScope(access).(CapturingScope).inherits(name, declScope) + enclosingScope(access).inherits(name, declScope) ) ) } @@ -399,7 +371,32 @@ module VariableScope { abstract class Range extends TScope { abstract string toString(); - abstract AstNode getScopeElement(); + abstract Generated::AstNode getScopeElement(); + + abstract predicate isCapturing(); + + VariableScope::Range getOuterScope() { result = enclosingScope(this.getScopeElement()) } + + /** Holds if this scope inherits `name` from an outer scope `outer`. */ + predicate inherits(string name, VariableScope::Range outer) { + this.isCapturing() and + not scopeDefinesParameterVariable(this, name, _) and + ( + outer = this.getOuterScope() and + ( + scopeDefinesParameterVariable(outer, name, _) + or + exists(Generated::Identifier i | + scopeAssigns(outer, name, i) and + strictlyBefore(i.getLocation(), this.getLocation()) + ) + ) + or + this.getOuterScope().inherits(name, outer) + ) + } + + final Location getLocation() { result = getScopeElement().getLocation() } } } @@ -407,7 +404,9 @@ module GlobalScope { class Range extends VariableScope::Range, TGlobalScope { override string toString() { result = "global scope" } - override AstNode getScopeElement() { none() } + override Generated::AstNode getScopeElement() { none() } + + override predicate isCapturing() { none() } } } @@ -415,7 +414,9 @@ module TopLevelScope { class Range extends VariableScope::Range, TTopLevelScope { override string toString() { result = "top-level scope" } - override AstNode getScopeElement() { TTopLevelScope(result) = this } + override Generated::AstNode getScopeElement() { TTopLevelScope(result) = this } + + override predicate isCapturing() { none() } } } @@ -423,7 +424,9 @@ module ModuleScope { class Range extends VariableScope::Range, TModuleScope { override string toString() { result = "module scope" } - override AstNode getScopeElement() { TModuleScope(result) = this } + override Generated::AstNode getScopeElement() { TModuleScope(result) = this } + + override predicate isCapturing() { none() } } } @@ -431,13 +434,15 @@ module ClassScope { class Range extends VariableScope::Range, TClassScope { override string toString() { result = "class scope" } - override AstNode getScopeElement() { TClassScope(result) = this } + override Generated::AstNode getScopeElement() { TClassScope(result) = this } + + override predicate isCapturing() { none() } } } module CallableScope { class Range extends VariableScope::Range, TCallableScope { - private Callable::Range c; + private Generated::AstNode c; Range() { this = TCallableScope(c) } @@ -452,7 +457,15 @@ module CallableScope { result = "block scope" } - override Callable::Range getScopeElement() { TCallableScope(result) = this } + override Generated::AstNode getScopeElement() { TCallableScope(result) = this } + + override predicate isCapturing() { + c instanceof Generated::Lambda + or + c instanceof Generated::Block + or + c instanceof Generated::DoBlock + } } } @@ -540,6 +553,8 @@ module ClassVariable { module VariableAccess { abstract class Range extends Expr::Range { abstract Variable getVariable(); + + final override string toString() { result = this.getVariable().getName() } } } From 85c13a1190868377b644540111a646874f0c6282 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 9 Feb 2021 16:34:25 +0100 Subject: [PATCH 2/2] Make entries in `RemoveWhenFullCoverage` explicit --- ql/src/codeql_ruby/ast/internal/AST.qll | 93 ++++++++++++++++++- ql/src/codeql_ruby/ast/internal/Expr.qll | 6 +- ql/src/codeql_ruby/ast/internal/Parameter.qll | 6 ++ ql/src/codeql_ruby/ast/internal/Pattern.qll | 4 +- ql/src/codeql_ruby/controlflow/CfgNodes.qll | 8 +- .../controlflow/ControlFlowGraph.qll | 2 +- 6 files changed, 110 insertions(+), 9 deletions(-) diff --git a/ql/src/codeql_ruby/ast/internal/AST.qll b/ql/src/codeql_ruby/ast/internal/AST.qll index 0b5b63976d0..4073265a4d2 100644 --- a/ql/src/codeql_ruby/ast/internal/AST.qll +++ b/ql/src/codeql_ruby/ast/internal/AST.qll @@ -8,13 +8,100 @@ module AstNode { Range() { this = generated } cached - string toString() { result = "AstNode" } + abstract string toString(); Location getLocation() { result = generated.getLocation() } } - // TODO: Remove, and when removed make Range::toString() above abstract + // TODO: Remove private class RemoveWhenFullCoverage extends Range { - RemoveWhenFullCoverage() { this instanceof Generated::AstNode } + // Lists the entities that are currently used in tests but do not yet + // have an external ASTNode. Perhaps not all entities below need to be + // an AST node, for example we include the `in` keyword in `for` loops + // in the CFG, but not the AST + RemoveWhenFullCoverage() { + this instanceof Generated::Program + or + this = any(Generated::Method m).getName() + or + this = any(Generated::SingletonMethod m).getName() + or + this instanceof Generated::BeginBlock + or + this instanceof Generated::EndBlock + or + this = any(Generated::Call c).getMethod() and + not this instanceof Generated::ScopeResolution + or + this instanceof Generated::Ensure + or + this instanceof Generated::Rescue + or + this instanceof Generated::Constant + or + this instanceof Generated::RestAssignment + or + this = any(Generated::RestAssignment ra).getChild() + or + this instanceof Generated::Return + or + this instanceof Generated::Break + or + this instanceof Generated::Alias + or + this instanceof Generated::SymbolArray + or + this instanceof Generated::Interpolation + or + this instanceof Generated::StringArray + or + this instanceof Generated::BareString + or + this instanceof Generated::Self + or + this instanceof Generated::Float + or + this instanceof Generated::Superclass + or + this instanceof Generated::EmptyStatement + or + this instanceof Generated::Next + or + this instanceof Generated::Redo + or + this instanceof Generated::Hash + or + this instanceof Generated::Array + or + this instanceof Generated::ElementReference + or + this instanceof Generated::Complex + or + this instanceof Generated::Character + or + this = any(Generated::Alias a).getName() + or + this = any(Generated::Alias a).getAlias() + or + this instanceof Generated::HeredocBody + or + this instanceof Generated::HeredocBeginning + or + this instanceof Generated::HeredocEnd + or + this instanceof Generated::Range + or + this instanceof Generated::Rational + or + this instanceof Generated::RescueModifier + or + this instanceof Generated::Subshell + or + this instanceof Generated::Undef + or + this = any(Generated::Undef u).getChild(_) + } + + override string toString() { result = "AstNode" } } } diff --git a/ql/src/codeql_ruby/ast/internal/Expr.qll b/ql/src/codeql_ruby/ast/internal/Expr.qll index 27e944c8aff..4b93421ce7f 100644 --- a/ql/src/codeql_ruby/ast/internal/Expr.qll +++ b/ql/src/codeql_ruby/ast/internal/Expr.qll @@ -68,7 +68,7 @@ module RegexLiteral { final override string toString() { result = - concat(AstNode c, int i, string s | + concat(Generated::AstNode c, int i, string s | c = generated.getChild(i) and if c instanceof Generated::Token then s = c.(Generated::Token).getValue() @@ -92,7 +92,7 @@ module StringLiteral { final override string toString() { result = - concat(AstNode c, int i, string s | + concat(Generated::AstNode c, int i, string s | c = generated.getChild(i) and if c instanceof Generated::Token then s = c.(Generated::Token).getValue() @@ -127,7 +127,7 @@ module SymbolLiteral { private string summaryString() { result = - concat(AstNode c, int i, string s | + concat(Generated::AstNode c, int i, string s | c = this.getChild(i) and if c instanceof Generated::Token then s = c.(Generated::Token).getValue() diff --git a/ql/src/codeql_ruby/ast/internal/Parameter.qll b/ql/src/codeql_ruby/ast/internal/Parameter.qll index b62adfcc59f..a891df165f0 100644 --- a/ql/src/codeql_ruby/ast/internal/Parameter.qll +++ b/ql/src/codeql_ruby/ast/internal/Parameter.qll @@ -21,6 +21,8 @@ module Parameter { final int getPosition() { result = pos } LocalVariable getAVariable() { none() } + + override string toString() { none() } } } @@ -49,12 +51,16 @@ module SimpleParameter { module PatternParameter { class Range extends Parameter::Range, Pattern::Range { override LocalVariable getAVariable() { result = this.(Pattern::Range).getAVariable() } + + override string toString() { none() } } } module TuplePatternParameter { class Range extends PatternParameter::Range, TuplePattern::Range { override LocalVariable getAVariable() { result = TuplePattern::Range.super.getAVariable() } + + override string toString() { result = TuplePattern::Range.super.toString() } } } diff --git a/ql/src/codeql_ruby/ast/internal/Pattern.qll b/ql/src/codeql_ruby/ast/internal/Pattern.qll index d99b96ccd1e..808031f751d 100644 --- a/ql/src/codeql_ruby/ast/internal/Pattern.qll +++ b/ql/src/codeql_ruby/ast/internal/Pattern.qll @@ -51,6 +51,8 @@ module Pattern { } Variable getAVariable() { none() } + + override string toString() { none() } } } @@ -81,6 +83,6 @@ module TuplePattern { override Variable getAVariable() { result = this.getElement(_).getAVariable() } - final override string toString() { result = "(..., ...)" } + override string toString() { result = "(..., ...)" } } } diff --git a/ql/src/codeql_ruby/controlflow/CfgNodes.qll b/ql/src/codeql_ruby/controlflow/CfgNodes.qll index c08df48c8a6..6a8410d8434 100644 --- a/ql/src/codeql_ruby/controlflow/CfgNodes.qll +++ b/ql/src/codeql_ruby/controlflow/CfgNodes.qll @@ -71,14 +71,20 @@ class AstCfgNode extends CfgNode, TAstNode { final override AstNode getNode() { result = n } + override Location getLocation() { result = n.getLocation() } + final override string toString() { exists(string s | - // TODO: Remove once the SSA implementation is based on the AST layer + // TODO: Replace the two disjuncts below with `s = n.(AstNode).toString()` once + // `RemoveWhenFullCoverage` has been removed s = n.(AstNode).toString() and s != "AstNode" or n.(AstNode).toString() = "AstNode" and s = n.toString() + or + n = any(Generated::For f).getValue() and + s = "In" | result = "[" + this.getSplitsString() + "] " + s or diff --git a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll index 43668d3e248..a5026c6085f 100644 --- a/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll +++ b/ql/src/codeql_ruby/controlflow/ControlFlowGraph.qll @@ -38,7 +38,7 @@ class CfgNode extends TCfgNode { AST::AstNode getNode() { none() } /** Gets the location of this control flow node. */ - Location getLocation() { result = this.getNode().getLocation() } + Location getLocation() { none() } /** Holds if this control flow node has conditional successors. */ final predicate isCondition() { exists(this.getASuccessor(any(BooleanSuccessor bs))) }