From 3c8153ca1e149414d385515a63d69d7c4d7b0683 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Wed, 17 Jun 2020 16:58:56 +0100 Subject: [PATCH] Clean up @field and @fieldparent usage * Centralise use of raw types and database predicates in FieldParent and FieldBase classes * Deduplicate type predicates common to all fields * Deduplicate predicates common to function parameters and results --- ql/src/semmle/go/AST.qll | 25 ++++++- ql/src/semmle/go/Decls.qll | 149 +++++++++++++------------------------ ql/src/semmle/go/Expr.qll | 30 ++------ 3 files changed, 82 insertions(+), 122 deletions(-) diff --git a/ql/src/semmle/go/AST.qll b/ql/src/semmle/go/AST.qll index a58a8c3d78a..216ca76eca3 100644 --- a/ql/src/semmle/go/AST.qll +++ b/ql/src/semmle/go/AST.qll @@ -20,7 +20,7 @@ class AstNode extends @node, Locatable { result = this.(StmtParent).getChildStmt(i) or result = this.(DeclParent).getDecl(i) or result = this.(GenDecl).getSpec(i) or - fields(result, this, i) + result = this.(FieldParent).getField(i) } /** @@ -140,6 +140,29 @@ class DeclParent extends @declparent, AstNode { int getNumDecl() { result = count(getADecl()) } } +/** + * An AST node whose children include fields. + */ +class FieldParent extends @fieldparent, AstNode { + /** + * Gets the `i`th field of this node. + * + * Note that the precise indices of fields are considered an implementation detail + * and are subject to change without notice. + */ + FieldBase getField(int i) { fields(result, this, i) } + + /** + * Gets a child field of this node in the AST. + */ + FieldBase getAField() { result = getField(_) } + + /** + * Gets the number of child fields of this node. + */ + int getNumFields() { result = count(getAField()) } +} + /** * An AST node which may induce a scope. * diff --git a/ql/src/semmle/go/Decls.qll b/ql/src/semmle/go/Decls.qll index eec7457df44..487fd32097f 100644 --- a/ql/src/semmle/go/Decls.qll +++ b/ql/src/semmle/go/Decls.qll @@ -7,7 +7,7 @@ import go /** * A declaration. */ -class Decl extends @decl, ExprParent, StmtParent { +class Decl extends @decl, ExprParent, StmtParent, FieldParent { /** * Gets the kind of this declaration, which is an integer value representing the declaration's * node type. @@ -363,11 +363,10 @@ class TypeSpec extends @typespec, Spec { } /** - * A field declaration in a struct type. + * A field declaration, of a struct, a function (in which case this is a parameter or result variable), + * or an interface (in which case this is a method or embedding spec). */ -class FieldDecl extends @field, Documentable, ExprParent { - FieldDecl() { fields(this, any(StructTypeExpr st), _) } - +class FieldBase extends @field, ExprParent { /** * Gets the expression representing the type of the fields declared in this declaration. */ @@ -377,6 +376,15 @@ class FieldDecl extends @field, Documentable, ExprParent { * Gets the type of the fields declared in this declaration. */ Type getType() { result = getTypeExpr().getType() } +} + +/** + * A field declaration in a struct type. + */ +class FieldDecl extends FieldBase, Documentable, ExprParent { + StructTypeExpr st; + + FieldDecl() { this = st.getField(_) } /** * Gets the expression representing the name of the `i`th field declared in this declaration @@ -391,7 +399,7 @@ class FieldDecl extends @field, Documentable, ExprParent { Expr getTag() { result = getChildExpr(-1) } /** Gets the struct type expression to which this field declaration belongs. */ - StructTypeExpr getDeclaringStructTypeExpr() { fields(this, result, _) } + StructTypeExpr getDeclaringStructTypeExpr() { result = st } /** Gets the struct type to which this field declaration belongs. */ StructType getDeclaringType() { result = getDeclaringStructTypeExpr().getType() } @@ -407,44 +415,26 @@ class EmbeddedFieldDecl extends FieldDecl { } /** - * A parameter declaration. + * A function parameter or result variable declaration. */ -class ParameterDecl extends @field, Documentable, ExprParent { - ParameterDecl() { - exists(int i | - fields(this, any(FuncTypeExpr ft), i) and - i >= 0 - ) - } +class ParameterOrResultDecl extends FieldBase, Documentable, ExprParent { + int rawIndex; + FuncTypeExpr ft; + + ParameterOrResultDecl() { this = ft.getField(rawIndex) } /** - * Gets the function type expression to which this parameter declaration belongs. + * Gets the function type expression to which this declaration belongs. */ - FuncTypeExpr getFunctionTypeExpr() { fields(this, result, _) } + FuncTypeExpr getFunctionTypeExpr() { result = ft } /** - * Gets the function to which this parameter declaration belongs. + * Gets the function to which this declaration belongs. */ FuncDef getFunction() { result.getTypeExpr() = getFunctionTypeExpr() } /** - * Gets the index of this parameter declarations among all parameter declarations of - * its associated function type. - */ - int getIndex() { fields(this, _, result) } - - /** - * Gets the expression representing the type of the parameters declared in this declaration. - */ - Expr getTypeExpr() { result = getChildExpr(0) } - - /** - * Gets the type of the parameters declared in this declaration. - */ - Type getType() { result = getTypeExpr().getType() } - - /** - * Gets the expression representing the name of the `i`th parameter declared in this declaration + * Gets the expression representing the name of the `i`th variable declared in this declaration * (0-based). */ Expr getNameExpr(int i) { @@ -452,29 +442,39 @@ class ParameterDecl extends @field, Documentable, ExprParent { result = getChildExpr(i + 1) } + /** + * Gets an expression representing the name of a variable declared in this declaration. + */ + Expr getANameExpr() { result = getNameExpr(_) } +} + +/** + * A parameter declaration. + */ +class ParameterDecl extends ParameterOrResultDecl { + ParameterDecl() { rawIndex >= 0 } + + /** + * Gets the index of this parameter declarations among all parameter declarations of + * its associated function type. + */ + int getIndex() { result = rawIndex } + override string toString() { result = "parameter declaration" } } /** * A receiver declaration in a function declaration. */ -class ReceiverDecl extends @field, Documentable, ExprParent { - ReceiverDecl() { fields(this, any(FuncDecl fd), -1) } +class ReceiverDecl extends FieldBase, Documentable, ExprParent { + FuncDecl fd; + + ReceiverDecl() { fd.getField(-1) = this } /** * Gets the function declaration to which this receiver belongs. */ - FuncDecl getFunction() { fields(this, result, _) } - - /** - * Gets the expression representing the type of the receiver declared in this declaration. - */ - Expr getTypeExpr() { result = getChildExpr(0) } - - /** - * Gets the type of the receiver declared in this declaration. - */ - Type getType() { result = getTypeExpr().getType() } + FuncDecl getFunction() { result = fd } /** * Gets the expression representing the name of the receiver declared in this declaration. @@ -487,48 +487,14 @@ class ReceiverDecl extends @field, Documentable, ExprParent { /** * A result variable declaration. */ -class ResultVariableDecl extends @field, Documentable, ExprParent { - ResultVariableDecl() { - exists(int i | - fields(this, any(FuncTypeExpr ft), i) and - i < 0 - ) - } - - /** - * Gets the expression representing the type of the result variables declared in this declaration. - */ - Expr getTypeExpr() { result = getChildExpr(0) } - - /** - * Gets the type of the result variables declared in this declaration. - */ - Type getType() { result = getTypeExpr().getType() } - - /** - * Gets the expression representing the name of the `i`th result variable declared in this declaration - * (0-based). - */ - Expr getNameExpr(int i) { - i >= 0 and - result = getChildExpr(i + 1) - } - - /** - * Gets an expression representing the name of a result variable declared in this declaration. - */ - Expr getANameExpr() { result = getNameExpr(_) } - - /** - * Gets the function type expression to which this result variable declaration belongs. - */ - FuncTypeExpr getFunctionTypeExpr() { fields(this, result, _) } +class ResultVariableDecl extends ParameterOrResultDecl { + ResultVariableDecl() { rawIndex < 0 } /** * Gets the index of this result variable declaration among all result variable declarations of * its associated function type. */ - int getIndex() { fields(this, _, -(result + 1)) } + int getIndex() { result = -(rawIndex + 1) } override string toString() { result = "result variable declaration" } } @@ -536,11 +502,11 @@ class ResultVariableDecl extends @field, Documentable, ExprParent { /** * A method or embedding specification in an interface type expression. */ -class InterfaceMemberSpec extends @field, Documentable, ExprParent { +class InterfaceMemberSpec extends FieldBase, Documentable, ExprParent { InterfaceTypeExpr ite; int idx; - InterfaceMemberSpec() { fields(this, ite, idx) } + InterfaceMemberSpec() { this = ite.getField(idx) } /** * Gets the interface type expression to which this member specification belongs. @@ -552,17 +518,6 @@ class InterfaceMemberSpec extends @field, Documentable, ExprParent { * its associated interface type expression. */ int getIndex() { result = idx } - - /** - * Gets the expression representing the type of the method or embedding declared in - * this specification. - */ - Expr getTypeExpr() { result = getChildExpr(0) } - - /** - * Gets the type of the method or embedding declared in this specification. - */ - Type getType() { result = getTypeExpr().getType() } } /** diff --git a/ql/src/semmle/go/Expr.qll b/ql/src/semmle/go/Expr.qll index 633c46d64a9..a9e86868bcf 100644 --- a/ql/src/semmle/go/Expr.qll +++ b/ql/src/semmle/go/Expr.qll @@ -792,16 +792,7 @@ class ArrayTypeExpr extends @arraytypeexpr, TypeExpr { * struct {x, y int; z float32} * ``` */ -class StructTypeExpr extends @structtypeexpr, TypeExpr { - /** Gets the `i`th field declared in this struct type expression (0-based). */ - FieldDecl getField(int i) { fields(result, this, i) } - - /** Gets a field declared in this struct type expression. */ - FieldDecl getAField() { result = getField(_) } - - /** Gets the number of fields declared in this struct type expression. */ - int getNumField() { result = count(getAField()) } - +class StructTypeExpr extends @structtypeexpr, TypeExpr, FieldParent { override string toString() { result = "struct type" } } @@ -814,12 +805,9 @@ class StructTypeExpr extends @structtypeexpr, TypeExpr { * func(a, b int, c float32) (float32, bool) * ``` */ -class FuncTypeExpr extends @functypeexpr, TypeExpr, ScopeNode { +class FuncTypeExpr extends @functypeexpr, TypeExpr, ScopeNode, FieldParent { /** Gets the `i`th parameter of this function type (0-based). */ - ParameterDecl getParameterDecl(int i) { - result.getFunctionTypeExpr() = this and - result.getIndex() = i - } + ParameterDecl getParameterDecl(int i) { result = getField(i) and i >= 0 } /** Gets a parameter of this function type. */ ParameterDecl getAParameterDecl() { result = getParameterDecl(_) } @@ -828,10 +816,7 @@ class FuncTypeExpr extends @functypeexpr, TypeExpr, ScopeNode { int getNumParameter() { result = count(getAParameterDecl()) } /** Gets the `i`th result of this function type (0-based). */ - ResultVariableDecl getResultDecl(int i) { - result.getFunctionTypeExpr() = this and - result.getIndex() = i - } + ResultVariableDecl getResultDecl(int i) { result = getField(-(i + 1)) } /** Gets a result of this function type. */ ResultVariableDecl getAResultDecl() { result = getResultDecl(_) } @@ -854,12 +839,9 @@ class FuncTypeExpr extends @functypeexpr, TypeExpr, ScopeNode { * interface { Read(p []byte) (n int, err error); Close() error} * ``` */ -class InterfaceTypeExpr extends @interfacetypeexpr, TypeExpr { +class InterfaceTypeExpr extends @interfacetypeexpr, TypeExpr, FieldParent { /** Gets the `i`th method specification of this interface type. */ - MethodSpec getMethod(int i) { - result.getInterfaceTypeExpr() = this and - result.getIndex() = i - } + MethodSpec getMethod(int i) { result = getField(i) } /** Gets a method of this interface type. */ MethodSpec getAMethod() { result = getMethod(_) }