From 69cf373f21b20b366cd600d62dddcebc120fca30 Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Wed, 13 Oct 2021 13:43:52 +0000 Subject: [PATCH] QL: Refactor --- ql/src/codeql_ql/ast/Ast.qll | 50 +++++++--- ql/src/codeql_ql/ast/internal/AstNodes.qll | 9 +- ql/src/codeql_ql/ast/internal/Predicate.qll | 104 +++----------------- ql/src/codeql_ql/ast/internal/Type.qll | 3 +- 4 files changed, 59 insertions(+), 107 deletions(-) diff --git a/ql/src/codeql_ql/ast/Ast.qll b/ql/src/codeql_ql/ast/Ast.qll index dad0dec9ae9..b5f3a9dafca 100644 --- a/ql/src/codeql_ql/ast/Ast.qll +++ b/ql/src/codeql_ql/ast/Ast.qll @@ -31,6 +31,20 @@ class AstNode extends TAstNode { ) } + predicate hasLocationInfo( + string filepath, int startline, int startcolumn, int endline, int endcolumn + ) { + if exists(getLocation()) + then getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) + else ( + filepath = "" and + startline = 0 and + startcolumn = 0 and + endline = 0 and + endcolumn = 0 + ) + } + /** * Gets the parent in the AST for this node. */ @@ -181,7 +195,7 @@ class Select extends TSelect, AstNode { * A QL predicate. * Either a classless predicate, a class predicate, or a characteristic predicate. */ -class Predicate extends TPredicate, AstNode, Declaration { +class Predicate extends TPredicate, AstNode, PredicateOrBuiltin, Declaration { /** * Gets the body of the predicate. */ @@ -200,7 +214,7 @@ class Predicate extends TPredicate, AstNode, Declaration { /** * Gets the number of parameters. */ - int getArity() { + override int getArity() { not this.(ClasslessPredicate).getAlias() instanceof PredicateExpr and result = count(getParameter(_)) or @@ -209,12 +223,19 @@ class Predicate extends TPredicate, AstNode, Declaration { ) } + /** + * Holds if this predicate is private. + */ + override predicate isPrivate() { hasAnnotation("private") } + /** * Gets the return type (if any) of the predicate. */ TypeExpr getReturnTypeExpr() { none() } - Type getReturnType() { result = this.getReturnTypeExpr().getResolvedType() } + override Type getReturnType() { result = this.getReturnTypeExpr().getResolvedType() } + + override Type getParameterType(int i) { result = this.getParameter(i).getType() } override AstNode getAChild(string pred) { result = super.getAChild(pred) @@ -376,6 +397,8 @@ class ClasslessPredicate extends TClasslessPredicate, Predicate, ModuleDeclarati or pred_name = directMember("getReturnTypeExpr") and result = this.getReturnTypeExpr() } + + override predicate isPrivate() { Predicate.super.isPrivate() } } /** @@ -394,11 +417,6 @@ class ClassPredicate extends TClassPredicate, Predicate { override Class getParent() { result.getAClassPredicate() = this } - /** - * Holds if this predicate is private. - */ - predicate isPrivate() { hasAnnotation("private") } - /** * Holds if this predicate is annotated as overriding another predicate. */ @@ -416,7 +434,7 @@ class ClassPredicate extends TClassPredicate, Predicate { /** * Gets the type representing this class. */ - ClassType getDeclaringType() { result.getDeclaration() = getParent() } + override ClassType getDeclaringType() { result.getDeclaration() = getParent() } predicate overrides(ClassPredicate other) { predOverrides(this, other) } @@ -453,7 +471,7 @@ class CharPred extends TCharPred, Predicate { pred_name = directMember("getBody") and result = this.getBody() } - ClassType getDeclaringType() { result.getDeclaration() = getParent() } + override ClassType getDeclaringType() { result.getDeclaration() = getParent() } } /** @@ -764,7 +782,7 @@ class NewType extends TNewType, TypeDeclaration, ModuleDeclaration { * A branch in a `newtype`. * E.g. `Bar()` or `Baz()` in `newtype Foo = Bar() or Baz()`. */ -class NewTypeBranch extends TNewTypeBranch, TypeDeclaration { +class NewTypeBranch extends TNewTypeBranch, PredicateOrBuiltin, TypeDeclaration { Generated::DatatypeBranch branch; NewTypeBranch() { this = TNewTypeBranch(branch) } @@ -786,6 +804,16 @@ class NewTypeBranch extends TNewTypeBranch, TypeDeclaration { /** Gets the body of this branch. */ Formula getBody() { toGenerated(result) = branch.getChild(_).(Generated::Body).getChild() } + override NewTypeBranchType getReturnType() { result.getDeclaration() = this } + + override Type getParameterType(int i) { result = this.getField(i).getType() } + + override int getArity() { result = count(this.getField(_)) } + + override Type getDeclaringType() { none() } + + predicate isPrivate() { this.getNewType().isPrivate() } + override QLDoc getQLDoc() { toGenerated(result) = branch.getChild(_) } NewType getNewType() { result.getABranch() = this } diff --git a/ql/src/codeql_ql/ast/internal/AstNodes.qll b/ql/src/codeql_ql/ast/internal/AstNodes.qll index 5aeffd620ad..89e48335927 100644 --- a/ql/src/codeql_ql/ast/internal/AstNodes.qll +++ b/ql/src/codeql_ql/ast/internal/AstNodes.qll @@ -1,5 +1,6 @@ import codeql_ql.ast.Ast as AST import TreeSitter +private import Builtins cached newtype TAstNode = @@ -65,7 +66,11 @@ newtype TAstNode = TYamlEntry(Generated::YamlEntry ye) or TYamlKey(Generated::YamlKey yk) or TYamlListitem(Generated::YamlListitem yli) or - TYamlValue(Generated::YamlValue yv) + TYamlValue(Generated::YamlValue yv) or + TBuiltinClassless(string ret, string name, string args) { isBuiltinClassless(ret, name, args) } or + TBuiltinMember(string qual, string ret, string name, string args) { + isBuiltinMember(qual, ret, name, args) + } class TFormula = TDisjunction or TConjunction or TComparisonFormula or TQuantifier or TNegation or TIfFormula or @@ -194,6 +199,8 @@ Generated::AstNode toGenerated(AST::AstNode n) { class TPredicate = TCharPred or TClasslessPredicate or TClassPredicate or TDBRelation; +class TPredOrBuiltin = TPredicate or TNewTypeBranch or TBuiltinClassless or TBuiltinMember; + class TModuleMember = TModuleDeclaration or TImport or TSelect or TQLDoc; class TDeclaration = TTypeDeclaration or TModuleDeclaration or TPredicate or TVarDecl; diff --git a/ql/src/codeql_ql/ast/internal/Predicate.qll b/ql/src/codeql_ql/ast/internal/Predicate.qll index 9279b93a4ea..668fd5cb2b0 100644 --- a/ql/src/codeql_ql/ast/internal/Predicate.qll +++ b/ql/src/codeql_ql/ast/internal/Predicate.qll @@ -1,12 +1,11 @@ import ql private import Builtins private import codeql_ql.ast.internal.Module -private import codeql_ql.ast.internal.AstNodes as AstNodes +private import codeql_ql.ast.internal.AstNodes -private class TClasslessPredicateOrNewTypeBranch = - AstNodes::TClasslessPredicate or AstNodes::TNewTypeBranch; +private class TClasslessPredicateOrNewTypeBranch = TClasslessPredicate or TNewTypeBranch; -string getPredicateName(TClasslessPredicateOrNewTypeBranch p) { +private string getPredicateName(TClasslessPredicateOrNewTypeBranch p) { result = p.(ClasslessPredicate).getName() or result = p.(NewTypeBranch).getName() } @@ -69,8 +68,7 @@ private module Cached { m = pc.getQualifier().getResolvedModule() and public = true | - definesPredicate(m, pc.getPredicateName(), pc.getNumberOfArguments(), p.getDeclaration(), - public) + definesPredicate(m, pc.getPredicateName(), pc.getNumberOfArguments(), p, public) ) } @@ -120,49 +118,13 @@ private module Cached { not resolvePredicateCall(c, _) and resolveDBRelation(c, p) } - - cached - module NewTypeDef { - cached - newtype TPredOrBuiltin = - TPred(Predicate p) or - TNewTypeBranch(NewTypeBranch b) or - TBuiltinClassless(string ret, string name, string args) { - isBuiltinClassless(ret, name, args) - } or - TBuiltinMember(string qual, string ret, string name, string args) { - isBuiltinMember(qual, ret, name, args) - } - } } import Cached -private import NewTypeDef -class PredicateOrBuiltin extends TPredOrBuiltin { +class PredicateOrBuiltin extends TPredOrBuiltin, AstNode { string getName() { none() } - string toString() { result = getName() } - - predicate hasLocationInfo( - string filepath, int startline, int startcolumn, int endline, int endcolumn - ) { - if exists(getDeclaration()) - then - getDeclaration() - .getLocation() - .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) - else ( - filepath = "" and - startline = 0 and - startcolumn = 0 and - endline = 0 and - endcolumn = 0 - ) - } - - AstNode getDeclaration() { none() } - Type getDeclaringType() { none() } Type getParameterType(int i) { none() } @@ -174,57 +136,13 @@ class PredicateOrBuiltin extends TPredOrBuiltin { predicate isPrivate() { none() } } -private class DefinedPredicate extends PredicateOrBuiltin, TPred { - Predicate decl; - - DefinedPredicate() { this = TPred(decl) } - - override Predicate getDeclaration() { result = decl } - - override string getName() { result = decl.getName() } - - override Type getReturnType() { result = decl.getReturnType() } - - override Type getParameterType(int i) { result = decl.getParameter(i).getType() } - - // Can be removed when all types can be resolved - override int getArity() { result = decl.getArity() } - - override Type getDeclaringType() { - result = decl.(ClassPredicate).getDeclaringType() - or - result = decl.(CharPred).getDeclaringType() - } - - override predicate isPrivate() { - decl.(ClassPredicate).isPrivate() or decl.(ClassPredicate).isPrivate() - } -} - -private class DefinedNewTypeBranch extends PredicateOrBuiltin, TNewTypeBranch { - NewTypeBranch b; - - DefinedNewTypeBranch() { this = TNewTypeBranch(b) } - - override NewTypeBranch getDeclaration() { result = b } - - override string getName() { result = b.getName() } - - override NewTypeBranchType getReturnType() { result.getDeclaration() = b } - - override Type getParameterType(int i) { result = b.getField(i).getType() } - - // Can be removed when all types can be resolved - override int getArity() { result = count(b.getField(_)) } - - override Type getDeclaringType() { none() } - - override predicate isPrivate() { b.getNewType().isPrivate() } -} - private class TBuiltin = TBuiltinClassless or TBuiltinMember; -class BuiltinPredicate extends PredicateOrBuiltin, TBuiltin { } +class BuiltinPredicate extends PredicateOrBuiltin, TBuiltin { + override string toString() { result = getName() } + + override string getAPrimaryQlClass() { result = "BuiltinPredicate" } +} private class BuiltinClassless extends BuiltinPredicate, TBuiltinClassless { string name; @@ -287,7 +205,7 @@ module PredConsistency { strictcount(PredicateOrBuiltin p0 | resolveCall(call, p0) and // aliases are expected to resolve to multiple. - not exists(p0.getDeclaration().(ClasslessPredicate).getAlias()) + not exists(p0.(ClasslessPredicate).getAlias()) ) and c > 1 and resolveCall(call, p) diff --git a/ql/src/codeql_ql/ast/internal/Type.qll b/ql/src/codeql_ql/ast/internal/Type.qll index 32e1000f426..18ccb7219f7 100644 --- a/ql/src/codeql_ql/ast/internal/Type.qll +++ b/ql/src/codeql_ql/ast/internal/Type.qll @@ -127,8 +127,7 @@ private PredicateOrBuiltin inherClassPredCandidate(Type ty, string name, int ari } predicate predOverrides(ClassPredicate sub, ClassPredicate sup) { - sup = - inherClassPredCandidate(sub.getDeclaringType(), sub.getName(), sub.getArity()).getDeclaration() + sup = inherClassPredCandidate(sub.getDeclaringType(), sub.getName(), sub.getArity()) } private VarDecl declaredField(ClassType ty, string name) {