Add AST class reference #164

owen-mc · 2020-06-04T16:35:12Z

Here is a draft AST class reference for codeql-go. The ordering is probably a bit weird, because I based it on the java one and added go-specific classes at the end of each table.

max-schaefer

Great stuff, thank you very much for tackling this! I have a bunch of fairly minor comments, but on the whole this looks great.

max-schaefer · 2020-06-08T15:33:34Z

docs/language/learn-ql/go/ast-class-reference.rst

+---------------------------+--------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------+
+| ``^x``                    | `ComplementExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$ComplementExpr.html>`__ | `BitwiseUnaryExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$BitwiseUnaryExpr.html>`__       |
+---------------------------+--------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------+
+| ``*x``                    | `DerefExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$DerefExpr.html>`__           |                                                                                                                  |


This is actually a StarExpr. Class DerefExpr exists and corresponds to something the Go compiler frontend claims to emit, but it actually never does.

It may be worth adding a cautionary note to that effect to the qldoc of DerefExpr. Could you do that in this PR?

max-schaefer · 2020-06-08T15:34:31Z

docs/language/learn-ql/go/ast-class-reference.rst

+| .. _VariableName: https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$VariableName.html             | .. _DefineStmt: https://help.semmle.com/qldoc/go/semmle/go/Stmt.qll/type.Stmt$DefineStmt.html                     | .. _SimpleAssignStmt: https://help.semmle.com/qldoc/go/semmle/go/Stmt.qll/type.Stmt$SimpleAssignStmt.html     |                                                                                                                   |
+| .. _Expr: https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$Expr.html                             |                                                                                                                   | .. _Assignment: https://help.semmle.com/qldoc/go/semmle/go/Stmt.qll/type.Stmt$Assignment.html                 |                                                                                                                   |
+---------------------------------------------------------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+---------------------------------------------------------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+
+| Expr_ ``...`` ``+=`` Expr_ ``...``                                                                            | AddAssignStmt_                                                                                                    | CompoundAssignStmt_, Assignment_                                                                              |                                                                                                                   |


I believe lhs and rhs for compound assignments must always be singletons (cf https://golang.org/ref/spec#Assignments).

max-schaefer · 2020-06-08T15:38:21Z

docs/language/learn-ql/go/ast-class-reference.rst

+==================================
+AST class reference — Learn CodeQL
+==================================


Suggested change

==================================

AST class reference — Learn CodeQL

==================================

Abstract syntax tree classes for Go

===================================

CodeQL has a large selection of classes for working with the abstract syntax tree representation of Go programs.

The abstract syntax tree (AST) represents the syntactic structure of a program. Nodes in the AST represent elements such as statements and expressions.

max-schaefer · 2020-06-08T15:39:28Z

docs/language/learn-ql/go/ast-class-reference.rst

+-------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------+
+| ``struct {x, y int; z float32}``                                  | `StructTypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$StructTypeExpr.html>`__             |                                                                                                    |
+-------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------+
+| ``fun c(a, b int, c float32) (float32, bool)``                    | `FuncTypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$FuncTypeExpr.html>`__                 |                                                                                                    |


Suggested change

| ``fun c(a, b int, c float32) (float32, bool)`` | `FuncTypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$FuncTypeExpr.html>`__ | |

| ``func c(a, b int, c float32) (float32, bool)`` | `FuncTypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$FuncTypeExpr.html>`__ | |

max-schaefer · 2020-06-08T15:40:31Z

docs/language/learn-ql/go/ast-class-reference.rst

+| ``chan float64``                                                  | `SendRecvChanTypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$SendRecvChanTypeExpr.html>`__ | `ChanTypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$ChanTypeExpr.html>`__ |
+-------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------+
+
+Expressions representing syntactic context


It's not very clear what this means. Could you try to rephrase?

I can't think of a good way of expressing it. "Expression classes which require syntactic context"?

Then maybe remove this section and instead add remarks for Ident and SelectorExpr pointing people at these more specialised classes. I think those are the most important ones.

It would be a shame to lose all that info because it seems useful. I've reworked it as a "Name expressions" section and another table in the Miscellaneous section. Let me know what you think.

max-schaefer · 2020-06-08T15:42:04Z

docs/language/learn-ql/go/ast-class-reference.rst

+-  `ValueExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$ValueExpr.html>`__:
+   an expression that can be evaluated to a value (as opposed to
+   expressions that refer to a package, a type, or a statement
+   label). This generalises


Suggested change

label). This generalises

