From 6dfcb654413c0425264601089a3d68389afb1709 Mon Sep 17 00:00:00 2001 From: Vincent Potucek Date: Wed, 6 Aug 2025 12:14:49 +0200 Subject: [PATCH] feat: Add `ReplaceObsoletesStep` --- .gitignore | 1 + .idea/icon.png | Bin 0 -> 1879 bytes .../googlejavaformat/java/Formatter.java | 15 +- .../java/RemoveUnusedDeclarations.java | 222 ++++++++++++++ .../googlejavaformat/java/FormatterTest.java | 133 ++++++++ .../java/RemoveUnusedDeclarationsTest.java | 290 ++++++++++++++++++ 6 files changed, 656 insertions(+), 5 deletions(-) create mode 100644 .idea/icon.png create mode 100644 core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedDeclarations.java create mode 100644 core/src/test/java/com/google/googlejavaformat/java/RemoveUnusedDeclarationsTest.java diff --git a/.gitignore b/.gitignore index 235f69d3d..ee6ba32b5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .idea/ +!.idea/icon.png *.ims *.iml diff --git a/.idea/icon.png b/.idea/icon.png new file mode 100644 index 0000000000000000000000000000000000000000..0cfd67eed86cc1f6e7ee744fb26d2907078ebc51 GIT binary patch literal 1879 zcmV-d2dMaoP) z+0<@O#@d3$ziBJ$5>~OXftnRk(r#-AvM(+9U3@VDm zPde%Dec$`O-%sz|yZ646V2oiVgSy`Y?iLRO+;7Zy0TQl8hrem`0PqN4D2c`|e>^tPe9|-8c*^JhAc2A9%TZljElz+Cq~>02 zp<&(DV#|vbh0HFb4*~>6xomEYDKB;X&=ZaK?jCUMJnZxw<5iJ{hiE*iRac%}SpLy3 z=au|fD0waz<>AaQ6ASFE zwFQQCV*3gfAVrAUUp{Nk&a$FIG(rl|EcxP7Nd590>j9fuX%W*;@H?>2Uh=CNlYVPP z>;xd1WXHeP(fd>n6LyK|CrH31t7$`xNxw}@V2p7BAsD@N4?2VpF;^1faD7+b>a(MP z^I~#<5CMy{>dK#&6}}0GKbR*PyYjBvclgYWz^TtBm@X#C#!~HSnvzy%RQ9juWk3Cl zSzl*S$ZWU!#8@`)qt3onC%yh=vE+FLq+XEMY$b2;&G{r|L%;5K?btnZ{jCJX(^f<# z$;Lk!4|b%k*9Kg#*IP|>J4!9PB?w9GhCHu;4fgEy^Rm*fHm-2gQP+88&BZ{-AtYe} zWBi*T_rdf2iB_FVqWg!}|DjmYQ$Z9Up#lBudFBO0pA#a@763eb4yC5d$~MOn?pWL> zHt~PPgO@VkJDn%VuxtS=E;1lH7`#9mBEbB$7$7p~jxi*+bPYV43Hf^o;&))pSE;qP z?`5+mKwEnm=={kY&buz}o-<>C)-Ql#S`Y;+TY#2NeWrCW$g1rH*;l*}XIW-fzw_lU zgdq<&r;tep+JYQ3ZI6R!4C<$y@K45f!hJUG7m~obWXtxuC8;rMP6I#}myJ_Ec?p`2 z#u0IJK6EopLX(%ReD!sqz9i&l_bN#OI7wRbp(tIz&-!rZ-I5XmMRp;P52pZ{x^1Z|slz#!J^!J+Gh>+y&Pmlec&xe*Oq{fPx?~v?~v1@|> zCQcitfa{|WaK^Et(1^;Xw#PAoH<$|e%bem>(!5hS@f#7|Z-Jxn2eo`Yd2^pyluiKS zadZMa!f7ME#Leya%+zOZQV#XwkHhiEETANm+O*pK6Fy=>VXR#9Od2!mcV8fGS%?CT zpJR{TzHf`=L7o2z=t}4&lYV2?cFlalmouY038i6fLH?K4KS#2Md{^3{-hSeSktpDF zE1bLL!Hz$KX2X`iV(?+*|Jf0Rco@LJ$Qc zl7Op^8h8FSR1TPMo|NS%X4_s_dQM%IE4nsB%QUj`Zy)_#PVu5@k`J4V2b~v=q+JBk ze#pAtcQu5rj(?QSN3K)ZZ7+ST&AisO8PVkZ(^RO+bhgSJKW#QvtgcJvH`38?Fy2P>-Y( zrLbg`Xl(i=y2lFEY3AuyB6N`s2ls-)`+cC;=`$!94~-3e@anF#{Df2`nE#Gm8a{Su zbJ4Svhk&OvAh z&uZi0^IOtOFr1B249Tk^h{k)B{Z6OAuGJectBn{`*1a zzux|nkUWAbb3^+tY)3SJ?lJ2&B%nql3RD!rrhSm~uYgdS;Y4@CbDITs73zo+EIY9G z{EvtK*s%e#IGizAQhZ>+-Lrph`_Y;;$ykuMdAk#$x6>mhdyn+I*u0Q$brORF6T3ci z=+pHBZ=BzX6Kry-O(v5;RaF(SFXH-7eBi(V{y#JbK)< z{=KgEdVY}%M1x#DX0sVwE^@s$<3BX1CDzx}-{3nl(3o9iTB=`a{jT;Q(>H0kBtLC8 z7KcNF{*TB0J>2L!H~3CsJT{Rr2hs?%x3@!6li+jvWH1_mqCi`!uxM;joyvkJ$htLF z5R8vSJW+3`FFfS$VWY{o(3onq8tUuo#R-^6m?>vsA|431- */ public String formatSourceAndFixImports(String input) throws FormatterException { - input = ImportOrderer.reorderImports(input, options.style()); - input = RemoveUnusedImports.removeUnusedImports(input); - String formatted = formatSource(input); - formatted = StringWrapper.wrap(formatted, this); - return formatted; + return wrap( + formatSource( + removeUnusedDeclarations( + removeUnusedImports( + reorderImports(input, options.style())))), + this); } /** diff --git a/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedDeclarations.java b/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedDeclarations.java new file mode 100644 index 000000000..8f490bd75 --- /dev/null +++ b/core/src/main/java/com/google/googlejavaformat/java/RemoveUnusedDeclarations.java @@ -0,0 +1,222 @@ +package com.google.googlejavaformat.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.*; +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 javax.lang.model.element.Modifier; +import javax.tools.Diagnostic; +import javax.tools.DiagnosticCollector; +import javax.tools.JavaFileObject; +import javax.tools.SimpleJavaFileObject; +import java.io.IOException; +import java.net.URI; +import java.util.*; +import java.util.stream.Collectors; + +/** + * 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<>(); + var 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)); + } + } + + var 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 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.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) { + var parentPath = getCurrentPath().getParentPath(); + if (node.getKind() == Tree.Kind.INTERFACE) { + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC, Modifier.ABSTRACT)); + } 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 { + checkForRedundantModifiers(node, Set.of()); // Always sort + } + + return super.visitClass(node, unused); + } + + @Override + public Void visitMethod(MethodTree node, Void unused) { + var parentPath = getCurrentPath().getParentPath(); + var 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, unused); + } + + @Override + public Void visitVariable(VariableTree node, Void unused) { + var parentPath = getCurrentPath().getParentPath(); + var parentKind = parentPath != null ? parentPath.getLeaf().getKind() : null; + + if (node.getKind() == Tree.Kind.ENUM) { + return super.visitVariable(node, unused); + } + + if (parentKind == Tree.Kind.INTERFACE || parentKind == Tree.Kind.ANNOTATION_TYPE) { + checkForRedundantModifiers(node, Set.of(Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)); + } else { + checkForRedundantModifiers(node, Set.of()); // Always sort + } + + return super.visitVariable(node, unused); + } + + private void checkForRedundantModifiers(Tree node, Set redundantModifiers) { + var modifiers = getModifiers(node); + if (modifiers == null) return; + try { + addReplacementForModifiers(node, new LinkedHashSet<>(modifiers.getFlags()).stream() + .filter(redundantModifiers::contains) + .collect(Collectors.toSet())); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + 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)) + .collect(Collectors.toCollection(LinkedHashSet::new)).stream() + .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/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java index 3835673d9..58e4a5404 100644 --- a/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java +++ b/core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java @@ -510,4 +510,137 @@ public void removeTrailingTabsInComments() throws Exception { + " }\n" + "}\n"); } + +// @Test +// @Disabled +// public void removesRedundantPublicInterfaceModifiers() throws FormatterException { +// String input = """ +// interface Test { +// public static final int CONST = 1; +// public abstract void method(); +// } +// """; +// String expected = """ +// interface Test { +// int CONST = 1; +// void method(); +// } +// """; +// assertThat(new Formatter().formatSource(input)).isEqualTo(expected); +// } + + @Test + public void preservesFinalParameters() throws FormatterException { + String input = """ + class Test { + void method(final String param1, @Nullable final String param2) {} + } + """; + String expected = """ + class Test { + void method(final String param1, @Nullable final String param2) {} + } + """; + assertThat(new Formatter().formatSource(input)).isEqualTo(expected); + } + +// @Test +// @Disabled +// 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(new Formatter().formatSource(input)).isEqualTo(expected); +// } + +// @Test +// @Disabled +// 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(new Formatter().formatSource(input)).isEqualTo(expected); +// } + + @Test + public void preservesMeaningfulModifiers() throws FormatterException { + String input = """ + class Test { + private int field; + protected abstract void method(); + public static final class Inner {} + } + """; + String expected = """ + class Test { + private int field; + + protected abstract void method(); + + public static final class Inner {} + } + """; + assertThat(new Formatter().formatSource(input)).isEqualTo(expected); + } + +// @Test +// @Disabled +// public void handlesRecords() throws FormatterException { +// String input = """ +// public record TestRecord( +// public final String name, +// public static final int MAX = 100 +// ) { +// public static void doSomething() {} +// } +// """; +// String expected = """ +// public record TestRecord( +// String name, +// int MAX = 100 +// ) { +// static void doSomething() {} +// } +// """; +// assertThat(new Formatter().formatSource(input)).isEqualTo(expected); +// } + +// @Test +// @Disabled +// public void handlesSealedClasses() throws FormatterException { +// String input = """ +// public sealed abstract class Shape +// permits public final class Circle, public non-sealed class Rectangle { +// public abstract double area(); +// } +// """; +// String expected = """ +// public sealed abstract class Shape +// permits Circle, Rectangle { +// public abstract double area(); +// } +// """; +// assertThat(new Formatter().formatSource(input)).isEqualTo(expected); +// } } diff --git a/core/src/test/java/com/google/googlejavaformat/java/RemoveUnusedDeclarationsTest.java b/core/src/test/java/com/google/googlejavaformat/java/RemoveUnusedDeclarationsTest.java new file mode 100644 index 000000000..a01594b4b --- /dev/null +++ b/core/src/test/java/com/google/googlejavaformat/java/RemoveUnusedDeclarationsTest.java @@ -0,0 +1,290 @@ +/* + * 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.google.googlejavaformat.java; + +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +import java.util.Collection; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.googlejavaformat.java.RemoveUnusedDeclarations.removeUnusedDeclarations; + +@RunWith(Parameterized.class) +public record RemoveUnusedDeclarationsTest(String input, String expected) { + + @Parameters(name = "{index}: {0}") + public static Collection parameters() { + return ImmutableList.builder() + .addAll(workingCases()) + //.addAll(todoCases()) + .build(); + } + + private static Collection workingCases() { + 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 {} + } + """ + } + }, + + // Class with redundant modifiers + { + { + """ + public class TestClass { + public final static String VALUE = "test"; + public abstract void doSomething(); + } + """ + }, + { + """ + public class TestClass { + public static final String VALUE = "test"; + public abstract void doSomething(); + } + """ + } + }, + + // 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 {} + } + """ + } + } + }; + + return buildTestCases(inputsOutputs); + } + + private static Collection todoCases() { + String[][][] inputsOutputs = { + // Enum constants + { + { + """ + public enum TestEnum { + public static final VALUE1, VALUE2; + public static void doSomething() {} + } + """ + }, + { + """ + public enum TestEnum { + VALUE1, VALUE2; + static void doSomething() {} + } + """ + } + }, + + // 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; + } + } + """ + } + }, + + // Records + { + { + """ + public record TestRecord( + public final String name, + public static final int MAX = 100 + ) { + public static void doSomething() {} + } + """ + }, + { + """ + public record TestRecord( + String name, + int MAX = 100 + ) { + static void doSomething() {} + } + """ + } + }, + + // Sealed classes + { + { + """ + public sealed abstract class Shape + permits public final class Circle, public non-sealed class Rectangle { + public abstract double area(); + } + """ + }, + { + """ + public sealed abstract class Shape + permits Circle, Rectangle { + abstract double area(); + } + """ + } + } + }; + + return buildTestCases(inputsOutputs); + } + + private static Collection buildTestCases(String[][][] inputsOutputs) { + ImmutableList.Builder builder = ImmutableList.builder(); + for (String[][] inputAndOutput : inputsOutputs) { + assertThat(inputAndOutput).hasLength(2); + builder.add(new String[]{ + Joiner.on('\n').join(inputAndOutput[0]) + '\n', + Joiner.on('\n').join(inputAndOutput[1]) + '\n', + }); + } + return builder.build(); + } + + @Test + public void removeUnused() throws Exception { + assertThat(removeUnusedDeclarations(input)).isEqualTo(expected); + } +} \ No newline at end of file