diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/Modes.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/Modes.java index 32135d9c..10bbe5a0 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/Modes.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/Modes.java @@ -27,12 +27,13 @@ import com.palantir.gradle.suppressibleerrorprone.modes.common.ModeName; import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption; import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.CombinedValue; +import com.palantir.gradle.suppressibleerrorprone.modes.interferences.ApplyingAndSuppressingOrRemoveUnusedInterference; import com.palantir.gradle.suppressibleerrorprone.modes.interferences.DisableModeInterference; -import com.palantir.gradle.suppressibleerrorprone.modes.interferences.RemovingAndSuppressingInterference; -import com.palantir.gradle.suppressibleerrorprone.modes.interferences.SuppressingAndApplyingInterference; +import com.palantir.gradle.suppressibleerrorprone.modes.interferences.RemoveRolloutAndSuppressingInterference; import com.palantir.gradle.suppressibleerrorprone.modes.modes.ApplyMode; import com.palantir.gradle.suppressibleerrorprone.modes.modes.DisableMode; import com.palantir.gradle.suppressibleerrorprone.modes.modes.RemoveRolloutMode; +import com.palantir.gradle.suppressibleerrorprone.modes.modes.RemoveUnusedMode; import com.palantir.gradle.suppressibleerrorprone.modes.modes.SuppressMode; import com.palantir.gradle.suppressibleerrorprone.modes.modes.TimingsMode; import java.util.List; @@ -60,13 +61,14 @@ public abstract class Modes { ModeName.APPLY, new ApplyMode(), ModeName.DISABLE, new DisableMode(), ModeName.REMOVE_ROLLOUT, new RemoveRolloutMode(), + ModeName.REMOVE_UNUSED, new RemoveUnusedMode(), ModeName.SUPPRESS, new SuppressMode(), ModeName.TIMINGS, new TimingsMode()); private final Set interferences = Set.of( new DisableModeInterference(), - new RemovingAndSuppressingInterference(), - new SuppressingAndApplyingInterference()); + new RemoveRolloutAndSuppressingInterference(), + new ApplyingAndSuppressingOrRemoveUnusedInterference()); public final CombinedValue modifyCheckApi() { return ModifyCheckApiOption.combine( diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/CommonOptions.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/CommonOptions.java index 796db385..730c7c6d 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/CommonOptions.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/CommonOptions.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.Map; import java.util.function.Supplier; +import java.util.stream.Collectors; import one.util.streamex.EntryStream; /** @@ -56,7 +57,8 @@ public PatchChecksOption patchChecks() { public Map extraErrorProneCheckOptions() { return EntryStream.of(CommonOptions.this.extraErrorProneCheckOptions()) .append(other.extraErrorProneCheckOptions()) - .toMap(); + .collect(Collectors.toMap( + Map.Entry::getKey, Map.Entry::getValue, (val1, val2) -> val1 + "," + val2)); } @Override diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/Mode.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/Mode.java index 137452f2..c7cbd1d6 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/Mode.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/Mode.java @@ -31,6 +31,8 @@ * (see {@link CommonOptions}). Modes can interfere with each other, see {@link ModeInterference}. */ public interface Mode { + String SUPPRESSIBLE_ERROR_PRONE_MODE = "SuppressibleErrorProne:Mode"; + default ModifyCheckApiOption modifyCheckApi() { return ModifyCheckApiOption.noEffect(); } diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/ModeName.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/ModeName.java index d4863897..8e7f3c4c 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/ModeName.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/common/ModeName.java @@ -26,6 +26,7 @@ public enum ModeName { APPLY("errorProneApply"), SUPPRESS("errorProneSuppress"), REMOVE_ROLLOUT("errorProneRemoveRollout"), + REMOVE_UNUSED("errorProneRemoveUnused"), TIMINGS("errorProneTimings"), // Historically, the logic of this plugin lived in baseline, so we need to support the old disable flag. DISABLE("errorProneDisable", "com.palantir.baseline-error-prone.disable"), diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/SuppressingAndApplyingInterference.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/ApplyingAndSuppressingOrRemoveUnusedInterference.java similarity index 54% rename from gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/SuppressingAndApplyingInterference.java rename to gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/ApplyingAndSuppressingOrRemoveUnusedInterference.java index 397bf38d..43847f7d 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/SuppressingAndApplyingInterference.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/ApplyingAndSuppressingOrRemoveUnusedInterference.java @@ -16,22 +16,42 @@ package com.palantir.gradle.suppressibleerrorprone.modes.interferences; +import com.google.common.collect.Sets; import com.palantir.gradle.suppressibleerrorprone.modes.common.CommonOptions; +import com.palantir.gradle.suppressibleerrorprone.modes.common.ModeInterference; +import com.palantir.gradle.suppressibleerrorprone.modes.common.ModeInterferenceResult; import com.palantir.gradle.suppressibleerrorprone.modes.common.ModeName; -import com.palantir.gradle.suppressibleerrorprone.modes.common.SpecificModeInterference; import java.util.Map; import java.util.Set; +import one.util.streamex.EntryStream; + +/** + * Interference between Apply && (Suppress || RemoveUnused). + */ +public final class ApplyingAndSuppressingOrRemoveUnusedInterference implements ModeInterference { -public final class SuppressingAndApplyingInterference extends SpecificModeInterference { @Override - protected Set interferingModes() { - return Set.of(ModeName.SUPPRESS, ModeName.APPLY); + public ModeInterferenceResult interferesWith(Set modeNames) { + if (!modeNames.contains(ModeName.APPLY)) { + return ModeInterferenceResult.noInterference(); + } + + Set interferingWithApply = + Sets.intersection(modeNames, Set.of(ModeName.SUPPRESS, ModeName.REMOVE_UNUSED)); + + if (interferingWithApply.isEmpty()) { + return ModeInterferenceResult.noInterference(); + } + + return ModeInterferenceResult.interferenceBetween(Sets.union(Set.of(ModeName.APPLY), interferingWithApply)); } @Override public CommonOptions interfere(Map modeCommonOptions) { - CommonOptions suppress = modeCommonOptions.get(ModeName.SUPPRESS); CommonOptions apply = modeCommonOptions.get(ModeName.APPLY); + CommonOptions naivelyCombined = EntryStream.of(modeCommonOptions) + .values() + .reduce(CommonOptions.empty(), CommonOptions::naivelyCombinedWith); // If we're applying suggested patches at the same time as suppressing, we still need to tell // errorprone to patch all checks, so we can make suggested fixes for suppressions in any check. @@ -40,9 +60,8 @@ public CommonOptions interfere(Map modeCommonOptions) { // fixes for and which to suppress. So we add the PreferPatchChecks argument here, which we can // use inside error-prone/the compiler. - return suppress.naivelyCombinedWith(apply) - .withExtraErrorProneCheckFlag( - "SuppressibleErrorProne:PreferPatchChecks", - () -> apply.patchChecks().asCommaSeparated().orElse("")); + return naivelyCombined.withExtraErrorProneCheckFlag( + "SuppressibleErrorProne:PreferPatchChecks", + () -> apply.patchChecks().asCommaSeparated().orElse("")); } } diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/RemovingAndSuppressingInterference.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/RemoveRolloutAndSuppressingInterference.java similarity index 95% rename from gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/RemovingAndSuppressingInterference.java rename to gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/RemoveRolloutAndSuppressingInterference.java index db25a1b6..29508b5b 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/RemovingAndSuppressingInterference.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/interferences/RemoveRolloutAndSuppressingInterference.java @@ -25,7 +25,7 @@ * Removing and suppressing cannot run at the same time, as removing just runs an errorprone to remove the for-rollout: * suppressions - if suppressing happened at the same time, this errorprone would just get suppressed. */ -public final class RemovingAndSuppressingInterference implements ModeInterference { +public final class RemoveRolloutAndSuppressingInterference implements ModeInterference { @Override public ModeInterferenceResult interferesWith(Set modeNames) { if (modeNames.containsAll(Set.of(ModeName.REMOVE_ROLLOUT, ModeName.SUPPRESS))) { diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/ApplyMode.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/ApplyMode.java index 513b5cb3..c35df4b5 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/ApplyMode.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/ApplyMode.java @@ -20,6 +20,7 @@ import com.palantir.gradle.suppressibleerrorprone.modes.common.CommonOptions; import com.palantir.gradle.suppressibleerrorprone.modes.common.Mode; import com.palantir.gradle.suppressibleerrorprone.modes.common.PatchChecksOption; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -31,6 +32,11 @@ public CommonOptions configureAndReturnCommonOptions(ModeOptionContext context) public PatchChecksOption patchChecks() { return PatchChecksOption.someChecks(() -> checksToApplySuggestedPatchesFor(context)); } + + @Override + public Map extraErrorProneCheckOptions() { + return Map.of(SUPPRESSIBLE_ERROR_PRONE_MODE, "Apply"); + } }; } diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/RemoveRolloutMode.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/RemoveRolloutMode.java index 9d74af3c..531cb1f6 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/RemoveRolloutMode.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/RemoveRolloutMode.java @@ -49,7 +49,9 @@ public Map extraErrorProneCheckOptions() { return Map.of( "SuppressibleErrorProne:RemoveRolloutSuppressions", - context.flagValue().orElse(ALL_CHECKS)); + context.flagValue().orElse(ALL_CHECKS), + SUPPRESSIBLE_ERROR_PRONE_MODE, + "RemoveRollout"); } @Override diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/RemoveUnusedMode.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/RemoveUnusedMode.java new file mode 100644 index 00000000..11994699 --- /dev/null +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/RemoveUnusedMode.java @@ -0,0 +1,54 @@ +/* + * (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.gradle.suppressibleerrorprone.modes.modes; + +import com.palantir.gradle.suppressibleerrorprone.modes.common.CommonOptions; +import com.palantir.gradle.suppressibleerrorprone.modes.common.Mode; +import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption; +import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.ModifiedFile; +import com.palantir.gradle.suppressibleerrorprone.modes.common.PatchChecksOption; +import java.util.Map; + +public final class RemoveUnusedMode implements Mode { + private static final String ALL_CHECKS = ""; + + @Override + public ModifyCheckApiOption modifyCheckApi() { + return ModifyCheckApiOption.mustModify( + ModifiedFile.BUG_CHECKER_INFO, ModifiedFile.SUPPRESSIBLE_TREE_PATH_SCANNER, ModifiedFile.VISITOR_STATE); + } + + @Override + public CommonOptions configureAndReturnCommonOptions(ModeOptionContext context) { + return new CommonOptions() { + @Override + public PatchChecksOption patchChecks() { + return PatchChecksOption.allChecks(); + } + + @Override + public Map extraErrorProneCheckOptions() { + return Map.of(SUPPRESSIBLE_ERROR_PRONE_MODE, "RemoveUnused"); + } + + @Override + public boolean ignoreSuppressionAnnotations() { + return true; + } + }; + } +} diff --git a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/SuppressMode.java b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/SuppressMode.java index f4b177f5..6ab6b064 100644 --- a/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/SuppressMode.java +++ b/gradle-suppressible-error-prone/src/main/java/com/palantir/gradle/suppressibleerrorprone/modes/modes/SuppressMode.java @@ -21,6 +21,7 @@ import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption; import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.ModifiedFile; import com.palantir.gradle.suppressibleerrorprone.modes.common.PatchChecksOption; +import java.util.Map; public final class SuppressMode implements Mode { @Override @@ -35,6 +36,11 @@ public CommonOptions configureAndReturnCommonOptions(ModeOptionContext context) public PatchChecksOption patchChecks() { return PatchChecksOption.allChecks(); } + + @Override + public Map extraErrorProneCheckOptions() { + return Map.of(SUPPRESSIBLE_ERROR_PRONE_MODE, "Suppress"); + } }; } } diff --git a/suppressible-error-prone/build.gradle b/suppressible-error-prone/build.gradle index ece60e95..3327a9f2 100644 --- a/suppressible-error-prone/build.gradle +++ b/suppressible-error-prone/build.gradle @@ -4,6 +4,8 @@ apply plugin: 'com.palantir.external-publish-jar' dependencies { implementation 'com.google.errorprone:error_prone_annotation' implementation 'com.google.errorprone:error_prone_check_api' + implementation 'com.google.errorprone:error_prone_core' + implementation 'one.util:streamex' annotationProcessor 'com.google.auto.service:auto-service' compileOnly 'com.google.auto.service:auto-service' @@ -45,6 +47,6 @@ publishing { tasks.withType(JavaCompile) { options.errorprone { - disable('StrictUnusedVariable') + disable('StrictUnusedVariable', 'PreferSafeLoggableExceptions') } } diff --git a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/AllErrorprones.java b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/AllErrorprones.java new file mode 100644 index 00000000..c7ed15ce --- /dev/null +++ b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/AllErrorprones.java @@ -0,0 +1,49 @@ +/* + * (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.suppressibleerrorprone; + +import com.google.common.base.Suppliers; +import com.google.errorprone.BugCheckerInfo; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.scanner.BuiltInCheckerSuppliers; +import java.util.ServiceLoader; +import java.util.Set; +import java.util.function.Supplier; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import one.util.streamex.StreamEx; + +public final class AllErrorprones { + private static Supplier> cachedAllBugcheckerNames = Suppliers.memoize(() -> { + Stream pluginChecks = + ServiceLoader.load(BugChecker.class).stream().map(ServiceLoader.Provider::get); + Stream pluginCheckNames = + StreamEx.of(pluginChecks).flatMap(bugchecker -> bugchecker.allNames().stream()); + + Stream builtInChecks = BuiltInCheckerSuppliers.allChecks().getAllChecks().values().stream(); + Stream builtInCheckNames = + StreamEx.of(builtInChecks).flatMap(bugchecker -> bugchecker.allNames().stream()); + + return Stream.concat(pluginCheckNames, builtInCheckNames).collect(Collectors.toSet()); + }); + + public static Set allBugcheckerNames() { + return cachedAllBugcheckerNames.get(); + } + + private AllErrorprones() {} +} diff --git a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/AnnotationUtils.java b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/AnnotationUtils.java index 12f7ef9b..a64bbf7b 100644 --- a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/AnnotationUtils.java +++ b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/AnnotationUtils.java @@ -18,12 +18,16 @@ import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.AssignmentTree; +import com.sun.source.tree.ClassTree; import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.IdentifierTree; import com.sun.source.tree.LiteralTree; import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodTree; +import com.sun.source.tree.ModifiersTree; import com.sun.source.tree.NewArrayTree; import com.sun.source.tree.Tree; +import com.sun.source.tree.VariableTree; import java.util.stream.Stream; import javax.lang.model.element.Name; @@ -67,5 +71,21 @@ static Name annotationName(Tree annotationType) { "Unsupported annotation type: " + annotationType.getClass().getCanonicalName()); } + static boolean isSuppressWarningsAnnotation(AnnotationTree annotation) { + return AnnotationUtils.annotationName(annotation.getAnnotationType()).contentEquals("SuppressWarnings"); + } + + static ModifiersTree getModifiers(Tree suppressible) { + if (suppressible instanceof ClassTree classTree) { + return classTree.getModifiers(); + } else if (suppressible instanceof MethodTree methodTree) { + return methodTree.getModifiers(); + } else if (suppressible instanceof VariableTree variableTree) { + return variableTree.getModifiers(); + } else { + throw new IllegalArgumentException("Unsupported tree type: " + suppressible.getClass()); + } + } + private AnnotationUtils() {} } diff --git a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/ReportedFixCache.java b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/ReportedFixCache.java new file mode 100644 index 00000000..754672bb --- /dev/null +++ b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/ReportedFixCache.java @@ -0,0 +1,104 @@ +/* + * (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.suppressibleerrorprone; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.matchers.Description; +import com.sun.source.tree.AnnotationTree; +import com.sun.source.tree.ModifiersTree; +import com.sun.source.tree.Tree; +import com.sun.source.util.TreePath; +import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition; +import java.util.Optional; +import java.util.Set; +import java.util.WeakHashMap; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.lang.model.element.Name; + +final class ReportedFixCache { + // Weak map so that we don't leak memory by keeping hold of references to the source element tree keys and our + // mutable fixes values around forever, once error-prone has finished with the source element tree used as a key + // here (once the file has been visited by all the error-prone checks), our SuppressingFixes can be safely + // garbage collected. + private final WeakHashMap cache = new WeakHashMap<>(); + + public static final Predicate REMOVE_EVERYTHING = bc -> false; + public static final Predicate KEEP_EVERYTHING = bc -> true; + + ReportedFixCache() {} + + /** + * When first called on a {@code declaration}, initialize a {@code LazySuppressionFix} on it, choosing which + * suppressions to keep with {@code filterExisting}. Subsequent calls on this {@code declaration} will return the + * initialized {@code LazySuppressionFix}. + */ + public LazySuppressionFix getOrReportNew( + TreePath declaration, VisitorState visitorState, Predicate filterExisting) { + return cache.computeIfAbsent( + declaration.getLeaf(), _ignored -> createAndReportFix(declaration, visitorState, filterExisting)); + } + + /** + * Initialize a {@code LazySuppressionFix} on {@code declaration}, choosing which suppressions to keep with + * {@code filterExisting}. + */ + @SuppressWarnings("PreferSafeLoggableExceptions") // It doesn't matter in our internal codebases + public LazySuppressionFix reportNew( + TreePath declaration, VisitorState visitorState, Predicate filterExisting) { + if (cache.containsKey(declaration.getLeaf())) { + throw new IllegalArgumentException("A fix on this declaration already exists"); + } + return cache.put(declaration.getLeaf(), createAndReportFix(declaration, visitorState, filterExisting)); + } + + private LazySuppressionFix createAndReportFix( + TreePath declaration, VisitorState state, Predicate filterExisting) { + if (!SuppressWarningsUtils.suppressibleTreePath(declaration)) { + throw new IllegalArgumentException("Not suppressible: " + declaration); + } + ModifiersTree modifiersTree = AnnotationUtils.getModifiers(declaration.getLeaf()); + Optional suppressWarnings = modifiersTree.getAnnotations().stream() + .filter(annotation -> { + Name annotationName = AnnotationUtils.annotationName(annotation.getAnnotationType()); + return annotationName.contentEquals(CommonConstants.SUPPRESS_WARNINGS_ANNOTATION); + }) + .findFirst(); + + Stream allExistingSuppressions = + suppressWarnings.map(AnnotationUtils::annotationStringValues).orElseGet(Stream::of); + Set filteredExistingSuppressions = + allExistingSuppressions.filter(filterExisting).collect(Collectors.toSet()); + + LazySuppressionFix fix = new LazySuppressionFix( + Optional.ofNullable(state.getSourceCode()), + suppressWarnings, + declaration.getLeaf(), + filteredExistingSuppressions); + @SuppressWarnings("RestrictedApi") + Description description = Description.builder( + (DiagnosticPosition) declaration.getLeaf(), + SuppressibleErrorProne.class.getSimpleName(), + "https://github.com/palantir/suppressible-error-prone", + "A fix on a suppressible by suppressible-error-prone") + .addFix(fix) + .build(); + state.reportMatch(description); + return fix; + } +} diff --git a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressWarningsUtils.java b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressWarningsUtils.java index ed390993..c5773298 100644 --- a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressWarningsUtils.java +++ b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressWarningsUtils.java @@ -16,6 +16,7 @@ package com.palantir.suppressibleerrorprone; +import com.sun.source.tree.AnnotationTree; import com.sun.source.tree.ClassTree; import com.sun.source.tree.LambdaExpressionTree; import com.sun.source.tree.MethodTree; @@ -26,6 +27,7 @@ import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -130,5 +132,15 @@ private static boolean isLambdaParameter(TreePath tree) { return lambdaExpressionTree.getParameters().contains(variableTree); } + public static Optional getSuppressWarnings(TreePath suppressible) { + if (!suppressibleTreePath(suppressible)) { + throw new IllegalArgumentException("Suppress annotations not allowed in " + suppressible); + } + + return AnnotationUtils.getModifiers(suppressible.getLeaf()).getAnnotations().stream() + .filter(AnnotationUtils::isSuppressWarningsAnnotation) + .findFirst(); + } + private SuppressWarningsUtils() {} } diff --git a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressibleErrorProne.java b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressibleErrorProne.java new file mode 100644 index 00000000..347fc098 --- /dev/null +++ b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/SuppressibleErrorProne.java @@ -0,0 +1,37 @@ +/* + * (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.suppressibleerrorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.BugPattern.SeverityLevel; +import com.google.errorprone.bugpatterns.BugChecker; + +/** + * Used by {@link ReportedFixCache} to register {@link LazySuppressionFix}es. + * We need this "default" bugchecker type, as using the name of any individual suppression being removed/added is + * inappropriate — a {@link LazySuppressionFix} may remove or add multiple suppressions. + */ +@AutoService(BugChecker.class) +@BugPattern( + link = "https://github.com/palantir/suppressible-error-prone", + linkType = BugPattern.LinkType.CUSTOM, + severity = SeverityLevel.WARNING, + summary = "A fix on a suppressible by suppressible-error-prone", + // Make it unsuppressible so that it can actually remove itself + suppressionAnnotations = {}) +public class SuppressibleErrorProne extends BugChecker {} diff --git a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/VisitorStateModifications.java b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/VisitorStateModifications.java index d6f17b21..82a47850 100644 --- a/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/VisitorStateModifications.java +++ b/suppressible-error-prone/src/main/java/com/palantir/suppressibleerrorprone/VisitorStateModifications.java @@ -21,39 +21,31 @@ import com.google.errorprone.BugPattern.SeverityLevel; import com.google.errorprone.VisitorState; import com.google.errorprone.matchers.Description; -import com.sun.source.tree.AnnotationTree; -import com.sun.source.tree.ClassTree; -import com.sun.source.tree.MethodTree; -import com.sun.source.tree.ModifiersTree; -import com.sun.source.tree.Tree; -import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; -import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.WeakHashMap; import java.util.logging.Logger; -import java.util.stream.Collectors; import java.util.stream.Stream; -import javax.lang.model.element.Name; // CHECKSTYLE:ON public final class VisitorStateModifications { private static final Logger log = Logger.getLogger(VisitorStateModifications.class.getName()); - // Weak map so that we don't leak memory by keeping hold of references to the source element tree keys and our - // mutable fixes values around forever, once error-prone has finished with the source element tree used as a key - // here (once the file has been visited by all the error-prone checks), our SuppressingFixes can be safely - // garbage collected. - private static final Map FIXES = new WeakHashMap<>(); + private static final ReportedFixCache FIXES = new ReportedFixCache(); @SuppressWarnings("RestrictedApi") public static Description interceptDescription(VisitorState visitorState, Description description) { - if (description == Description.NO_MATCH) { + // Prevent infinite recursion on reported fixes + if (description == Description.NO_MATCH + || description.checkName.equals(SuppressibleErrorProne.class.getSimpleName())) { return description; } + if (getModes(visitorState).contains("RemoveUnused")) { + throw new UnsupportedOperationException("RemoveUnused mode is not yet supported"); + } + // If both -PerrorProneSuppress and -PerrorProneApply are used at the same time, for the checks configured as // "patchChecks" in the extension we need to use their suggested fixes instead of suppressing, so we can do // both in one pass as if you'd run -PerrorProneApply and -PerrorProneSuppress separately. We pass the checks @@ -98,71 +90,21 @@ public static Description interceptDescription(VisitorState visitorState, Descri return Description.NO_MATCH; } - Tree firstSuppressibleParent = firstSuppressible.get().getLeaf(); - - ModifiersTree modifiersTree = modifiersTree(firstSuppressibleParent).get(); - - Optional suppressWarnings = modifiersTree.getAnnotations().stream() - .filter(annotation -> { - Name annotationName = AnnotationUtils.annotationName(annotation.getAnnotationType()); - return annotationName.contentEquals(CommonConstants.SUPPRESS_WARNINGS_ANNOTATION); - }) - .findFirst(); - // In order to be able to suppress multiple errors in one pass on the same element, we need to do a single // Fix/Replacement in error-prone. It's not possible to do this bit by bit with multiple Replacements. To do // this, we make sure we only make one fix per source element we put the suppression on by using a Map. This // way we have our own mutable Fix that we can add errors to, and only once the file has been visited by all // the error-prone checks it will then produce a replacement with all the checks suppressed. - boolean alreadyReportedFix = FIXES.containsKey(firstSuppressibleParent); - - LazySuppressionFix suppressingFix = FIXES.computeIfAbsent(firstSuppressibleParent, _ignored -> { - // Initialize every fix with all remove-rollout: suppressions removed - Stream existingSuppressions = suppressWarnings - .map(AnnotationUtils::annotationStringValues) - .orElseGet(Stream::of); - Set existingNonRolloutSuppressions = existingSuppressions - .filter(sup -> !sup.startsWith(CommonConstants.AUTOMATICALLY_ADDED_PREFIX)) - .collect(Collectors.toSet()); - return new LazySuppressionFix( - Optional.ofNullable(visitorState.getSourceCode()), - suppressWarnings, - firstSuppressibleParent, - existingNonRolloutSuppressions); - }); - + LazySuppressionFix suppressingFix = FIXES.getOrReportNew( + firstSuppressible.get(), + visitorState, + bugchecker -> !bugchecker.startsWith(CommonConstants.AUTOMATICALLY_ADDED_PREFIX)); suppressingFix.addSuppression(CommonConstants.AUTOMATICALLY_ADDED_PREFIX + description.checkName); - - // If we already submitted our mutable fix, we don't need to do so again, just need to add the error to the fix. - if (alreadyReportedFix) { - return Description.NO_MATCH; - } - - return Description.builder( - description.position, - description.checkName, - description.getLink(), - description.getMessageWithoutCheckName()) - .addFix(suppressingFix) - .build(); + return Description.NO_MATCH; } - private static Optional modifiersTree(Tree tree) { - // This covers all type definitions eg class, interface, enum, record, annotation, future kinds - // of class-like type definitions. - if (tree instanceof ClassTree classTree) { - return Optional.of(classTree.getModifiers()); - } - - if (tree instanceof MethodTree methodTree) { - return Optional.of(methodTree.getModifiers()); - } - - if (tree instanceof VariableTree variableTree) { - return Optional.of(variableTree.getModifiers()); - } - - return Optional.empty(); + private static Set getModes(VisitorState state) { + return state.errorProneOptions().getFlags().getSetOrEmpty("SuppressibleErrorProne:Mode"); } private VisitorStateModifications() {} diff --git a/versions.lock b/versions.lock index 8c06c735..c68c6397 100644 --- a/versions.lock +++ b/versions.lock @@ -4,35 +4,41 @@ com.github.ben-manes.caffeine:caffeine:3.0.5 (1 constraints: e312a21b) com.github.kevinstern:software-and-algorithms:1.0 (1 constraints: 7e12fcf5) -com.google.auto:auto-common:1.2.1 (2 constraints: b321cc9d) +com.google.auto:auto-common:1.2.2 (3 constraints: a432fc93) com.google.auto.service:auto-service:1.1.1 (1 constraints: 0505f435) -com.google.auto.service:auto-service-annotations:1.1.1 (1 constraints: 9c0f6a86) +com.google.auto.service:auto-service-annotations:1.1.1 (2 constraints: 8a20341c) -com.google.auto.value:auto-value-annotations:1.10.4 (2 constraints: 141da40a) +com.google.auto.value:auto-value-annotations:1.10.4 (3 constraints: ac2df2f5) -com.google.errorprone:error_prone_annotation:2.41.0 (3 constraints: db2cc946) +com.google.errorprone:error_prone_annotation:2.41.0 (4 constraints: fe3de95e) -com.google.errorprone:error_prone_annotations:2.41.0 (5 constraints: 9e4c911f) +com.google.errorprone:error_prone_annotations:2.41.0 (6 constraints: c15ddb8c) -com.google.errorprone:error_prone_check_api:2.41.0 (2 constraints: ca195cd6) +com.google.errorprone:error_prone_check_api:2.41.0 (3 constraints: ed2a1f5b) + +com.google.errorprone:error_prone_core:2.41.0 (1 constraints: 3e054c3b) + +com.google.googlejavaformat:google-java-format:1.27.0 (2 constraints: b6258eff) com.google.guava:failureaccess:1.0.3 (1 constraints: 160ae3b4) -com.google.guava:guava:33.5.0-jre (13 constraints: 4bf2c3cf) +com.google.guava:guava:33.5.0-jre (14 constraints: ef047572) com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava (1 constraints: bd17c918) com.google.j2objc:j2objc-annotations:3.1 (1 constraints: b809f1a0) +com.google.protobuf:protobuf-java:3.25.5 (1 constraints: 2c1160c9) + com.palantir.gradle.utils:environment-variables:0.19.0 (1 constraints: 3c053e3b) -io.github.eisop:dataflow-errorprone:3.41.0-eisop1 (2 constraints: 9c2c4f1c) +io.github.eisop:dataflow-errorprone:3.41.0-eisop1 (3 constraints: 3e40ddde) io.github.java-diff-utils:java-diff-utils:4.12 (1 constraints: b412c908) -javax.inject:javax.inject:1 (1 constraints: 201230d1) +javax.inject:javax.inject:1 (2 constraints: 51221d52) net.ltgt.gradle:gradle-errorprone-plugin:4.3.0 (1 constraints: 09050836) @@ -40,10 +46,12 @@ one.util:streamex:0.8.4 (1 constraints: 0e050736) org.checkerframework:checker-qual:3.42.0 (3 constraints: 102d771b) -org.jspecify:jspecify:1.0.0 (9 constraints: 8497a3d2) +org.jspecify:jspecify:1.0.0 (10 constraints: 71a8e1bc) org.ow2.asm:asm:9.9 (2 constraints: be0e7759) +org.pcollections:pcollections:4.0.1 (1 constraints: f21028b8) + [Test dependencies] @@ -68,8 +76,6 @@ com.google.code.findbugs:jsr305:3.0.2 (1 constraints: d8120c26) com.google.errorprone:error_prone_test_helpers:2.41.0 (1 constraints: 3e054c3b) -com.google.googlejavaformat:google-java-format:1.27.0 (1 constraints: 9014ad75) - com.google.jimfs:jimfs:1.3.0 (1 constraints: 5a141061) com.google.testing.compile:compile-testing:0.21.0 (1 constraints: 89149575)