Renaming nodes to be shorter and more consistent

[konradweiss]

This PR introduces some renamings with the aim to make the node names more intuitive and more fluently usable when working with the query language and any console scripting interface. The main idea i followed was to name more consistently and omit unnecessary details

• ParamVariableDeclaration to ParameterDeclaration
• DeclaredReferenceExpression to Reference to make it shorter and omitting the Expression consistent with Literal.
• ArraySubscriptionExpression to SubscriptionExpr shorten further and because we use it for other subscriptable constructs that are no arrays.
• ClassTemplateDeclaration to RecordTemplateDeclaration to be consistent with RecordDeclaration
• ArrayCreationExpression to NewArrayExpression: to better reflect that is only used in the new context and can be
• Rename CompoundStatement to Block: I think would be more intuitive.
• Unify CompoundStatement and CompoundStatementExpression
• UsingDirective to UsingDeclaration due to it being a declaration
• ExplicitConstructorInvocation to ConstructorCallExpression to remove unnecessary explicit and invocation and signal that it is a call expression
• TypeParamDeclaration to TypeParameterDeclaration for consistency

[oxisto]

Personally, I wouldn't shorten Statement to Stmt, etc. I kinda like that our classes are complete words and not just abbreviations, which sometimes could be misleading (Decl -> Declaration? Declarator? Declared?)

I do agree with

• Reference
• ParameterDeclaration
• SubscriptionExpression
• CompoundStatement -> BlockStatement

One question we could explore is whether to drop the added Statement, etc. et all. Do we have a benefit of CallExpression vs. Call? Block instead of CompoundStatement?

[KuechA]

Tmp for Template is too confusing to me, the rest sounds reasonable. I wouldn't be declined to abbreviate Statement, Declaration, Expression since they occur quite often but I'd also be fine with dropping this information completely. I think the reason to keep it was to know quickly whether it inherits from Statement, Declaration or Expression but at the same time, assigning a wrong type wouldn't be possible anyway

[konradweiss]

@oxisto @KuechA I don't see us dropping the suffix of Statement, Expression, Declaration completely because there are some node names that would be misleading or collide, e.g. CompoundStatement and CompoundStatementExpression and as @KuechA its also good for developers to know what type of node it is. I would incorporate the feedback and still keep the shortening of these three names as my objection to the Decl-Declaration-Declarator Issue is that Decl always stands for Declaration and users who differentiate between Declarator, Declaration and something Declared are so involved that they know our convention. That ofc means we do not abbreviate Declarator and Declared. Unless you have a objection @oxisto ?

[KuechA]

e.g. CompoundStatement and CompoundStatementExpression

Is this something that we can get rid of? Why can't Block inherit from Expression (which extends Statement)

[konradweiss]

e.g. CompoundStatement and CompoundStatementExpression

Is this something that we can get rid of? Why can't Block inherit from Expression (which extends Statement)

This would make EVERY block of statements an expression, but although I had an initial repulsion to the thought I see it as less problematic the more I think of it. @oxisto What do you think?

[oxisto]

e.g. CompoundStatement and CompoundStatementExpression

Is this something that we can get rid of? Why can't Block inherit from Expression (which extends Statement)

This would make EVERY block of statements an expression, but although I had an initial repulsion to the thought I see it as less problematic the more I think of it. @oxisto What do you think?

We could do it similar to the AssignExpression, which is also used as a statement in 90% of the time, but it derives from expression (because of C++ of course).

We then have a property that denotes the usage as expression:

cpg/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt

Lines 79 to 84 in 463ac60

	/**
	* This property specifies, that this is actually used as an expression. Not many languages
	* support that. In the regular case, an assignment is a simple statement and does not hold any
	* value itself.
	*/
	var usedAsExpression = false

We afaik then check in the DFG (EOG?) passes if this property is set and set an additional DFG edge to itself.

[konradweiss]

e.g. CompoundStatement and CompoundStatementExpression

