codeconsole opened a new pull request, #3207: URL: https://github.com/apache/groovy/pull/3207
JIRA: [GROOVY-12688](https://issues.apache.org/jira/browse/GROOVY-12688) ### The bug With `groovy.typecheckers.FormatStringChecker` enabled, a class that declares an enum or a record does not compile. The checker throws a `NullPointerException` inside the compiler and compilation ends with a general error, even though the source contains no format call: ```text error: General error during canonicalization: Cannot get property 'annotations' on null object ``` The crash also stops the checker before it reaches the rest of the class, so a real format-string mistake in a record method goes unreported. A second, related failure happens in methods without a body: an abstract method, an interface method, or the abstract method of an enum with constant bodies fails with `Cannot invoke method visit() on null object`. That one affects `RegexChecker` as well. ### Reproducer ```groovy import groovy.transform.TypeChecked @TypeChecked(extensions = 'groovy.typecheckers.FormatStringChecker') enum Size { S, M } ``` ```groovy import groovy.transform.TypeChecked @TypeChecked(extensions = 'groovy.typecheckers.FormatStringChecker') record Point(int x, int y) {} ``` `groovyc --check Size.groovy` (or `Point.groovy`) exits 1 with the error above, while `@TypeChecked enum Size { S, M }` without the extension compiles. The same failure happens when the enum or record is nested in a checked class. It also happens when static compilation with the extension is applied from a config script (`ast(CompileStatic, extensions: ['groovy.typecheckers.FormatStringChecker'])`) to a script that declares an enum. The failure in methods without a body: ```groovy @groovy.transform.TypeChecked(extensions = 'groovy.typecheckers.RegexChecker') enum Op { PLUS { int apply(int a, int b) { a + b } } abstract int apply(int a, int b) } ``` ### Cause When the type checker cannot resolve a constructor call in generated code, it stores no target method: it drops errors for nodes that have no source position. It still fires `afterMethodCall`, and `getTargetMethod` is documented to return the selected target "if any". In an enum, that call is the synthetic `$INIT` helper's `this(*para)`. In a record, it is the `this(...)` call by which the named-argument constructor delegates. `FormatStringChecker` read `annotations` from the null target. Separately, `beforeVisitMethod` and `afterVisitMethod` fire for every method, including abstract and interface methods, which have no code. `FormatStringChecker` and `RegexChecker` called `method.code.visit(...)` without a null check. ### Fix - `FormatStringChecker`: return from the `afterMethodCall` handler when the call has no target method, and visit `method.code` null-safely. - `RegexChecker`: visit `method.code` null-safely in both hooks. In both checkers this just skips what they cannot analyse. `NullChecker`, `SqlInjectionChecker` and `PurityChecker` already call `method.code?.visit(...)` and check their target lookups. I ran every other checker in the module (`NullChecker`, `SqlInjectionChecker`, `PurityChecker`, `ModifiesChecker`, `CombinerChecker`, `MonadicChecker`, `MonadicShapeChecker`) against an enum, a record, an enum with constant bodies and an interface. None of them fails, before or after this change. ### Tests New tests in `FormatStringCheckerTest` and `RegexCheckerTest`: - `testEnumAndRecordDeclarations`: an enum and a record compile in two forms: at script level, with the extension applied by the test shell's customizer, and nested in a class annotated `@TypeChecked(extensions = 'groovy.typecheckers.FormatStringChecker')`. - `testFormatErrorsReportedWithEnumAndRecordDeclarations`: in such a class, mismatches in the class itself, in the enum and in the record are all reported, with no general error. Before the fix, the record's mismatch was not reported. - `testMethodsWithoutBody` (in both test classes): an interface and an enum with constant bodies compile, and a mistake inside a constant body is still reported. All four tests failed on master before the change and pass after it. ```text ./gradlew :groovy-typecheckers:test --rerun 390 tests, 0 failures ./gradlew :groovy-typecheckers:lintGate :groovy-typecheckers:rat BUILD SUCCESSFUL ``` ### Affected versions Reproduced with JDK 21 on the Groovy 6.0.0 and 6.0.0-beta-1 distributions and on master (7.0.0-SNAPSHOT at 9a1a0d0ccb). The same failures reproduce with 5.0.0 and 5.0.8, where the checker was incubating. The affected code dates from GROOVY-11261. ### Backport Suggested target: `GROOVY_6_0_X`. The production change applies there without changes. If a 5.0.x backport is wanted, `GROOVY_5_0_X` has the same three dereferences in slightly different code, so the edits would need to be made by hand. ### Not in this change `FormatStringChecker` still fails with `BUG! ... ClassNode#getTypeClass for Foo called before the type class is set` when a non-constant format argument has a type declared in the same compilation. For example, `class Foo {}` with `String.format('%s', new Foo())`, or a constant of an enum declared in the same source passed as an argument. That failure has a separate cause (`checkFormatStringTypes` compares runtime classes), occurs on 6.0.0 and master alike, and is left for its own issue. --- -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
