This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-7971alt in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f33e9b5efdf1fd74e0a13cb4b7e4b80f725442ee Author: Eric Milles <[email protected]> AuthorDate: Wed Dec 10 10:12:13 2025 -0600 GROOVY-7971, GROOVY-8337, GROOVY-8965: logical-or `instanceof` guards --- .../classgen/asm/sc/StaticTypesTypeChooser.java | 4 +- .../transform/stc/StaticTypeCheckingSupport.java | 6 +- .../transform/stc/StaticTypeCheckingVisitor.java | 151 ++++++++++++++++----- .../groovy/transform/stc/TypeCheckingContext.java | 4 + .../groovy/transform/stc/UnionTypeClassNode.java | 16 +-- src/test/groovy/bugs/Groovy8337Bug.groovy | 52 ------- .../groovy/transform/stc/MethodCallsSTCTest.groovy | 59 ++++---- .../transform/stc/TypeInferenceSTCTest.groovy | 110 ++++++++------- .../asm/sc/TypeInferenceStaticCompileTest.groovy | 9 +- 9 files changed, 230 insertions(+), 181 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java index 4918ca9012..419bc4b606 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java @@ -43,7 +43,9 @@ public class StaticTypesTypeChooser extends StatementMetaTypeChooser { var ast = getTarget(exp); // GROOVY-9344, GROOVY-9607, GROOVY-11375 inferredType = ast.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE); } - if (inferredType != null && !isPrimitiveVoid(inferredType)) { + if (inferredType != null && !isPrimitiveVoid(inferredType) + // prevent union and intersection types from escaping into classgen + && !inferredType.getName().startsWith("(") && !inferredType.getName().startsWith("<")) { return inferredType; } 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 ea9de4082b..9eb04af018 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java @@ -883,12 +883,12 @@ public abstract class StaticTypeCheckingSupport { return true; } if (superOrInterface instanceof WideningCategories.LowestUpperBoundClassNode) { - if (implementsInterfaceOrIsSubclassOf(type, superOrInterface.getSuperClass()) + if (implementsInterfaceOrIsSubclassOf(type, superOrInterface.getUnresolvedSuperClass()) && Arrays.stream(superOrInterface.getInterfaces()).allMatch(type::implementsInterface)) { return true; } - } else if (superOrInterface instanceof UnionTypeClassNode) { - for (ClassNode delegate : ((UnionTypeClassNode) superOrInterface).getDelegates()) { + } else if (superOrInterface instanceof UnionTypeClassNode union) { + for (ClassNode delegate : union.getDelegates()) { if (implementsInterfaceOrIsSubclassOf(type, delegate)) { return true; } 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 bf4451ecae..1ab31d73f7 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -266,6 +266,7 @@ import static org.codehaus.groovy.syntax.Types.INTDIV; import static org.codehaus.groovy.syntax.Types.INTDIV_EQUAL; import static org.codehaus.groovy.syntax.Types.KEYWORD_IN; import static org.codehaus.groovy.syntax.Types.KEYWORD_INSTANCEOF; +import static org.codehaus.groovy.syntax.Types.LOGICAL_OR; import static org.codehaus.groovy.syntax.Types.MINUS_MINUS; import static org.codehaus.groovy.syntax.Types.MOD; import static org.codehaus.groovy.syntax.Types.MOD_EQUAL; @@ -848,10 +849,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { return; } + // GROOVY-7971, GROOVY-8965, GROOVY-10096, GROOVY-10702, et al. + if (op == LOGICAL_OR) typeCheckingContext.pushTemporaryTypeInfo(); + leftExpression.visit(this); ClassNode lType = getType(leftExpression); var setterInfo = removeSetterInfo(leftExpression); - if (setterInfo != null) { + if (setterInfo != null) { assert op != LOGICAL_OR ; if (ensureValidSetter(expression, leftExpression, rightExpression, setterInfo)) { return; } @@ -860,7 +864,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { lType = getOriginalDeclarationType(leftExpression); applyTargetType(lType, rightExpression); } - rightExpression.visit(this); + if (op != LOGICAL_OR) { + rightExpression.visit(this); + } else { + var lhs = typeCheckingContext.temporaryIfBranchTypeInformation.pop(); + typeCheckingContext.pushTemporaryTypeInfo(); + rightExpression.visit(this); + + var rhs = typeCheckingContext.temporaryIfBranchTypeInformation.pop(); + propagateTemporaryTypeInfo(lhs, rhs); // `instanceof` on either side? + } } ClassNode rType = isNullConstant(rightExpression) @@ -973,7 +986,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } else if (op == KEYWORD_INSTANCEOF) { pushInstanceOfTypeInfo(leftExpression, rightExpression); } else if (op == COMPARE_NOT_INSTANCEOF) { // GROOVY-6429, GROOVY-8321, GROOVY-8412, GROOVY-8523, GROOVY-9931 - putNotInstanceOfTypeInfo(extractTemporaryTypeInfoKey(leftExpression), Collections.singleton(rightExpression.getType())); + putNotInstanceOfTypeInfo(extractTemporaryTypeInfoKey(leftExpression), Set.of(rightExpression.getType())); } if (!isEmptyDeclaration) { storeType(expression, resultType); @@ -989,6 +1002,55 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } } + private void propagateTemporaryTypeInfo(final Map<Object, List<ClassNode>> lhs, + final Map<Object, List<ClassNode>> rhs) { + // TODO: deal with (x !instanceof T) + lhs.keySet().removeIf(k -> k instanceof Object[]); + rhs.keySet().removeIf(k -> k instanceof Object[]); + + for (var entry : lhs.entrySet()) { + if (rhs.containsKey(entry.getKey())) { + // main case: (x instanceof A || x instanceof B) produces A|B type + ClassNode t = unionize(entry.getValue(), rhs.get(entry.getKey())); + typeCheckingContext.peekTemporaryTypeInfo(entry.getKey()).add(t); + } else if (entry.getKey() instanceof Variable v) { + // edge case: (x instanceof A || ...) produces A|typeof(x) type + ClassNode t = unionize(entry.getValue(), List.of(v instanceof ASTNode n ? getType(n) : v.getType())); + typeCheckingContext.peekTemporaryTypeInfo(v).add(t); + } + } + + rhs.keySet().removeAll(lhs.keySet()); + + for (var entry : rhs.entrySet()) { + if (entry.getKey() instanceof Variable v) { + // edge case: (... || x instanceof B) produces typeof(x)|B type + ClassNode t = unionize(List.of(v instanceof ASTNode n ? getType(n) : v.getType()), entry.getValue()); + typeCheckingContext.peekTemporaryTypeInfo(v).add(t); + } + } + } + + private static ClassNode unionize(final List<ClassNode> a, final List<ClassNode> b) { + var types = new LinkedHashSet<ClassNode>(); + for (ClassNode t : a) { + if (t instanceof UnionTypeClassNode u) { + Collections.addAll(types, u.getDelegates()); + } else { + types.add(t); + } + } + for (ClassNode t : b) { + if (t instanceof UnionTypeClassNode u) { + Collections.addAll(types, u.getDelegates()); + } else { + types.add(t); + } + } + assert types.size() > 1; + return new UnionTypeClassNode(types.toArray(ClassNode[]::new)); + } + private void validateResourceInARM(final BinaryExpression expression, final ClassNode lType) { if (expression instanceof DeclarationExpression && TryCatchStatement.isResource(expression) @@ -1170,8 +1232,8 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0 } addStaticTypeError(message, leftExpression); } else { - ClassNode[] tergetTypes = visibleSetters.stream().map(setterType).toArray(ClassNode[]::new); - addAssignmentError(tergetTypes.length == 1 ? tergetTypes[0] : new UnionTypeClassNode(tergetTypes), getType(valueExpression), expression); + ClassNode[] targetTypes = visibleSetters.stream().map(setterType).toArray(ClassNode[]::new); + addAssignmentError(targetTypes.length == 1 ? targetTypes[0] : new UnionTypeClassNode(targetTypes), getType(valueExpression), expression); } return true; } @@ -1492,7 +1554,7 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0 ClassNode valueType = getType(valueExpression); BinaryExpression kv = typeCheckingContext.popEnclosingBinaryExpression(); if (propertyTypes.stream().noneMatch(targetType -> checkCompatibleAssignmentTypes(targetType, getResultType(targetType, ASSIGN, valueType, kv), valueExpression))) { - ClassNode targetType = propertyTypes.size() == 1 ? propertyTypes.iterator().next() : new UnionTypeClassNode(propertyTypes.toArray(ClassNode.EMPTY_ARRAY)); + ClassNode targetType = propertyTypes.size() == 1 ? propertyTypes.iterator().next() : new UnionTypeClassNode(propertyTypes.toArray(ClassNode[]::new)); if (!extension.handleIncompatibleAssignment(targetType, valueType, entryExpression)) { addAssignmentError(targetType, valueType, entryExpression); } @@ -2381,10 +2443,9 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0 super.visitExpressionStatement(statement); Map<?,List<ClassNode>> tti = typeCheckingContext.temporaryIfBranchTypeInformation.pop(); if (!tti.isEmpty() && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) { - tti.forEach((k, tempTypes) -> { + tti.forEach((key, tempTypes) -> { if (tempTypes.contains(VOID_TYPE)) - typeCheckingContext.temporaryIfBranchTypeInformation.peek() - .computeIfAbsent(k, x -> new LinkedList<>()).add(VOID_TYPE); + typeCheckingContext.peekTemporaryTypeInfo(key).add(VOID_TYPE); }); } } @@ -4005,18 +4066,16 @@ out: if (mn.size() != 1) { ClassNode receiver = getType(objectExpression); List<Receiver<String>> owners = new ArrayList<>(); if (typeCheckingContext.delegationMetadata != null - && objectExpression instanceof VariableExpression - && "owner".equals(((Variable) objectExpression).getName()) - && /*isNested:*/typeCheckingContext.delegationMetadata.getParent() != null) { + && /*isNested:*/typeCheckingContext.delegationMetadata.getParent() != null + && objectExpression instanceof VariableExpression v && v.getName().equals("owner")) { owners.add(new Receiver<>(receiver, "owner")); // if nested, Closure is the first owner List<Receiver<String>> thisType = new ArrayList<>(2); // and the enclosing class is the addDelegateReceiver(thisType, makeThis(), null); // end of the closure owner chain addReceivers(owners, thisType, typeCheckingContext.delegationMetadata.getParent(), "owner."); } else { - List<ClassNode> temporaryTypes = getTemporaryTypesForExpression(objectExpression); - int temporaryTypesCount = (temporaryTypes != null ? temporaryTypes.size() : 0); - if (temporaryTypesCount > 0) { // GROOVY-8965, GROOVY-10180, GROOVY-10668 - owners.add(Receiver.make(lowestUpperBound(temporaryTypes))); + // GROOVY-8965, GROOVY-10180, GROOVY-10668: `instanceof` type before declared + for (ClassNode tempType : getTemporaryTypesForExpression(objectExpression)) { + if (tempType != receiver) owners.add(Receiver.make(tempType)); } if (isClassClassNodeWrappingConcreteType(receiver)) { ClassNode staticType = receiver.getGenericsTypes()[0].getType(); @@ -4033,9 +4092,6 @@ out: if (mn.size() != 1) { } } } - if (temporaryTypesCount > 1 && !(objectExpression instanceof VariableExpression)) { - owners.add(Receiver.make(new UnionTypeClassNode(temporaryTypes.toArray(ClassNode.EMPTY_ARRAY)))); - } } return owners; } @@ -4248,9 +4304,11 @@ trying: for (ClassNode[] signature : signatures) { @Override protected void afterSwitchCaseStatementsVisited(final SwitchStatement statement) { - // GROOVY-8411: if any "case Type:" then "default:" contributes condition type - if (!statement.getDefaultStatement().isEmpty() && !typeCheckingContext.temporaryIfBranchTypeInformation.peek().isEmpty()) - pushInstanceOfTypeInfo(statement.getExpression(), new ClassExpression(statement.getExpression().getNodeMetaData(TYPE))); + // GROOVY-8411: if any "case Type:" then "default:" contributes alternate type + if (!statement.getDefaultStatement().isEmpty()) { + Expression selectable = statement.getExpression(); + optInstanceOfTypeInfo(selectable, selectable.getNodeMetaData(TYPE)); + } } @Override @@ -4258,7 +4316,9 @@ trying: for (ClassNode[] signature : signatures) { Expression selectable = typeCheckingContext.getEnclosingSwitchStatement().getExpression(); Expression expression = statement.getExpression(); if (expression instanceof ClassExpression) { // GROOVY-8411: refine the switch type - pushInstanceOfTypeInfo(selectable, expression); + if (!optInstanceOfTypeInfo(selectable, expression.getType())) { + pushInstanceOfTypeInfo(selectable, expression); + } } else if (expression instanceof ClosureExpression) { // GROOVY-9854: propagate the switch type ClassNode inf = selectable.getNodeMetaData(TYPE); expression.putNodeMetaData(CLOSURE_ARGUMENTS, new ClassNode[]{inf}); @@ -4423,7 +4483,7 @@ trying: for (ClassNode[] signature : signatures) { } else { typeOfFalse = checkForTargetType(falseExpression, typeOfFalse); typeOfTrue = checkForTargetType(trueExpression, typeOfTrue); - resultType = lowestUpperBound(typeOfTrue, typeOfFalse); + resultType = lowestUpperBound(typeOfTrue, typeOfFalse); // TODO: union instead of intersect } storeType(expression, resultType); popAssignmentTracking(oldTracker); @@ -4643,7 +4703,7 @@ trying: for (ClassNode[] signature : signatures) { } if (!var.isThisExpression() && !var.isSuperExpression() && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) { // GROOVY-5226, GROOVY-11290: assignment voids instanceof - pushInstanceOfTypeInfo(var, classX(VOID_TYPE)); + pushInstanceOfTypeInfo(var, VOID_TYPE); } } } @@ -5341,7 +5401,7 @@ trying: for (ClassNode[] signature : signatures) { if (!selfTypes.isEmpty()) { // TODO: reduce to the most-specific type(s) ClassNode superclass = selfTypes.stream().filter(t -> !t.isInterface()).findFirst().orElse(OBJECT_TYPE); selfTypes.remove(superclass); selfTypes.add(trait); - return new WideningCategories.LowestUpperBoundClassNode("TypesOf$" + trait.getNameWithoutPackage(), superclass, selfTypes.toArray(ClassNode.EMPTY_ARRAY)); + return new WideningCategories.LowestUpperBoundClassNode("TypesOf$" + trait.getNameWithoutPackage(), superclass, selfTypes.toArray(ClassNode[]::new)); } return trait; } @@ -6158,19 +6218,33 @@ out: for (ClassNode type : todo) { // temporaryIfBranchTypeInformation support; migrate to TypeCheckingContext? /** - * Stores information about types when [objectOfInstanceof instanceof typeExpression] is visited. + * Stores information about types when <i>[sourceExpression instanceof typeExpression]</i> is visited. * - * @param objectOfInstanceOf the expression to be checked against instanceof - * @param typeExpression the expression which represents the target type + * @param sourceExpression the expression to be checked against instanceof + * @param typeExpression the expression which represents the target type */ - protected void pushInstanceOfTypeInfo(final Expression objectOfInstanceOf, final Expression typeExpression) { - Object ttiKey = extractTemporaryTypeInfoKey(objectOfInstanceOf); ClassNode type = typeExpression.getType(); - typeCheckingContext.temporaryIfBranchTypeInformation.peek().computeIfAbsent(ttiKey, x -> new LinkedList<>()).add(type); + protected void pushInstanceOfTypeInfo(final Expression sourceExpression, final Expression typeExpression) { + pushInstanceOfTypeInfo(sourceExpression, typeExpression.getType()); + } + + private void pushInstanceOfTypeInfo(final Expression sourceExpression, final ClassNode type) { + typeCheckingContext.peekTemporaryTypeInfo(extractTemporaryTypeInfoKey(sourceExpression)).add(type); } private void putNotInstanceOfTypeInfo(final Object key, final Collection<ClassNode> types) { Object notKey = key instanceof Object[] ? ((Object[]) key)[1] : new Object[]{"!instanceof", key}; // stash negative type(s) - typeCheckingContext.temporaryIfBranchTypeInformation.peek().computeIfAbsent(notKey, x -> new LinkedList<>()).addAll(types); + typeCheckingContext.peekTemporaryTypeInfo(notKey).addAll(types); + } + + private boolean optInstanceOfTypeInfo(final Expression expression, final ClassNode type) { + List<ClassNode> types = typeCheckingContext.temporaryIfBranchTypeInformation.peek().get(extractTemporaryTypeInfoKey(expression)); + if (types != null) { assert !types.isEmpty(); + ClassNode t = unionize(types, List.of(type)); + types.clear(); + types.add(t); + return true; + } + return false; } /** @@ -6209,7 +6283,7 @@ out: for (ClassNode type : todo) { } private ClassNode getInferredTypeFromTempInfo(final Expression expression, final ClassNode expressionType) { - if (expression instanceof VariableExpression) { + if (expression instanceof VariableExpression && !isPrimitiveType(expressionType)) { List<ClassNode> tempTypes = getTemporaryTypesForExpression(expression); if (!tempTypes.isEmpty()) { ClassNode superclass; @@ -6249,10 +6323,15 @@ out: for (ClassNode type : todo) { } int typesCount = types.size(); - if (typesCount == 1) { + if (typesCount == 1 || (typesCount != 0 && types.stream().filter(ClassNode::isInterface).count() == 0)) { return types.get(0); } else if (typesCount > 1) { - return new UnionTypeClassNode(types.toArray(ClassNode.EMPTY_ARRAY)); + String names = types.stream().map(StaticTypeCheckingSupport::prettyPrintType).collect(Collectors.joining(" & ", "(", ")")); + Map<Boolean, List<ClassNode>> ifacesAndClasses = types.stream().collect(Collectors.partitioningBy(ClassNode::isInterface)); + ClassNode sc = !ifacesAndClasses.get(Boolean.FALSE).isEmpty() ? ifacesAndClasses.get(Boolean.FALSE).get(0) : OBJECT_TYPE; + ClassNode[] is = ifacesAndClasses.get(Boolean.TRUE).toArray(ClassNode[]::new); +assert !isObjectType(sc) || (is.length > 1) : "expecting specific super class or multiple interfaces"; + return new ClassNode(names, Opcodes.ACC_ABSTRACT, sc, is, null); } } } diff --git a/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java b/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java index 5d6da2eb7f..e061ecef25 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/TypeCheckingContext.java @@ -116,6 +116,10 @@ public class TypeCheckingContext { */ protected Stack<Map<Object, List<ClassNode>>> temporaryIfBranchTypeInformation = new Stack<>(); + List<ClassNode> peekTemporaryTypeInfo(final Object o) { + return temporaryIfBranchTypeInformation.peek().computeIfAbsent(o, x -> new LinkedList<>()); + } + public void pushTemporaryTypeInfo() { temporaryIfBranchTypeInformation.push(new HashMap<>()); } diff --git a/src/main/java/org/codehaus/groovy/transform/stc/UnionTypeClassNode.java b/src/main/java/org/codehaus/groovy/transform/stc/UnionTypeClassNode.java index cb1377b9b5..108c12447a 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/UnionTypeClassNode.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/UnionTypeClassNode.java @@ -51,14 +51,15 @@ import static org.codehaus.groovy.ast.tools.WideningCategories.LowestUpperBoundC /** * This class node type is very special and should only be used by the static type checker * to represent types which are the union of other types. This is useful when, for example, - * we enter a section like : + * we enter a section like: * <pre>if (x instanceof A || x instanceof B)</pre> * where the type of <i>x</i> can be represented as one of <i>A</i> or <i>B</i>. - * + * <p> * This class node type should never leak outside of the type checker. More precisely, it should * only be used to check method call arguments, and nothing more. */ class UnionTypeClassNode extends ClassNode { + private final ClassNode[] delegates; UnionTypeClassNode(final ClassNode... classNodes) { @@ -68,7 +69,7 @@ class UnionTypeClassNode extends ClassNode { } private static String makeName(final ClassNode[] nodes) { - StringJoiner sj = new StringJoiner("+", "<UnionType:", ">"); + var sj = new StringJoiner("+", "<UnionType:", ">"); for (ClassNode node : nodes) { sj.add(node.getText()); } @@ -78,7 +79,7 @@ class UnionTypeClassNode extends ClassNode { private static ClassNode makeSuper(final ClassNode[] nodes) { ClassNode upper = lowestUpperBound(Arrays.asList(nodes)); if (upper instanceof LowestUpperBoundClassNode) { - upper = upper.getUnresolvedSuperClass(); + upper = upper.getUnresolvedSuperClass(false); } else if (upper.isInterface()) { upper = OBJECT_TYPE; } @@ -87,7 +88,7 @@ class UnionTypeClassNode extends ClassNode { //-------------------------------------------------------------------------- - public ClassNode[] getDelegates() { + ClassNode[] getDelegates() { return delegates; } @@ -384,10 +385,7 @@ class UnionTypeClassNode extends ClassNode { @Override public boolean isDerivedFrom(final ClassNode type) { - for (ClassNode delegate : delegates) { - if (delegate.isDerivedFrom(type)) return true; - } - return false; + return getUnresolvedSuperClass(false).isDerivedFrom(type); } @Override diff --git a/src/test/groovy/bugs/Groovy8337Bug.groovy b/src/test/groovy/bugs/Groovy8337Bug.groovy deleted file mode 100644 index 365fa7d0aa..0000000000 --- a/src/test/groovy/bugs/Groovy8337Bug.groovy +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package bugs - -import gls.CompilableTestSupport -import groovy.test.NotYetImplemented - -class Groovy8337Bug extends CompilableTestSupport { - void testGroovy8337() { - assertScript ''' - import groovy.transform.CompileStatic - @CompileStatic - class Static { - private Number n - BigDecimal meth() { - return n == null || n instanceof BigDecimal ? n : new BigDecimal(n.toString()) - } - } - assert null == new Static().meth() - ''' - } - - void testGroovy8337WithFieldValue() { - assertScript ''' - import groovy.transform.CompileStatic - @CompileStatic - class Static { - private Number n = new BigDecimal('1.23') - BigDecimal meth() { - return n == null || n instanceof BigDecimal ? n : new BigDecimal(n.toString()) - } - } - assert new BigDecimal('1.23') == new Static().meth() - ''' - } -} diff --git a/src/test/groovy/groovy/transform/stc/MethodCallsSTCTest.groovy b/src/test/groovy/groovy/transform/stc/MethodCallsSTCTest.groovy index 2b430400eb..15a06889ed 100644 --- a/src/test/groovy/groovy/transform/stc/MethodCallsSTCTest.groovy +++ b/src/test/groovy/groovy/transform/stc/MethodCallsSTCTest.groovy @@ -608,9 +608,10 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { ''' } + // GROOVY-5226 void testMethodCallArgumentUsingInstanceOf() { assertScript ''' - void foo(String str) { 'String' } + void foo(String str) { } def o if (o instanceof String) { foo(o) @@ -643,7 +644,8 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { 'Cannot find matching method' } - void testShouldNotFailThanksToInstanceOfChecks() { + // GROOVY-5226 + void testShouldNotFailWithExplicitCasts() { assertScript ''' static String foo(String s) { 'String' @@ -656,17 +658,18 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { } ['foo',123,true].each { if (it instanceof String) { - foo((String)it) - } else if (it instanceof Boolean) { - foo((Boolean)it) + foo((String) it) } else if (it instanceof Integer) { - foo((Integer)it) + foo((Integer) it) + } else if (it instanceof Boolean) { + foo((Boolean) it) } } ''' } - void testShouldNotFailThanksToInstanceOfChecksAndWithoutExplicitCasts() { + // GROOVY-5226 + void testShouldNotFailWithInstanceOfChecks() { assertScript ''' static String foo(String s) { 'String' @@ -680,16 +683,17 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { ['foo',123,true].each { if (it instanceof String) { foo(it) - } else if (it instanceof Boolean) { - foo(it) } else if (it instanceof Integer) { foo(it) + } else if (it instanceof Boolean) { + foo(it) } } ''' } - void testShouldNotFailThanksToInstanceOfChecksAndWithoutExplicitCasts2() { + // GROOVY-5226 + void testShouldNotFailWithInstanceOfChecks2() { assertScript ''' static String foo(String s) { 'String' @@ -703,15 +707,16 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { ['foo',123,true].each { argument -> if (argument instanceof String) { foo(argument) - } else if (argument instanceof Boolean) { - foo(argument) } else if (argument instanceof Integer) { foo(argument) + } else if (argument instanceof Boolean) { + foo(argument) } } ''' } + // GROOVY-5226 void testShouldFailWithMultiplePossibleMethods() { shouldFailWithMessages ''' static String foo(String s) { @@ -724,14 +729,15 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { 'Boolean' } ['foo',123,true].each { - if (it instanceof String || it instanceof Boolean || it instanceof Integer) { + if (it instanceof String || it instanceof Integer || it instanceof Boolean) { foo(it) } } ''', - 'Reference to method is ambiguous' + 'foo' } + // GROOVY-5226 void testShouldFailWithMultiplePossibleMethods2() { shouldFailWithMessages ''' static String foo(String s) { @@ -744,12 +750,12 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { 'Boolean' } ['foo',123,true].each { argument -> - if (argument instanceof String || argument instanceof Boolean || argument instanceof Integer) { + if (argument instanceof String || argument instanceof Integer || argument instanceof Boolean) { foo(argument) } } ''', - 'Reference to method is ambiguous' + 'foo' } // GROOVY-5703 @@ -875,6 +881,17 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { ''', 'Cannot find matching method','#foo(java.util.Date)', 'Cannot find matching method','#bar(java.util.Date)' + + shouldFailWithMessages foo + ''' + def it = ~/regexp/ + if (it !instanceof String) { + it = 123 + Integer.toHextString(it) + } else { // cannot be String + foo(it) + } + ''', + 'Cannot find matching method','#foo(java.util.regex.Pattern)' } // GROOVY-5226, GROOVY-11290 @@ -903,16 +920,6 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase { } } ''' - - assertScript foobar + ''' - def it = ~/regexp/ - if (it !instanceof String) { - it = 123 - foo(it) - } else { - bar(it) - } - ''' } // GROOVY-5226, GROOVY-11290 diff --git a/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy b/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy index dbf1cf374e..57995cfee9 100644 --- a/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy +++ b/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy @@ -18,7 +18,6 @@ */ package groovy.transform.stc -import groovy.test.NotYetImplemented import org.codehaus.groovy.ast.ClassHelper import org.codehaus.groovy.ast.ClassNode import org.codehaus.groovy.ast.MethodNode @@ -145,8 +144,7 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { void bar() { } } - - void test(o) { + void test(Object o) { if (o instanceof A) { o.bar() } @@ -164,10 +162,10 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { assertScript ''' class A { } - A test(Object x) { - if (x instanceof A) { - def y = x - return y + A test(Object o) { + if (o instanceof A) { + def v = o + return v } else { new A() } @@ -176,8 +174,26 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { ''' } - // GROOVY-9454 + // GROOVY-8828 void testInstanceOf7() { + assertScript ''' + interface Foo { + } + interface Bar { + String name() + } + Map<String, Foo> map = [:] + map.values().each { foo -> + if (foo instanceof Bar) { + String name = foo.name() // method available through Bar + map.put(name, foo) // second parameter expects Foo + } + } + ''' + } + + // GROOVY-9454 + void testInstanceOf8() { assertScript ''' interface Face { } @@ -186,7 +202,6 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { } class Task<R extends Face> implements java.util.concurrent.Callable<String> { R request - @Override String call() { if (request instanceof Impl) { @@ -204,29 +219,27 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { } // GROOVY-10667 - void testInstanceOf8() { + void testInstanceOf9() { assertScript ''' trait Tagged { String tag } class TaggedException extends Exception implements Tagged { } - static void doSomething1(Exception e) { if (e instanceof Tagged) { - //println e.tag + assert e.tag == 'Test' doSomething2(e) // Cannot find matching method #doSomething2(Tagged) } } static void doSomething2(Exception e) { } - doSomething1(new TaggedException(tag:'Test')) ''' } // GROOVY-7971 - void testInstanceOf9() { + void testInstanceOf10() { assertScript ''' import groovy.json.JsonOutput def test(value) { @@ -242,9 +255,33 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { ''' } + // GROOVY-8337 + void testInstanceOf11() { + assertScript ''' + class C { + private Number n + BigDecimal m() { + (n == null || n instanceof BigDecimal) + ? n : new BigDecimal(n.toString()) + } + } + assert new C().m() == null + ''' + + assertScript ''' + class C { + private Number n = new BigDecimal('1.23') + BigDecimal m() { + (n == null || n instanceof BigDecimal) + ? n : new BigDecimal(n.toString()) + } + } + assert new C().m() == 1.23G + ''' + } + // GROOVY-10096 - @NotYetImplemented - void testInstanceOf10() { + void testInstanceOf12() { shouldFailWithMessages ''' class Foo { void foo() { @@ -262,14 +299,13 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { } // GROOVY-11007 - void testInstanceOf11() { + void testInstanceOf13() { assertScript ''' interface I { CharSequence getCharSequence() } - - void accept(CharSequence cs) { } - + void accept(CharSequence cs) { + } void test(I i) { i.with { if (charSequence instanceof String) { @@ -278,27 +314,25 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { } } } - test({ -> 'works' } as I) ''' } // GROOVY-11290 - void testInstanceOf12() { + void testInstanceOf14() { assertScript ''' def test(List<String> list) { if (list instanceof List) { (list*.toLowerCase()).join() } } - String result = test(['foo', 'bar']) assert result == 'foobar' ''' } // GROOVY-11815 - void testInstanceOf13() { + void testInstanceOf15() { assertScript ''' def c = true ? new ArrayDeque() : new Stack() if (c instanceof Deque) { @@ -313,7 +347,7 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { // ^ (AbstractCollection<String> & Serializable & ... & Deque) } ''', - 'Cannot call <UnionType:java.util.AbstractCollection','#add(java.lang.String) with arguments [int]' + 'Cannot call (java.util.AbstractCollection','#add(java.lang.String) with arguments [int]' } // GROOVY-5226 @@ -497,22 +531,6 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { } } - // GROOVY-8828 - void testMultipleInstanceOf8() { - assertScript ''' - interface Foo { } - interface Bar { String name() } - - Map<String, Foo> map = [:] - map.values().each { foo -> - if (foo instanceof Bar) { - String name = foo.name() // method available through Bar - map.put(name, foo) // second parameter expects Foo - } - } - ''' - } - // GROOVY-6429 void testNotInstanceof1() { String types = ''' @@ -679,11 +697,11 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { void testInstanceOfInferenceWithImplicitIt() { assertScript ''' - ['a', 'b', 'c'].each { - if (it instanceof String) { - println it.toUpperCase() + ['a', 'b', 'c'].each { + if (it instanceof String) { + println it.toUpperCase() + } } - } ''' } @@ -1249,7 +1267,7 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase { case File: something.toString() default: - something.getCanonicalName() + something.canonicalName // TODO: GROOVY-10702 } } ''', diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy index b7ae38f82b..ee3a9b19d8 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy @@ -18,17 +18,10 @@ */ package org.codehaus.groovy.classgen.asm.sc -import groovy.test.NotYetImplemented import groovy.transform.stc.TypeInferenceSTCTest /** * Unit tests for static compilation : type inference. */ -class TypeInferenceStaticCompileTest extends TypeInferenceSTCTest implements StaticCompilationTestSupport { - - @Override - @NotYetImplemented - void testInstanceOf9() { - super.testInstanceOf9() // GROOVY-7971 - } +final class TypeInferenceStaticCompileTest extends TypeInferenceSTCTest implements StaticCompilationTestSupport { }