Is this something that we can get rid of? Why can't Block inherit from Expression (which extends Statement)

This would make EVERY block of statements an expression, but although I had an initial repulsion to the thought I see it as less problematic the more I think of it. @oxisto What do you think?

We could do it similar to the AssignExpression, which is also used as a statement in 90% of the time, but it derives from expression (because of C++ of course).

We then have a property that denotes the usage as expression:

cpg/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt

Lines 79 to 84 in 463ac60

	/**
	* This property specifies, that this is actually used as an expression. Not many languages
	* support that. In the regular case, an assignment is a simple statement and does not hold any
	* value itself.
	*/
	var usedAsExpression = false

We afaik then check in the DFG (EOG?) passes if this property is set and set an additional DFG edge to itself.

Ok, I see we ar edoing this, so I would say we just rename CompoundStatementExpression to Block and remove CompundStatement. Here I am fine with removing the suffix cause we use them as Expression and Statement so it does not matter that much.

Can it work without using a variable like usedAsExpression?

[oxisto]

e.g. CompoundStatement and CompoundStatementExpression

Is this something that we can get rid of? Why can't Block inherit from Expression (which extends Statement)

This would make EVERY block of statements an expression, but although I had an initial repulsion to the thought I see it as less problematic the more I think of it. @oxisto What do you think?

We could do it similar to the AssignExpression, which is also used as a statement in 90% of the time, but it derives from expression (because of C++ of course).
We then have a property that denotes the usage as expression:

cpg/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt

Lines 79 to 84 in 463ac60

	/**
	* This property specifies, that this is actually used as an expression. Not many languages
	* support that. In the regular case, an assignment is a simple statement and does not hold any
	* value itself.
	*/
	var usedAsExpression = false

We afaik then check in the DFG (EOG?) passes if this property is set and set an additional DFG edge to itself.

Ok, I see we ar edoing this, so I would say we just rename CompoundStatementExpression to Block and remove CompundStatement. Here I am fine with removing the suffix cause we use them as Expression and Statement so it does not matter that much.

Can it work without using a variable like usedAsExpression?

I think we need some kind of property to make the differentiation in the EOG and/or DFG because probably the last statement flows to the expression itself or something. Not sure how the EOG/DFG is implemented for CompoundStatementExpression.

[konradweiss]

e.g. CompoundStatement and CompoundStatementExpression

Is this something that we can get rid of? Why can't Block inherit from Expression (which extends Statement)

This would make EVERY block of statements an expression, but although I had an initial repulsion to the thought I see it as less problematic the more I think of it. @oxisto What do you think?

We could do it similar to the AssignExpression, which is also used as a statement in 90% of the time, but it derives from expression (because of C++ of course).
We then have a property that denotes the usage as expression:

cpg/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/statements/expressions/AssignExpression.kt

Lines 79 to 84 in 463ac60

	/**
	* This property specifies, that this is actually used as an expression. Not many languages
	* support that. In the regular case, an assignment is a simple statement and does not hold any
	* value itself.
	*/
	var usedAsExpression = false

We afaik then check in the DFG (EOG?) passes if this property is set and set an additional DFG edge to itself.

Ok, I see we ar edoing this, so I would say we just rename CompoundStatementExpression to Block and remove CompundStatement. Here I am fine with removing the suffix cause we use them as Expression and Statement so it does not matter that much.
Can it work without using a variable like usedAsExpression?

I think we need some kind of property to make the differentiation in the EOG and/or DFG because probably the last statement flows to the expression itself or something. Not sure how the EOG/DFG is implemented for CompoundStatementExpression.

I will see if it is necessary

[oxisto]

• DesignatedInitializerExpression to InitializerExpression : @oxisto Does this still reflect what it is, is there a better name?

I would keep it as DesignatedInitializerExpression for now and merge it as a combination of the initializerlist and the key value expression at some point.

[oxisto]

Some smaller things. Looks good overall, I support the renamings

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
7 Code Smells

71.5% Coverage
0.1% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint