codeconsole opened a new pull request, #3214:
URL: https://github.com/apache/groovy/pull/3214
Placing `@TypeChecked` or `@CompileStatic` on a local variable declaration
failed with "[Static type checking] - Unimplemented node type" instead of the
standard annotation target diagnostic. The same happened on a field, a property
or a parameter, and on a loop statement the transformation threw a
`ClassCastException` (`ForStatement cannot be cast to AnnotatedNode`), reported
as a general compiler error.
Both annotations declare `@Target({TYPE, METHOD, CONSTRUCTOR})` and
`ExtendedVerifier` checks that target for every placement, but it runs at class
generation. `StaticTypesTransformation.visit` and
`StaticCompileTransformation.visit` run earlier, at instruction selection; they
cast `nodes[1]` to `AnnotatedNode` and added their own "Unimplemented node
type" error for anything other than a class, method or property node, so
compilation stopped before the verifier could report the actual mistake.
The change makes the two transformations handle a `ClassNode` or
`MethodNode` as before and otherwise return without doing anything, so a
misplaced annotation reaches `ExtendedVerifier` and is reported as "Annotation
@groovy.transform.CompileStatic is not allowed on element LOCAL_VARIABLE" (or
FIELD, PARAMETER, STATEMENT), exactly like `@Override` in the same position and
at the same source position as before. `nodes[1]` is no longer cast to
`AnnotatedNode`, which also removes the crash for loop statements. The existing
special case that ignored the annotation on a `PropertyNode` (GROOVY-9993) is
subsumed by the general case. The unused `SyntaxException`, `AnnotatedNode` and
`PropertyNode` imports are removed; `STATIC_ERROR_PREFIX` stays, it is used
elsewhere.
Tests: two regression tests in `BugsSTCTest` (local variable at script level
and in a method body; field, parameter and loop statement), which
`BugsStaticCompileTest` inherits so both transformations are exercised. They
fail on master with "Unimplemented node type" and pass with the change. Also
run: the `groovy.transform.stc`, `org.codehaus.groovy.classgen.asm.sc` and
`gls.annotations` test packages and `checkstyleMain`. Verified with the command
line tools on the installed distribution: `groovy` and `groovyc --check
--error-format=short` now print the target diagnostic for all the placements
listed above.
Not covered here: inside a `@CompileStatic` class, annotations on a
multiple-assignment declaration (`@X def (a, b) = [1, 2]`) are dropped by
`BinaryExpressionTransformer.transformMultipleAssignment`, which rebuilds the
declaration without them, so no target error is reported for that form even for
`@Override`; that is an existing, separate defect.
Suggested backport: GROOVY_6_0_X.
--
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]
