Skip to content

Commit

Permalink
GROOVY-11276: STC: drop type args after applying generics connections
Browse files Browse the repository at this point in the history 
GROOVY-5482, GROOVY-8609

3_0_X backport
  • Loading branch information
@eric-milles
eric-milles committed Jan 12, 2024
1 parent b702e6b commit a1f5078
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 173 deletions.
3 changes: 1 addition & 2 deletions src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,7 @@ public static ClassNode nonGeneric(final ClassNode type) {
}
if (temp instanceof DecompiledClassNode // GROOVY-10461: check without resolving supers
? ((DecompiledClassNode) temp).isParameterized() : temp.isUsingGenerics()) {
ClassNode result = ClassHelper.makeWithoutCaching(temp.getName());
result.setRedirect(temp);
ClassNode result = temp.getPlainNodeReference();
result.setGenericsTypes(null);
result.setUsingGenerics(false);
while (dims > 0) { dims -= 1;
Expand Down
83 changes: 34 additions & 49 deletions src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList;
Expand Down Expand Up @@ -1025,7 +1024,7 @@ public static List<MethodNode> chooseBestMethod(final ClassNode receiver, final
Iterable<MethodNode> candidates = noCulling ? methods : removeCovariantsAndInterfaceEquivalents(methods);

for (MethodNode candidate : candidates) {
MethodNode safeNode = candidate;
MethodNode safeNode = candidate;
ClassNode[] safeArgs = argumentTypes;
boolean isExtensionMethod = candidate instanceof ExtensionMethodNode;
if (isExtensionMethod) {
Expand All @@ -1036,35 +1035,11 @@ public static List<MethodNode> chooseBestMethod(final ClassNode receiver, final
safeNode = ((ExtensionMethodNode) candidate).getExtensionMethodNode();
}

/* TODO: corner case
class B extends A {}
Animal foo(A a) {}
Person foo(B b) {}
B b = new B()
Person p = foo(b)
*/

ClassNode declaringClass = candidate.getDeclaringClass();
ClassNode actualReceiver = receiver != null ? receiver : declaringClass;
Parameter[] simpleParams = getSafeParameters(safeNode, declaringClass, actualReceiver);

Map<GenericsType, GenericsType> spec;
if (candidate.isStatic()) {
spec = Collections.emptyMap(); // none visible
} else {
spec = GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType(declaringClass, actualReceiver);
GenericsType[] methodGenerics = candidate.getGenericsTypes();
if (methodGenerics != null) { // GROOVY-10322: remove hidden type parameters
for (int i = 0, n = methodGenerics.length; i < n && !spec.isEmpty(); i += 1) {
for (Iterator<GenericsType> it = spec.keySet().iterator(); it.hasNext(); ) {
if (it.next().getName().equals(methodGenerics[i].getName())) it.remove();
}
}
}
}

Parameter[] params = makeRawTypes(safeNode.getParameters(), spec);
int dist = measureParametersAndArgumentsDistance(params, safeArgs);
int dist = measureParametersAndArgumentsDistance(simpleParams, safeArgs);
if (dist >= 0) {
dist += getClassDistance(declaringClass, actualReceiver);
dist += getExtensionDistance(isExtensionMethod);
Expand Down Expand Up @@ -1092,6 +1067,37 @@ Person foo(B b) {}
return bestChoices;
}

private static Parameter[] getSafeParameters(final MethodNode methodNode, final ClassNode declaringClass, final ClassNode actualReceiver) {
Parameter[] params = methodNode.getParameters();
if (params.length > 0) {
Map<GenericsTypeName, GenericsType> spec;
if (methodNode.isStatic()) {
spec = Collections.emptyMap(); // not visible
} else {
extractGenericsConnections(spec = new HashMap<>(), actualReceiver, declaringClass);

GenericsType[] methodGenerics = methodNode.getGenericsTypes();
if (methodGenerics != null) { // GROOVY-10322: remove hidden type parameters
for (GenericsType tp : methodGenerics) spec.remove(new GenericsTypeName(tp.getName()));
}
}

params = params.clone();

for (int i = 0; i < params.length; i += 1) {
ClassNode t = params[i].getOriginType(), x=t;
while (x.isArray()) x = x.getComponentType();
if (x.isGenericsPlaceHolder()) {
// GROOVY-7204, GROOVY-8059, GROOVY-8609, GROOVY-10820, GROOVY-11276: "T" to "Foo" (via spec) or erasure
params[i] = new Parameter(GenericsUtils.nonGeneric(applyGenericsContext(spec, t)), params[i].getName());
} else if (x.getGenericsTypes() != null) { // reduce "List<Type>" to just "List"
params[i] = new Parameter(GenericsUtils.nonGeneric(t), params[i].getName());
}
}
}
return params;
}

private static int measureParametersAndArgumentsDistance(final Parameter[] parameters, final ClassNode[] argumentTypes) {
int dist = -1;
if (parameters.length == argumentTypes.length) {
Expand Down Expand Up @@ -1154,27 +1160,6 @@ private static int getExtensionDistance(final boolean isExtensionMethodNode) {
return isExtensionMethodNode ? 0 : 1;
}

private static Parameter[] makeRawTypes(final Parameter[] parameters, final Map<GenericsType, GenericsType> genericsPlaceholderAndTypeMap) {
return Arrays.stream(parameters).map(param -> {
String name = param.getType().getUnresolvedName();
Optional<GenericsType> value = genericsPlaceholderAndTypeMap.entrySet().stream()
.filter(e -> e.getKey().getName().equals(name)).findFirst().map(Map.Entry::getValue);
ClassNode type = value.map(gt -> !gt.isPlaceholder() ? gt.getType() : makeRawType(gt.getType())).orElseGet(() -> makeRawType(param.getType()));

return new Parameter(type, param.getName());
}).toArray(Parameter[]::new);
}

private static ClassNode makeRawType(final ClassNode receiver) {
if (receiver.isArray()) {
return makeRawType(receiver.getComponentType()).makeArray();
}
ClassNode raw = receiver.getPlainNodeReference();
raw.setUsingGenerics(false);
raw.setGenericsTypes(null);
return raw;
}

private static List<MethodNode> removeCovariantsAndInterfaceEquivalents(final Collection<MethodNode> collection) {
List<MethodNode> list = new ArrayList<>(new LinkedHashSet<>(collection)), toBeRemoved = new ArrayList<>();
for (int i = 0, n = list.size(); i < n - 1; i += 1) {
Expand Down
171 changes: 84 additions & 87 deletions src/test/groovy/bugs/Groovy8609Bug.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,121 +26,118 @@ final class Groovy8609Bug extends GroovyTestCase {

void testUpperBoundWithGenerics() {
assertScript '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map<String, Integer>> {
E getFirstRecord(T recordList) {
return recordList.get(0)
}
static void main(args) {
def list = new ArrayList<HashMap<String, Integer>>()
def record = new HashMap<String, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map<String, Integer>> {
E getFirstRecord(T recordList) {
return recordList.get(0)
}
static void main(args) {
def list = new ArrayList<HashMap<String, Integer>>()
def record = new HashMap<String, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
}
}
}
'''
}

void testUpperBoundWithoutGenerics() {
assertScript '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map> {
E getFirstRecord(T recordList) {
return recordList.get(0);
}
static void main(args) {
def list = new ArrayList<HashMap<String, Integer>>()
def record = new HashMap<String, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map> {
E getFirstRecord(T recordList) {
return recordList.get(0);
}
static void main(args) {
def list = new ArrayList<HashMap<String, Integer>>()
def record = new HashMap<String, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
}
}
}
'''
}

void testNoUpperBound() {
assertScript '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E> {
E getFirstRecord(T recordList) {
return recordList.get(0);
}
static void main(args) {
def list = new ArrayList<HashMap<String, Integer>>()
def record = new HashMap<String, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
@groovy.transform.CompileStatic
public class A<T extends List<E>, E> {
E getFirstRecord(T recordList) {
return recordList.get(0);
}
static void main(args) {
def list = new ArrayList<HashMap<String, Integer>>()
def record = new HashMap<String, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
}
}
}
'''
}

void testUpperBoundWithGenericsThroughWrongType() {
def errMsg = shouldFail '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map<String, Integer>> {
E getFirstRecord(T recordList) {
return recordList.get(0)
}
static void main(args) {
def list = new ArrayList<TreeMap<String, Integer>>()
def record = new TreeMap<String, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
def err = shouldFail '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map<String, Integer>> {
E getFirstRecord(T recordList) {
return recordList.get(0)
}
static void main(args) {
def list = new ArrayList<TreeMap<String, Integer>>()
def record = new TreeMap<String, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
}
}
}
'''

assert errMsg.contains('[Static type checking] - Cannot find matching method A#getFirstRecord(java.util.ArrayList <TreeMap>)')
assert err.contains('Cannot call A <ArrayList, HashMap>#getFirstRecord(java.util.ArrayList <HashMap>) with arguments [java.util.ArrayList <TreeMap>]')
}

void testUpperBoundWithGenericsThroughWrongType2() {
def errMsg = shouldFail '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map<String, Integer>> {
E getFirstRecord(T recordList) {
return recordList.get(0)
}
static void main(args) {
def list = new ArrayList<HashMap<String, Long>>()
def record = new HashMap<String, Long>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
def err = shouldFail '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map<String, Integer>> {
E getFirstRecord(T recordList) {
return recordList.get(0)
}
static void main(args) {
def list = new ArrayList<HashMap<String, Long>>()
def record = new HashMap<String, Long>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
}
}
}
'''

assert errMsg.contains('[Static type checking] - Cannot find matching method A#getFirstRecord(java.util.ArrayList <HashMap>)')
assert err.contains('Cannot call A <ArrayList, HashMap>#getFirstRecord(java.util.ArrayList <HashMap>) with arguments [java.util.ArrayList <HashMap>]')
}

void testUpperBoundWithGenericsThroughWrongType3() {
def errMsg = shouldFail '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map<String, Integer>> {
E getFirstRecord(T recordList) {
return recordList.get(0)
}
static void main(args) {
def list = new ArrayList<HashMap<StringBuffer, Integer>>()
def record = new HashMap<StringBuffer, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
def err = shouldFail '''
@groovy.transform.CompileStatic
public class A<T extends List<E>, E extends Map<String, Integer>> {
E getFirstRecord(T recordList) {
return recordList.get(0)
}
static void main(args) {
def list = new ArrayList<HashMap<StringBuffer, Integer>>()
def record = new HashMap<StringBuffer, Integer>()
list.add(record)
def a = new A<ArrayList<HashMap<String, Integer>>, HashMap<String, Integer>>()
assert record.is(a.getFirstRecord(list))
}
}
}
'''

assert errMsg.contains('[Static type checking] - Cannot find matching method A#getFirstRecord(java.util.ArrayList <HashMap>)')
assert err.contains('Cannot call A <ArrayList, HashMap>#getFirstRecord(java.util.ArrayList <HashMap>) with arguments [java.util.ArrayList <HashMap>]')
}
}
25 changes: 9 additions & 16 deletions src/test/groovy/transform/stc/BugsSTCTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,22 +208,15 @@ class BugsSTCTest extends StaticTypeCheckingTestCase {
}

void testGroovy5482ListsAndFlowTyping() {
assertScript '''
class StaticGroovy2 {
def bar() {
def foo = [new Date(), 1, new C()]
foo.add( 2 ) // Compiles
foo.add( new Date() )
foo.add( new C() )
foo = [new Date(), 1]
foo.add( 2 ) // Does not compile
}
}
class C{
}
new StaticGroovy2()'''
assertScript '''class C {}
def foo = [new Date(), 1, new C()]
foo.add(new Date())
foo.add(new C())
foo.add(2)
foo = [new Date(), 1]
foo.add(2) // STC err
'''
}

void testClosureThisObjectDelegateOwnerProperty() {
Expand Down
Loading

0 comments on commit a1f5078

Please sign in to comment.