label). This generalizes

(Our public-facing documentation generally uses US spelling.)

max-schaefer · 2020-06-08T15:43:46Z

docs/language/learn-ql/go/ast-class-reference.rst

+------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------+
+| ``x.( `TypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$TypeExpr.html>`__ )`` | `TypeAssertExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$TypeAssertExpr.html>`__ |                                                                                                                    |
+------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------+
+| ``*x``                                                                                         | `StarExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$StarExpr.html>`__             |                                                                                                                    |


Perhaps worth remarking here that a StarExpr can either be a TypeExpr or a ValueExpr, depending on context.

max-schaefer

Almost good to go, just a few minor comments left.

max-schaefer · 2020-06-09T16:08:09Z

ql/src/semmle/go/Expr.qll

- * ```go
- * *p
- * ```
+ * A unary pointer-dereference expression. Never actually emitted by the go compiler - StarExpr is used instead.


Suggested change

* A unary pointer-dereference expression. Never actually emitted by the go compiler - StarExpr is used instead.

* A unary pointer-dereference expression.

*

* This class exists for compatibility reasons only and should not normally be used directly. Use `StarExpr` instead.

max-schaefer · 2020-06-09T16:12:51Z

docs/language/learn-ql/go/ast-class-reference.rst

-      or a
-      `FunctionName <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$FunctionName.html>`__,
-      depending on what sort of entity the name refers to.
+The following classes relate to what sort of entity the name refers to.


I think I would prefer organizing this as a nested list rather than a table.

max-schaefer · 2020-06-09T16:14:03Z

docs/language/learn-ql/go/ast-class-reference.rst

+| ``f(x)``                                                                                                 | `CallExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$CallExpr.html>`__             | `CallOrConversionExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$CallOrConversionExpr.html>`__ |                                                                                                                                                                                                                          |
+----------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+
+The following classes do not have a standard syntax.


This makes it sound like something is wrong or broken. Perhaps rephrase as "The following classes organize expressions by the kind of entity they refer to" or something like that, and use a list instead of a table.

max-schaefer · 2020-06-09T16:14:29Z

docs/language/learn-ql/go/ast-class-reference.rst

+------------------------------------------------------------------------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+| `ValueExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$BadExpr.html>`__           | an expression that can be evaluated to a value (as opposed to expressions that refer to a package, a type, or a statement label). This generalizes `ReferenceExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$ReferenceExpr.html>`__ |
+------------------------------------------------------------------------------------------------------+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+| `BadExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$BadExpr.html>`__             | an expression that cannot be parsed                                                                                                                                                                                                                     |


Perhaps put into one of the other tables, similar to BadStmt?

max-schaefer · 2020-06-10T08:52:56Z

Looking good; thanks for bearing with me!

I just noticed one thing that may be worth addressing: for statements, you provide schematic syntax (as in "for Expr BlockStmt"), while for expressions you provide concrete syntax examples (as in "x + y").

Now I can see that it is not helpful to give schematic syntax for literals, but perhaps we could do it for the other expressions? So instead of "x + y" use "Expr + Expr". That is, for example, how the JavaScript AST class reference does it.

owen-mc · 2020-06-10T10:46:49Z

No problem @max-schaefer , I'm very happy to improve it as much as possible now so I don't need to come back to it later. I learned how to avoid unwanted spaces in RST, so I've done that as well as carrying out your suggestion.

max-schaefer

Great stuff, thanks!

One last organisational thing: could you rebase this onto the rc/1.24 branch and change the PR to target that branch? As discussed with @felicitymay, we'll want to put a copy of this reference onto the rc/1.24 branch of https://github.com/github/codeql to expedite publication, and it would probably make sense to do the same in this repo for consistency.

