From 978b393eea5eb5b475773d563b8b26bba15367c9 Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran <131454720+srikanth-sankaran@users.noreply.github.com> Date: Wed, 26 Jun 2024 10:21:28 +0530 Subject: [PATCH] [sealed types] ECJ accepts a cast from a disjoint interface to a sealed interface(#2634) * Fixes https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2595 --- .../jdt/internal/compiler/ast/Expression.java | 8 +- .../compiler/ast/SwitchStatement.java | 27 +--- .../compiler/lookup/ReferenceBinding.java | 88 +++++++++++ .../internal/compiler/lookup/TypeBinding.java | 9 ++ .../tests/compiler/regression/EnumTest.java | 42 ++++-- .../compiler/regression/SealedTypesTests.java | 139 ++++++++++++++++++ 6 files changed, 275 insertions(+), 38 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java index f5883da658b..778c906c217 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java @@ -462,6 +462,8 @@ public final boolean checkCastTypesCompatibility(Scope scope, TypeBinding castTy if (match != null) { return checkUnsafeCast(scope, castType, interfaceType, match, true); } + if (((ReferenceBinding) castType).isDisjointFrom(interfaceType)) + return false; if (use15specifics) { checkUnsafeCast(scope, castType, expressionType, null /*no match*/, true); // ensure there is no collision between both interfaces: i.e. I1 extends List, I2 extends List @@ -504,7 +506,7 @@ public final boolean checkCastTypesCompatibility(Scope scope, TypeBinding castTy if (match != null) { return checkUnsafeCast(scope, castType, expressionType, match, true); } - if (((ReferenceBinding) castType).isFinal()) { + if (((ReferenceBinding) castType).isDisjointFrom((ReferenceBinding) expressionType)) { // no subclass for castType, thus compile-time check is invalid return false; } @@ -554,8 +556,8 @@ public final boolean checkCastTypesCompatibility(Scope scope, TypeBinding castTy if (match != null) { return checkUnsafeCast(scope, castType, expressionType, match, false); } - // unless final a subclass may implement the interface ==> no check at compile time - if (refExprType.isFinal()) { + // unless final, a subclass may implement the interface ==> no check at compile time + if (refExprType.isDisjointFrom((ReferenceBinding) castType)) { return false; } tagAsNeedCheckCast(); diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java index a9cd89c4ab9..bd57ce20c6c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java @@ -21,9 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.function.Function; import java.util.function.IntPredicate; import org.eclipse.jdt.internal.compiler.ASTVisitor; @@ -350,8 +348,8 @@ public boolean visit(TNode node) { availableTypes.add(child.type); } } - if (node.type instanceof ReferenceBinding && ((ReferenceBinding)node.type).isSealed()) { - List allAllowedTypes = getAllPermittedTypes((ReferenceBinding) node.type); + if (node.type instanceof ReferenceBinding ref && ref.isSealed()) { + List allAllowedTypes = ref.getAllEnumerableReferenceTypes(); this.covers &= isExhaustiveWithCaseTypes(allAllowedTypes, availableTypes); return this.covers; } @@ -1375,7 +1373,7 @@ private boolean checkAndFlagDefaultSealed(BlockScope skope, CompilerOptions comp return checkAndFlagDefaultRecord(skope, compilerOptions, ref); } if (!ref.isSealed()) return false; - if (!isExhaustiveWithCaseTypes(getAllPermittedTypes(ref), this.caseLabelElementTypes)) { + if (!isExhaustiveWithCaseTypes(ref.getAllEnumerableReferenceTypes(), this.caseLabelElementTypes)) { if (this instanceof SwitchExpression) // non-exhaustive switch expressions will be flagged later. return false; skope.problemReporter().enhancedSwitchMissingDefaultCase(this.expression); @@ -1384,25 +1382,6 @@ private boolean checkAndFlagDefaultSealed(BlockScope skope, CompilerOptions comp this.switchBits |= SwitchStatement.Exhaustive; return false; } - List getAllPermittedTypes(ReferenceBinding ref) { - if (!ref.isSealed()) - return new ArrayList<>(0); - - Set permSet = new HashSet<>(Arrays.asList(ref.permittedTypes())); - if (ref.isClass() && (!ref.isAbstract())) - permSet.add(ref); - Set oldSet = new HashSet<>(permSet); - do { - for (ReferenceBinding type : permSet) { - oldSet.addAll(Arrays.asList(type.permittedTypes())); - } - Set tmp = oldSet; - oldSet = permSet; - permSet = tmp; - } while (oldSet.size() != permSet.size()); - return Arrays.asList(permSet.toArray(new ReferenceBinding[0])); - } - private boolean checkAndFlagDefaultRecord(BlockScope skope, CompilerOptions compilerOptions, ReferenceBinding ref) { RecordComponentBinding[] comps = ref.components(); List allallowedTypes = new ArrayList<>(); 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 0beb1853e2a..dcdc2d644c0 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 @@ -53,7 +53,11 @@ package org.eclipse.jdt.internal.compiler.lookup; import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import org.eclipse.jdt.core.compiler.CharOperation; import org.eclipse.jdt.internal.compiler.ast.LambdaExpression; @@ -1558,6 +1562,7 @@ public final boolean isNonSealed() { /** * Answer true if the receiver has sealed modifier */ +@Override public boolean isSealed() { return (this.modifiers & ExtraCompilerModifiers.AccSealed) != 0; } @@ -2544,6 +2549,89 @@ public boolean hasEnclosingInstanceContext() { return !enclosingMethod.isStatic(); return false; } + +@Override +public List getAllEnumerableReferenceTypes() { + if (!isSealed()) + return Collections.emptyList(); + + Set permSet = new HashSet<>(Arrays.asList(permittedTypes())); + if (isClass() && (!isAbstract())) + permSet.add(this); + Set oldSet = new HashSet<>(permSet); + do { + for (ReferenceBinding type : permSet) { + oldSet.addAll(Arrays.asList(type.permittedTypes())); + } + Set tmp = oldSet; + oldSet = permSet; + permSet = tmp; + } while (oldSet.size() != permSet.size()); + return Arrays.asList(permSet.toArray(new ReferenceBinding[0])); +} + +// 5.1.6.1 Allowed Narrowing Reference Conversion +public boolean isDisjointFrom(ReferenceBinding that) { + if (this.isInterface()) { + if (that.isInterface()) { + /* • An interface named I is disjoint from another interface named J if (i) it is not that case that I <: J, and (ii) it is not the case that J <: I, and + * (iii) one of the following cases applies: + – I is sealed, and all of the permitted direct subclasses and subinterfaces of I are disjoint from J. + – J is sealed, and I is disjoint from all the permitted direct subclasses and subinterfaces of J. + */ + if (this.findSuperTypeOriginatingFrom(that) != null || that.findSuperTypeOriginatingFrom(this) != null) + return false; + if (this.isSealed()) { + for (ReferenceBinding directSubType : this.permittedTypes()) { + if (!directSubType.isDisjointFrom(that)) + return false; + } + return true; + } + if (that.isSealed()) { + for (ReferenceBinding directSubType : that.permittedTypes()) { + if (!this.isDisjointFrom(directSubType)) + return false; + } + return true; + } + return false; + } else { + // • An interface named I is disjoint from a class named C if C is disjoint from I. + return that.isDisjointFrom(this); + } + } else { + if (that.isInterface()) { + /* • A class named C is disjoint from an interface named I if (i) it is not the case that C <: I, and (ii) one of the following cases applies: + – C is final. + – C is sealed, and all of the permitted direct subclasses of C are disjoint from I. + – C is freely extensible (§8.1.1.2), and I is sealed, and C is disjoint from all of the permitted direct subclasses and subinterfaces of I + */ + if (this.findSuperTypeOriginatingFrom(that) != null) + return false; + if (this.isFinal()) + return true; + if (this.isSealed()) { + for (ReferenceBinding directSubclass : this.permittedTypes()) { + if (!directSubclass.isDisjointFrom(that)) + return false; + } + return true; + } + if (that.isSealed()) { + for (ReferenceBinding directSubType : that.permittedTypes()) { + if (!this.isDisjointFrom(directSubType)) + return false; + } + return true; + } + return false; + } else { + // • A class named C is disjoint from another class named D if (i) it is not the case that C <: D, and (ii) it is not the case that D <: C. + return this.findSuperTypeOriginatingFrom(that) == null && that.findSuperTypeOriginatingFrom(this) == null; + } + } +} static class InvalidBindingException extends Exception { private static final long serialVersionUID = 1L; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java index fc924b9b506..ce91f97244a 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeBinding.java @@ -38,6 +38,7 @@ *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -1806,4 +1807,12 @@ public boolean isNonDenotable() { return false; } +public boolean isSealed() { + return false; +} + +public List getAllEnumerableReferenceTypes() { + return Collections.emptyList(); +} + } diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/EnumTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/EnumTest.java index a5d45448c24..951a8dcb2f2 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/EnumTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/EnumTest.java @@ -5077,17 +5077,37 @@ public void test139() { } //check final modifier public void test140() { - this.runConformTest( - new String[] { - "X.java", - "public enum X {\n" + - " PLUS {/*ANONYMOUS*/}, MINUS;\n" + - " void bar(X x) {\n" + - " Runnable r = (Runnable)x;\n" + - " }\n" + - "}", // ================= - }, - ""); + if (this.complianceLevel < ClassFileConstants.JDK17) { + this.runConformTest( + new String[] { + "X.java", + "public enum X {\n" + + " PLUS {/*ANONYMOUS*/}, MINUS;\n" + + " void bar(X x) {\n" + + " Runnable r = (Runnable)x;\n" + + " }\n" + + "}", // ================= + }, + ""); + } else { + // An enum class E is implicitly sealed if its declaration contains at least one enum constant that has a class body + this.runNegativeTest( + new String[] { + "X.java", + "public enum X {\n" + + " PLUS {/*ANONYMOUS*/}, MINUS;\n" + + " void bar(X x) {\n" + + " Runnable r = (Runnable)x;\n" + + " }\n" + + "}", // ================= + }, + "----------\n" + + "1. ERROR in X.java (at line 4)\n" + + " Runnable r = (Runnable)x;\n" + + " ^^^^^^^^^^^\n" + + "Cannot cast from X to Runnable\n" + + "----------\n"); + } } //check final modifier public void test141() { diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SealedTypesTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SealedTypesTests.java index dc26c3ecc47..d22c48707e3 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SealedTypesTests.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SealedTypesTests.java @@ -6039,4 +6039,143 @@ public static void main(String [] args) { "Compiled and ran fine!"); } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2595 + // [sealed types] ECJ accepts a cast from a disjoint interface to a sealed interface + public void testIssue2595_0() { + runNegativeTest( + new String[] { + "X.java", + """ + interface I { + } + + final class C { + } + + public class X { + void test(C c) { + if (c instanceof I) // Compile-time error! + System.out.println("It's an I"); + } + void test(I i) { + if (i instanceof C) // Compile-time error! + System.out.println("It's a C"); + } + } + """ + }, + "----------\n" + + "1. ERROR in X.java (at line 9)\n" + + " if (c instanceof I) // Compile-time error!\n" + + " ^^^^^^^^^^^^^^\n" + + "Incompatible conditional operand types C and I\n" + + "----------\n" + + "2. ERROR in X.java (at line 13)\n" + + " if (i instanceof C) // Compile-time error!\n" + + " ^^^^^^^^^^^^^^\n" + + "Incompatible conditional operand types I and C\n" + + "----------\n"); + } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2595 + // [sealed types] ECJ accepts a cast from a disjoint interface to a sealed interface + public void testIssue2595_1() { + runNegativeTest( + new String[] { + "X.java", + """ + public class X { + interface I { + } + + sealed class C permits D { + } + + final class D extends C { + } + + void test(C c) { + if (c instanceof I) // Compile-time error! + System.out.println("It's an I"); + } + + void test(I i) { + if (i instanceof C) // Compile-time error! + System.out.println("It's a C"); + } + } + """ + }, + "----------\n" + + "1. ERROR in X.java (at line 12)\n" + + " if (c instanceof I) // Compile-time error!\n" + + " ^^^^^^^^^^^^^^\n" + + "Incompatible conditional operand types X.C and X.I\n" + + "----------\n" + + "2. ERROR in X.java (at line 17)\n" + + " if (i instanceof C) // Compile-time error!\n" + + " ^^^^^^^^^^^^^^\n" + + "Incompatible conditional operand types X.I and X.C\n" + + "----------\n"); + } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2595 + // [sealed types] ECJ accepts a cast from a disjoint interface to a sealed interface + public void testIssue2595_2() { + runConformTest( + new String[] { + "X.java", + """ + public class X { + interface I {} + sealed class C permits D, E {} + non-sealed class D extends C {} + final class E extends C {} + class F extends D implements I {} + + void test (C c) { + if (c instanceof I) + System.out.println("It's an I"); + } + + void test (I i) { + if (i instanceof C) + System.out.println("It's a C"); + } + + public static void main(String [] args) { + new X().test(((C) new X().new F())); + new X().test(((I) new X().new F())); + } + } + """ + }, + "It's an I\nIt's a C"); + } + + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2595 + // [sealed types] ECJ accepts a cast from a disjoint interface to a sealed interface + public void testIssue2595_3() { + runNegativeTest( + new String[] { + "X.java", + """ + sealed interface Intf permits PermittedA {} + final class PermittedA implements Intf {} + interface Standalone {} + public class X { + public Intf foo(Standalone st) { + return (Intf) st; + } + } + """ + }, + "----------\n" + + "1. ERROR in X.java (at line 6)\r\n" + + " return (Intf) st;\r\n" + + " ^^^^^^^^^\n" + + "Cannot cast from Standalone to Intf\n" + + "----------\n"); + } }