Skip to content

Commit

Permalink
Remove BugPattern.suppressibility and BugPattern.customSuppressionAnn…
Browse files Browse the repository at this point in the history
…otations

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=178278597
  • Loading branch information
cushon authored and ronshapiro committed Dec 8, 2017
1 parent 306acd5 commit 49eabf7
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 138 deletions.
24 changes: 0 additions & 24 deletions annotation/src/main/java/com/google/errorprone/BugPattern.java
Expand Up @@ -207,33 +207,9 @@ 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: 16 additions & 40 deletions check_api/src/main/java/com/google/errorprone/BugCheckerInfo.java
Expand Up @@ -22,14 +22,12 @@
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 @@ -76,16 +74,12 @@ 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 custom suppression annotations for this check. Computed from the {@code * A set of suppression annotations for this check. Computed from the {@code
* suppressibility} and {@code customSuppressionAnnotations} attributes from its {@code * suppressionAnnotations} attributes from its {@code BugPattern}. May be empty if there are no
* BugPattern}. May be empty if there are no custom suppression annotations for this check. * suppression annotations for this check.
*/ */
private final Set<Class<? extends Annotation>> customSuppressionAnnotations; private final Set<Class<? extends Annotation>> customSuppressionAnnotations;


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


private static Suppressibility suppressibility(BugPattern pattern) { private static boolean isSuppressWarnings(Class<? extends Annotation> annotation) {
if (pattern.suppressibility() == Suppressibility.DEFAULT) { return annotation.getSimpleName().equals("SuppressWarnings");
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 @@ -155,7 +131,7 @@ private BugCheckerInfo(
String message, String message,
SeverityLevel defaultSeverity, SeverityLevel defaultSeverity,
String linkUrl, String linkUrl,
Suppressibility suppressibility, boolean supportsSuppressWarnings,
Set<Class<? extends Annotation>> customSuppressionAnnotations, Set<Class<? extends Annotation>> customSuppressionAnnotations,
ImmutableSet<String> tags, ImmutableSet<String> tags,
boolean disableable) { boolean disableable) {
Expand All @@ -165,7 +141,7 @@ private BugCheckerInfo(
this.message = message; this.message = message;
this.defaultSeverity = defaultSeverity; this.defaultSeverity = defaultSeverity;
this.linkUrl = linkUrl; this.linkUrl = linkUrl;
this.suppressibility = suppressibility; this.supportsSuppressWarnings = supportsSuppressWarnings;
this.customSuppressionAnnotations = customSuppressionAnnotations; this.customSuppressionAnnotations = customSuppressionAnnotations;
this.tags = tags; this.tags = tags;
this.disableable = disableable; this.disableable = disableable;
Expand All @@ -187,7 +163,7 @@ public BugCheckerInfo withCustomDefaultSeverity(SeverityLevel defaultSeverity) {
message, message,
defaultSeverity, defaultSeverity,
linkUrl, linkUrl,
suppressibility, supportsSuppressWarnings,
customSuppressionAnnotations, customSuppressionAnnotations,
tags, tags,
disableable); disableable);
Expand Down Expand Up @@ -255,16 +231,16 @@ public String linkUrl() {
return linkUrl; return linkUrl;
} }


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


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


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


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


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 @@ -164,21 +163,15 @@ 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;
} }
switch (suppressible.suppressibility()) { if (suppressible.supportsSuppressWarnings()
case CUSTOM_ANNOTATION: && !Collections.disjoint(suppressible.allNames(), suppressionsOnCurrentPath)) {
return !Collections.disjoint( return true;
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,7 +21,6 @@
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 @@ -176,8 +175,8 @@ public String linkUrl() {
} }


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


@Override @Override
Expand Down Expand Up @@ -422,11 +421,16 @@ 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.suppressibility().equals(that.suppressibility()); && this.supportsSuppressWarnings() == that.supportsSuppressWarnings()
&& this.customSuppressionAnnotations().equals(that.customSuppressionAnnotations());
} }


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


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 @@ -31,11 +30,8 @@ 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}. */
* Returns how this checker can be suppressed (e.g., via {@code @SuppressWarnings} or a custom boolean supportsSuppressWarnings();
* 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,7 +20,6 @@
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 @@ -251,9 +250,7 @@ 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) {
if (checker.suppressibility() == Suppressibility.CUSTOM_ANNOTATION) { customSuppressionAnnotations.addAll(checker.customSuppressionAnnotations());
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 = true disableable = false
) )
public static class UnsuppressiblePackageLocation extends PackageLocation {} public static class UnsuppressiblePackageLocation extends PackageLocation {}


Expand Down
Expand Up @@ -19,7 +19,6 @@
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 @@ -59,51 +58,15 @@ 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());
Suppressibility suppressibility; Object suppression = keyValues.get("suppressionAnnotations");
try { if (suppression == null) {
suppressibility = annotation.suppressibility(); instance.suppressionAnnotations = new String[] {SuppressWarnings.class.getName()};
} catch (EnumConstantNotPresentException e) { } else {
suppressibility = Suppressibility.DEFAULT; Preconditions.checkState(suppression instanceof List);
} @SuppressWarnings("unchecked") // Always List<? extends AnnotationValue>, see above.

List<? extends AnnotationValue> resultList = (List<? extends AnnotationValue>) suppression;
switch (suppressibility) { instance.suppressionAnnotations =
case UNSUPPRESSIBLE: resultList.stream().map(AnnotationValue::toString).toArray(String[]::new);
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 49eabf7

Please sign in to comment.