Skip to content

Commit

Permalink
Add in ordered modifiers, workaround JavaParser issue #713
Browse files Browse the repository at this point in the history
  • Loading branch information
blacelle committed Dec 6, 2023
1 parent 6b9d478 commit 5ed2f95
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 21 deletions.
1 change: 1 addition & 0 deletions CHANGES.MD
Original file line number Diff line number Diff line change
Expand Up @@ -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:/solven-eu/cleanthat/issues/713).

### Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,6 +40,8 @@
* Order modifiers according the the Java specification.
*
* @author Benoit Lacelle
* @see https:/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);
Expand All @@ -49,6 +52,8 @@ public class ModifierOrder extends AJavaparserNodeMutator {
"abstract",
"default",
"static",
"sealed",
"non-sealed",
"final",
"transient",
"volatile",
Expand Down Expand Up @@ -120,18 +125,35 @@ private int compare2(String left, String right) {
var changed = areSameReferences(modifiers, mutableModifiers);

if (changed) {
// https:/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<Modifier> originalModifiers,
NodeList<Modifier> 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:/javaparser/javaparser/issues/4245");
return false;
}

// https:/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<Modifier> modifiers, NodeList<Modifier> mutableModifiers) {
var changed = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
}
};
Expand All @@ -78,28 +81,39 @@ public static class Post {
}

// https:/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"
+ "\n"
+ "public abstract class GCRootInfo implements Serializable {\n"
+ " private static final long serialVersionUID = 2L;\n"
+ "\n"
+ "}\n"
+ "")
+ "}\n")
public static class Issue_MissingWhitespaceAfterLastModifier {
}

// https:/solven-eu/cleanthat/issues/713
@UnmodifiedCompilationUnitAsString(
pre = "public sealed interface IUpdatePortCommand permits UpdateScheduleCommand, UpdateStateCommand {}")
public static class Issue_SealedClass_rightOrder {
}

// https:/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 {
}
}
Original file line number Diff line number Diff line change
@@ -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:/javaparser/javaparser/issues/4245
// https:/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);
}
}

0 comments on commit 5ed2f95

Please sign in to comment.