diff --git a/CHANGES.MD b/CHANGES.MD index de9ce8e5c..812dcb47d 100644 --- a/CHANGES.MD +++ b/CHANGES.MD @@ -12,6 +12,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ### Fixed * `UnnecessaryFullyQualifiedName` now handle FullyQualifiedName on anonymous/nested classes when the nesting class is imported. +* `ModifierOrder` should not crash on use of `sealed` keyword (JDK17, https://github.com/solven-eu/cleanthat/issues/713). ### Changes diff --git a/java/src/main/java/eu/solven/cleanthat/engine/java/refactorer/mutators/ModifierOrder.java b/java/src/main/java/eu/solven/cleanthat/engine/java/refactorer/mutators/ModifierOrder.java index 6f312e240..d0aa72c22 100644 --- a/java/src/main/java/eu/solven/cleanthat/engine/java/refactorer/mutators/ModifierOrder.java +++ b/java/src/main/java/eu/solven/cleanthat/engine/java/refactorer/mutators/ModifierOrder.java @@ -25,6 +25,7 @@ import org.slf4j.LoggerFactory; import com.github.javaparser.ast.Modifier; +import com.github.javaparser.ast.Modifier.Keyword; import com.github.javaparser.ast.Node; import com.github.javaparser.ast.NodeList; import com.github.javaparser.ast.nodeTypes.NodeWithModifiers; @@ -39,6 +40,8 @@ * Order modifiers according the the Java specification. * * @author Benoit Lacelle + * @see https://github.com/checkstyle/checkstyle/blob/master/src/xdocs/checks/modifier/modifierorder.xml + * @see */ public class ModifierOrder extends AJavaparserNodeMutator { private static final Logger LOGGER = LoggerFactory.getLogger(ModifierOrder.class); @@ -49,6 +52,8 @@ public class ModifierOrder extends AJavaparserNodeMutator { "abstract", "default", "static", + "sealed", + "non-sealed", "final", "transient", "volatile", @@ -120,18 +125,35 @@ private int compare2(String left, String right) { var changed = areSameReferences(modifiers, mutableModifiers); if (changed) { - // https://github.com/javaparser/javaparser/issues/3935 - nodeWithModifiers.setModifiers(); - - LOGGER.debug("We fixed the ordering of modifiers"); - nodeWithModifiers.setModifiers(mutableModifiers); - return true; + return applyModifiers(nodeWithModifiers, modifiers, mutableModifiers); } } return false; } + private boolean applyModifiers(NodeWithModifiers nodeWithModifiers, + NodeList originalModifiers, + NodeList sortedModifiers) { + if (sortedModifiers.stream() + .map(m -> m.getKeyword()) + .anyMatch(m -> m == Keyword.SEALED || m == Keyword.NON_SEALED)) { + LOGGER.warn("We do not re-order {} into {} due to {}", + originalModifiers, + sortedModifiers, + "https://github.com/javaparser/javaparser/issues/4245"); + return false; + } + + // https://github.com/javaparser/javaparser/issues/3935 + nodeWithModifiers.setModifiers(); + + LOGGER.debug("We fixed the ordering of modifiers"); + nodeWithModifiers.setModifiers(sortedModifiers); + + return true; + } + @SuppressWarnings("PMD.CompareObjectsWithEquals") private boolean areSameReferences(NodeList modifiers, NodeList mutableModifiers) { var changed = false; diff --git a/java/src/test/java/eu/solven/cleanthat/engine/java/refactorer/cases/do_not_format_me/TestModifierOrderCases.java b/java/src/test/java/eu/solven/cleanthat/engine/java/refactorer/cases/do_not_format_me/TestModifierOrderCases.java index 47bab82c0..7bb3b0808 100644 --- a/java/src/test/java/eu/solven/cleanthat/engine/java/refactorer/cases/do_not_format_me/TestModifierOrderCases.java +++ b/java/src/test/java/eu/solven/cleanthat/engine/java/refactorer/cases/do_not_format_me/TestModifierOrderCases.java @@ -1,8 +1,11 @@ package eu.solven.cleanthat.engine.java.refactorer.cases.do_not_format_me; +import org.junit.jupiter.api.Disabled; + import eu.solven.cleanthat.engine.java.refactorer.annotations.CompareCompilationUnitsAsStrings; import eu.solven.cleanthat.engine.java.refactorer.annotations.CompareMethods; import eu.solven.cleanthat.engine.java.refactorer.annotations.CompareTypes; +import eu.solven.cleanthat.engine.java.refactorer.annotations.UnmodifiedCompilationUnitAsString; import eu.solven.cleanthat.engine.java.refactorer.meta.IJavaparserAstMutator; import eu.solven.cleanthat.engine.java.refactorer.mutators.ModifierOrder; import eu.solven.cleanthat.engine.java.refactorer.test.AJavaparserRefactorerCases; @@ -59,7 +62,7 @@ synchronized protected final void staticMethod() { public Object post() { return new Object() { @SuppressWarnings("unused") - protected final synchronized void staticMethod() { + protected final synchronized void staticMethod() { // Empty } }; @@ -78,19 +81,18 @@ public static class Post { } // https://github.com/javaparser/javaparser/issues/3935 - @CompareCompilationUnitsAsStrings(pre = "package org.eclipse.mat.snapshot.model;\n" - + "\n" - + "import java.io.Serializable;\n" - + "\n" - + "import org.eclipse.mat.internal.Messages;\n" - + "\n" - + "abstract public class GCRootInfo implements Serializable {\n" - + " private static final long serialVersionUID = 2L;\n" - + "\n" - + "}\n" - + "", - post = "package org.eclipse.mat.snapshot.model;\n" + @CompareCompilationUnitsAsStrings( + pre = "package org.eclipse.mat.snapshot.model;\n" + "\n" + + "import java.io.Serializable;\n" + + "\n" + + "import org.eclipse.mat.internal.Messages;\n" + + "\n" + + "abstract public class GCRootInfo implements Serializable {\n" + + " private static final long serialVersionUID = 2L;\n" + "\n" + + "}\n" + + "", + post = "package org.eclipse.mat.snapshot.model;\n" + "\n" + "import java.io.Serializable;\n" + "\n" + "import org.eclipse.mat.internal.Messages;\n" @@ -98,8 +100,20 @@ public static class Post { + "public abstract class GCRootInfo implements Serializable {\n" + " private static final long serialVersionUID = 2L;\n" + "\n" - + "}\n" - + "") + + "}\n") public static class Issue_MissingWhitespaceAfterLastModifier { } + + // https://github.com/solven-eu/cleanthat/issues/713 + @UnmodifiedCompilationUnitAsString( + pre = "public sealed interface IUpdatePortCommand permits UpdateScheduleCommand, UpdateStateCommand {}") + public static class Issue_SealedClass_rightOrder { + } + + // https://github.com/javaparser/javaparser/issues/4245 + @UnmodifiedCompilationUnitAsString( + pre = "sealed public interface IUpdatePortCommand permits UpdateScheduleCommand, UpdateStateCommand {}", + post = "public sealed interface IUpdatePortCommand permits UpdateScheduleCommand, UpdateStateCommand {}") + public static class Issue_SealedClass_wrongOrder { + } } diff --git a/java/src/test/java/eu/solven/cleanthat/engine/java/refactorer/report_javaparser/SealedPermitsParsing.java b/java/src/test/java/eu/solven/cleanthat/engine/java/refactorer/report_javaparser/SealedPermitsParsing.java new file mode 100644 index 000000000..ec3f15114 --- /dev/null +++ b/java/src/test/java/eu/solven/cleanthat/engine/java/refactorer/report_javaparser/SealedPermitsParsing.java @@ -0,0 +1,48 @@ +/* + * Copyright 2023 Benoit Lacelle - SOLVEN + * + * Licensed 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 eu.solven.cleanthat.engine.java.refactorer.report_javaparser; + +import com.github.javaparser.ParserConfiguration; +import com.github.javaparser.ParserConfiguration.LanguageLevel; +import com.github.javaparser.StaticJavaParser; +import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; +import com.github.javaparser.printer.lexicalpreservation.LexicalPreservingPrinter; +import com.github.javaparser.symbolsolver.JavaSymbolSolver; +import com.github.javaparser.symbolsolver.resolution.typesolvers.ReflectionTypeSolver; + +// https://github.com/javaparser/javaparser/issues/4245 +// https://github.com/solven-eu/cleanthat/issues/713 +public class SealedPermitsParsing { + static final String testCase = + "public sealed interface IUpdatePortCommand permits UpdateScheduleCommand, UpdateStateCommand {}"; + + public static void main(String[] args) { + System.out.println(testCase); + + ParserConfiguration parserConfiguration = new ParserConfiguration(); + parserConfiguration.setLanguageLevel(LanguageLevel.JAVA_17); + StaticJavaParser.setConfiguration( + parserConfiguration.setSymbolResolver(new JavaSymbolSolver(new ReflectionTypeSolver(true)))); + var unit = StaticJavaParser.parse(testCase); + + unit = LexicalPreservingPrinter.setup(unit); + + ClassOrInterfaceDeclaration classOrInterface = unit.findFirst(ClassOrInterfaceDeclaration.class).get(); + classOrInterface.setModifiers(); + + System.out.println(classOrInterface); + } +}