Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions COMPATIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,32 @@ which compiles a type checking DSL script — therefore pass `false`.
configuration for a nested compilation**, and null-check the `SourceUnit`
in any customizer that might be applied to one.

### Groovy 6 — method-level type-checking annotations override class-level SKIP (GROOVY-12292)

A method (or constructor) whose own `@TypeChecked` or `@CompileStatic`
annotation has the default non-`SKIP` mode is now type checked — and for
`@CompileStatic`, statically compiled — even when its declaring class is
annotated with `@CompileDynamic`, `@CompileStatic(TypeCheckingMode.SKIP)`
or `@TypeChecked(TypeCheckingMode.SKIP)`. Previously the class-level
`SKIP` silently won and the method-level annotation was ignored. Nested
classes already behaved this way (GROOVY-10238); this aligns methods
with them: the most specific annotation wins, and a class-level `SKIP`
is the default only for members without their own annotation.

**Who is affected.** Code with a method-level opt-in under a class-level
opt-out. Such methods may now raise type-checking errors that were
previously not reported, and their bodies are statically compiled where
they were previously dynamic. Remove the method-level annotation (or
change it to `SKIP` mode) to retain the old behaviour.

**What is unchanged.** The opt-out direction is untouched:
`@CompileDynamic` / `SKIP`-mode methods inside checked classes are
skipped exactly as before. Cross-family behaviour is also unchanged: a
method-level `@CompileStatic(TypeCheckingMode.SKIP)` disables static
compilation but does not exempt the method from an enclosing class's
`@TypeChecked` checking; only `@TypeChecked(TypeCheckingMode.SKIP)`
does that.

## The binary-compatibility check

The [`subprojects/binary-compatibility/`](subprojects/binary-compatibility)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -603,9 +603,14 @@ protected boolean shouldSkipMethodNode(final MethodNode node) {

/**
* Indicates whether the annotated node is configured for {@link TypeCheckingMode#SKIP}.
* <p>
* The most specific annotation wins: a node whose own type-checking annotation has a
* non-SKIP mode is never skipped, whatever an enclosing class or method declares.
* Without an annotation of its own, a node inherits skip mode from its enclosing scope.
*/
public boolean isSkipMode(final AnnotatedNode node) {
if (node == null) return false;
boolean explicitNonSkip = false;
for (ClassNode tca : getTypeCheckingAnnotations()) {
List<AnnotationNode> annotations = node.getAnnotations(tca);
if (annotations != null) {
Expand All @@ -618,9 +623,11 @@ public boolean isSkipMode(final AnnotatedNode node) {
if (TypeCheckingMode.SKIP.toString().equals(pe.getPropertyAsString())) return true;
}
}
explicitNonSkip = true;
}
}
}
if (explicitNonSkip) return false; // GROOVY-12292
if (node instanceof MethodNode) {
return isSkipMode(node.getDeclaringClass());
}
Expand Down
7 changes: 7 additions & 0 deletions src/spec/doc/core-semantics.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1326,6 +1326,13 @@ type checker would normally complain and compilation would fail. Since the metho
`TypeCheckingMode.SKIP`, type checking is _skipped_ for this method, so the code will compile, even if the rest of the
class is type checked.

The most specific annotation wins: a method or nested class whose own `@TypeChecked` or `@CompileStatic` annotation
has the default (non-`SKIP`) mode is checked even when an enclosing class is marked with `SKIP` mode (or with
`@CompileDynamic`). Conversely, a class-level `SKIP` acts as the default for members that carry no type-checking
annotation of their own. Note that the two annotations opt out of different things: `@CompileStatic(TypeCheckingMode.SKIP)`
(or `@CompileDynamic`) on a method disables static _compilation_ but does not exempt that method from an enclosing
class's `@TypeChecked` checking; only `@TypeChecked(TypeCheckingMode.SKIP)` does that.

The following sections describe the semantics of type checking in Groovy.

==== Type checking assignments
Expand Down
47 changes: 47 additions & 0 deletions src/test/groovy/groovy/transform/stc/TypeCheckingModeTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
*/
package groovy.transform.stc

import org.codehaus.groovy.control.MultipleCompilationErrorsException
import org.junit.jupiter.api.Test

import static groovy.test.GroovyAssert.shouldFail

/**
* Unit tests for static type checking : type checking mode.
*/
Expand Down Expand Up @@ -142,4 +145,48 @@ class TypeCheckingModeTest extends StaticTypeCheckingTestCase {
}
'''
}

// GROOVY-12292
@Test
void testTypeCheckedMethodInTypeCheckedSkipClass() {
def err = shouldFail(MultipleCompilationErrorsException) {
new GroovyShell().evaluate '''
import groovy.transform.TypeChecked
import static groovy.transform.TypeCheckingMode.SKIP

@TypeChecked(SKIP)
class C {
@TypeChecked
def m() {
"".toStrings()
}
}
new C()
'''
}
assert err.message.contains('Cannot find matching method java.lang.String#toStrings()')
}

// GROOVY-12292: @CompileStatic(SKIP) opts out of static compilation only;
// it does not exempt a method from an enclosing class's type checking
@Test
void testCompileStaticSkipMethodInTypeCheckedClassIsStillTypeChecked() {
def err = shouldFail(MultipleCompilationErrorsException) {
new GroovyShell().evaluate '''
import groovy.transform.CompileStatic
import groovy.transform.TypeChecked
import static groovy.transform.TypeCheckingMode.SKIP

@TypeChecked
class C {
@CompileStatic(SKIP)
def m() {
"".toStrings()
}
}
new C()
'''
}
assert err.message.contains('Cannot find matching method java.lang.String#toStrings()')
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,63 @@ final class CompileDynamicTest extends StaticTypeCheckingTestCase implements Sta
new C()
'''
}

// GROOVY-12292
@Test
void testCompileStaticMethodInCompileDynamicClass() {
shouldFailWithMessages '''
@CompileDynamic
class C {
@CompileStatic
def m() {
"".toStrings()
}
}
''', 'Cannot find matching method java.lang.String#toStrings()'
}

// GROOVY-12292
@Test
void testCompileStaticMethodInCompileStaticSkipClass() {
shouldFailWithMessages '''
@CompileStatic(groovy.transform.TypeCheckingMode.SKIP)
class C {
@CompileStatic
def m() {
"".toStrings()
}
}
''', 'Cannot find matching method java.lang.String#toStrings()'
}

// GROOVY-12292
@Test
void testCompileStaticMethodInTypeCheckedSkipClass() {
shouldFailWithMessages '''
@groovy.transform.TypeChecked(groovy.transform.TypeCheckingMode.SKIP)
class C {
@CompileStatic
def m() {
"".toStrings()
}
}
''', 'Cannot find matching method java.lang.String#toStrings()'
}

// GROOVY-12292
@Test
void testCompileStaticMethodInCompileDynamicClassIsStaticallyCompiled() {
assertScript '''
@CompileDynamic
class C {
@CompileStatic
String m() {
'works'.toUpperCase()
}
}
assert new C().m() == 'WORKS'
'''
String bytecode = astTrees['C'][1]
assert bytecode.contains('INVOKEVIRTUAL java/lang/String.toUpperCase ()Ljava/lang/String;')
}
}
Loading