Skip to content

Commit

Permalink
Automated rollback of 49eabf7
Browse files Browse the repository at this point in the history
*** Original change description ***

Remove BugPattern.suppressibility and BugPattern.customSuppressionAnnotations

suppressionAnnotations should be used instead.

RELNOTES: Remove BugPattern.suppressibility and BugPattern.customSuppressionAnnotations; suppressionAnnotations should be used instead.

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=178294356
  • Loading branch information
tap-prod authored and cushon committed Dec 13, 2017
1 parent 49eabf7 commit 44a1774
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 43 deletions.
24 changes: 24 additions & 0 deletions annotation/src/main/java/com/google/errorprone/BugPattern.java
Expand Up @@ -207,9 +207,33 @@ public enum SeverityLevel {
SUGGESTION SUGGESTION
} }


/** @deprecated use {@link #suppressionAnnotations} instead. */
@Deprecated
Suppressibility suppressibility() default Suppressibility.DEFAULT;

/** @deprecated use {@link #suppressionAnnotations} instead. */
@Deprecated
public enum Suppressibility {
/**
* Can be suppressed using the standard {@code SuppressWarnings("foo")} mechanism. This setting
* should be used unless there is a good reason otherwise, e.g. security.
*/
SUPPRESS_WARNINGS,
/** Can be suppressed with a custom annotation on a parent AST node. */
CUSTOM_ANNOTATION,
/** Cannot be suppressed. */
UNSUPPRESSIBLE,
/** The default - uses {@link #suppressionAnnotations} instead. */
DEFAULT;
}

/** True if the check can be disabled using command-line flags. */ /** True if the check can be disabled using command-line flags. */
boolean disableable() default true; boolean disableable() default true;


/** @deprecated use {@link #suppressionAnnotations} instead. */
@Deprecated
Class<? extends Annotation>[] customSuppressionAnnotations() default {};

/** /**
* A set of annotation types that can be used to suppress the check. * A set of annotation types that can be used to suppress the check.
* *
Expand Down
56 changes: 40 additions & 16 deletions check_api/src/main/java/com/google/errorprone/BugCheckerInfo.java
Expand Up @@ -22,12 +22,14 @@
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern.SeverityLevel; import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.BugPattern.Suppressibility;
import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Description;
import com.sun.source.tree.Tree; import com.sun.source.tree.Tree;
import java.io.Serializable; import java.io.Serializable;
import java.lang.annotation.Annotation; import java.lang.annotation.Annotation;
import java.lang.reflect.Modifier; import java.lang.reflect.Modifier;
import java.util.Arrays;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.stream.Stream; import java.util.stream.Stream;
Expand Down Expand Up @@ -74,12 +76,16 @@ public class BugCheckerInfo implements Serializable {
*/ */
private final String linkUrl; private final String linkUrl;


private final boolean supportsSuppressWarnings; /**
* Whether this check may be suppressed. Corresponds to the {@code suppressibility} attribute from
* its {@code BugPattern}.
*/
private final Suppressibility suppressibility;


/** /**
* A set of suppression annotations for this check. Computed from the {@code * A set of custom suppression annotations for this check. Computed from the {@code
* suppressionAnnotations} attributes from its {@code BugPattern}. May be empty if there are no * suppressibility} and {@code customSuppressionAnnotations} attributes from its {@code
* suppression annotations for this check. * BugPattern}. May be empty if there are no custom suppression annotations for this check.
*/ */
private final Set<Class<? extends Annotation>> customSuppressionAnnotations; private final Set<Class<? extends Annotation>> customSuppressionAnnotations;


Expand Down Expand Up @@ -112,16 +118,34 @@ private BugCheckerInfo(Class<? extends BugChecker> checker, BugPattern pattern)
pattern.summary(), pattern.summary(),
pattern.severity(), pattern.severity(),
createLinkUrl(pattern), createLinkUrl(pattern),
Stream.of(pattern.suppressionAnnotations()).anyMatch(a -> isSuppressWarnings(a)), suppressibility(pattern),
Stream.of(pattern.suppressionAnnotations()) customSuppressionAnnotations(pattern),
.filter(a -> !isSuppressWarnings(a))
.collect(toImmutableSet()),
ImmutableSet.copyOf(pattern.tags()), ImmutableSet.copyOf(pattern.tags()),
pattern.disableable()); pattern.disableable());
} }


