diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java index 561d74895..b3eb7fff4 100644 --- a/palantir-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/Formatter.java @@ -14,6 +14,9 @@ package com.palantir.javaformat.java; +import static com.palantir.javaformat.java.ImportOrderer.reorderImports; +import static com.palantir.javaformat.java.RemoveUnusedDeclarations.removeUnusedDeclarations; +import static com.palantir.javaformat.java.RemoveUnusedImports.removeUnusedImports; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; @@ -215,18 +218,8 @@ private static JavaInputAstVisitor createVisitor( } static boolean errorDiagnostic(Diagnostic input) { - if (input.getKind() != Diagnostic.Kind.ERROR) { - return false; - } - switch (input.getCode()) { - case "compiler.err.invalid.meth.decl.ret.type.req": - // accept constructor-like method declarations that don't match the name of their - // enclosing class - return false; - default: - break; - } - return true; + return input.getKind() == Diagnostic.Kind.ERROR + && !input.getCode().equals("compiler.err.invalid.meth.decl.ret.type.req"); } /** @@ -272,6 +265,21 @@ public String formatSourceAndFixImports(String input) throws FormatterException return formatted; } + /** + * Formats an input string (a Java compilation unit) and fixes imports and redundant declarations. + * + *

Fixing imports includes ordering, spacing, and removal of unused import statements. + * + * @param input the input string + * @return the output string + * @throws FormatterException if the input string cannot be parsed + * @see Google Java + * Style Guide - 3.3.3 Import ordering and spacing + */ + public String formatSourceAndFixImportsAndDeclarations(String input) throws FormatterException { + return formatSourceAndFixImports(removeUnusedDeclarations(input)); + } + /** * Fixes imports (e.g. ordering, spacing, and removal of unused import statements). * @@ -282,7 +290,7 @@ public String formatSourceAndFixImports(String input) throws FormatterException * Style Guide - 3.3.3 Import ordering and spacing */ public String fixImports(String input) throws FormatterException { - return ImportOrderer.reorderImports(RemoveUnusedImports.removeUnusedImports(input), options.style()); + return reorderImports(removeUnusedImports(input), options.style()); } /** diff --git a/palantir-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedDeclarations.java b/palantir-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedDeclarations.java new file mode 100644 index 000000000..b81cc15a5 --- /dev/null +++ b/palantir-java-format/src/main/java/com/palantir/javaformat/java/RemoveUnusedDeclarations.java @@ -0,0 +1,264 @@ +/* + * (c) Copyright 2025 Palantir Technologies Inc. All rights reserved. + * + * 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 com.palantir.javaformat.java; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Range; +import com.google.common.collect.RangeMap; +import com.google.common.collect.TreeRangeMap; +import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ModifiersTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.Tree.Kind; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.JavacTask; +import com.sun.source.util.SourcePositions; +import com.sun.source.util.TreePath; +import com.sun.source.util.TreePathScanner; +import com.sun.source.util.Trees; +import com.sun.tools.javac.api.JavacTool; +import com.sun.tools.javac.file.JavacFileManager; +import com.sun.tools.javac.util.Context; +import java.io.IOException; +import java.net.URI; +import java.util.Comparator; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import javax.annotation.Nullable; +import javax.lang.model.element.Modifier; +import javax.tools.Diagnostic; +import javax.tools.DiagnosticCollector; +import javax.tools.JavaFileObject; +import javax.tools.SimpleJavaFileObject; + +/** + * Removes unused declarations from Java source code, including: + * - Redundant modifiers in interfaces (public, static, final, abstract) + * - Redundant modifiers in classes, enums, and annotations + * - Redundant final modifiers on method parameters (preserved now) + */ +public class RemoveUnusedDeclarations { + public static String removeUnusedDeclarations(String source) throws FormatterException { + DiagnosticCollector diagnostics = new DiagnosticCollector<>(); + JavacTask task = JavacTool.create() + .getTask( + null, + new JavacFileManager(new Context(), true, null), + diagnostics, + ImmutableList.of("-Xlint:-processing"), + null, + ImmutableList.of((JavaFileObject) + new SimpleJavaFileObject(URI.create("source"), JavaFileObject.Kind.SOURCE) { + @Override + public CharSequence getCharContent(boolean ignoreEncodingErrors) { + return source; + } + })); + + try { + Iterable units = task.parse(); + if (!units.iterator().hasNext()) { + throw new FormatterException("No compilation units found"); + } + + for (Diagnostic diagnostic : diagnostics.getDiagnostics()) { + if (diagnostic.getKind() == Diagnostic.Kind.ERROR) { + throw new FormatterException("Syntax error in source: " + diagnostic.getMessage(null)); + } + } + + UnusedDeclarationScanner scanner = new UnusedDeclarationScanner(task); + scanner.scan(units.iterator().next(), null); + + return applyReplacements(source, scanner.getReplacements()); + } catch (IOException e) { + throw new FormatterException("Error processing source file: " + e.getMessage()); + } + } + + private static final class UnusedDeclarationScanner extends TreePathScanner { + private final RangeMap replacements = TreeRangeMap.create(); + private final SourcePositions sourcePositions; + private final Trees trees; + + private static final ImmutableList CANONICAL_MODIFIER_ORDER = ImmutableList.of( + Modifier.PUBLIC, + Modifier.PROTECTED, + Modifier.PRIVATE, + Modifier.ABSTRACT, + Modifier.STATIC, + Modifier.FINAL, + Modifier.SEALED, + Modifier.NON_SEALED, + Modifier.TRANSIENT, + Modifier.VOLATILE, + Modifier.SYNCHRONIZED, + Modifier.NATIVE, + Modifier.STRICTFP); + + private UnusedDeclarationScanner(JavacTask task) { + this.sourcePositions = Trees.instance(task).getSourcePositions(); + this.trees = Trees.instance(task); + } + + public RangeMap getReplacements() { + return replacements; + } + + @Override + public Void visitClass(ClassTree node, Void _unused) { + TreePath parentPath = getCurrentPath().getParentPath(); + Kind parentKind = parentPath != null ? parentPath.getLeaf().getKind() : null; + + if (node.getKind() == Tree.Kind.INTERFACE) { + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC, Modifier.ABSTRACT, Modifier.STATIC)); + } else if ((parentPath != null ? parentPath.getLeaf().getKind() : null) == Tree.Kind.INTERFACE) { + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC, Modifier.STATIC)); + } else if (node.getKind() == Tree.Kind.ANNOTATION_TYPE) { + checkForRedundantModifiers(node, Set.of(Modifier.ABSTRACT)); + } else if (node.getModifiers().getFlags().contains(Modifier.SEALED)) { + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC)); + } else { + checkForRedundantModifiers(node, Set.of()); // Always sort + } + + return super.visitClass(node, null); + } + + @Override + public Void visitMethod(MethodTree node, Void _unused) { + TreePath parentPath = getCurrentPath().getParentPath(); + Kind parentKind = parentPath != null ? parentPath.getLeaf().getKind() : null; + + if (parentKind == Tree.Kind.INTERFACE) { + if (!node.getModifiers().getFlags().contains(Modifier.DEFAULT) + && !node.getModifiers().getFlags().contains(Modifier.STATIC)) { + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC, Modifier.ABSTRACT)); + } else { + checkForRedundantModifiers(node, Set.of()); + } + } else if (parentKind == Tree.Kind.ANNOTATION_TYPE) { + checkForRedundantModifiers(node, Set.of(Modifier.ABSTRACT)); + } else { + checkForRedundantModifiers(node, Set.of()); // Always sort + } + + return super.visitMethod(node, null); + } + + @Override + public Void visitVariable(VariableTree node, Void _unused) { + TreePath parentPath = getCurrentPath().getParentPath(); + Kind parentKind = parentPath != null ? parentPath.getLeaf().getKind() : null; + + if (node.getKind() == Tree.Kind.ENUM) { + // Enum constants should have no modifiers + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)); + } else if (parentKind == Tree.Kind.INTERFACE || parentKind == Tree.Kind.ANNOTATION_TYPE) { + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)); + } else if (node.getKind() == Kind.RECORD) { + // Record components should have no modifiers + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC, Modifier.FINAL)); + } else { + checkForRedundantModifiers(node, Set.of()); // Always sort + } + + return super.visitVariable(node, null); + } + + private void checkForRedundantModifiers(Tree node, Set redundantModifiers) { + ModifiersTree modifiers = getModifiers(node); + if (modifiers == null) return; + try { + addReplacementForModifiers( + node, + modifiers.getFlags().stream() + .filter(redundantModifiers::contains) + .collect(Collectors.toSet())); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Nullable + private ModifiersTree getModifiers(Tree node) { + if (node instanceof ClassTree) return ((ClassTree) node).getModifiers(); + if (node instanceof MethodTree) return ((MethodTree) node).getModifiers(); + if (node instanceof VariableTree) return ((VariableTree) node).getModifiers(); + return null; + } + + private void addReplacementForModifiers(Tree node, Set toRemove) throws IOException { + TreePath path = trees.getPath(getCurrentPath().getCompilationUnit(), node); + if (path == null) return; + + CompilationUnitTree unit = path.getCompilationUnit(); + String source = unit.getSourceFile().getCharContent(true).toString(); + + ModifiersTree modifiers = getModifiers(node); + if (modifiers == null) return; + + long modifiersStart = sourcePositions.getStartPosition(unit, modifiers); + long modifiersEnd = sourcePositions.getEndPosition(unit, modifiers); + if (modifiersStart == -1 || modifiersEnd == -1) return; + + String newModifiersText = modifiers.getFlags().stream() + .filter(m -> !toRemove.contains(m)) + .sorted(Comparator.comparingInt(mod -> { + int idx = CANONICAL_MODIFIER_ORDER.indexOf(mod); + return idx == -1 ? Integer.MAX_VALUE : idx; + })) + .map(Modifier::toString) + .collect(Collectors.joining(" ")); + + long annotationsEnd = modifiersStart; + for (AnnotationTree annotation : modifiers.getAnnotations()) { + long end = sourcePositions.getEndPosition(unit, annotation); + if (end > annotationsEnd) annotationsEnd = end; + } + + int effectiveStart = (int) annotationsEnd; + while (effectiveStart < modifiersEnd && Character.isWhitespace(source.charAt(effectiveStart))) { + effectiveStart++; + } + + String current = source.substring(effectiveStart, (int) modifiersEnd); + if (!newModifiersText.trim().equals(current.trim())) { + int globalEnd = (int) modifiersEnd; + if (newModifiersText.isEmpty()) { + while (globalEnd < source.length() && Character.isWhitespace(source.charAt(globalEnd))) { + globalEnd++; + } + } + replacements.put(Range.closedOpen(effectiveStart, globalEnd), newModifiersText); + } + } + } + + private static String applyReplacements(String source, RangeMap replacements) { + StringBuilder sb = new StringBuilder(source); + for (Map.Entry, String> entry : + replacements.asDescendingMapOfRanges().entrySet()) { + Range range = entry.getKey(); + sb.replace(range.lowerEndpoint(), range.upperEndpoint(), entry.getValue()); + } + return sb.toString(); + } +} diff --git a/palantir-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java b/palantir-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java index 7bf54dc38..cab6f027e 100644 --- a/palantir-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java +++ b/palantir-java-format/src/test/java/com/palantir/javaformat/java/FormatterTest.java @@ -31,6 +31,8 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.api.parallel.Execution; @@ -446,10 +448,84 @@ public void dontWrapMoeLineComments() throws Exception { @Test void canParse_java9_private_interface_methods() { assertThatCode(() -> Formatter.create() - .formatSourceAndFixImports("" + .formatSourceAndFixImportsAndDeclarations("" + "interface T {\n" + " private static void foo() {}\n" // + "}")) .doesNotThrowAnyException(); } + + @Nested + class formatSourceAndFixImportsAndDeclarations { + + @Test + public void removesRedundantPublicInterfaceModifiers() throws FormatterException { + String input = + """ + interface TestInterface { + public static final int CONSTANT = 1; + + public abstract void method(); + + public static class InnerClass {} + } + """; + String expected = + """ + interface TestInterface { + int CONSTANT = 1; + + void method(); + + class InnerClass {} + } + """; + assertThat(Formatter.create().formatSourceAndFixImportsAndDeclarations(input)) + .isEqualTo(expected); + } + + @Test + @Disabled("fixme") + public void reordersModifiers() throws FormatterException { + String input = + """ + class Test { + public final static String VALUE = "test"; + protected final abstract void doSomething(); + } + """; + String expected = + """ + class Test { + public static final String VALUE = "test"; + + protected abstract void doSomething(); + } + """; + assertThat(Formatter.create().formatSourceAndFixImportsAndDeclarations(input)) + .isEqualTo(expected); + } + + @Test + public void handlesNestedClasses() throws FormatterException { + String input = + """ + class Outer { + public static interface Inner { + public static final int VAL = 1; + } + } + """; + String expected = + """ + class Outer { + interface Inner { + int VAL = 1; + } + } + """; + assertThat(Formatter.create().formatSourceAndFixImportsAndDeclarations(input)) + .isEqualTo(expected); + } + } } diff --git a/palantir-java-format/src/test/java/com/palantir/javaformat/java/RemoveUnusedDeclarationsTest.java b/palantir-java-format/src/test/java/com/palantir/javaformat/java/RemoveUnusedDeclarationsTest.java new file mode 100644 index 000000000..f144c2472 --- /dev/null +++ b/palantir-java-format/src/test/java/com/palantir/javaformat/java/RemoveUnusedDeclarationsTest.java @@ -0,0 +1,190 @@ +/* + * Copyright 2016 Google Inc. + * + * 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 com.palantir.javaformat.java; + +import static com.google.common.truth.Truth.assertThat; +import static com.palantir.javaformat.java.RemoveUnusedDeclarations.removeUnusedDeclarations; + +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; +import com.google.common.truth.Truth; +import com.palantir.javaformat.jupiter.ParameterizedClass; +import java.util.List; +import org.junit.jupiter.api.TestTemplate; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; + +/** {@link RemoveUnusedImports}Test */ +@Execution(ExecutionMode.CONCURRENT) +@ExtendWith(ParameterizedClass.class) +public record RemoveUnusedDeclarationsTest(String input, String expected) { + + @ParameterizedClass.Parameters(name = "{index}: {0}") + public static List parameters() { + String[][][] inputsOutputs = { + // Interface members + { + { + """ + interface TestInterface { + public static final int CONSTANT = 1; + public abstract void method(); + public static class InnerClass {} + } + """ + }, + { + """ + interface TestInterface { + int CONSTANT = 1; + void method(); + class InnerClass {} + } + """ + } + }, + + // Final parameters (should be preserved) + { + { + """ + class Test { + void method(final String param1, @Nullable final String param2) {} + } + """ + }, + { + """ + class Test { + void method(final String param1, @Nullable final String param2) {} + } + """ + } + }, + + // Code that shouldn't change + { + { + """ + class NoChanges { + private int field; + void method(String param) {} + static final class Inner {} + } + """ + }, + { + """ + class NoChanges { + private int field; + void method(String param) {} + static final class Inner {} + } + """ + } + }, + + // Annotation declarations + // { + // { + // """ + // public @interface TestAnnotation { + // public abstract String value(); + // public static final int DEFAULT = 0; + // } + // """ + // }, + // { + // """ + // public @interface TestAnnotation { + // String value(); + // int DEFAULT = 0; + // } + // """ + // } + // }, + + // Nested interfaces and classes + // { + // { + // """ + // class Outer { + // public static interface InnerInterface { + // public static final int VAL = 1; + // } + // public static class InnerClass { + // public static final int VAL = 1; + // } + // } + // """ + // }, + // { + // """ + // class Outer { + // interface InnerInterface { + // int VAL = 1; + // } + // static class InnerClass { + // static final int VAL = 1; + // } + // } + // """ + // } + // }, + + // Static interfaces in abstract classes + { + { + """ + public abstract class Test { + public static final int CONST1 = 1; + private static final int CONST2 = 2; + protected abstract void doSomething(final String param); + public static interface Inner { + public static final int INNER_CONST = 3; + } + } + """ + }, + { + """ + public abstract class Test { + public static final int CONST1 = 1; + private static final int CONST2 = 2; + protected abstract void doSomething(final String param); + interface Inner { + int INNER_CONST = 3; + } + } + """ + } + } + }; + ImmutableList.Builder builder = ImmutableList.builder(); + for (String[][] inputAndOutput : inputsOutputs) { + assertThat(inputAndOutput.length).isEqualTo(2); + builder.add(new String[] { + Joiner.on('\n').join(inputAndOutput[0]) + '\n', Joiner.on('\n').join(inputAndOutput[1]) + '\n', + }); + } + return builder.build(); + } + + @TestTemplate + public void removeUnused() throws FormatterException { + Truth.assertThat(removeUnusedDeclarations(input)).isEqualTo(expected); + } +}