From 5bcaa7f2d21b284da1df28f5901356ff363c66d1 Mon Sep 17 00:00:00 2001 From: Martin Kellogg Date: Thu, 3 Apr 2025 10:59:47 -0400 Subject: [PATCH 1/8] better tests --- .../specimin/SwitchEnumConstant2Test.java | 19 ++++++++++ .../specimin/SwitchEnumConstantTest.java | 18 ++++++++++ .../expected/com/example/Simple.java | 21 +++++++++++ .../expected/org/example/MyEnum.java | 8 +++++ .../input/com/example/Simple.java | 35 ++++++++++++++++++ .../expected/com/example/Foo.java | 7 ++++ .../expected/com/example/Simple.java | 21 +++++++++++ .../expected/org/example/MyEnum.java | 8 +++++ .../input/com/example/Simple.java | 36 +++++++++++++++++++ 9 files changed, 173 insertions(+) create mode 100644 src/test/java/org/checkerframework/specimin/SwitchEnumConstant2Test.java create mode 100644 src/test/java/org/checkerframework/specimin/SwitchEnumConstantTest.java create mode 100644 src/test/resources/switchenumconstant/expected/com/example/Simple.java create mode 100644 src/test/resources/switchenumconstant/expected/org/example/MyEnum.java create mode 100644 src/test/resources/switchenumconstant/input/com/example/Simple.java create mode 100644 src/test/resources/switchenumconstant2/expected/com/example/Foo.java create mode 100644 src/test/resources/switchenumconstant2/expected/com/example/Simple.java create mode 100644 src/test/resources/switchenumconstant2/expected/org/example/MyEnum.java create mode 100644 src/test/resources/switchenumconstant2/input/com/example/Simple.java diff --git a/src/test/java/org/checkerframework/specimin/SwitchEnumConstant2Test.java b/src/test/java/org/checkerframework/specimin/SwitchEnumConstant2Test.java new file mode 100644 index 00000000..9b6502bf --- /dev/null +++ b/src/test/java/org/checkerframework/specimin/SwitchEnumConstant2Test.java @@ -0,0 +1,19 @@ +package org.checkerframework.specimin; + +import java.io.IOException; +import org.junit.Test; + +/** + * This test checks that Specimin properly handles the scoping of enum constants used in switch + * statements and expressions, which have special (weird) rules. This variant checks that only + * constants used in the case part of the switch go into the enum. + */ +public class SwitchEnumConstant2Test { + @Test + public void runTest() throws IOException { + SpeciminTestExecutor.runTestWithoutJarPaths( + "switchenumconstant2", + new String[] {"com/example/Simple.java"}, + new String[] {"com.example.Simple#bar(MyEnum)"}); + } +} diff --git a/src/test/java/org/checkerframework/specimin/SwitchEnumConstantTest.java b/src/test/java/org/checkerframework/specimin/SwitchEnumConstantTest.java new file mode 100644 index 00000000..8ace68c0 --- /dev/null +++ b/src/test/java/org/checkerframework/specimin/SwitchEnumConstantTest.java @@ -0,0 +1,18 @@ +package org.checkerframework.specimin; + +import java.io.IOException; +import org.junit.Test; + +/** + * This test checks that Specimin properly handles the scoping of enum constants used in switch + * statements and expressions, which have special (weird) rules. + */ +public class SwitchEnumConstantTest { + @Test + public void runTest() throws IOException { + SpeciminTestExecutor.runTestWithoutJarPaths( + "switchenumconstant", + new String[] {"com/example/Simple.java"}, + new String[] {"com.example.Simple#bar(MyEnum)"}); + } +} diff --git a/src/test/resources/switchenumconstant/expected/com/example/Simple.java b/src/test/resources/switchenumconstant/expected/com/example/Simple.java new file mode 100644 index 00000000..ec3a3c55 --- /dev/null +++ b/src/test/resources/switchenumconstant/expected/com/example/Simple.java @@ -0,0 +1,21 @@ +package com.example; + +import org.example.MyEnum; + +class Simple { + void bar(MyEnum m) { + switch(m) { + case CONSTANT1: + int x = 5 + 8; + break; + case CONSTANT2: + int y = 3 * 2; + break; + } + int z = switch (m) { + case CONSTANT3 -> 5; + case CONSTANT4 -> 6; + default -> 0; + }; + } +} diff --git a/src/test/resources/switchenumconstant/expected/org/example/MyEnum.java b/src/test/resources/switchenumconstant/expected/org/example/MyEnum.java new file mode 100644 index 00000000..a2406a9c --- /dev/null +++ b/src/test/resources/switchenumconstant/expected/org/example/MyEnum.java @@ -0,0 +1,8 @@ +package org.example; + +public enum MyEnum { + CONSTANT1, + CONSTANT2, + CONSTANT3, + CONSTANT4; +} \ No newline at end of file diff --git a/src/test/resources/switchenumconstant/input/com/example/Simple.java b/src/test/resources/switchenumconstant/input/com/example/Simple.java new file mode 100644 index 00000000..f2f06a8b --- /dev/null +++ b/src/test/resources/switchenumconstant/input/com/example/Simple.java @@ -0,0 +1,35 @@ +package com.example; + +import org.example.MyEnum; + +// The Java scoping rules for switches over enum constants +// are surprisingly complicated and poorly documented by the JLS. +// Specifically, Java permits an unqualified enum constant that +// is a member of the type used as the selector expression in +// a switch statement to be used either as a rule or as a case, +// without an explicit import. Specimin needs to correctly +// identify those cases as enum constants, because otherwise +// there usually is not _anything_ in scope that matches, which +// causes us to produce non-sensical output. However, I don't +// _think_ that the unqualified enum constant names can be used +// in the bodies of the expressions in the rest of the switch, +// and have implemented Specimin's logic assuming that is so. +// This test case checks that we get this right. +class Simple { + // Target method. + void bar(MyEnum m) { + switch(m) { + case CONSTANT1: + int x = 5 + 8; // nonsense code + break; + case CONSTANT2: + int y = 3 * 2; + break; + } + int z = switch (m) { + case CONSTANT3 -> 5; + case CONSTANT4 -> 6; + default -> 0; + }; + } +} diff --git a/src/test/resources/switchenumconstant2/expected/com/example/Foo.java b/src/test/resources/switchenumconstant2/expected/com/example/Foo.java new file mode 100644 index 00000000..73e71e6b --- /dev/null +++ b/src/test/resources/switchenumconstant2/expected/com/example/Foo.java @@ -0,0 +1,7 @@ +package com.example; + +public class Foo { + public int CONSTANT5; + + public int CONSTANT6; +} \ No newline at end of file diff --git a/src/test/resources/switchenumconstant2/expected/com/example/Simple.java b/src/test/resources/switchenumconstant2/expected/com/example/Simple.java new file mode 100644 index 00000000..e777d075 --- /dev/null +++ b/src/test/resources/switchenumconstant2/expected/com/example/Simple.java @@ -0,0 +1,21 @@ +package com.example; + +import org.example.MyEnum; + +class Simple extends Foo { + void bar(MyEnum m) { + switch(m) { + case CONSTANT1: + int x = CONSTANT5; + break; + case CONSTANT2: + int y = 3 * 2; + break; + } + int z = switch (m) { + case CONSTANT3 -> CONSTANT6; + case CONSTANT4 -> 6; + default -> 0; + }; + } +} diff --git a/src/test/resources/switchenumconstant2/expected/org/example/MyEnum.java b/src/test/resources/switchenumconstant2/expected/org/example/MyEnum.java new file mode 100644 index 00000000..a2406a9c --- /dev/null +++ b/src/test/resources/switchenumconstant2/expected/org/example/MyEnum.java @@ -0,0 +1,8 @@ +package org.example; + +public enum MyEnum { + CONSTANT1, + CONSTANT2, + CONSTANT3, + CONSTANT4; +} \ No newline at end of file diff --git a/src/test/resources/switchenumconstant2/input/com/example/Simple.java b/src/test/resources/switchenumconstant2/input/com/example/Simple.java new file mode 100644 index 00000000..62bf38c1 --- /dev/null +++ b/src/test/resources/switchenumconstant2/input/com/example/Simple.java @@ -0,0 +1,36 @@ +package com.example; + +import org.example.MyEnum; + +// The Java scoping rules for switches over enum constants +// are surprisingly complicated and poorly documented by the JLS. +// Specifically, Java permits an unqualified enum constant that +// is a member of the type used as the selector expression in +// a switch statement to be used either as a rule or as a case, +// without an explicit import. Specimin needs to correctly +// identify those cases as enum constants, because otherwise +// there usually is not _anything_ in scope that matches, which +// causes us to produce non-sensical output. However, I don't +// _think_ that the unqualified enum constant names can be used +// in the bodies of the expressions in the rest of the switch, +// and have implemented Specimin's logic assuming that is so. +// This test case checks that we get this right; this variant +// differs from the other because it checks the superclass case. +class Simple extends Foo { + // Target method. + void bar(MyEnum m) { + switch(m) { + case CONSTANT1: + int x = CONSTANT5; // CONSTANT5 should go into the super class, not into the enum + break; + case CONSTANT2: + int y = 3 * 2; + break; + } + int z = switch (m) { + case CONSTANT3 -> CONSTANT6; // CONSTANT6 should go into the super class, not into the enum + case CONSTANT4 -> 6; + default -> 0; + }; + } +} From 356a81d2b3193fe97d5245c1787c038d3a7841b8 Mon Sep 17 00:00:00 2001 From: Martin Kellogg Date: Thu, 3 Apr 2025 11:29:50 -0400 Subject: [PATCH 2/8] checkpoint of todos --- .../specimin/UnsolvedSymbolVisitor.java | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 2fc231b8..455df912 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -43,6 +43,7 @@ import com.github.javaparser.ast.stmt.ReturnStmt; import com.github.javaparser.ast.stmt.Statement; import com.github.javaparser.ast.stmt.SwitchEntry; +import com.github.javaparser.ast.stmt.SwitchStmt; import com.github.javaparser.ast.stmt.TryStmt; import com.github.javaparser.ast.stmt.WhileStmt; import com.github.javaparser.ast.type.ArrayType; @@ -635,11 +636,39 @@ public Visitable visit(ForEachStmt node, Void p) { return result; } + // private @Nullable UnsolvedClassOrInterface enumSwitchSelector = null; + private boolean inSwitch = false; + @Override public Visitable visit(SwitchExpr node, Void p) { HashSet currentLocalVariables = new HashSet<>(); localVariables.addFirst(currentLocalVariables); + // workaround JavaParser visit order bug: we should visit the + // selector expression first, but JavaParser for some reason + // visits the _entries_ (i.e., the expression in the cases) first! + node.getSelector().accept(this, p); + inSwitch = true; + // TODO: same problem as the next method Visitable result = super.visit(node, p); + inSwitch = false; + localVariables.removeFirst(); + return result; + } + + @Override + public Visitable visit(SwitchStmt node, Void p) { + HashSet currentLocalVariables = new HashSet<>(); + localVariables.addFirst(currentLocalVariables); + // workaround JavaParser visit order bug: we should visit the + // selector expression first, but JavaParser for some reason + // visits the _entries_ (i.e., the expression in the cases) first! + node.getSelector().accept(this, p); + inSwitch = true; + // TODO: need to store the name of the enum's type here (ideally, an FQN?), + // so that we can look it up in the list of synthetic classes later. Or, + // maybe we should just create it as an enum here? Hard to say, hard to say. + Visitable result = super.visit(node, p); + inSwitch = false; localVariables.removeFirst(); return result; } @@ -648,6 +677,9 @@ public Visitable visit(SwitchExpr node, Void p) { public Visitable visit(SwitchEntry node, Void p) { HashSet currentLocalVariables = new HashSet<>(); localVariables.addFirst(currentLocalVariables); + // TODO: need to save and then restore whatever state we were using in the methods above, + // to defend against the lousy visit order, I think. Maybe? Or maybe just when visiting + // the RHS of the entry? We may need to rewrite the superclass method :( Visitable result = super.visit(node, p); localVariables.removeFirst(); return result; @@ -975,9 +1007,13 @@ else if (staticImportedMembersMap.containsKey(name)) { if (parentNode.isEmpty() || !(parentNode.get() instanceof MethodCallExpr || parentNode.get() instanceof FieldAccessExpr)) { - if (!isALocalVar(name)) { + if (!isALocalVar(name) && !inSwitch) { updateSyntheticClassForSuperCall(node); } + // TODO after lunch: if inSwitch, need to create a synthetic enum constant here + // How would we know the correct enum type to use? Need to store that elsewhere + // (when visiting the switch?) and then write an updateSyntheticEnumWithNewConstant + // method. } } return super.visit(node, arg); @@ -2377,6 +2413,10 @@ public void updateSyntheticClassForSuperCall(Expression expr) { // If we're inside an object creation, this is an anonymous class. Locate any super things // in the class that's being extended. + System.out.println("updating sythetic class based on a super call"); + System.out.println("expr: " + expr); + System.out.println("className: " + className); + String parentClassName; try { parentClassName = insideAnObjectCreation(expr) ? className : getParentClass(className); @@ -3845,7 +3885,7 @@ private String lookupFQNs(String javacType) { typeVarDecl = ""; rest = javacType; } - + // need to also remove annotations before parsing, because they aren't in the right // format. E.g., the string might look like this: // WeakReference<@org.checkerframework.checker.nullness.qual.Nullable,@org.checkerframework.checker.interning.qual.Interned String []> @@ -4017,8 +4057,8 @@ private void migrateType(String incorrectTypeName, String correctTypeName) { } /** - * Given the name of a class, this method will based on the map classAndItsParent to find the name - * of the super class of the input class + * Given the name of a class, this method look up the name in the map classAndItsParent to find + * the name of the super class of the input class. Otherwise, a RunTime exception is thrown. * * @param className the name of the class to be taken as the input * @return the name of the super class of the input class From 39d8f3dec95e122017594ee418b5dd16d4694442 Mon Sep 17 00:00:00 2001 From: Martin Kellogg Date: Thu, 3 Apr 2025 12:44:57 -0400 Subject: [PATCH 3/8] fix the simplest version of this test --- .../specimin/UnsolvedClassOrInterface.java | 66 ++++++++++++++++++- .../specimin/UnsolvedSymbolVisitor.java | 47 ++++++++++--- .../expected/com/example/Simple.java | 9 ++- .../expected/org/example/MyEnum.java | 5 +- 4 files changed, 109 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedClassOrInterface.java b/src/main/java/org/checkerframework/specimin/UnsolvedClassOrInterface.java index 8ae3fad8..772b3d5d 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedClassOrInterface.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedClassOrInterface.java @@ -12,8 +12,9 @@ import org.checkerframework.checker.signature.qual.ClassGetSimpleName; /** - * An UnsolvedClassOrInterface instance is a representation of a class or an interface that can not - * be solved by SymbolSolver. The reason is that the class file is not in the root directory. + * An UnsolvedClassOrInterface instance is a representation of a class, enum, or an interface that + * can not be solved by SymbolSolver. The reason is that the class file is not in the root + * directory. */ public class UnsolvedClassOrInterface { /** @@ -58,6 +59,15 @@ public class UnsolvedClassOrInterface { /** Is this class an annotation? */ private boolean isAnAnnotation = false; + /** Is this an enum? */ + private boolean isAnEnum = false; + + /** + * The enum constants of this enum. Must be a linked set to ensure deterministic iteration order + * when writing out the constants for synthetic classes. + */ + private final LinkedHashSet enumConstants; + /** * This class' constructor should be used for creating inner classes. Frankly, this design is a * mess (sorry) - controlling whether this is an inner class via inheritance is probably bad. @@ -123,6 +133,7 @@ public UnsolvedClassOrInterface( this.extendsClause = " extends Exception"; } this.isAnInterface = isAnInterface; + this.enumConstants = new LinkedHashSet<>(); } /** @@ -142,6 +153,41 @@ public void setIsAnInterfaceToTrue() { this.isAnInterface = true; } + /** + * Returns the value of isAnEnum. + * + * @return return true if the current UnsolvedClassOrInterface instance represents an enum. + */ + public boolean isAnEnum() { + return isAnEnum; + } + + /** + * Adds the given name to this enum's list of enum constants. + * + * @param enumConstantName the name of the constant + */ + public void addEnumConstant(String enumConstantName) { + if (!isAnEnum) { + throw new RuntimeException( + "cannot add an enum constant to a synthetic class that is not an enum"); + } + this.enumConstants.add(enumConstantName); + } + + /** + * Sets isAnEnum to true. isAnEnum is monotonic: it can start as false and become true (because we + * encounter something that forces the class to be a enum, such as being used in a switch + * selector), but it can never go from true to false. + */ + public void setIsAnEnumToTrue() { + this.isAnEnum = true; + if (extendsClause != null) { + throw new RuntimeException( + "trying to make a synthetic class that already has an extends clause into" + "an enum"); + } + } + /** * Sets isAnAnnotation to true. isAnAnnotation is monotonic: it can start as false and become true * (because we encounter evidence that this is an annotation), but it can never go from true to @@ -302,6 +348,9 @@ public void implement(String interfaceName) { */ public void extend(String className) { this.extendsClause = "extends " + className; + if (isAnEnum) { + throw new RuntimeException("enums cannot extend another class"); + } } /** @@ -505,6 +554,8 @@ public String toString() { sb.append("interface "); } else if (isAnAnnotation) { sb.append("@interface "); + } else if (isAnEnum) { + sb.append("enum "); } else { sb.append("class "); } @@ -526,6 +577,17 @@ public String toString() { } } sb.append(" {\n"); + if (isAnEnum) { + Iterator constants = enumConstants.iterator(); + while (constants.hasNext()) { + sb.append(constants.next()); + if (constants.hasNext()) { + sb.append(",\n"); + } + } + sb.append(";\n"); + } + if (innerClasses != null) { for (UnsolvedClassOrInterface innerClass : innerClasses) { sb.append(innerClass.toString()); diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 455df912..326c0bbe 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -636,7 +636,7 @@ public Visitable visit(ForEachStmt node, Void p) { return result; } - // private @Nullable UnsolvedClassOrInterface enumSwitchSelector = null; + private @Nullable String enumSwitchSelectorFQN = null; private boolean inSwitch = false; @Override @@ -648,7 +648,13 @@ public Visitable visit(SwitchExpr node, Void p) { // visits the _entries_ (i.e., the expression in the cases) first! node.getSelector().accept(this, p); inSwitch = true; - // TODO: same problem as the next method + try { + ResolvedType resolved = node.getSelector().calculateResolvedType(); + enumSwitchSelectorFQN = resolved.describe(); + } catch (UnsolvedSymbolException e) { + // We'll deal with this next time. + gotException(); + } Visitable result = super.visit(node, p); inSwitch = false; localVariables.removeFirst(); @@ -664,11 +670,16 @@ public Visitable visit(SwitchStmt node, Void p) { // visits the _entries_ (i.e., the expression in the cases) first! node.getSelector().accept(this, p); inSwitch = true; - // TODO: need to store the name of the enum's type here (ideally, an FQN?), - // so that we can look it up in the list of synthetic classes later. Or, - // maybe we should just create it as an enum here? Hard to say, hard to say. + try { + ResolvedType resolved = node.getSelector().calculateResolvedType(); + enumSwitchSelectorFQN = resolved.describe(); + } catch (UnsolvedSymbolException e) { + // We'll deal with this next time. + gotException(); + } Visitable result = super.visit(node, p); inSwitch = false; + enumSwitchSelectorFQN = null; localVariables.removeFirst(); return result; } @@ -1010,10 +1021,9 @@ else if (staticImportedMembersMap.containsKey(name)) { if (!isALocalVar(name) && !inSwitch) { updateSyntheticClassForSuperCall(node); } - // TODO after lunch: if inSwitch, need to create a synthetic enum constant here - // How would we know the correct enum type to use? Need to store that elsewhere - // (when visiting the switch?) and then write an updateSyntheticEnumWithNewConstant - // method. + if (inSwitch && enumSwitchSelectorFQN != null) { + updateSyntheticEnumWithNewConstant(enumSwitchSelectorFQN, name); + } } } return super.visit(node, arg); @@ -2400,6 +2410,25 @@ public void updateSyntheticClassWithNonStaticFields(FieldAccessExpr field) { updateClassSetWithQualifiedFieldSignature(fieldQualifedSignature, false, false); } + /** + * Updates the synthetic enum with the given FQN by adding a new enum constant with the given + * name. This method is idempotent, and the enum may not yet exist (but if not, this will trigger + * another round of unsolved symbol visiting). + * + * @param enumFQN the fully-qualified name of a synthetic enum + * @param constantName the name of one of the enum constants of that enum + */ + public void updateSyntheticEnumWithNewConstant(String enumFQN, String constantName) { + for (UnsolvedClassOrInterface synthEnum : missingClass) { + if (!synthEnum.getQualifiedClassName().equals(enumFQN)) { + continue; + } + synthEnum.setIsAnEnumToTrue(); + synthEnum.addEnumConstant(constantName); + return; + } + } + /** * For a super call, this method will update the corresponding synthetic class * diff --git a/src/test/resources/switchenumconstant/expected/com/example/Simple.java b/src/test/resources/switchenumconstant/expected/com/example/Simple.java index ec3a3c55..c09ccbee 100644 --- a/src/test/resources/switchenumconstant/expected/com/example/Simple.java +++ b/src/test/resources/switchenumconstant/expected/com/example/Simple.java @@ -13,9 +13,12 @@ void bar(MyEnum m) { break; } int z = switch (m) { - case CONSTANT3 -> 5; - case CONSTANT4 -> 6; - default -> 0; + case CONSTANT3 -> + 5; + case CONSTANT4 -> + 6; + default -> + 0; }; } } diff --git a/src/test/resources/switchenumconstant/expected/org/example/MyEnum.java b/src/test/resources/switchenumconstant/expected/org/example/MyEnum.java index a2406a9c..bf26eef1 100644 --- a/src/test/resources/switchenumconstant/expected/org/example/MyEnum.java +++ b/src/test/resources/switchenumconstant/expected/org/example/MyEnum.java @@ -1,8 +1,5 @@ package org.example; public enum MyEnum { - CONSTANT1, - CONSTANT2, - CONSTANT3, - CONSTANT4; + CONSTANT1, CONSTANT2, CONSTANT3, CONSTANT4 } \ No newline at end of file From 9e5b81390494a04e5002cca4a9842abfd9dec0cd Mon Sep 17 00:00:00 2001 From: Martin Kellogg Date: Thu, 3 Apr 2025 13:15:36 -0400 Subject: [PATCH 4/8] more bookkeeping, now the second test almost works --- .../specimin/UnsolvedSymbolVisitor.java | 35 +++++++++++++++---- .../expected/com/example/Simple.java | 9 +++-- .../expected/org/example/MyEnum.java | 5 +-- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 326c0bbe..4d50533e 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -688,10 +688,32 @@ public Visitable visit(SwitchStmt node, Void p) { public Visitable visit(SwitchEntry node, Void p) { HashSet currentLocalVariables = new HashSet<>(); localVariables.addFirst(currentLocalVariables); - // TODO: need to save and then restore whatever state we were using in the methods above, - // to defend against the lousy visit order, I think. Maybe? Or maybe just when visiting - // the RHS of the entry? We may need to rewrite the superclass method :( - Visitable result = super.visit(node, p); + + // The following code is copied from super.visit(SwitchEntry, ...), + // because we need to differentiate between the expressions in the "labels" + // (which may be unqualified enum constants) and the "statements", which may not be. + SwitchEntry n = node; + Void arg = p; + Expression guard = n.getGuard().map(s -> (Expression) s.accept(this, arg)).orElse(null); + NodeList labels = (NodeList) n.getLabels().accept(this, arg); + + // Here, we need to remove the special enum switch handling temporarily. + String oldEnumSwitchSelectorFQN = enumSwitchSelectorFQN; + enumSwitchSelectorFQN = null; + inSwitch = false; + + NodeList statements = (NodeList) n.getStatements().accept(this, arg); + Comment comment = n.getComment().map(s -> (Comment) s.accept(this, arg)).orElse(null); + n.setGuard(guard); + n.setLabels(labels); + n.setStatements(statements); + n.setComment(comment); + Visitable result = n; + + // and then restore it + enumSwitchSelectorFQN = oldEnumSwitchSelectorFQN; + inSwitch = true; + localVariables.removeFirst(); return result; } @@ -1018,11 +1040,10 @@ else if (staticImportedMembersMap.containsKey(name)) { if (parentNode.isEmpty() || !(parentNode.get() instanceof MethodCallExpr || parentNode.get() instanceof FieldAccessExpr)) { - if (!isALocalVar(name) && !inSwitch) { - updateSyntheticClassForSuperCall(node); - } if (inSwitch && enumSwitchSelectorFQN != null) { updateSyntheticEnumWithNewConstant(enumSwitchSelectorFQN, name); + } else if (!isALocalVar(name) && !inSwitch) { + updateSyntheticClassForSuperCall(node); } } } diff --git a/src/test/resources/switchenumconstant2/expected/com/example/Simple.java b/src/test/resources/switchenumconstant2/expected/com/example/Simple.java index e777d075..e88ec8ef 100644 --- a/src/test/resources/switchenumconstant2/expected/com/example/Simple.java +++ b/src/test/resources/switchenumconstant2/expected/com/example/Simple.java @@ -13,9 +13,12 @@ void bar(MyEnum m) { break; } int z = switch (m) { - case CONSTANT3 -> CONSTANT6; - case CONSTANT4 -> 6; - default -> 0; + case CONSTANT3 -> + CONSTANT6; + case CONSTANT4 -> + 6; + default -> + 0; }; } } diff --git a/src/test/resources/switchenumconstant2/expected/org/example/MyEnum.java b/src/test/resources/switchenumconstant2/expected/org/example/MyEnum.java index a2406a9c..bf26eef1 100644 --- a/src/test/resources/switchenumconstant2/expected/org/example/MyEnum.java +++ b/src/test/resources/switchenumconstant2/expected/org/example/MyEnum.java @@ -1,8 +1,5 @@ package org.example; public enum MyEnum { - CONSTANT1, - CONSTANT2, - CONSTANT3, - CONSTANT4; + CONSTANT1, CONSTANT2, CONSTANT3, CONSTANT4 } \ No newline at end of file From c46e4c33d68168ee60be28d81f4a2425b5086b40 Mon Sep 17 00:00:00 2001 From: Martin Kellogg Date: Thu, 3 Apr 2025 13:22:50 -0400 Subject: [PATCH 5/8] make the next test pass --- .../checkerframework/specimin/JavaTypeCorrect.java | 13 +++++++++++-- .../specimin/UnsolvedSymbolVisitor.java | 4 ---- .../expected/com/example/Foo.java | 3 ++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/checkerframework/specimin/JavaTypeCorrect.java b/src/main/java/org/checkerframework/specimin/JavaTypeCorrect.java index d0984765..70e2a99a 100644 --- a/src/main/java/org/checkerframework/specimin/JavaTypeCorrect.java +++ b/src/main/java/org/checkerframework/specimin/JavaTypeCorrect.java @@ -333,7 +333,8 @@ else if (line.contains("void cannot be converted to")) { } if (line.contains("error: incompatible types") - || line.contains("error: incomparable types")) { + || line.contains("error: incomparable types") + || line.contains("cannot be converted to")) { if (line.contains("invalid method reference") || line.contains("bad return type in method reference")) { lookingForInvalidMethodReference = true; @@ -478,7 +479,15 @@ private void updateTypeToChange(String errorMessage, String filePath) { * 1. error: incompatible types: cannot be converted to */ if (errorMessage.contains("cannot be converted to")) { - String rhs = getTypeFrom(splitErrorMessage, 4, "cannot"); + // There are two forms of this error message: the typical one (detailed above) and a + // special version from bad switch expressions, in which case this part appears on its + // own line, like this: + // com/example/Simple.java:18: error: incompatible types: bad type in switch expression + // CONSTANT6; + // ^ + // SyntheticTypeForCONSTANT6 cannot be converted to int + int startIndex = errorMessage.contains("error:") ? 4 : 0; + String rhs = getTypeFrom(splitErrorMessage, startIndex, "cannot"); int toIndex = splitErrorMessage.indexOf("to"); String lhs = getTypeFrom(splitErrorMessage, toIndex + 1, null); if ("Throwable".equals(lhs)) { diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index 4d50533e..cc92cc5f 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -2463,10 +2463,6 @@ public void updateSyntheticClassForSuperCall(Expression expr) { // If we're inside an object creation, this is an anonymous class. Locate any super things // in the class that's being extended. - System.out.println("updating sythetic class based on a super call"); - System.out.println("expr: " + expr); - System.out.println("className: " + className); - String parentClassName; try { parentClassName = insideAnObjectCreation(expr) ? className : getParentClass(className); diff --git a/src/test/resources/switchenumconstant2/expected/com/example/Foo.java b/src/test/resources/switchenumconstant2/expected/com/example/Foo.java index 73e71e6b..d4c28540 100644 --- a/src/test/resources/switchenumconstant2/expected/com/example/Foo.java +++ b/src/test/resources/switchenumconstant2/expected/com/example/Foo.java @@ -1,7 +1,8 @@ package com.example; public class Foo { - public int CONSTANT5; public int CONSTANT6; + + public int CONSTANT5; } \ No newline at end of file From 8f9b9f398bb6cd0e58d58c098c64cd5d6e55df19 Mon Sep 17 00:00:00 2001 From: Martin Kellogg Date: Thu, 3 Apr 2025 13:53:55 -0400 Subject: [PATCH 6/8] fix CF errors --- JavaParser.astub | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/JavaParser.astub b/JavaParser.astub index 8f541aff..a7c51f3a 100644 --- a/JavaParser.astub +++ b/JavaParser.astub @@ -37,3 +37,13 @@ package com.github.javaparser.ast.stmt; class IfStmt { IfStmt setElseStmt(@Nullable Statement elseStmt); } + +class SwitchEntry { + public SwitchEntry setGuard(@Nullable Expression guard); +} + +package com.github.javaparser.ast; + +class Node { + public Node setComment(@Nullable Comment comment); +} \ No newline at end of file From fa91095016a3ef2e01c997ed3ce2005ee61bc42e Mon Sep 17 00:00:00 2001 From: Martin Kellogg Date: Thu, 3 Apr 2025 14:56:58 -0400 Subject: [PATCH 7/8] missing javadoc --- .../checkerframework/specimin/UnsolvedSymbolVisitor.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index cc92cc5f..e0b8a98f 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -636,7 +636,13 @@ public Visitable visit(ForEachStmt node, Void p) { return result; } + /** + * The name of the enum type used as a switch selector, if + * one is currently in scope. + */ private @Nullable String enumSwitchSelectorFQN = null; + + /** True iff we are visiting a switch control structure (a label, guard, or selector). */ private boolean inSwitch = false; @Override From 0460d90870679c962dcafb56b81c026d99864850 Mon Sep 17 00:00:00 2001 From: Martin Kellogg Date: Thu, 3 Apr 2025 14:57:22 -0400 Subject: [PATCH 8/8] formatting --- .../org/checkerframework/specimin/UnsolvedSymbolVisitor.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java index e0b8a98f..90715c7f 100644 --- a/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java +++ b/src/main/java/org/checkerframework/specimin/UnsolvedSymbolVisitor.java @@ -636,10 +636,7 @@ public Visitable visit(ForEachStmt node, Void p) { return result; } - /** - * The name of the enum type used as a switch selector, if - * one is currently in scope. - */ + /** The name of the enum type used as a switch selector, if one is currently in scope. */ private @Nullable String enumSwitchSelectorFQN = null; /** True iff we are visiting a switch control structure (a label, guard, or selector). */