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 2026/09/09 22:50:22 UTC
(groovy) branch master updated: GROOVY-12000: STC: instanceof or combined with not
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new ca07019cab GROOVY-12000: STC: instanceof or combined with not
ca07019cab is described below
commit ca07019cab843c865a79bc6c368529fa59a1ca27
Author: Paul King <pa...@asert.com.au>
AuthorDate: Thu Sep 10 00:15:55 2026 +1000
GROOVY-12000: STC: instanceof or combined with not
---
.../transform/stc/StaticTypeCheckingVisitor.java | 102 ++++++++++++-
.../transform/stc/TypeInferenceSTCTest.groovy | 161 +++++++++++++++++++++
2 files changed, 257 insertions(+), 6 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 b5292ecba1..50709d20ff 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -936,8 +936,23 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
public void visitNotExpression(final NotExpression expression) {
typeCheckingContext.pushTemporaryTypeInfo();
super.visitNotExpression(expression);
+ var tti = typeCheckingContext.temporaryIfBranchTypeInformation.pop();
+ // GROOVY-12000: a negative ("!instanceof") entry may only be re-flipped into a positive
+ // smart-cast when the operand is itself a single negated check, or an "||" whose operands
+ // pin their negatives down; "!(L && R)" only guarantees "!L || !R", so drop them there
+ Expression operand = stripParentheses(expression.getExpression());
+ if (!(operand instanceof NotExpression) && !isBinaryOperation(operand, COMPARE_NOT_INSTANCEOF, LOGICAL_OR)) {
+ List<ClassNode> types = new ArrayList<>();
+ Object key = negatedInstanceOfConjunctionKey(operand, types);
+ if (key != null) { // !(x !instanceof A && x !instanceof B) is (x instanceof A || x instanceof B)
+ tti.clear();
+ typeCheckingContext.peekTemporaryTypeInfo(key).add(newUnionTypeClassNode(types));
+ } else {
+ tti.keySet().removeIf(k -> k instanceof Object[]);
+ }
+ }
// GROOVY-9455: !(x instanceof T) shouldn't propagate T as inferred type
- typeCheckingContext.temporaryIfBranchTypeInformation.pop().forEach(this::putNotInstanceOfTypeInfo);
+ tti.forEach(this::putNotInstanceOfTypeInfo);
}
/** {@inheritDoc} */
@@ -1036,7 +1051,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
rightExpression.visit(this);
var rhs = typeCheckingContext.temporaryIfBranchTypeInformation.pop();
- propagateTemporaryTypeInfo(lhs, rhs); // `instanceof` on either side?
+ propagateTemporaryTypeInfo(lhs, rhs, leftExpression, rightExpression); // `instanceof` on either side?
}
}
@@ -1160,6 +1175,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
boolean compatible = rType.isInterface();
for (int i = 0; !compatible && i < union.getDelegates().length; i += 1) {
ClassNode delegate = union.getDelegates()[i];
+ if (delegate instanceof WideningCategories.LowestUpperBoundClassNode) delegate = delegate.getUnresolvedSuperClass(); // GROOVY-12000
compatible = delegate.isInterface() || delegate.isDerivedFrom(rType) || rType.isDerivedFrom(delegate);
}
if (!compatible) {
@@ -1186,9 +1202,14 @@ 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)
+ private void propagateTemporaryTypeInfo(final Map<Object, List<ClassNode>> lhs, final Map<Object, List<ClassNode>> rhs,
+ final Expression leftExpression, final Expression rightExpression) {
+ // GROOVY-12000: (x !instanceof A || x !instanceof B) narrows x to A & B in the else branch; keep the
+ // negative entries of each operand whose falsity pins them down, merged per key so that the else-branch
+ // sign-flip yields the intersection -- the entries are inert in the then branch (never read positively)
+ var negatives = new LinkedHashMap<Object, List<ClassNode>>();
+ if (canInvertNarrowingForElseBranch(leftExpression)) collectNegativeTypeInfo(lhs, negatives);
+ if (canInvertNarrowingForElseBranch(rightExpression)) collectNegativeTypeInfo(rhs, negatives);
lhs.keySet().removeIf(k -> k instanceof Object[]);
rhs.keySet().removeIf(k -> k instanceof Object[]);
@@ -1214,6 +1235,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
typeCheckingContext.peekTemporaryTypeInfo(v).add(newUnionTypeClassNode(types));
}
}
+
+ negatives.forEach(this::putNotInstanceOfTypeInfo);
+ }
+
+ private static void collectNegativeTypeInfo(final Map<Object, List<ClassNode>> tti, final Map<Object, List<ClassNode>> negatives) {
+ for (var entry : tti.entrySet()) {
+ if (entry.getKey() instanceof Object[] arr) {
+ negatives.computeIfAbsent(arr[1], k -> new ArrayList<>()).addAll(entry.getValue());
+ }
+ }
}
private void validateResourceInARM(final BinaryExpression expression, final ClassNode lType) {
@@ -7592,6 +7623,47 @@ out: for (ClassNode type : todo) {
return true;
}
+ private static Expression stripParentheses(Expression expression) {
+ while (expression instanceof BooleanExpression be && !(expression instanceof NotExpression)) {
+ expression = be.getExpression();
+ }
+ return expression;
+ }
+
+ private static boolean isBinaryOperation(final Expression expression, final int... operations) {
+ if (expression instanceof BinaryExpression be) {
+ int op = be.getOperation().getType();
+ for (int operation : operations) {
+ if (op == operation) return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * GROOVY-12000: If {@code expression} is a conjunction ({@code &&} or {@code &})
+ * of negated instanceof checks -- {@code !(x instanceof T)} or {@code x !instanceof T} --
+ * that all test the same variable or property, returns that variable's temporary type
+ * info key and adds the tested types to {@code types} in source order; otherwise returns
+ * null. Such a conjunction under a {@code !} is, by De Morgan, a disjunction of positive
+ * checks: {@code !(!(x instanceof A) && !(x instanceof B))} narrows {@code x} to {@code A | B}.
+ */
+ private Object negatedInstanceOfConjunctionKey(final Expression expression, final List<ClassNode> types) {
+ Expression expr = stripParentheses(expression);
+ if (expr instanceof NotExpression ne) {
+ expr = stripParentheses(ne.getExpression());
+ if (!isBinaryOperation(expr, KEYWORD_INSTANCEOF)) return null;
+ } else if (isBinaryOperation(expr, LOGICAL_AND, BITWISE_AND)) {
+ Object left = negatedInstanceOfConjunctionKey(((BinaryExpression) expr).getLeftExpression(), types);
+ return left != null && left.equals(negatedInstanceOfConjunctionKey(((BinaryExpression) expr).getRightExpression(), types)) ? left : null;
+ } else if (!isBinaryOperation(expr, COMPARE_NOT_INSTANCEOF)) {
+ return null;
+ }
+ BinaryExpression check = (BinaryExpression) expr;
+ types.add(check.getRightExpression().getType());
+ return extractTemporaryTypeInfoKey(check.getLeftExpression());
+ }
+
/**
* Computes the key to use for {@link TypeCheckingContext#temporaryIfBranchTypeInformation}.
*/
@@ -7682,7 +7754,25 @@ out: for (ClassNode type : todo) {
}
private static ClassNode newIntersectionTypeClassNode(final Collection<ClassNode> types) {
- Map<Boolean, List<ClassNode>> spec = types.stream().collect(Collectors.partitioningBy(ClassNode::isInterface));
+ int i = 0; // GROOVY-12000: distribute over a union so each alternative stands alone: T & (A | B) is (T & A) | (T & B)
+ for (ClassNode type : types) {
+ if (type instanceof UnionTypeClassNode union) {
+ List<ClassNode> alternatives = new ArrayList<>();
+ for (ClassNode delegate : union.getDelegates()) {
+ List<ClassNode> copy = new ArrayList<>(types);
+ copy.set(i, delegate);
+ ClassNode alternative = newIntersectionTypeClassNode(copy);
+ if (alternatives.stream().noneMatch(a -> implementsInterfaceOrIsSubclassOf(alternative, a))) {
+ alternatives.removeIf(a -> implementsInterfaceOrIsSubclassOf(a, alternative)); // subsumed
+ alternatives.add(alternative);
+ }
+ }
+ return newUnionTypeClassNode(alternatives);
+ }
+ i += 1;
+ }
+ Map<Boolean, List<ClassNode>> spec = types.stream().filter(t -> types.stream().noneMatch(o -> o != t // drop members implied by another member
+ && implementsInterfaceOrIsSubclassOf(o, t) && !implementsInterfaceOrIsSubclassOf(t, o))).collect(Collectors.partitioningBy(ClassNode::isInterface));
ClassNode[] interfaces = spec.get(Boolean.TRUE ).toArray(ClassNode[]::new);
List<ClassNode> supers = spec.get(Boolean.FALSE);
if (interfaces.length == 0) return supers.get(0);
diff --git a/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy b/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy
index 5ee12e50c6..9445a88dcc 100644
--- a/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy
+++ b/src/test/groovy/groovy/transform/stc/TypeInferenceSTCTest.groovy
@@ -1166,6 +1166,167 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-12000: a !instanceof inside !(... && ...) must not produce a positive smart-cast in the then branch;
+ // !(cond && !(x instanceof T)) is true when cond is false, whatever x is
+ @Test
+ void testNotInstanceof10() {
+ for (test in ['!(x instanceof String)', 'x !instanceof String']) {
+ assertScript """
+ @groovy.transform.CompileStatic
+ class T {
+ static int f(Object x, boolean cond) {
+ if (!(cond && $test)) {
+ return x.hashCode() // x is some Object -- no checkcast
+ } else {
+ return -1
+ }
+ }
+ }
+ assert T.f(42, false) == Integer.valueOf(42).hashCode()
+ assert T.f('s', false) == 's'.hashCode()
+ assert T.f('s', true) == 's'.hashCode()
+ assert T.f(42, true) == -1
+ """
+ // ternary form
+ assertScript """
+ @groovy.transform.CompileStatic
+ class T {
+ static int g(Object x, boolean cond) {
+ return !(cond && $test) ? x.hashCode() : -1
+ }
+ }
+ assert T.g(42, false) == Integer.valueOf(42).hashCode()
+ assert T.g(42, true) == -1
+ """
+ shouldFailWithMessages """
+ @groovy.transform.TypeChecked
+ void test(Object x, boolean cond) {
+ if (!(cond && $test)) {
+ x.length()
+ }
+ }
+ """,
+ 'Cannot find matching method java.lang.Object#length()'
+ }
+ }
+
+ // GROOVY-12000: De Morgan: !(!(x instanceof A) && !(x instanceof B)) is (x instanceof A || x instanceof B)
+ @Test
+ void testNotInstanceof11() {
+ for (test in [['!(x instanceof CharSequence)', '!(x instanceof Number)'], ['x !instanceof CharSequence', 'x !instanceof Number']]) {
+ assertScript """
+ @groovy.transform.CompileStatic
+ class T {
+ static String f(Object x) {
+ if (!(${test[0]} && ${test[1]})) {
+ return x.toString() // (CharSequence | Number)
+ } else {
+ return 'neither'
+ }
+ }
+ }
+ assert T.f('s') == 's'
+ assert T.f(42) == '42'
+ assert T.f([]) == 'neither'
+ """
+ shouldFailWithMessages """
+ @groovy.transform.TypeChecked
+ void test(Object x) {
+ if (!(${test[0]} && ${test[1]})) {
+ x.intValue() // not an intersection
+ }
+ }
+ """,
+ 'Cannot find matching method (java.lang.CharSequence | java.lang.Number)#intValue()'
+ }
+ }
+
+ // GROOVY-12000: (x !instanceof A || x !instanceof B) narrows x to A & B in the else branch
+ @Test
+ void testNotInstanceof12() {
+ for (test in ['!(x instanceof CharSequence) || !(x instanceof Comparable)', 'x !instanceof CharSequence || x !instanceof Comparable']) {
+ assertScript """
+ @groovy.transform.CompileStatic
+ class T {
+ static int f(Object x) {
+ if ($test) {
+ return -1
+ } else {
+ return x.length() + x.compareTo('ab')
+ }
+ }
+ static int g(Object x) {
+ if ($test) return -1
+ return x.length()
+ }
+ }
+ assert T.f('ab') == 2
+ assert T.f(42) == -1
+ assert T.g('ab') == 2
+ assert T.g([]) == -1
+ """
+ shouldFailWithMessages """
+ @groovy.transform.TypeChecked
+ void test(Object x) {
+ if ($test) {
+ x.length()
+ }
+ }
+ """,
+ 'Cannot find matching method java.lang.Object#length()'
+ }
+ }
+
+ // GROOVY-12000: union of interfaces on a class-typed receiver is checked per alternative:
+ // Number & (Cloneable | Closeable) is (Number & Cloneable) | (Number & Closeable)
+ @Test
+ void testMultipleInstanceOfWithClassTypedReceiver() {
+ for (test in ['number instanceof Cloneable || number instanceof Closeable', '!(number instanceof Cloneable || number instanceof Closeable)']) {
+ String then = test.startsWith('!') ? 'return -number.intValue()' : 'return number.intValue()'
+ String other = test.startsWith('!') ? 'return number.intValue()' : 'return -number.intValue()'
+ assertScript """
+ class N extends Number implements Cloneable {
+ int intValue() { 7 }
+ long longValue() { 7 }
+ float floatValue() { 7 }
+ double doubleValue() { 7 }
+ }
+ @groovy.transform.CompileStatic
+ int test(Number number) {
+ if ($test) {
+ $then
+ } else {
+ $other
+ }
+ }
+ assert test(new N()) == 7
+ assert test(42) == -42
+ """
+ String call = test.startsWith('!') ? 'if ($test) { } else { number.close() }' : 'if ($test) { number.close() }'
+ shouldFailWithMessages """
+ @groovy.transform.TypeChecked
+ void test(Number number) {
+ ${call.replace('$test', test)}
+ }
+ """,
+ 'Cannot find matching method ((java.lang.Number & java.lang.Cloneable) | (java.lang.Number & java.io.Closeable))#close()'
+ }
+ // a nested instanceof narrows every alternative; (Integer & Cloneable) | Integer is Integer
+ assertScript '''
+ @groovy.transform.CompileStatic
+ int test(Number number) {
+ if (number instanceof Cloneable || number instanceof Comparable) {
+ if (number instanceof Integer) {
+ return number + 2
+ }
+ }
+ return -1
+ }
+ assert test(3) == 5
+ assert test(3.5d) == -1
+ '''
+ }
+
// GROOVY-10217
@Test
void testInstanceOfThenSubscriptOperator() {