Skip to content

Commit

Permalink
GROOVY-8084, GROOVY-9074, GROOVY-10588: STC: implement wildcard capture
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-milles committed Nov 13, 2024
1 parent e6c95b7 commit 71490fb
Show file tree
Hide file tree
Showing 9 changed files with 241 additions and 223 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1346,6 +1346,7 @@ protected static ClassNode fullyResolveType(final ClassNode type, final Map<Gene
if (type.isGenericsPlaceHolder()) {
GenericsType gt = placeholders.get(new GenericsTypeName(type.getUnresolvedName()));
if (gt != null) {
if (gt.isWildcard()) return new ClassNode("capture-of " + gt, 0, getCombinedBoundType(gt));
return gt.getType();
}
ClassNode cn = extractType(type.asGenericsType()); // GROOVY-10756
Expand Down Expand Up @@ -1524,9 +1525,9 @@ && isUsingGenericsOrIsArrayUsingGenerics(type.getComponentType())) {
resolvedMethodGenerics.put(entry.getKey(), candidate);
continue;
} else if (!candidate.isPlaceholder() && !candidate.isWildcard()) {
// combine "T=Integer" and "T=String" to produce "T=? extends Serializable & Comparable<...>"
// combine "T=Integer" and "T=String" to produce "T=(Serializable & ... )"
ClassNode lub = lowestUpperBound(candidate.getType(), resolved.getType());
resolvedMethodGenerics.put(entry.getKey(), lub.asGenericsType());
resolvedMethodGenerics.put(entry.getKey(), new GenericsType(lub));
continue;
}
}
Expand Down Expand Up @@ -1559,6 +1560,11 @@ private static int dimensions(ClassNode cn) {
}

private static boolean compatibleConnection(final GenericsType resolved, final GenericsType connection) {
if (resolved.isWildcard()
// TODO: figure out capture of super
&& resolved.getLowerBound() == null) {
return false; // GROOVY-8084, GROOVY-9074, GROOVY-10588
}
if (resolved.isPlaceholder()
&& resolved.getUpperBounds() != null
&& resolved.getUpperBounds().length == 1
Expand Down
17 changes: 10 additions & 7 deletions src/spec/doc/core-semantics.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1777,8 +1777,8 @@ can assign to the variable:
----
include::../test/typing/TypeCheckingTest.groovy[tags=flowtyping_typeconstraints,indent=0]
----
<1> `list` is declared as an unchecked `List` and assigned a list literal of `String`s
<2> this line passes compilation because of flow typing: the type checker knows that `list` is at this point a `List<String>`
<1> `list` is declared as an unchecked `List` and assigned a list literal of strings
<2> this line passes compilation because of flow typing: the type checker knows that `list` is at this point an `ArrayList<String>`
<3> but you can't assign a `String` to a `List` so this is a type checking error
You can also note that even if the variable is declared *without* generics information, the type checker knows what is
Expand All @@ -1788,18 +1788,21 @@ the component type. Therefore, such code would fail compilation:
----
include::../test/typing/TypeCheckingTest.groovy[tags=flowtyping_typeconstraints_failure,indent=0]
----
<1> `list` is inferred as `List<String>`
<2> so adding an `int` to a `List<String>` is a compile-time error
<1> `list` is inferred as `ArrayList<String>`
<2> so adding an `int` to a `ArrayList<String>` is a compile-time error
<3> `list` is declared as `List<?>`
<4> the inferred type of `list` here is `List<capture-of ?>`, so calling `addAll` with a list of anything is a compile-time error
<5> and calling `add` with an `int` is also a compile-time error for the same reason; only `add(null)` is allowed
Fixing this requires adding an explicit generic type to the declaration:
Fixing this requires adding an explicit, non-wildcard type argument:
[source,groovy]
----
include::../test/typing/TypeCheckingTest.groovy[tags=flowtyping_typeconstraints_fixed,indent=0]
----
<1> `list` declared as `List<? extends Serializable>` and initialized with an empty list
<1> `list` is declared as `List<Serializable>` and initialized with an empty list
<2> elements added to the list conform to the declaration type of the list
<3> so adding an `int` to a `List<? extends Serializable>` is allowed
<3> and adding an integer is allowed
Flow typing has been introduced to reduce the difference in semantics between classic and static Groovy. In particular,
consider the behavior of this code in Java:
Expand Down
30 changes: 19 additions & 11 deletions src/spec/test/typing/TypeCheckingTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -688,32 +688,40 @@ import static org.codehaus.groovy.ast.tools.WideningCategories.lowestUpperBound
}
// end::flowtyping_typeconstraints[]
flowTypingWithExplicitType()
''', 'Cannot assign value of type java.lang.String to variable of type java.util.List'
''',
'Cannot assign value of type java.lang.String to variable of type java.util.List'
}

