From 1376361bb9089b1d54e9938a7055610b67b8df0e Mon Sep 17 00:00:00 2001 From: John Wagenleitner Date: Thu, 24 May 2018 14:59:20 -0700 Subject: [PATCH 1/2] Fix parameter matching for parameterized types Revert of change 57cfd2a3e4d985b3 and fixes issues with Nextflow CI builds. --- .../stc/StaticTypeCheckingSupport.java | 52 +++++++++++++++++-- .../transform/stc/GenericsSTCTest.groovy | 10 +++- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java index cf92a6e0508..6a5245bb2af 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java @@ -62,6 +62,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -1099,7 +1100,8 @@ Person foo(B i){...} Person p = foo(b) */ - Parameter[] params = makeRawTypes(safeNode.getParameters(), declaringClassForDistance, actualReceiverForDistance); + Map declaringAndActualGenericsTypeMap = makeDeclaringAndActualGenericsTypeMap(declaringClassForDistance, actualReceiverForDistance); + Parameter[] params = makeRawTypes(safeNode.getParameters(), declaringAndActualGenericsTypeMap); int dist = measureParametersAndArgumentsDistance(params, safeArgs); if (dist >= 0) { dist += getClassDistance(declaringClassForDistance, actualReceiverForDistance); @@ -1189,20 +1191,62 @@ private static int getExtensionDistance(boolean isExtensionMethodNode) { return isExtensionMethodNode ? 0 : 1; } + private static ClassNode findActualTypeByGenericsPlaceholderName(String placeholderName, Map genericsPlaceholderAndTypeMap) { + for (Map.Entry entry : genericsPlaceholderAndTypeMap.entrySet()) { + GenericsType declaringGenericsType = entry.getKey(); + + if (placeholderName.equals(declaringGenericsType.getName())) { + return entry.getValue().getType(); + } + } + + return null; + } + public static ClassNode findActualTypeByPlaceholderName(String placeholderName, Map placeholderInfo) { GenericsType gt = placeholderInfo.get(placeholderName); return null == gt ? null : gt.getType().redirect(); } - private static Parameter[] makeRawTypes(Parameter[] params, ClassNode declaringClassForDistance, ClassNode actualReceiverForDistance) { - Map placeholderInfo = GenericsUtils.extractPlaceholders(GenericsUtils.findParameterizedTypeFromCache(declaringClassForDistance, actualReceiverForDistance)); + /** + * map declaring generics type to actual generics type, e.g. GROOVY-7204: + * declaring generics types: T, S extends Serializable + * actual generics types : String, Long + * + * the result map is [ + * T: String, + * S: Long + * ] + * + * The resolved types can not help us to choose methods correctly if the argument is a string: T: Object, S: Serializable + * so we need actual types: T: String, S: Long + */ + private static Map makeDeclaringAndActualGenericsTypeMap(ClassNode declaringClass, ClassNode actualReceiver) { + ClassNode parameterizedType = GenericsUtils.findParameterizedType(declaringClass, actualReceiver); + + if (null == parameterizedType) { + return Collections.emptyMap(); + } + + GenericsType[] declaringGenericsTypes = declaringClass.getGenericsTypes(); + GenericsType[] actualGenericsTypes = parameterizedType.getGenericsTypes(); + + Map result = new LinkedHashMap<>(); + for (int i = 0, n = declaringGenericsTypes.length; i < n; i++) { + result.put(declaringGenericsTypes[i], actualGenericsTypes[i]); + } + + return result; + } + + private static Parameter[] makeRawTypes(Parameter[] params, Map genericsPlaceholderAndTypeMap) { Parameter[] newParam = new Parameter[params.length]; for (int i = 0; i < params.length; i++) { Parameter oldP = params[i]; - ClassNode actualType = findActualTypeByPlaceholderName(oldP.getType().getUnresolvedName(), placeholderInfo); + ClassNode actualType = findActualTypeByGenericsPlaceholderName(oldP.getType().getUnresolvedName(), genericsPlaceholderAndTypeMap); Parameter newP = new Parameter(makeRawType(null == actualType ? oldP.getType() : actualType), oldP.getName()); newParam[i] = newP; } diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy index 4b1055b7936..69a43a77f1b 100644 --- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy +++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy @@ -76,7 +76,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase { shouldFailWithMessages ''' List list = [] list << 1 - ''', '[Static type checking] - Cannot call java.util.List #leftShift(T) with arguments [int] ' + ''', '[Static type checking] - Cannot find matching method java.util.List#leftShift(int)' } void testAddOnList2UsingLeftShift() { @@ -91,6 +91,14 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase { ''' } + void testAddOnListWithParameterizedTypeLeftShift() { + assertScript ''' + class Trie {} + List> list = [] + list << new Trie() + ''' + } + void testAddOnListWithDiamondUsingLeftShift() { assertScript ''' List list = new LinkedList<>() From c7d3e6d8916f78544e7354837fa0ff4b811931f6 Mon Sep 17 00:00:00 2001 From: John Wagenleitner Date: Fri, 25 May 2018 13:16:18 -0700 Subject: [PATCH 2/2] Fix NPE if accessed property not a member of the owning class --- .../stc/StaticTypeCheckingVisitor.java | 11 +++++++---- .../TypeCheckingExtensionSpecTest.groovy | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index 9a13abe2b8c..96faeb9ca9e 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -611,10 +611,13 @@ public void visitVariableExpression(VariableExpression vexp) { if (vexp.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER) == null) { ClassNode owner = (ClassNode) vexp.getNodeMetaData(StaticCompilationMetadataKeys.PROPERTY_OWNER); if (owner != null) { - boolean lhsOfEnclosingAssignment = isLHSOfEnclosingAssignment(vexp); - fieldNode = owner.getField(vexp.getName()); - vexp.setAccessedVariable(fieldNode); - checkOrMarkPrivateAccess(vexp, fieldNode, lhsOfEnclosingAssignment); + FieldNode veFieldNode = owner.getField(vexp.getName()); + if (veFieldNode != null) { + fieldNode = veFieldNode; + boolean lhsOfEnclosingAssignment = isLHSOfEnclosingAssignment(vexp); + vexp.setAccessedVariable(fieldNode); + checkOrMarkPrivateAccess(vexp, fieldNode, lhsOfEnclosingAssignment); + } } } } diff --git a/src/spec/test/typing/TypeCheckingExtensionSpecTest.groovy b/src/spec/test/typing/TypeCheckingExtensionSpecTest.groovy index e6b310e69ec..cb1e4fa2d70 100644 --- a/src/spec/test/typing/TypeCheckingExtensionSpecTest.groovy +++ b/src/spec/test/typing/TypeCheckingExtensionSpecTest.groovy @@ -587,6 +587,25 @@ new DelegateTest().delegate() ''' } + void testDelegateVariableFromDifferentOwningClass() { + assertScript ''' + @groovy.transform.CompileStatic + class A { + static private int MAX_LINES = 2 + static class B { + @Delegate + private Map delegate = [:] + void m(int c) { + if (c > MAX_LINES) { + return + } + } + } + } + null + ''' + } + private static class SpecSupport { static int getLongueur(String self) { self.length() } static int longueur(String self) { self.length() }