Can ConstantIfExpressionRule detect more complex cases? If so how?

[markjschreiber]

I want to create/ extend a rule that is similar to the ConstantIfExpressionRule but covers cases where the constant may be inferred.

For example

def final b = false;

// later
if (b) {
  // do something
}

Arguably, this might not be static analysis but it's not far off as it only involves a lookup of a defined variable. Being new to CodeNarc and the Groovy AST I don't know if there is any existing support in either to determine the value of b while checking the if statement. Is this possible?

[chrismair]

Hmmm, that is an interesting question/request.

I think that would involve creating an AST Visitor that processed each variable definition (see VariableNameRule /VariableNameAstVisitor), looked for ones that were final and were also assigned "constant" values, and then do a check for IF expressions like in ConstantIfExpressionAstVisitor.

I think that would work. I think extending ConstantIfExpressionRule with that behavior could be reasonable, perhaps behind a boolean flag something like checkFinalVariablesWithConstantValues that defaulted to false (to avoid breaking existing users). Hmmm...

[markjschreiber]

You would need to build up some kind of table of declared variables in a scope (lets call it a context) and then when you get to if(b) { ... } you would look up b and if it has a resolvable constant value of false you get a violation. This context would allow another check for undeclared variables if b is not defined at all. Having some kind of context available to all rules would be pretty cool.

The full context would actually require the full AST to be built and some processing of it to happen. I don't know Groovy's parser well but presumably this actually happens at some point when compiling/ running a groovy script. Perhaps there is something already there that can be reused?

[markjschreiber]

One possible way of doing this might be to collect the relevant statements/ code blocks and then use the GroovyShell to parse/ evaluate. This might be challenging and potentially dangerous?

[chrismair]

I'm pretty sure you don't want to use GroovyShell to re-parse/evaluate the code .. that would be unpleasant and complicated and is unnecessary since the Groovy AST should already provide the relevant information.

Most of the CodeNarc rules use the Groovy AST as-is. The approach uses the Visitor pattern. Many of those rules are implemented by a "main" XxxRule class and then an associated XxxAstVisitor class.

We already have several rules where the AST Vistor builds up "context" information as it visits components in the AST, such as classes, methods, variables, expressions, etc.. The typical pattern instantiates the AST Visitor class for each file it is processing, so that "context" in specific to a single source file.

See CodeNarc/src/main/groovy/org/codenarc/rule/design/AssignmentToStaticFieldFromInstanceMethodRule.groovy at master · CodeNarc/CodeNarc as an example. That holds on to the static field names and local variable names.

The AST elements are visited in the order of how the source is parsed/compiled/processed, so you will visit field definitions before the fields are referenced, for instance. So I think in the scenario you describe above, you could process/visit variable definitions, save the names if they are initialized to "constant" values. Then also process/visit if expressions and check if the conditional is (only?) referencing one of those variable names .. something like that.