void testFlowTypingTypeConstraintsFailure() {
void testFlowTypingTypeConstraints2() {
shouldFailWithMessages '''
// tag::flowtyping_typeconstraints_failure[]
@groovy.transform.TypeChecked
void flowTypingWithExplicitType() {
List list = ['a','b','c'] // <1>
void flowTypingTypeConstraints1() {
def list = ['a','b','c'] // <1>
list.add(1) // <2>
}
@groovy.transform.TypeChecked
void flowTypingTypeConstraints2() {
List<?> list = [] // <3>
list.addAll(['a','b','c']) // <4>
list.add(1) // <5>
}
// end::flowtyping_typeconstraints_failure[]
flowTypingWithExplicitType()
''',
'Cannot call java.util.ArrayList#add(java.lang.String) with arguments [int]'
'Cannot call java.util.ArrayList#add(java.lang.String) with arguments [int]',
'Cannot call java.util.ArrayList#addAll(java.util.Collection<? extends capture-of ?>) with arguments [java.util.ArrayList<java.lang.String>]',
'Cannot call java.util.ArrayList#add(capture-of ?) with arguments [int]'

assertScript '''
// tag::flowtyping_typeconstraints_fixed[]
@groovy.transform.TypeChecked
void flowTypingWithExplicitType() {
List<? extends Serializable> list = [] // <1>
list.addAll(['a','b','c']) // <2>
list.add(1) // <3>
void flowTypingTypeConstraints3() {
List<Serializable> list = [] // <1>
list.addAll(['a','b','c']) // <2>
list.add(1) // <3>
}
// end::flowtyping_typeconstraints_fixed[]
flowTypingWithExplicitType()
flowTypingTypeConstraints3()
'''
}

Expand Down
39 changes: 0 additions & 39 deletions src/test/groovy/bugs/Groovy8084Bug.groovy

This file was deleted.

81 changes: 7 additions & 74 deletions src/test/groovy/bugs/Groovy9074.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -18,84 +18,16 @@
*/
package groovy.bugs

import groovy.test.GroovyTestCase
import groovy.transform.CompileStatic
import org.codehaus.groovy.control.CompilationUnit
import org.junit.Test

import static groovy.test.GroovyAssert.shouldFail
import static org.codehaus.groovy.control.Phases.CLASS_GENERATION

@CompileStatic
final class Groovy9074 extends GroovyTestCase {
final class Groovy9074 {

void _FIXME_testWildcardCapture() {
def err = shouldFail '''
@groovy.transform.TypeChecked
class Main {
private static Collection<?> c = new ArrayList<String>()
static main(args) {
c.add(new Object())
}
}
'''

// TODO: This is just a sample message; Java produces this for the equivalent code.
assert err =~ / The method add\(capture#1-of \?\) in the type Collection<capture#1-of \?> is not applicable for the arguments \(Object\)/
}

void _FIXME_testWildcardExtends() {
def err = shouldFail '''
import java.awt.Canvas
abstract class Shape {
abstract void draw(Canvas c)
}
class Circle extends Shape {
private int x, y, radius
@Override void draw(Canvas c) {}
}
class Rectangle extends Shape {
private int x, y, width, height
@Override void draw(Canvas c) {}
}
@groovy.transform.TypeChecked
void addRectangle(List<? extends Shape> shapes) {
shapes.add(0, new Rectangle()) // TODO: compile-time error!
}
'''

// TODO: This is just a sample message; Java produces this for the equivalent code.
assert err =~ / The method add(capture#1-of \?) in the type List<capture#1-of \?> is not applicable for the arguments \(Rectangle\)/
}

void testWildcardSuper() {
assertScript '''
import java.awt.Canvas
abstract class Shape {
abstract void draw(Canvas c)
}
class Circle extends Shape {
private int x, y, radius
@Override void draw(Canvas c) {}
}
class Rectangle extends Shape {
private int x, y, width, height
@Override void draw(Canvas c) {}
}
@groovy.transform.TypeChecked
void addRectangle(List<? super Shape> shapes) {
shapes.add(0, new Rectangle())
}
List<Shape> list = []
addRectangle(list)
assert list.size() == 1
assert list.get(0) instanceof Rectangle
'''
}

void testWildcardExtends2() {
@Test
void testWildcardExtends() {
new CompilationUnit().with {
addSource 'Main.groovy', '''
class Factory {
Expand Down Expand Up @@ -124,7 +56,8 @@ final class Groovy9074 extends GroovyTestCase {
}
}

void testWildcardSuper2() {
@Test
void testWildcardSuper() {
new CompilationUnit().with {
addSource 'Main.groovy', '''
class Factory {
Expand Down
8 changes: 0 additions & 8 deletions src/test/groovy/transform/stc/BugsSTCTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -279,14 +279,6 @@ class BugsSTCTest extends StaticTypeCheckingTestCase {
'''
}

void testGroovyObjectInGenerics() {
assertScript '''
class A {}
List<? extends GroovyObject> list = new LinkedList<? extends GroovyObject>()
list.add(new A())
'''
}

// GROOVY-5656
void testShouldNotThrowAmbiguousMethodError() {
assertScript '''import groovy.transform.*
Expand Down
Loading

0 comments on commit 71490fb

Please sign in to comment.