You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2015/12/23 09:56:38 UTC
groovy git commit: GROOVY-6787: Compare arguments with the generic
bounds (closes #221)
Repository: groovy
Updated Branches:
refs/heads/master 5c8d97b2c -> bc392629e
GROOVY-6787: Compare arguments with the generic bounds (closes #221)
The type inference on arguments shouldn't override the types of the
parameters, especially their bounds. This prevented the bound checks from
occurring.
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/bc392629
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/bc392629
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/bc392629
Branch: refs/heads/master
Commit: bc392629e9ba4f968a4799af2d7f574deeb9901c
Parents: 5c8d97b
Author: Frank Pavageau <fp...@ekino.com>
Authored: Tue Dec 22 22:47:40 2015 +0100
Committer: pascalschumacher <pa...@gmx.net>
Committed: Wed Dec 23 09:56:01 2015 +0100
----------------------------------------------------------------------
.../org/codehaus/groovy/ast/GenericsType.java | 2 +
.../stc/StaticTypeCheckingSupport.java | 42 +++++--
.../groovy/transform/stc/GenericsSTCTest.groovy | 110 +++++++++++++++++++
3 files changed, 146 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/bc392629/src/main/org/codehaus/groovy/ast/GenericsType.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/GenericsType.java b/src/main/org/codehaus/groovy/ast/GenericsType.java
index 053c7c5..51e10c5 100644
--- a/src/main/org/codehaus/groovy/ast/GenericsType.java
+++ b/src/main/org/codehaus/groovy/ast/GenericsType.java
@@ -281,6 +281,8 @@ public class GenericsType extends ASTNode {
// but with reversed arguments
return implementsInterfaceOrIsSubclassOf(lowerBound, classNode) && checkGenerics(classNode);
}
+ // If there are no bounds, the generic type is basically Object, and everything is compatible.
+ return true;
}
// if this is not a generics placeholder, first compare that types represent the same type
if ((type!=null && !type.equals(classNode))) {
http://git-wip-us.apache.org/repos/asf/groovy/blob/bc392629/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index e560490..d28d5fd 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1396,7 +1396,7 @@ public abstract class StaticTypeCheckingSupport {
Set<String> result = new HashSet<String>();
for (Map.Entry<String, GenericsType> entry : resolvedMethodGenerics.entrySet()) {
GenericsType value = entry.getValue();
- if (value.isPlaceholder() || value.isPlaceholder()) continue;
+ if (value.isPlaceholder()) continue;
result.add(entry.getKey());
}
return result;
@@ -1418,7 +1418,7 @@ public abstract class StaticTypeCheckingSupport {
// since it is possible that the callsite uses some generics as well,
// we may have to add additional elements here
addMissingEntries(connections, resolvedMethodGenerics);
- // to finally see if the parameter and the argument fit togehter,
+ // to finally see if the parameter and the argument fit together,
// we use the provided information to transform the parameter
// into something that can exist in the callsite context
type = applyGenericsContext(resolvedMethodGenerics, type);
@@ -1535,20 +1535,46 @@ public abstract class StaticTypeCheckingSupport {
GenericsType original = entry.getValue();
if (!original.isWildcard() && !original.isPlaceholder()) {
continue;
- } else if (!replacement.isPlaceholder()) {
+ }
+ boolean placeholderReplacement = replacement.isPlaceholder();
+ if (placeholderReplacement) {
+ GenericsType connectedType = resolvedPlaceholders.get(replacement.getName());
+ if (replacement==connectedType) continue;
+ }
+ // GROOVY-6787: Don't override the original if the replacement placeholder doesn't respect the bounds,
+ // otherwise the original bounds are lost which can result in accepting an incompatible type as an
+ // argument, for example.
+ ClassNode replacementType = extractType(replacement);
+ if (original.isCompatibleWith(replacementType)) {
entry.setValue(replacement);
- continue;
+ if (placeholderReplacement) {
+ checkForMorePlaceHolders = checkForMorePlaceHolders || !equalIncludingGenerics(original,replacement);
+ }
}
- GenericsType connectedType = resolvedPlaceholders.get(replacement.getName());
- if (replacement==connectedType) continue;
- entry.setValue(replacement);
- checkForMorePlaceHolders = checkForMorePlaceHolders || !equalIncludingGenerics(original,replacement);
}
if (!checkForMorePlaceHolders) break;
}
if (count>=10000) throw new GroovyBugError("unable to handle generics in "+resolvedPlaceholders+" with connections "+connections);
}
+ private static ClassNode extractType(GenericsType gt) {
+ if (!gt.isPlaceholder()) {
+ return gt.getType();
+ }
+ // For a placeholder, a type based on the generics type is used for the compatibility check, to match on
+ // the actual bounds and not the name of the placeholder.
+ ClassNode replacementType = OBJECT_TYPE;
+ if (gt.getType().getGenericsTypes() != null) {
+ GenericsType realGt = gt.getType().getGenericsTypes()[0];
+ if (realGt.getLowerBound() != null) {
+ replacementType = realGt.getLowerBound();
+ } else if (realGt.getUpperBounds() != null && realGt.getUpperBounds().length > 0) {
+ replacementType = realGt.getUpperBounds()[0];
+ }
+ }
+ return replacementType;
+ }
+
private static boolean equalIncludingGenerics(GenericsType orig, GenericsType copy) {
if (orig==copy) return true;
if (orig.isPlaceholder()!=copy.isPlaceholder()) return false;
http://git-wip-us.apache.org/repos/asf/groovy/blob/bc392629/src/test/groovy/transform/stc/GenericsSTCTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index 6b3ab57..8b0f901 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -989,6 +989,116 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ void testCorrectlyBoundedByWildcardGenericParameterType() {
+ assertScript '''
+ class Foo {
+ static <T extends List<?>> void bar(T a) {}
+ }
+ Foo.bar(['abc'])
+ '''
+ }
+
+ void testCorrectlyBoundedByExtendsGenericParameterType() {
+ assertScript '''
+ class Foo {
+ static <T extends List<? extends CharSequence>> void bar(T a) {}
+ }
+ Foo.bar(['abc'])
+ '''
+ }
+
+ void testCorrectlyBoundedBySuperGenericParameterType() {
+ assertScript '''
+ class Foo {
+ static <T extends List<? super CharSequence>> void bar(T a) {}
+ }
+ Foo.bar([new Object()])
+ '''
+ }
+
+ void testCorrectlyBoundedByExtendsPlaceholderParameterType() {
+ assertScript '''
+ class Foo {
+ static <T extends List<? extends CharSequence>> void bar(T a) {}
+ }
+ class Baz {
+ static <T extends List<? extends String>> void qux(T a) {
+ Foo.bar(a)
+ }
+ }
+ Baz.qux(['abc'])
+ '''
+ }
+
+ void testCorrectlyBoundedBySuperPlaceholderParameterType() {
+ assertScript '''
+ class Foo {
+ static <T extends List<? super CharSequence>> void bar(T a) {}
+ }
+ class Baz {
+ static <T extends List<? super Object>> void qux(T a) {
+ Foo.bar(a)
+ }
+ }
+ Baz.qux([new Object()])
+ '''
+ }
+
+ void testCorrectlyBoundedSubtypeGenericParameterType() {
+ assertScript '''
+ class Foo {
+ static <T extends Collection<? extends CharSequence>> void bar(T a) {}
+ }
+ Foo.bar(['abc'])
+ '''
+ }
+
+ void testOutOfBoundsByExtendsGenericParameterType() {
+ shouldFailWithMessages '''
+ class Foo {
+ static <T extends List<? extends CharSequence>> void bar(T a) {}
+ }
+ Foo.bar([new Object()])
+ ''', 'Cannot call <T extends java.util.List<? extends java.lang.CharSequence>> Foo#bar(T) with arguments [java.util.List <java.lang.Object>]'
+ }
+
+ void testOutOfBoundsBySuperGenericParameterType() {
+ shouldFailWithMessages '''
+ class Foo {
+ static <T extends List<? super CharSequence>> void bar(T a) {}
+ }
+ Foo.bar(['abc'])
+ ''', 'Cannot call <T extends java.util.List<? super java.lang.CharSequence>> Foo#bar(T) with arguments [java.util.List <java.lang.String>]'
+ }
+
+ void testOutOfBoundsByExtendsPlaceholderParameterType() {
+ shouldFailWithMessages '''
+ class Foo {
+ static <T extends List<? extends CharSequence>> void bar(T a) {}
+ }
+ class Baz {
+ static <T extends List<Object>> void qux(T a) {
+ Foo.bar(a)
+ }
+ }
+ Baz.qux([new Object()])
+ ''', 'Cannot call <T extends java.util.List<? extends java.lang.CharSequence>> Foo#bar(T) with arguments [T]'
+ }
+
+ void testOutOfBoundsBySuperPlaceholderParameterType() {
+ shouldFailWithMessages '''
+ class Foo {
+ static <T extends List<? super CharSequence>> void bar(T a) {}
+ }
+ class Baz {
+ static <T extends List<String>> void qux(T a) {
+ Foo.bar(a)
+ }
+ }
+ Baz.qux(['abc'])
+ ''', 'Cannot call <T extends java.util.List<? super java.lang.CharSequence>> Foo#bar(T) with arguments [T]'
+ }
+
// GROOVY-5721
void testExtractComponentTypeFromSubclass() {
assertScript '''