I didn't know how to do that in rst before

max-schaefer · 2020-06-10T15:01:17Z

🎉

Next, could you make a PR against the rc/124 branch of https://github.com/github/codeql to put a copy of the reference into https://github.com/github/codeql/tree/master/docs/language/learn-ql/go?

owen-mc · 2020-06-10T15:43:13Z

I have now done that: github/codeql#3675

owen-mc requested a review from a team June 4, 2020 16:35

owen-mc force-pushed the ast-class-reference branch from 8023b53 to 3e40b99 Compare June 8, 2020 11:08

owen-mc marked this pull request as ready for review June 8, 2020 11:09

max-schaefer suggested changes Jun 8, 2020

View reviewed changes

max-schaefer suggested changes Jun 9, 2020

View reviewed changes

max-schaefer reviewed Jun 10, 2020

View reviewed changes

owen-mc force-pushed the ast-class-reference branch from 20844db to 7fc4d1d Compare June 10, 2020 11:21

owen-mc changed the base branch from master to rc/1.24 June 10, 2020 11:24

owen-mc added 2 commits June 10, 2020 14:16

Add AST class reference

4ae626c

Address review comments

65f2d38

owen-mc force-pushed the ast-class-reference branch from 7fc4d1d to ec53bd9 Compare June 10, 2020 13:20

owen-mc added 3 commits June 10, 2020 14:27

Address more review comments

ed68c09

Remove unwanted spaces

573f654

I didn't know how to do that in rst before

Give generic syntax for expressions

69eb043

owen-mc force-pushed the ast-class-reference branch from ec53bd9 to 69eb043 Compare June 10, 2020 13:28

max-schaefer approved these changes Jun 10, 2020

View reviewed changes

max-schaefer merged commit c30893a into github:rc/1.24 Jun 10, 2020

owen-mc deleted the ast-class-reference branch June 10, 2020 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AST class reference #164

Add AST class reference #164

owen-mc commented Jun 4, 2020 •

edited

Loading

max-schaefer left a comment

max-schaefer Jun 8, 2020

max-schaefer Jun 8, 2020

max-schaefer Jun 8, 2020

max-schaefer Jun 8, 2020

max-schaefer Jun 8, 2020

max-schaefer Jun 8, 2020

owen-mc Jun 8, 2020

max-schaefer Jun 8, 2020

owen-mc Jun 9, 2020

max-schaefer Jun 8, 2020

max-schaefer Jun 8, 2020

max-schaefer left a comment

max-schaefer Jun 9, 2020

max-schaefer Jun 9, 2020

max-schaefer Jun 9, 2020

max-schaefer Jun 9, 2020

max-schaefer commented Jun 10, 2020

owen-mc commented Jun 10, 2020

max-schaefer left a comment

max-schaefer commented Jun 10, 2020

owen-mc commented Jun 10, 2020 •

edited

Loading

-==================================
-AST class reference — Learn CodeQL
-==================================
+Abstract syntax tree classes for Go
+===================================
+CodeQL has a large selection of classes for working with the abstract syntax tree representation of Go programs.
+The abstract syntax tree (AST) represents the syntactic structure of a program. Nodes in the AST represent elements such as statements and expressions.

	\| ``fun c(a, b int, c float32) (float32, bool)`` \| `FuncTypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$FuncTypeExpr.html>`__ \| \|
	\| ``func c(a, b int, c float32) (float32, bool)`` \| `FuncTypeExpr <https://help.semmle.com/qldoc/go/semmle/go/Expr.qll/type.Expr$FuncTypeExpr.html>`__ \| \|

Add AST class reference #164

Add AST class reference #164

Conversation

owen-mc commented Jun 4, 2020 • edited Loading

max-schaefer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

max-schaefer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

max-schaefer commented Jun 10, 2020

owen-mc commented Jun 10, 2020

max-schaefer left a comment

Choose a reason for hiding this comment

max-schaefer commented Jun 10, 2020

owen-mc commented Jun 10, 2020 • edited Loading

owen-mc commented Jun 4, 2020 •

edited

Loading

owen-mc commented Jun 10, 2020 •

edited

Loading