private static boolean isSuppressWarnings(Class<? extends Annotation> annotation) { private static Suppressibility suppressibility(BugPattern pattern) {
return annotation.getSimpleName().equals("SuppressWarnings"); if (pattern.suppressibility() == Suppressibility.DEFAULT) {
if (pattern.suppressionAnnotations().length == 0) {
return Suppressibility.UNSUPPRESSIBLE;
}
if (Arrays.asList(pattern.suppressionAnnotations()).contains(SuppressWarnings.class)) {
return Suppressibility.SUPPRESS_WARNINGS;
}
return Suppressibility.CUSTOM_ANNOTATION;
}
return pattern.suppressibility();
}

private static Set<Class<? extends Annotation>> customSuppressionAnnotations(BugPattern pattern) {
if (pattern.suppressibility() == Suppressibility.DEFAULT) {
return Stream.of(pattern.suppressionAnnotations())
.filter(a -> !a.equals(SuppressWarnings.class))
.collect(toImmutableSet());
}
return pattern.suppressibility() == Suppressibility.CUSTOM_ANNOTATION
? ImmutableSet.copyOf(pattern.customSuppressionAnnotations())
: ImmutableSet.of();
} }


private BugCheckerInfo( private BugCheckerInfo(
Expand All @@ -131,7 +155,7 @@ private BugCheckerInfo(
String message, String message,
SeverityLevel defaultSeverity, SeverityLevel defaultSeverity,
String linkUrl, String linkUrl,
boolean supportsSuppressWarnings, Suppressibility suppressibility,
Set<Class<? extends Annotation>> customSuppressionAnnotations, Set<Class<? extends Annotation>> customSuppressionAnnotations,
ImmutableSet<String> tags, ImmutableSet<String> tags,
boolean disableable) { boolean disableable) {
Expand All @@ -141,7 +165,7 @@ private BugCheckerInfo(
this.message = message; this.message = message;
this.defaultSeverity = defaultSeverity; this.defaultSeverity = defaultSeverity;
this.linkUrl = linkUrl; this.linkUrl = linkUrl;
this.supportsSuppressWarnings = supportsSuppressWarnings; this.suppressibility = suppressibility;
this.customSuppressionAnnotations = customSuppressionAnnotations; this.customSuppressionAnnotations = customSuppressionAnnotations;
this.tags = tags; this.tags = tags;
this.disableable = disableable; this.disableable = disableable;
Expand All @@ -163,7 +187,7 @@ public BugCheckerInfo withCustomDefaultSeverity(SeverityLevel defaultSeverity) {
message, message,
defaultSeverity, defaultSeverity,
linkUrl, linkUrl,
supportsSuppressWarnings, suppressibility,
customSuppressionAnnotations, customSuppressionAnnotations,
tags, tags,
disableable); disableable);
Expand Down Expand Up @@ -231,16 +255,16 @@ public String linkUrl() {
return linkUrl; return linkUrl;
} }


public boolean supportsSuppressWarnings() { public Suppressibility suppressibility() {
return supportsSuppressWarnings; return suppressibility;
} }


public Set<Class<? extends Annotation>> customSuppressionAnnotations() { public Set<Class<? extends Annotation>> customSuppressionAnnotations() {
return customSuppressionAnnotations; return customSuppressionAnnotations;
} }


public boolean disableable() { public boolean disableable() {
return disableable; return suppressibility() != Suppressibility.UNSUPPRESSIBLE && disableable;
} }


public ImmutableSet<String> getTags() { public ImmutableSet<String> getTags() {
Expand Down
Expand Up @@ -18,6 +18,7 @@


import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern.SeverityLevel; import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.BugPattern.Suppressibility;
import com.google.errorprone.matchers.Suppressible; import com.google.errorprone.matchers.Suppressible;
import com.google.errorprone.util.ASTHelpers; import com.google.errorprone.util.ASTHelpers;
import com.sun.tools.javac.code.Attribute; import com.sun.tools.javac.code.Attribute;
Expand Down Expand Up @@ -163,15 +164,21 @@ public static boolean isSuppressed(
SeverityLevel severityLevel, SeverityLevel severityLevel,
boolean inGeneratedCode, boolean inGeneratedCode,
boolean disableWarningsInGeneratedCode) { boolean disableWarningsInGeneratedCode) {
if (suppressible.suppressibility() == Suppressibility.UNSUPPRESSIBLE) {
return false;
}
if (inGeneratedCode && disableWarningsInGeneratedCode && severityLevel != SeverityLevel.ERROR) { if (inGeneratedCode && disableWarningsInGeneratedCode && severityLevel != SeverityLevel.ERROR) {
return true; return true;
} }
if (suppressible.supportsSuppressWarnings() switch (suppressible.suppressibility()) {
&& !Collections.disjoint(suppressible.allNames(), suppressionsOnCurrentPath)) { case CUSTOM_ANNOTATION:
return true; return !Collections.disjoint(
suppressible.customSuppressionAnnotations(), customSuppressionsOnCurrentPath);
case SUPPRESS_WARNINGS:
return !Collections.disjoint(suppressible.allNames(), suppressionsOnCurrentPath);
default:
throw new IllegalStateException("No case for: " + suppressible.suppressibility());
} }
return !Collections.disjoint(
suppressible.customSuppressionAnnotations(), customSuppressionsOnCurrentPath);
} }


private static boolean isGenerated(Symbol sym, VisitorState state) { private static boolean isGenerated(Symbol sym, VisitorState state) {
Expand Down
Expand Up @@ -21,6 +21,7 @@
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.errorprone.BugCheckerInfo; import com.google.errorprone.BugCheckerInfo;
import com.google.errorprone.BugPattern.SeverityLevel; import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.BugPattern.Suppressibility;
import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;
import com.google.errorprone.fixes.Fix; import com.google.errorprone.fixes.Fix;
import com.google.errorprone.matchers.Description; import com.google.errorprone.matchers.Description;
Expand Down Expand Up @@ -175,8 +176,8 @@ public String linkUrl() {
} }


@Override @Override
public boolean supportsSuppressWarnings() { public Suppressibility suppressibility() {
return info.supportsSuppressWarnings(); return info.suppressibility();
} }


@Override @Override
Expand Down Expand Up @@ -421,16 +422,11 @@ public boolean equals(Object obj) {
BugChecker that = (BugChecker) obj; BugChecker that = (BugChecker) obj;
return this.canonicalName().equals(that.canonicalName()) return this.canonicalName().equals(that.canonicalName())
&& this.defaultSeverity().equals(that.defaultSeverity()) && this.defaultSeverity().equals(that.defaultSeverity())
&& this.supportsSuppressWarnings() == that.supportsSuppressWarnings() && this.suppressibility().equals(that.suppressibility());
&& this.customSuppressionAnnotations().equals(that.customSuppressionAnnotations());
} }


@Override @Override
public int hashCode() { public int hashCode() {
return Objects.hash( return Objects.hash(canonicalName(), defaultSeverity(), suppressibility());
canonicalName(),
defaultSeverity(),
supportsSuppressWarnings(),
customSuppressionAnnotations());
} }
} }
Expand Up @@ -16,6 +16,7 @@


package com.google.errorprone.matchers; package com.google.errorprone.matchers;


import com.google.errorprone.BugPattern;
import java.lang.annotation.Annotation; import java.lang.annotation.Annotation;
import java.util.Set; import java.util.Set;


Expand All @@ -30,8 +31,11 @@ public interface Suppressible {
/** The canonical name of the check. */ /** The canonical name of the check. */
String canonicalName(); String canonicalName();


/** Returns true if this checker can be suppressed using {@code @SuppressWarnings}. */ /**
boolean supportsSuppressWarnings(); * Returns how this checker can be suppressed (e.g., via {@code @SuppressWarnings} or a custom
* suppression annotation.
*/
BugPattern.Suppressibility suppressibility();


/** Returns the custom suppression annotations for this checker, if custom suppression is used. */ /** Returns the custom suppression annotations for this checker, if custom suppression is used. */
Set<Class<? extends Annotation>> customSuppressionAnnotations(); Set<Class<? extends Annotation>> customSuppressionAnnotations();
Expand Down
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.SeverityLevel; import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.BugPattern.Suppressibility;
import com.google.errorprone.ErrorProneError; import com.google.errorprone.ErrorProneError;
import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker; import com.google.errorprone.bugpatterns.BugChecker;
Expand Down Expand Up @@ -250,7 +251,9 @@ protected Set<Class<? extends Annotation>> getCustomSuppressionAnnotations() {
private final List<WildcardTreeMatcher> wildcardMatchers = new ArrayList<>(); private final List<WildcardTreeMatcher> wildcardMatchers = new ArrayList<>();


private void registerNodeTypes(BugChecker checker) { private void registerNodeTypes(BugChecker checker) {
customSuppressionAnnotations.addAll(checker.customSuppressionAnnotations()); if (checker.suppressibility() == Suppressibility.CUSTOM_ANNOTATION) {
customSuppressionAnnotations.addAll(checker.customSuppressionAnnotations());
}


if (checker instanceof AnnotationTreeMatcher) { if (checker instanceof AnnotationTreeMatcher) {
annotationMatchers.add((AnnotationTreeMatcher) checker); annotationMatchers.add((AnnotationTreeMatcher) checker);
Expand Down
Expand Up @@ -474,7 +474,7 @@ public void disablingPackageLocation_suppressible() throws Exception {
category = JDK, category = JDK,
severity = ERROR, severity = ERROR,
suppressionAnnotations = {}, suppressionAnnotations = {},
disableable = false disableable = true
) )
public static class UnsuppressiblePackageLocation extends PackageLocation {} public static class UnsuppressiblePackageLocation extends PackageLocation {}


Expand Down
Expand Up @@ -19,6 +19,7 @@
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.errorprone.BugPattern.ProvidesFix; import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.BugPattern.SeverityLevel; import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.BugPattern.Suppressibility;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
Expand Down Expand Up @@ -58,15 +59,51 @@ public static BugPatternInstance fromElement(Element element) {
instance.providesFix = annotation.providesFix(); instance.providesFix = annotation.providesFix();


Map<String, Object> keyValues = getAnnotation(element, BugPattern.class.getName()); Map<String, Object> keyValues = getAnnotation(element, BugPattern.class.getName());
Object suppression = keyValues.get("suppressionAnnotations"); Suppressibility suppressibility;
if (suppression == null) { try {
instance.suppressionAnnotations = new String[] {SuppressWarnings.class.getName()}; suppressibility = annotation.suppressibility();
} else { } catch (EnumConstantNotPresentException e) {
Preconditions.checkState(suppression instanceof List); suppressibility = Suppressibility.DEFAULT;
@SuppressWarnings("unchecked") // Always List<? extends AnnotationValue>, see above. }
List<? extends AnnotationValue> resultList = (List<? extends AnnotationValue>) suppression;
instance.suppressionAnnotations = switch (suppressibility) {
resultList.stream().map(AnnotationValue::toString).toArray(String[]::new); case UNSUPPRESSIBLE:
instance.suppressionAnnotations = new String[0];
break;
case SUPPRESS_WARNINGS:
instance.suppressionAnnotations = new String[] {SuppressWarnings.class.getName()};
break;
case CUSTOM_ANNOTATION:
// Deprecated in favor of DEFAULT/suppressionAnnotations
Object customSuppression = keyValues.get("customSuppressionAnnotations");
if (customSuppression == null) {
instance.suppressionAnnotations = new String[0];
} else {
Preconditions.checkState(customSuppression instanceof List);
// The doc for AnnotationValue says that if the value is an array, then
// AnnotationValue#getValue() will return a List<? extends AnnotationValue>.
@SuppressWarnings("unchecked")
List<? extends AnnotationValue> resultList =
(List<? extends AnnotationValue>) customSuppression;
instance.suppressionAnnotations =
resultList.stream().map(AnnotationValue::toString).toArray(String[]::new);
}
break;
case DEFAULT:
Object suppression = keyValues.get("suppressionAnnotations");
if (suppression == null) {
instance.suppressionAnnotations = new String[] {SuppressWarnings.class.getName()};
} else {
Preconditions.checkState(suppression instanceof List);
@SuppressWarnings("unchecked") // Always List<? extends AnnotationValue>, see above.
List<? extends AnnotationValue> resultList =
(List<? extends AnnotationValue>) suppression;
instance.suppressionAnnotations =
resultList.stream().map(AnnotationValue::toString).toArray(String[]::new);
}
break;
default:
throw new AssertionError(suppressibility);
} }


instance.generateExamplesFromTestCases = instance.generateExamplesFromTestCases =
Expand Down

0 comments on commit 44a1774

Please sign in to comment.