Skip to content
Draft
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import com.palantir.gradle.suppressibleerrorprone.modes.Modes;
import com.palantir.gradle.suppressibleerrorprone.modes.common.CommonOptions;
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption;
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.ModifiedFile;
import com.palantir.gradle.suppressibleerrorprone.transform.ModifyErrorProneCheckApi;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import javax.inject.Inject;
import net.ltgt.gradle.errorprone.ErrorProneOptions;
Expand Down Expand Up @@ -75,7 +77,7 @@ private void applyToJavaProject(Project project) {
if (getModes().modifyCheckApi() instanceof ModifyCheckApiOption.MustModify mustModify) {
// When auto-suppressing, the logic will run a bytecode patched version of errorprone
// (via an artifact transform) that intercepts every error from every check and adds a custom fix
setupErrorProneArtifactTransform(project, mustModify.modifyVisitorState());
setupErrorProneArtifactTransform(project, mustModify.modifiedFiles());
}

project.getConfigurations().named(ErrorPronePlugin.CONFIGURATION_NAME).configure(errorProneConfiguration -> {
Expand Down Expand Up @@ -108,7 +110,7 @@ private void applyToJavaProject(Project project) {
});
}

private static void setupErrorProneArtifactTransform(Project project, boolean modifyVisitorState) {
private static void setupErrorProneArtifactTransform(Project project, Set<ModifiedFile> modifiedFiles) {
Attribute<Boolean> suppressible =
Attribute.of("com.palantir.suppressible-error-prone.suppressible", Boolean.class);
project.getDependencies().getAttributesSchema().attribute(suppressible);
Expand All @@ -120,7 +122,7 @@ private static void setupErrorProneArtifactTransform(Project project, boolean mo
.attribute(suppressible, false);

project.getDependencies().registerTransform(ModifyErrorProneCheckApi.class, spec -> {
spec.getParameters().getModifyVisitorState().set(modifyVisitorState);
spec.getParameters().getFilesToModify().set(modifiedFiles);

Attribute<String> artifactType = Attribute.of("artifactType", String.class);
spec.getFrom().attribute(suppressible, false).attribute(artifactType, "jar");
Expand Down Expand Up @@ -205,6 +207,10 @@ private void setupErrorProneOptions(CommonOptions commonOptions, ErrorProneOptio

errorProneOptions.getErrorproneArgumentProviders().add(patchChecksCommandLineArgumentProvider);

errorProneOptions
.getIgnoreSuppressionAnnotations()
.set(getProviderFactory().provider(commonOptions::ignoreSuppressionAnnotations));

errorProneOptions
.getCheckOptions()
.putAll(getProviderFactory().provider(commonOptions::extraErrorProneCheckOptions));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption;
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.CombinedValue;
import com.palantir.gradle.suppressibleerrorprone.modes.interferences.DisableModeInterference;
import com.palantir.gradle.suppressibleerrorprone.modes.interferences.RemovingAndSuppressingInterference;
import com.palantir.gradle.suppressibleerrorprone.modes.interferences.RemoveRolloutAndSuppressingInterference;
import com.palantir.gradle.suppressibleerrorprone.modes.interferences.RemoveUnusedAndApplyingInterference;
import com.palantir.gradle.suppressibleerrorprone.modes.interferences.RemoveUnusedAndSuppressInterference;
import com.palantir.gradle.suppressibleerrorprone.modes.interferences.SuppressingAndApplyingInterference;
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;
Expand Down Expand Up @@ -60,13 +63,16 @@ 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<ModeInterference> interferences = Set.of(
new DisableModeInterference(),
new RemovingAndSuppressingInterference(),
new SuppressingAndApplyingInterference());
new RemoveRolloutAndSuppressingInterference(),
new SuppressingAndApplyingInterference(),
new RemoveUnusedAndApplyingInterference(),
new RemoveUnusedAndSuppressInterference());

public final CombinedValue modifyCheckApi() {
return ModifyCheckApiOption.combine(
Expand Down Expand Up @@ -127,7 +133,8 @@ public final CommonOptions commonOptionsFor(JavaCompile javaCompile) {
.append(nonInterferingCommonOptions)
.toSet();

return CommonOptions.naivelyCombine(allCommonOptions);
CommonOptions commonOptions = CommonOptions.naivelyCombine(allCommonOptions);
return commonOptions;
}

private Map<ModeName, Optional<String>> modesEnabledAndFlagValues() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -41,6 +42,10 @@ default RemoveRolloutCheck removeRolloutCheck() {
return RemoveRolloutCheck.DISABLE;
}

default boolean ignoreSuppressionAnnotations() {
return false;
}

default CommonOptions naivelyCombinedWith(CommonOptions other) {
return new CommonOptions() {
@Override
Expand All @@ -52,32 +57,51 @@ public PatchChecksOption patchChecks() {
public Map<String, String> 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
public RemoveRolloutCheck removeRolloutCheck() {
return CommonOptions.this.removeRolloutCheck().or(other.removeRolloutCheck());
}

@Override
public boolean ignoreSuppressionAnnotations() {
return CommonOptions.this.ignoreSuppressionAnnotations() || other.ignoreSuppressionAnnotations();
}
};
}

static CommonOptions naivelyCombine(Collection<CommonOptions> commonOptions) {
return commonOptions.stream().reduce(CommonOptions.empty(), CommonOptions::naivelyCombinedWith);
}

// NOTE: Maybe CommonOptions itself should be an interface without default methods, while a
// DefaultCommonOptions provides sensible defaults.
// Or maybe CommonOptions should just be a record with methods
default CommonOptions withExtraErrorProneCheckFlag(String key, Supplier<String> value) {
return new CommonOptions() {
@Override
public Map<String, String> extraErrorProneCheckOptions() {
Map<String, String> map = new HashMap<>(CommonOptions.this.extraErrorProneCheckOptions());
map.put(key, value.get());
return Collections.unmodifiableMap(map);
}

@Override
public PatchChecksOption patchChecks() {
return CommonOptions.this.patchChecks();
}

@Override
public Map<String, String> extraErrorProneCheckOptions() {
Map<String, String> map = new HashMap<>(CommonOptions.this.extraErrorProneCheckOptions());
map.put(key, value.get());
return Collections.unmodifiableMap(map);
public RemoveRolloutCheck removeRolloutCheck() {
return CommonOptions.this.removeRolloutCheck();
}

@Override
public boolean ignoreSuppressionAnnotations() {
return CommonOptions.this.ignoreSuppressionAnnotations();
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.DoNotModify;
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.MustModify;
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModifyCheckApiOption.NoEffect;
import java.util.Arrays;
import java.util.Collection;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import one.util.streamex.StreamEx;

/**
Expand All @@ -47,19 +50,16 @@ static ModifyCheckApiOption noEffect() {
}

/**
* The {@code error_prone_check_api} jar must be modified to allow {@code `for-rollout:`} suppressions to work.
* Modify these files in the error-prone API.
*/
static ModifyCheckApiOption mustModify() {
return new MustModify(false);
static MustModify mustModify(ModifiedFile... modifiedFile) {
return new MustModify(Arrays.stream(modifiedFile).collect(Collectors.toSet()));
}

/**
* The {@code error_prone_check_api} jar must be modified to allow {@code `for-rollout:`} suppressions to work,
* and {@code VisitorState} must be modified to intercept reportMatch.
*/
static ModifyCheckApiOption mustModifyIncludingVisitorState() {
return new MustModify(true);
default Set<ModifiedFile> getModifiedFiles() {
return Set.of();
}
;

enum DoNotModify implements ModifyCheckApiOption, CombinedValue {
INSTANCE
Expand All @@ -69,9 +69,11 @@ enum NoEffect implements ModifyCheckApiOption {
INSTANCE
}

record MustModify(boolean modifyVisitorState) implements ModifyCheckApiOption, CombinedValue {
record MustModify(Set<ModifiedFile> modifiedFiles) implements ModifyCheckApiOption, CombinedValue {
public MustModify combine(MustModify other) {
return new MustModify(modifyVisitorState || other.modifyVisitorState);
Set<ModifiedFile> union = Stream.concat(modifiedFiles.stream(), other.modifiedFiles.stream())
.collect(Collectors.toSet());
return new MustModify(union);
}
}

Expand All @@ -84,7 +86,7 @@ static CombinedValue combine(Collection<ModifyCheckApiOption> options) {

if (withoutNoEffects.isEmpty()) {
// By default, we need to modify the check API to support for-rollout suppressions
return new MustModify(false);
return mustModify(ModifiedFile.BUG_CHECKER_INFO);
}

boolean doNotModify = withoutNoEffects.contains(DoNotModify.INSTANCE);
Expand All @@ -103,4 +105,20 @@ static CombinedValue combine(Collection<ModifyCheckApiOption> options) {
.reduce(MustModify::combine)
.get();
}

enum ModifiedFile {
BUG_CHECKER_INFO("BugCheckerInfo"),
VISITOR_STATE("VisitorState"),
SUPPRESSIBLE_TREE_PATH_SCANNER("SuppressibleTreePathScanner");

private final String className;

ModifiedFile(String className) {
this.className = className;
}

public String getClassName() {
return className;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<ModeName> modeNames) {
if (modeNames.containsAll(Set.of(ModeName.REMOVE_ROLLOUT, ModeName.SUPPRESS))) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.gradle.suppressibleerrorprone.modes.interferences;

import com.palantir.gradle.suppressibleerrorprone.modes.common.CommonOptions;
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModeName;
import com.palantir.gradle.suppressibleerrorprone.modes.common.SpecificModeInterference;
import java.util.Map;
import java.util.Set;

public final class RemoveUnusedAndApplyingInterference extends SpecificModeInterference {
@Override
protected Set<ModeName> interferingModes() {
return Set.of(ModeName.REMOVE_UNUSED, ModeName.APPLY);
}

@Override
public CommonOptions interfere(Map<ModeName, CommonOptions> modeCommonOptions) {
CommonOptions removeUnused = modeCommonOptions.get(ModeName.REMOVE_UNUSED);
CommonOptions apply = modeCommonOptions.get(ModeName.APPLY);

// 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.
// However, inside our changes to errorprone, we need to get the list of checks that we're going
// to use the default suggested fixes for, so we can work out which ones to use the suggested
// fixes for and which to suppress. So we add the PreferPatchChecks argument here, which we can
// use inside error-prone/the compiler.

return removeUnused
.naivelyCombinedWith(apply)
.withExtraErrorProneCheckFlag(
"SuppressibleErrorProne:PreferPatchChecks",
() -> apply.patchChecks().asCommaSeparated().orElse(""));
}
}
Original file line number Diff line number Diff line change
@@ -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.gradle.suppressibleerrorprone.modes.interferences;

import com.palantir.gradle.suppressibleerrorprone.modes.common.CommonOptions;
import com.palantir.gradle.suppressibleerrorprone.modes.common.ModeName;
import com.palantir.gradle.suppressibleerrorprone.modes.common.SpecificModeInterference;
import java.util.Map;
import java.util.Set;

public final class RemoveUnusedAndSuppressInterference extends SpecificModeInterference {
@Override
protected Set<ModeName> interferingModes() {
return Set.of(ModeName.REMOVE_UNUSED, ModeName.SUPPRESS);
}

@Override
public CommonOptions interfere(Map<ModeName, CommonOptions> modeCommonOptions) {
CommonOptions removeUnused = modeCommonOptions.get(ModeName.REMOVE_UNUSED);
CommonOptions suppress = modeCommonOptions.get(ModeName.SUPPRESS);
return removeUnused.naivelyCombinedWith(suppress);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -31,6 +32,11 @@ public CommonOptions configureAndReturnCommonOptions(ModeOptionContext context)
public PatchChecksOption patchChecks() {
return PatchChecksOption.someChecks(() -> checksToApplySuggestedPatchesFor(context));
}

@Override
public Map<String, String> extraErrorProneCheckOptions() {
return Map.of("SuppressibleErrorProne:Mode", "Apply");
}
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ public Map<String, String> extraErrorProneCheckOptions() {

return Map.of(
"SuppressibleErrorProne:RemoveRolloutSuppressions",
context.flagValue().orElse(ALL_CHECKS));
context.flagValue().orElse(ALL_CHECKS),
"SuppressibleErrorProne:Mode",
"RemoveRollout");
}

@Override
Expand Down
Loading