From 3e6c7aad7793ab82d65427a3dc790035f47494e7 Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran Date: Fri, 27 Sep 2024 11:28:01 +0530 Subject: [PATCH] A sealed interface with generic causes IllegalStateException and nothing can be done then * Fixes https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3009 --- .../compiler/lookup/BinaryTypeBinding.java | 36 +- .../internal/compiler/lookup/ClassScope.java | 2 +- .../compiler/lookup/MissingTypeBinding.java | 2 +- .../lookup/ParameterizedTypeBinding.java | 81 +++- .../compiler/lookup/ReferenceBinding.java | 5 - .../regression/SwitchPatternTest.java | 414 ++++++++++++++++++ 6 files changed, 498 insertions(+), 42 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java index 99bc6086c5c..0f19923e7ff 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/BinaryTypeBinding.java @@ -102,7 +102,7 @@ public class BinaryTypeBinding extends ReferenceBinding { protected ReferenceBinding superclass; protected ReferenceBinding enclosingType; protected ReferenceBinding[] superInterfaces; - protected ReferenceBinding[] permittedSubtypes; + protected ReferenceBinding[] permittedTypes; protected FieldBinding[] fields; protected RecordComponentBinding[] components; protected MethodBinding[] methods; @@ -269,7 +269,7 @@ public BinaryTypeBinding(BinaryTypeBinding prototype) { this.superclass = prototype.superclass; this.enclosingType = prototype.enclosingType; this.superInterfaces = prototype.superInterfaces; - this.permittedSubtypes = prototype.permittedSubtypes; + this.permittedTypes = prototype.permittedTypes; this.fields = prototype.fields; this.components = prototype.components; this.methods = prototype.methods; @@ -449,7 +449,7 @@ void cachePartsFrom(IBinaryType binaryType, boolean needFieldsAndMethods) { // and still want to use binaries passed that point (e.g. type hierarchy resolver, see bug 63748). this.typeVariables = Binding.NO_TYPE_VARIABLES; this.superInterfaces = Binding.NO_SUPERINTERFACES; - this.permittedSubtypes = Binding.NO_PERMITTEDTYPES; + this.permittedTypes = Binding.NO_PERMITTEDTYPES; // must retrieve member types in case superclass/interfaces need them this.memberTypes = Binding.NO_MEMBER_TYPES; @@ -556,7 +556,7 @@ void cachePartsFrom(IBinaryType binaryType, boolean needFieldsAndMethods) { this.tagBits |= TagBits.HasUnresolvedSuperinterfaces; } - this.permittedSubtypes = Binding.NO_PERMITTEDTYPES; + this.permittedTypes = Binding.NO_PERMITTEDTYPES; if (!wrapper.atEnd()) { // attempt to find each permitted type if it exists in the cache (otherwise - resolve it when requested) java.util.ArrayList types = new java.util.ArrayList(2); @@ -564,22 +564,22 @@ void cachePartsFrom(IBinaryType binaryType, boolean needFieldsAndMethods) { do { types.add(this.environment.getTypeFromTypeSignature(wrapper, typeVars, this, missingTypeNames, toplevelWalker.toSupertype(rank++, wrapper.peekFullType()))); } while (!wrapper.atEnd()); - this.permittedSubtypes = new ReferenceBinding[types.size()]; - types.toArray(this.permittedSubtypes); + this.permittedTypes = new ReferenceBinding[types.size()]; + types.toArray(this.permittedTypes); this.extendedTagBits |= ExtendedTagBits.HasUnresolvedPermittedSubtypes; } } // fall back, in case we haven't got them from signature char[][] permittedSubtypeNames = binaryType.getPermittedSubtypeNames(); - if (this.permittedSubtypes == Binding.NO_PERMITTEDTYPES && permittedSubtypeNames != null) { + if (this.permittedTypes == Binding.NO_PERMITTEDTYPES && permittedSubtypeNames != null) { this.modifiers |= ExtraCompilerModifiers.AccSealed; int size = permittedSubtypeNames.length; if (size > 0) { - this.permittedSubtypes = new ReferenceBinding[size]; + this.permittedTypes = new ReferenceBinding[size]; for (short i = 0; i < size; i++) // attempt to find each superinterface if it exists in the cache (otherwise - resolve it when requested) - this.permittedSubtypes[i] = this.environment.getTypeFromConstantPoolName(permittedSubtypeNames[i], 0, -1, false, missingTypeNames, toplevelWalker.toSupertype(i, null)); + this.permittedTypes[i] = this.environment.getTypeFromConstantPoolName(permittedSubtypeNames[i], 0, -1, false, missingTypeNames, toplevelWalker.toSupertype(i, null)); } } boolean canUseNullTypeAnnotations = this.environment.globalOptions.isAnnotationBasedNullAnalysisEnabled && this.environment.globalOptions.sourceLevel >= ClassFileConstants.JDK1_8; @@ -593,7 +593,7 @@ void cachePartsFrom(IBinaryType binaryType, boolean needFieldsAndMethods) { break; } } - for (TypeBinding permsub : this.permittedSubtypes) { + for (TypeBinding permsub : this.permittedTypes) { if (permsub.hasNullTypeAnnotations()) { this.externalAnnotationStatus = ExternalAnnotationStatus.TYPE_IS_ANNOTATED; break; @@ -2581,13 +2581,13 @@ public ReferenceBinding[] superInterfaces() { public ReferenceBinding[] permittedTypes() { if (!isPrototype()) { - return this.permittedSubtypes = this.prototype.permittedTypes(); + return this.permittedTypes = this.prototype.permittedTypes(); } - for (int i = this.permittedSubtypes.length; --i >= 0;) - this.permittedSubtypes[i] = (ReferenceBinding) resolveType(this.permittedSubtypes[i], this.environment, false); + for (int i = this.permittedTypes.length; --i >= 0;) + this.permittedTypes[i] = (ReferenceBinding) resolveType(this.permittedTypes[i], this.environment, false); // Note: unlike for superinterfaces() hierarchy check not required here since these are subtypes - return this.permittedSubtypes; + return this.permittedTypes; } @Override public TypeVariableBinding[] typeVariables() { @@ -2657,13 +2657,13 @@ public String toString() { buffer.append("NULL SUPERINTERFACES"); //$NON-NLS-1$ } - if (this.permittedSubtypes != null) { - if (this.permittedSubtypes != Binding.NO_PERMITTEDTYPES) { + if (this.permittedTypes != null) { + if (this.permittedTypes != Binding.NO_PERMITTEDTYPES) { buffer.append("\n\tpermits : "); //$NON-NLS-1$ - for (int i = 0, length = this.permittedSubtypes.length; i < length; i++) { + for (int i = 0, length = this.permittedTypes.length; i < length; i++) { if (i > 0) buffer.append(", "); //$NON-NLS-1$ - buffer.append((this.permittedSubtypes[i] != null) ? this.permittedSubtypes[i].debugName() : "NULL TYPE"); //$NON-NLS-1$ + buffer.append((this.permittedTypes[i] != null) ? this.permittedTypes[i].debugName() : "NULL TYPE"); //$NON-NLS-1$ } } } else { diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java index ff7316ff965..c505f0e2eb0 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ClassScope.java @@ -1698,7 +1698,7 @@ private ReferenceBinding findPermittedtype(TypeReference typeReference) { } typeReference.bits |= ASTNode.IgnoreRawTypeCheck; ReferenceBinding permittedType = (ReferenceBinding) typeReference.resolveType(this); - return permittedType; + return permittedType != null ? permittedType.actualType() : permittedType; // while permitted classes/interfaces cannot be parameterized with type arguments, they are not raw either } catch (AbortCompilation e) { SourceTypeBinding sourceType = this.referenceContext.binding; if (sourceType.permittedTypes == null) sourceType.setPermittedTypes(Binding.NO_PERMITTEDTYPES); diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MissingTypeBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MissingTypeBinding.java index aa1d0f17843..093d1c69ce3 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MissingTypeBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MissingTypeBinding.java @@ -35,7 +35,7 @@ public MissingTypeBinding(PackageBinding packageBinding, char[][] compoundName, this.modifiers = ClassFileConstants.AccPublic; this.superclass = null; // will be fixed up using #setMissingSuperclass(...) this.superInterfaces = Binding.NO_SUPERINTERFACES; - this.permittedSubtypes = Binding.NO_PERMITTEDTYPES; + this.permittedTypes = Binding.NO_PERMITTEDTYPES; this.typeVariables = Binding.NO_TYPE_VARIABLES; this.memberTypes = Binding.NO_MEMBER_TYPES; this.fields = Binding.NO_FIELDS; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java index 32df901927e..ec3365a642e 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java @@ -49,7 +49,9 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import org.eclipse.jdt.core.compiler.CharOperation; @@ -1108,26 +1110,71 @@ public boolean isRawSubstitution() { @Override public ReferenceBinding[] permittedTypes() { - ReferenceBinding[] permTypes = this.type.permittedTypes(); - List applicablePermTypes = new ArrayList<>(); - for (ReferenceBinding pt : permTypes) { - ReferenceBinding permittedTypeAvatar = pt; - if (pt.isRawType()) { - ReferenceBinding ptRef = pt.actualType(); - ReferenceBinding enclosingType1 = ptRef.enclosingType(); - if (enclosingType1 != null) { - // don't use TypeSystem.getParameterizedType below as this is just for temporary check. - ParameterizedTypeBinding ptb = new ParameterizedTypeBinding(ptRef, this.arguments, ptRef.enclosingType(), this.environment); - ptb.superclass(); - ptb.superInterfaces(); - permittedTypeAvatar = ptb; + List permittedTypes = new ArrayList<>(); +NextPermittedType: + for (ReferenceBinding pt : this.type.permittedTypes()) { + // Step 1: Gather all type variables that would need to be solved. + Map map = new HashMap<>(); + TypeBinding current = pt; + do { + if (current.kind() == Binding.GENERIC_TYPE) { + for (TypeVariableBinding tvb : current.typeVariables()) { + map.put(tvb, null); + } } - } - if (permittedTypeAvatar.isCompatibleWith(this)) - applicablePermTypes.add(pt); + current = current.enclosingType(); + } while (current != null); + + // Step 2: Collect substitutes + current = this; + TypeBinding sooper = pt.findSuperTypeOriginatingFrom(this); + do { + if (sooper.isParameterizedType()) { + if (current.isParameterizedType()) { + for (int i = 0, length = sooper.typeArguments().length; i < length; i++) { + TypeBinding t = sooper.typeArguments()[i]; + if (t instanceof TypeVariableBinding tvb) { + map.put(tvb, current.typeArguments()[i]); + } else if (TypeBinding.notEquals(t, this.typeArguments()[i])) { + continue NextPermittedType; + } + } + } + } + current = current.enclosingType(); + sooper = sooper.enclosingType(); + } while (current != null); + + Substitution substitution = new Substitution() { + @Override + public LookupEnvironment environment() { + return ParameterizedTypeBinding.this.environment; + } + @Override + public boolean isRawSubstitution() { + return false; + } + @Override + public TypeBinding substitute(TypeVariableBinding typeVariable) { + TypeBinding retVal = map.get(typeVariable.unannotated()); + if (retVal == null) { + retVal = ParameterizedTypeBinding.this.environment.createWildcard((ReferenceBinding) typeVariable.declaringElement, typeVariable.rank, null, null, Wildcard.UNBOUND); + map.put(typeVariable, retVal); + } + return retVal; + } + }; + + // Step 3: compute subtype with parameterizations if any. + pt = (ReferenceBinding) Scope.substitute(substitution, pt); + + if (pt.isCompatibleWith(this)) + permittedTypes.add(pt); } - return applicablePermTypes.toArray(new ReferenceBinding[0]); + + return permittedTypes.toArray(new ReferenceBinding[0]); } + @Override public TypeBinding unannotated() { return this.hasTypeAnnotations() ? this.environment.getUnannotatedType(this) : this; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java index 136dfe2c77a..52196d09c28 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java @@ -2105,11 +2105,6 @@ public ReferenceBinding superclass() { return null; } -@Override -public ReferenceBinding[] permittedTypes() { - return Binding.NO_PERMITTEDTYPES; -} - @Override public ReferenceBinding[] superInterfaces() { return Binding.NO_SUPERINTERFACES; diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java index 553b9044652..7b6bc2c4070 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchPatternTest.java @@ -8335,4 +8335,418 @@ public static void main(String[] args) { "Only record types are permitted in a record pattern\n" + "----------\n"); } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3009 + // A sealed interface with generic causes IllegalStateException and nothing can be done then + public void testIssue3009() { + runConformTest( + new String[] { + "X.java", + """ + public sealed interface X permits X.Y { + + default Object foo() { + return switch (this) { + case X x -> "OK!"; + }; + } + + sealed class Y implements X permits Y.Z { + static final class Z extends Y {} + } + + public static void main(String [] args) { + System.out.println(new Y().foo()); + } + } + """ + }, + "OK!"); + } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3009 + // A sealed interface with generic causes IllegalStateException and nothing can be done then + public void testIssue3009_2() { + runConformTest( + new String[] { + "Editable.java", + """ + import java.util.Objects; + import java.util.Optional; + import java.util.function.Consumer; + import java.util.function.Supplier; + + public sealed interface Editable permits Editable.NotEdited, Editable.Edited { + + public static void main(String [] args) { + System.out.println("OK!"); + } + + static Editable notEdited() { + return (Editable) NotEdited.NOT_EDITED; + } + + static Editable emptyValue() { + return (Editable) Edited.EditedWithoutValue.NO_VALUE; + } + + static Editable of(T value) { + return new Edited.EditedWithValue<>(value); + } + + static Editable ofNullable(T value) { + if (value == null) { + return emptyValue(); + } + + return new Edited.EditedWithValue<>(value); + } + + default boolean isEdited() { + return this instanceof Editable.Edited; + } + + default boolean isNotEdited() { + return this instanceof Editable.NotEdited; + } + + default Optional editedValue() { + return switch (this) { + case Edited edited -> Optional.of(edited).flatMap(Edited::value); + case NotEdited ignored -> Optional.empty(); + }; + } + + boolean hasNotChanged(T otherValue); + + Optional or(Supplier> supplier); + + void ifEdited(Consumer> action); + + final class NotEdited implements Editable { + + private static final Editable NOT_EDITED = new NotEdited<>(); + + private NotEdited() { + } + + @Override + public boolean hasNotChanged(T otherValue) { + return true; + } + + @Override + public Optional or(Supplier> supplier) { + Objects.requireNonNull(supplier); + + return supplier.get(); + } + + @Override + public void ifEdited(Consumer> action) { + // Nothing to do + } + } + + abstract sealed class Edited implements Editable permits Edited.EditedWithoutValue, Edited.EditedWithValue { + + protected abstract Optional value(); + + @Override + public void ifEdited(Consumer> action) { + Objects.requireNonNull(action); + + action.accept(value()); + } + + @Override + public Optional or(Supplier> supplier) { + return value(); + } + + static final class EditedWithoutValue extends Edited { + + private static final Edited NO_VALUE = new EditedWithoutValue<>(); + + private EditedWithoutValue() { + } + + @Override + protected Optional value() { + return Optional.empty(); + } + + @Override + public boolean hasNotChanged(T otherValue) { + return otherValue == null; + } + } + + static final class EditedWithValue extends Edited { + + private final T value; + + private EditedWithValue(T value) { + this.value = Objects.requireNonNull(value); + } + + @Override + protected Optional value() { + return Optional.of(value); + } + + @Override + public boolean hasNotChanged(T otherValue) { + return Objects.equals(value, otherValue); + } + } + } + } + """ + }, + "OK!"); + } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3009 + // A sealed interface with generic causes IllegalStateException and nothing can be done then + public void testIssue3009_3() { + runConformTest( + new String[] { + "X.java", + """ + sealed interface J permits D, E, F, G, H {} + final class D implements J {} + final class E implements J {} + final class F implements J {} + final class G implements J {} + final class H implements J {} + + public class X { + static int testExhaustive2(J ji) { + return switch (ji) { // Exhaustive! + case E e -> 42; + case G e -> 420; + case H e -> 4200; + }; + } + public static void main(String[] args) { + J ji = new E<>(); + System.out.println(X.testExhaustive2(ji)); + ji = new G<>(); + System.out.println(X.testExhaustive2(ji)); + ji = new H<>(); + System.out.println(X.testExhaustive2(ji)); + } + } + """ + }, + "42\n420\n4200"); + } + + public void testIssue3009_4() { // FIXME: this should not compile!!! + runConformTest( + new String[] { + "X.java", + """ + sealed interface J permits D, E, F, G, H {} + final class D implements J {} + final class E implements J {} + final class F implements J {} + final class G implements J {} + final class H implements J {} + + public class X { + static int testExhaustive2(J ji) { + return switch (ji) { // Exhaustive! + case E e -> 42; + case G e -> 420; + case H e -> 4200; + }; + } + public static void main(String[] args) { + J ji = new E<>(); + System.out.println(X.testExhaustive2(ji)); + ji = new G<>(); + System.out.println(X.testExhaustive2(ji)); + ji = new H<>(); + System.out.println(X.testExhaustive2(ji)); + } + } + """ + }, + "42\n420\n4200"); + } + + public void testIssue3009_5() { + runConformTest( + new String[] { + "X.java", + """ + public class X { + + class O { + class M { + class I { + abstract sealed class J permits S {} + } + } + } + + final class S extends O.M.I.J { + S(O.M.I ei) { + ei.super(); + } + } + + static int testExhaustive(O.M.I.J ji) { + return switch (ji) { // Exhaustive! + case S e -> 42; + }; + } + + public static void main(String[] args) { + System.out.println(X.testExhaustive(new X().new S(new X().new O().new M().new I()))); + } + } + """ + }, + "42"); + } + + public void testIssue3009_6() { + runConformTest( + new String[] { + "X.java", + """ + abstract sealed class J permits X.S {} + + public class X { + + final class S extends J {} + + int testExhaustive(J ji) { + return switch (ji) { // Exhaustive! + case X.S e -> 420; + }; + } + public static void main(String[] args) { + X.S xs = null; + System.out.println(new X().testExhaustive(new X().new S())); + } + } + """ + }, + "420"); + } + + public void testIssue3009_7() { + runConformTest( + new String[] { + "X.java", + """ + public class X { + + class O { + class M { + class I { + abstract sealed class J permits W.NG.I.S { + } + } + } + } + + class W { + class NG { + class I { + final class S extends O.M.I.J { + S(O.M.I ei) { + ei.super(); + } + } + } + } + } + + static int testExhaustive(O.M.I.J ji) { + return switch (ji) { // Exhaustive! + case W.NG.I.S e -> 42; + }; + } + + public static void main(String[] args) { + System.out.println(X.testExhaustive(new X().new W().new NG().new I().new S( + new X().new O().new M().new I()))); + O.M.I.J ji = (W.NG.I.S) null; + } + } + """ + }, + "42"); + } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3031 + // [Switch][Sealed types] Incorrect exhaustiveness check leads to MatchException at runtime + public void testIssue3031() { + runNegativeTest( + new String[] { + "X.java", + """ + abstract sealed class J permits X.S, A {} + + final class A extends J {} + + public class X { + + final class S extends J {} + + int testExhaustive(J ji) { + return switch (ji) { // Exhaustive! + case A a -> 42; + //case X.S e -> 42; + }; + } + public static void main(String[] args) { + X.S xs = null; + System.out.println(new X().testExhaustive(new X().new S())); + } + } + """ + }, + "----------\n" + + "1. ERROR in X.java (at line 10)\n" + + " return switch (ji) { // Exhaustive!\n" + + " ^^\n" + + "A switch expression should have a default case\n" + + "----------\n"); + } + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3031 + // [Switch][Sealed types] Incorrect exhaustiveness check leads to MatchException at runtime + public void testIssue3031_2() { + runConformTest( + new String[] { + "X.java", + """ + abstract sealed class J permits X.S, A {} + + final class A extends J {} + + public class X { + + final class S extends J {} + + int testExhaustive(J ji) { + return switch (ji) { // Exhaustive! + case A a -> 42; + case X.S e -> 4200; + }; + } + public static void main(String[] args) { + X.S xs = null; + System.out.println(new X().testExhaustive(new X().new S())); + } + } + """ + }, + "4200"); + } }