counters() {
}
public Name getName(String nameStr) {
- return Names.instance(context).fromString(nameStr);
+ return getNames().fromString(nameStr);
}
/**
@@ -331,7 +328,8 @@ public Name getName(String nameStr) {
*/
@Nullable
public Type getTypeFromString(String typeStr) {
- return typeCache
+ return sharedState
+ .typeCache
.computeIfAbsent(typeStr, key -> Optional.fromNullable(getTypeFromStringInternal(key)))
.orNull();
}
@@ -360,12 +358,11 @@ private Type getTypeFromStringInternal(String typeStr) {
public Symbol getSymbolFromString(String symStr) {
symStr = inferBinaryName(symStr);
Name name = getName(symStr);
- Modules modules = Modules.instance(context);
- boolean modular = modules.getDefaultModule() != getSymtab().noModule;
+ boolean modular = sharedState.modules.getDefaultModule() != getSymtab().noModule;
if (!modular) {
return getSymbolFromString(getSymtab().noModule, name);
}
- for (ModuleSymbol msym : Modules.instance(context).allModules()) {
+ for (ModuleSymbol msym : sharedState.modules.allModules()) {
ClassSymbol result = getSymbolFromString(msym, name);
if (result != null) {
// TODO(cushon): the path where we iterate over all modules is probably slow.
@@ -401,19 +398,32 @@ public ClassSymbol getSymbolFromString(ModuleSymbol msym, Name name) {
// TODO(cushon): consider migrating call sites to use binary names and removing this code.
// (But then we'd probably want error handling for probably-incorrect canonical names,
// so it may not end up being a performance win.)
- private static String inferBinaryName(String classname) {
- StringBuilder sb = new StringBuilder();
- boolean first = true;
- char sep = '.';
- for (String bit : Splitter.on('.').split(classname)) {
- if (!first) {
- sb.append(sep);
- }
- sb.append(bit);
- if (Character.isUpperCase(bit.charAt(0))) {
- sep = '$';
+ @VisibleForTesting
+ static String inferBinaryName(String classname) {
+ int len = classname.length();
+ checkArgument(!classname.isEmpty(), "class name must be non-empty");
+ checkArgument(classname.charAt(len - 1) != '.', "invalid class name: %s", classname);
+
+ int lastPeriod = classname.lastIndexOf('.');
+ if (lastPeriod == -1) {
+ return classname; // top level class in default package
+ }
+ int secondToLastPeriod = classname.lastIndexOf('.', lastPeriod - 1);
+ if (secondToLastPeriod != -1
+ && !Character.isUpperCase(classname.charAt(secondToLastPeriod + 1))) {
+ return classname; // top level class
+ }
+
+ StringBuilder sb = new StringBuilder(len);
+ boolean foundUppercase = false;
+ for (int i = 0; i < len; i++) {
+ char c = classname.charAt(i);
+ foundUppercase = foundUppercase || Character.isUpperCase(c);
+ if (c == '.') {
+ sb.append(foundUppercase ? '$' : '.');
+ } else {
+ sb.append(c);
}
- first = false;
}
return sb.toString();
}
@@ -605,6 +615,35 @@ public boolean isAndroidCompatible() {
/** Returns a timing span for the given {@link Suppressible}. */
public AutoCloseable timingSpan(Suppressible suppressible) {
- return timings.span(suppressible);
+ return sharedState.timings.span(suppressible);
+ }
+
+ /**
+ * Instances that every {@link VisitorState} instance can share.
+ *
+ * For the types that are typically stored in {@link Context}, caching the references over
+ * {@code SomeClass.instance(context)} has sizable performance improvements in aggregate.
+ */
+ private static final class SharedState {
+ private final Modules modules;
+ private final Names names;
+ private final Symtab symtab;
+ private final ErrorProneTimings timings;
+ private final Types types;
+ private final TreeMaker treeMaker;
+
+ // TODO(ronshapiro): should we presize this with a reasonable size? We can check for the
+ // smallest build and see how many types are loaded and use that. Or perhaps a heuristic
+ // based on number of files?
+ private final Map> typeCache = new HashMap<>();
+
+ SharedState(Context context) {
+ this.modules = Modules.instance(context);
+ this.names = Names.instance(context);
+ this.symtab = Symtab.instance(context);
+ this.timings = ErrorProneTimings.instance(context);
+ this.types = Types.instance(context);
+ this.treeMaker = TreeMaker.instance(context);
+ }
}
}
diff --git a/check_api/src/main/java/com/google/errorprone/bugpatterns/BugChecker.java b/check_api/src/main/java/com/google/errorprone/bugpatterns/BugChecker.java
index be1e971ee61..2870f95aa94 100644
--- a/check_api/src/main/java/com/google/errorprone/bugpatterns/BugChecker.java
+++ b/check_api/src/main/java/com/google/errorprone/bugpatterns/BugChecker.java
@@ -245,6 +245,10 @@ public boolean supportsSuppressWarnings() {
return info.supportsSuppressWarnings();
}
+ public boolean disableable() {
+ return info.disableable();
+ }
+
@Override
public Set> customSuppressionAnnotations() {
return info.customSuppressionAnnotations();
diff --git a/check_api/src/main/java/com/google/errorprone/matchers/Matchers.java b/check_api/src/main/java/com/google/errorprone/matchers/Matchers.java
index 7bd112334b9..2df77b1b9cc 100644
--- a/check_api/src/main/java/com/google/errorprone/matchers/Matchers.java
+++ b/check_api/src/main/java/com/google/errorprone/matchers/Matchers.java
@@ -23,6 +23,7 @@
import static com.google.errorprone.suppliers.Suppliers.typeFromClass;
import static com.google.errorprone.util.ASTHelpers.getType;
import static com.google.errorprone.util.ASTHelpers.stripParentheses;
+import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.VisitorState;
@@ -351,10 +352,11 @@ public static Matcher argumentCount(final int argumentCoun
* parentNode(kindIs(Kind.RETURN))} would match the {@code this} expression in {@code return
* this;}
*/
- public static Matcher parentNode(Matcher treeMatcher) {
- @SuppressWarnings("unchecked") // Safe contravariant cast
- Matcher matcher = (Matcher) treeMatcher;
- return new ParentNode(matcher);
+ public static Matcher parentNode(Matcher treeMatcher) {
+ return (tree, state) -> {
+ TreePath parent = requireNonNull(state.getPath().getParentPath());
+ return treeMatcher.matches(parent.getLeaf(), state.withPath(parent));
+ };
}
/**
@@ -468,14 +470,13 @@ public static Enclosing.Method enclosingMethod(MatcherTODO(eaftan): This could be used instead of enclosingBlock and enclosingClass.
*/
- public static Matcher enclosingNode(final Matcher matcher) {
- // TODO(cushon): this should take a Class
+ public static Matcher enclosingNode(Matcher matcher) {
return (t, state) -> {
TreePath path = state.getPath().getParentPath();
while (path != null) {
Tree node = path.getLeaf();
state = state.withPath(path);
- if (matcher.matches((T) node, state)) {
+ if (matcher.matches(node, state)) {
return true;
}
path = path.getParentPath();
diff --git a/check_api/src/main/java/com/google/errorprone/matchers/NextStatement.java b/check_api/src/main/java/com/google/errorprone/matchers/NextStatement.java
index 316f4982b9c..4aaec469ca4 100644
--- a/check_api/src/main/java/com/google/errorprone/matchers/NextStatement.java
+++ b/check_api/src/main/java/com/google/errorprone/matchers/NextStatement.java
@@ -53,7 +53,7 @@ public boolean matches(T statement, VisitorState state) {
nextStmt = blockStatements.get(statementIndex);
}
- // TODO(glorioso): return false always instead of allowing the matcher to fail to match null?
+ // TODO(b/134670335): return false always instead of allowing the matcher to fail to match null?
return matcher.matches(nextStmt, state);
}
}
diff --git a/check_api/src/main/java/com/google/errorprone/matchers/ParentNode.java b/check_api/src/main/java/com/google/errorprone/matchers/ParentNode.java
deleted file mode 100644
index 6a20c64ecee..00000000000
--- a/check_api/src/main/java/com/google/errorprone/matchers/ParentNode.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Copyright 2011 The Error Prone Authors.
- *
- * 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.errorprone.matchers;
-
-import com.google.errorprone.VisitorState;
-import com.sun.source.tree.Tree;
-
-/** @author alexeagle@google.com (Alex Eagle) */
-public class ParentNode implements Matcher {
- private final Matcher treeMatcher;
-
- public ParentNode(Matcher treeMatcher) {
- this.treeMatcher = treeMatcher;
- }
-
- @Override
- public boolean matches(Tree tree, VisitorState state) {
- Tree parent = state.getPath().getParentPath().getLeaf();
- try {
- return treeMatcher.matches(parent, state.withPath(state.getPath().getParentPath()));
- } catch (ClassCastException e) {
- return false;
- }
- }
-}
diff --git a/check_api/src/main/java/com/google/errorprone/predicates/TypePredicates.java b/check_api/src/main/java/com/google/errorprone/predicates/TypePredicates.java
index 79d83581b38..f40faa3d155 100644
--- a/check_api/src/main/java/com/google/errorprone/predicates/TypePredicates.java
+++ b/check_api/src/main/java/com/google/errorprone/predicates/TypePredicates.java
@@ -78,4 +78,30 @@ public static TypePredicate isDescendantOfAny(Iterable types) {
public static TypePredicate isDescendantOf(String type) {
return isDescendantOf(Suppliers.typeFromString(type));
}
+
+ public static TypePredicate allOf(TypePredicate... predicates) {
+ return (type, state) -> {
+ for (TypePredicate predicate : predicates) {
+ if (!predicate.apply(type, state)) {
+ return false;
+ }
+ }
+ return true;
+ };
+ }
+
+ public static TypePredicate anyOf(TypePredicate... predicates) {
+ return (type, state) -> {
+ for (TypePredicate predicate : predicates) {
+ if (predicate.apply(type, state)) {
+ return true;
+ }
+ }
+ return false;
+ };
+ }
+
+ public static TypePredicate not(TypePredicate predicate) {
+ return (type, state) -> !predicate.apply(type, state);
+ }
}
diff --git a/check_api/src/main/java/com/google/errorprone/scanner/ErrorProneScanner.java b/check_api/src/main/java/com/google/errorprone/scanner/ErrorProneScanner.java
index acc7bb5f740..04e713da349 100644
--- a/check_api/src/main/java/com/google/errorprone/scanner/ErrorProneScanner.java
+++ b/check_api/src/main/java/com/google/errorprone/scanner/ErrorProneScanner.java
@@ -16,6 +16,7 @@
package com.google.errorprone.scanner;
+
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
@@ -537,6 +538,7 @@ public Void visitClass(ClassTree tree, VisitorState visitorState) {
@Override
public Void visitCompilationUnit(CompilationUnitTree tree, VisitorState visitorState) {
+
VisitorState state =
processMatchers(
compilationUnitMatchers,
diff --git a/check_api/src/main/java/com/google/errorprone/scanner/ScannerSupplier.java b/check_api/src/main/java/com/google/errorprone/scanner/ScannerSupplier.java
index bec1ec27547..92944da3689 100644
--- a/check_api/src/main/java/com/google/errorprone/scanner/ScannerSupplier.java
+++ b/check_api/src/main/java/com/google/errorprone/scanner/ScannerSupplier.java
@@ -25,6 +25,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
+import com.google.common.collect.Streams;
import com.google.errorprone.BugCheckerInfo;
import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.ErrorProneFlags;
@@ -76,15 +77,14 @@ public static ScannerSupplier fromBugCheckerClasses(
/** Returns a {@link ScannerSupplier} built from a list of {@link BugCheckerInfo}s. */
public static ScannerSupplier fromBugCheckerInfos(Iterable checkers) {
- ImmutableBiMap.Builder builder = ImmutableBiMap.builder();
- for (BugCheckerInfo checker : checkers) {
- builder.put(checker.canonicalName(), checker);
- }
- ImmutableBiMap allChecks = builder.build();
+ ImmutableBiMap allChecks =
+ Streams.stream(checkers)
+ .collect(
+ ImmutableBiMap.toImmutableBiMap(BugCheckerInfo::canonicalName, checker -> checker));
return new ScannerSupplierImpl(
allChecks,
defaultSeverities(allChecks.values()),
- ImmutableSet.of(),
+ ImmutableSet.of(),
ErrorProneFlags.empty());
}
@@ -279,12 +279,11 @@ public ScannerSupplier plus(ScannerSupplier other) {
*/
@CheckReturnValue
public ScannerSupplier filter(Predicate predicate) {
- ImmutableSet.Builder disabled = ImmutableSet.builder();
- for (BugCheckerInfo check : getAllChecks().values()) {
- if (!predicate.apply(check)) {
- disabled.add(check.canonicalName());
- }
- }
- return new ScannerSupplierImpl(getAllChecks(), severities(), disabled.build(), getFlags());
+ ImmutableSet disabled =
+ getAllChecks().values().stream()
+ .filter(predicate.negate())
+ .map(BugCheckerInfo::canonicalName)
+ .collect(ImmutableSet.toImmutableSet());
+ return new ScannerSupplierImpl(getAllChecks(), severities(), disabled, getFlags());
}
}
diff --git a/check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java b/check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java
index b311d7ff6b8..1ea110850a8 100644
--- a/check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java
+++ b/check_api/src/test/java/com/google/errorprone/fixes/AppliedFixTest.java
@@ -20,7 +20,7 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertThrows;
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java
index 508cf91a36d..c5b6225787a 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractToString.java
@@ -21,7 +21,6 @@
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.staticMethod;
-import com.google.common.base.Optional;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.IdentifierTreeMatcher;
import com.google.errorprone.bugpatterns.BugChecker.MemberSelectTreeMatcher;
@@ -42,6 +41,7 @@
import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Type.MethodType;
+import java.util.Optional;
/**
* An abstract matcher for implicit and explicit calls to {@code Object.toString()}, for use on
@@ -65,7 +65,7 @@ public abstract class AbstractToString extends BugChecker
/** Adds the description message for match on the type without fixes. */
protected Optional descriptionMessageForDefaultMatch(Type type, VisitorState state) {
- return Optional.absent();
+ return Optional.empty();
}
/**
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ArrayToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/ArrayToString.java
index 06443de0dcf..8dc638a06f0 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/ArrayToString.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/ArrayToString.java
@@ -19,7 +19,6 @@
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
-import com.google.common.base.Optional;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.fixes.Fix;
@@ -30,6 +29,7 @@
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree;
+import java.util.Optional;
/**
* @author adgar@google.com (Mike Edgar)
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/BadImport.java b/core/src/main/java/com/google/errorprone/bugpatterns/BadImport.java
index 0ad4640bcc6..864930becbe 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/BadImport.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/BadImport.java
@@ -64,10 +64,18 @@ public class BadImport extends BugChecker implements ImportTreeMatcher {
private static final ImmutableSet BAD_NESTED_CLASSES =
ImmutableSet.of(
- "Builder");
+ "Builder",
+ "Class");
private static final ImmutableSet BAD_STATIC_IDENTIFIERS =
ImmutableSet.of(
- "copyOf", "of", "from", "INSTANCE", "builder", "newBuilder", "getDefaultInstance");
+ "copyOf",
+ "of",
+ "from",
+ "INSTANCE",
+ "builder",
+ "newBuilder",
+ "getDefaultInstance",
+ "valueOf");
private static final MultiMatcher HAS_TYPE_USE_ANNOTATION =
annotations(AT_LEAST_ONE, (t, state) -> isTypeAnnotation(t));
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ChainedAssertionLosesContext.java b/core/src/main/java/com/google/errorprone/bugpatterns/ChainedAssertionLosesContext.java
index 741dcd40af7..49b90aeac43 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/ChainedAssertionLosesContext.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/ChainedAssertionLosesContext.java
@@ -21,13 +21,15 @@
import static com.google.errorprone.BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.bugpatterns.ImplementAssertionWithChaining.makeCheckDescription;
-import static com.google.errorprone.bugpatterns.ProvideDescriptionToCheck.findThatCall;
import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.util.ASTHelpers.getDeclaredSymbol;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.isSubtype;
import static com.sun.source.tree.Tree.Kind.CLASS;
+import static com.sun.source.tree.Tree.Kind.METHOD_INVOCATION;
import static java.lang.String.format;
import static java.util.stream.Stream.concat;
import static javax.lang.model.element.Modifier.STATIC;
@@ -140,6 +142,38 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
}
}
+ /**
+ * Starting from a {@code VisitorState} pointing at part of a fluent assertion statement (like
+ * {@code check()} or {@code assertWithMessage()}, walks up the tree and returns the subsequent
+ * call to {@code that(...)}.
+ *
+ * Often, the call is made directly on the result of the given tree -- like when the input is
+ * {@code check()}, which is part of the expression {@code check().that(...)}. But sometimes there
+ * is an intervening call to {@code withMessage}, {@code about}, or both.
+ */
+ static MethodInvocationTree findThatCall(VisitorState state) {
+ TreePath path = state.getPath();
+ /*
+ * Each iteration walks 1 method call up the tree, but it's actually 2 steps in the tree because
+ * there's a MethodSelectTree between each pair of MethodInvocationTrees.
+ */
+ while (true) {
+ path = path.getParentPath().getParentPath();
+ Tree leaf = path.getLeaf();
+ if (leaf.getKind() != METHOD_INVOCATION) {
+ return null;
+ }
+ MethodInvocationTree maybeThatCall = (MethodInvocationTree) leaf;
+ if (WITH_MESSAGE_OR_ABOUT.matches(maybeThatCall, state)) {
+ continue;
+ } else if (SUBJECT_BUILDER_THAT.matches(maybeThatCall, state)) {
+ return maybeThatCall;
+ } else {
+ return null;
+ }
+ }
+ }
+
@FormatMethod
private Description replace(Tree tree, String format, Object... args) {
return describeMatch(tree, SuggestedFix.replace(tree, String.format(format, args)));
@@ -252,4 +286,21 @@ private static String findThatCallAndMakeCheckDescription(VisitorState state) {
staticMethod().onClass(TRUTH_CLASS).named("assertWithMessage");
private static final Matcher ASSERT =
staticMethod().onClass(TRUTH_CLASS).named("assert_");
+
+ private static final Matcher SUBJECT_BUILDER_THAT =
+ anyOf(
+ instanceMethod()
+ .onDescendantOf("com.google.common.truth.CustomSubjectBuilder")
+ .named("that"),
+ instanceMethod()
+ .onDescendantOf("com.google.common.truth.SimpleSubjectBuilder")
+ .named("that"),
+ instanceMethod()
+ .onDescendantOf("com.google.common.truth.StandardSubjectBuilder")
+ .named("that"));
+
+ private static final Matcher WITH_MESSAGE_OR_ABOUT =
+ instanceMethod()
+ .onDescendantOf("com.google.common.truth.StandardSubjectBuilder")
+ .namedAnyOf("withMessage", "about");
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/CheckNotNullMultipleTimes.java b/core/src/main/java/com/google/errorprone/bugpatterns/CheckNotNullMultipleTimes.java
new file mode 100644
index 00000000000..8d088ec58fe
--- /dev/null
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/CheckNotNullMultipleTimes.java
@@ -0,0 +1,137 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION;
+import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.Matchers.staticMethod;
+import static com.google.errorprone.util.ASTHelpers.getSymbol;
+
+import com.google.common.collect.HashMultiset;
+import com.google.common.collect.Multiset;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.IfTree;
+import com.sun.source.tree.LambdaExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.StatementTree;
+import com.sun.source.tree.SwitchTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.tree.TryTree;
+import com.sun.source.util.TreePathScanner;
+import com.sun.tools.javac.code.Flags;
+import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.VarSymbol;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/** Checks for the same variable being checked against null twice in a method. */
+@BugPattern(
+ name = "CheckNotNullMultipleTimes",
+ severity = ERROR,
+ summary = "A variable was checkNotNulled multiple times. Did you mean to check something else?",
+ providesFix = REQUIRES_HUMAN_ATTENTION)
+public final class CheckNotNullMultipleTimes extends BugChecker implements MethodTreeMatcher {
+
+ private static final Matcher CHECK_NOT_NULL =
+ staticMethod().onClass("com.google.common.base.Preconditions").named("checkNotNull");
+
+ @Override
+ public Description matchMethod(MethodTree tree, VisitorState state) {
+ Multiset variables = HashMultiset.create();
+ Map lastCheck = new HashMap<>();
+ new TreePathScanner() {
+ @Override
+ public Void visitMethodInvocation(MethodInvocationTree tree, Void unused) {
+ List arguments = tree.getArguments();
+ if (!arguments.isEmpty()
+ && arguments.get(0) instanceof IdentifierTree
+ // Only consider side-effects-only calls to checkNotNull: it turns out people often
+ // intentionally write repeated null-checks for the same variable when using the result
+ // as a value, and we would have many false positives if we counted those.
+ && getCurrentPath().getParentPath().getLeaf() instanceof StatementTree
+ && CHECK_NOT_NULL.matches(tree, state)) {
+ Symbol symbol = getSymbol(arguments.get(0));
+ if (symbol instanceof VarSymbol
+ && (symbol.flags() & (Flags.EFFECTIVELY_FINAL | Flags.FINAL)) != 0) {
+ variables.add((VarSymbol) symbol);
+ lastCheck.put((VarSymbol) symbol, tree);
+ }
+ }
+ return super.visitMethodInvocation(tree, null);
+ }
+
+ // Don't descend into ifs and switches, given people often repeat the same checks within
+ // top-level conditional branches.
+ @Override
+ public Void visitSwitch(SwitchTree tree, Void unused) {
+ return null;
+ }
+
+ @Override
+ public Void visitIf(IfTree tree, Void unused) {
+ return null;
+ }
+
+ @Override
+ public Void visitLambdaExpression(LambdaExpressionTree tree, Void unused) {
+ return null;
+ }
+
+ @Override
+ public Void visitClass(ClassTree tree, Void unused) {
+ return null;
+ }
+
+ // Sometimes a variable is checked in the try and the catch blocks of a try statement; don't
+ // descend into the catch.
+ // try {
+ //
+ // checkNotNull(frobnicator);
+ //
+ // } catch (Exception e) {
+ // checkNotNull(frobnicator);
+ // }
+ @Override
+ public Void visitTry(TryTree tree, Void unused) {
+ return scan(tree.getBlock(), null);
+ }
+ }.scan(state.getPath(), null);
+ for (Multiset.Entry entry : variables.entrySet()) {
+ if (entry.getCount() > 1) {
+ state.reportMatch(
+ buildDescription(lastCheck.get(entry.getElement()))
+ .setMessage(
+ String.format(
+ "checkNotNull(%s) was called more than once. Did you mean to check"
+ + " something else?",
+ entry.getElement()))
+ .build());
+ }
+ }
+ return NO_MATCH;
+ }
+}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/CheckReturnValue.java b/core/src/main/java/com/google/errorprone/bugpatterns/CheckReturnValue.java
index e4d0e33d2cb..17bffd8658a 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/CheckReturnValue.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/CheckReturnValue.java
@@ -21,7 +21,6 @@
import static com.google.errorprone.util.ASTHelpers.enclosingPackage;
import static com.google.errorprone.util.ASTHelpers.hasDirectAnnotationWithSimpleName;
-import com.google.common.base.Optional;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
@@ -36,6 +35,7 @@
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import java.util.Optional;
import javax.lang.model.element.ElementKind;
/** @author eaftan@google.com (Eddie Aftandilian) */
@@ -50,59 +50,34 @@ public class CheckReturnValue extends AbstractReturnValueIgnored
private static final String CHECK_RETURN_VALUE = "CheckReturnValue";
private static final String CAN_IGNORE_RETURN_VALUE = "CanIgnoreReturnValue";
- private static Optional shouldCheckReturnValue(Symbol sym, VisitorState state) {
+ // TODO(amalloy): After java 9, these two methods can be simplified with a Stream using
+ // iterate/takeWhile.
+ private static Optional shouldCheckReturnValue(Symbol sym) {
if (hasDirectAnnotationWithSimpleName(sym, CAN_IGNORE_RETURN_VALUE)) {
return Optional.of(false);
}
if (hasDirectAnnotationWithSimpleName(sym, CHECK_RETURN_VALUE)) {
return Optional.of(true);
}
- return Optional.absent();
+ return Optional.empty();
}
- private static Optional checkEnclosingClasses(MethodSymbol method, VisitorState state) {
+ private static Optional checkEnclosingClasses(MethodSymbol method) {
Symbol enclosingClass = enclosingClass(method);
while (enclosingClass instanceof ClassSymbol) {
- Optional result = shouldCheckReturnValue(enclosingClass, state);
+ Optional result = shouldCheckReturnValue(enclosingClass);
if (result.isPresent()) {
return result;
}
enclosingClass = enclosingClass.owner;
}
- return Optional.absent();
+ return Optional.empty();
}
- private static Optional checkPackage(MethodSymbol method, VisitorState state) {
- return shouldCheckReturnValue(enclosingPackage(method), state);
+ private static Optional checkPackage(MethodSymbol method) {
+ return shouldCheckReturnValue(enclosingPackage(method));
}
- private static final Matcher MATCHER =
- new Matcher() {
- @Override
- public boolean matches(ExpressionTree tree, VisitorState state) {
- Symbol sym = ASTHelpers.getSymbol(tree);
- if (!(sym instanceof MethodSymbol)) {
- return false;
- }
- MethodSymbol method = (MethodSymbol) sym;
- Optional result = shouldCheckReturnValue(method, state);
- if (result.isPresent()) {
- return result.get();
- }
-
- result = checkEnclosingClasses(method, state);
- if (result.isPresent()) {
- return result.get();
- }
-
- result = checkPackage(method, state);
- if (result.isPresent()) {
- return result.get();
- }
-
- return false;
- }
- };
/**
* Return a matcher for method invocations in which the method being called has the
@@ -110,7 +85,18 @@ public boolean matches(ExpressionTree tree, VisitorState state) {
*/
@Override
public Matcher specializedMatcher() {
- return MATCHER;
+ return (tree, state) -> {
+ Symbol sym = ASTHelpers.getSymbol(tree);
+ if (!(sym instanceof MethodSymbol)) {
+ return false;
+ }
+ MethodSymbol method = (MethodSymbol) sym;
+ return shouldCheckReturnValue(method)
+ .orElseGet(
+ () ->
+ checkEnclosingClasses(method)
+ .orElseGet(() -> checkPackage(method).orElse(false)));
+ };
}
private static final String BOTH_ERROR =
@@ -119,7 +105,7 @@ public Matcher specializedMatcher() {
/**
* Validate {@code @CheckReturnValue} and {@link CanIgnoreReturnValue} usage on methods.
*
- * The annotations should not both be appled to the same method.
+ *
The annotations should not both be applied to the same method.
*
*
The annotations should not be applied to void-returning methods. Doing so makes no sense,
* because there is no return value to check.
@@ -144,7 +130,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
return Description.NO_MATCH;
}
if (method.getKind() != ElementKind.METHOD) {
- // skip contructors (which javac thinks are void-returning)
+ // skip constructors (which javac thinks are void-returning)
return Description.NO_MATCH;
}
if (!ASTHelpers.isVoidType(method.getReturnType(), state)) {
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ComparisonContractViolated.java b/core/src/main/java/com/google/errorprone/bugpatterns/ComparisonContractViolated.java
index 837f53797fa..b1b73b24249 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/ComparisonContractViolated.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/ComparisonContractViolated.java
@@ -224,8 +224,8 @@ public Void visitReturn(ReturnTree node, VisitorState state) {
}
BinaryTree binaryExpr = (BinaryTree) conditionExpr;
Type ty = ASTHelpers.getType(binaryExpr.getLeftOperand());
- Types types = Types.instance(state.context);
- Symtab symtab = Symtab.instance(state.context);
+ Types types = state.getTypes();
+ Symtab symtab = state.getSymtab();
ExpressionTree first =
trueFirst ? binaryExpr.getLeftOperand() : binaryExpr.getRightOperand();
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/EmptyIfStatement.java b/core/src/main/java/com/google/errorprone/bugpatterns/EmptyIfStatement.java
index 4a92ed9aba1..dc2364b4083 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/EmptyIfStatement.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/EmptyIfStatement.java
@@ -17,9 +17,8 @@
package com.google.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
-import static com.google.errorprone.matchers.Matchers.nextStatement;
-import static com.google.errorprone.matchers.Matchers.parentNode;
-import static com.sun.source.tree.Tree.Kind.IF;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.Matchers.isLastStatementInBlock;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
@@ -27,11 +26,10 @@
import com.google.errorprone.bugpatterns.BugChecker.EmptyStatementTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
-import com.google.errorprone.matchers.Matchers;
import com.sun.source.tree.EmptyStatementTree;
import com.sun.source.tree.IfTree;
-import com.sun.source.tree.StatementTree;
import com.sun.source.tree.Tree;
+import com.sun.source.util.TreePath;
/**
* This checker finds and fixes empty statements after an if, with no else part. For example: if
@@ -56,16 +54,15 @@ public class EmptyIfStatement extends BugChecker implements EmptyStatementTreeMa
*/
@Override
public Description matchEmptyStatement(EmptyStatementTree tree, VisitorState state) {
- boolean matches = false;
- Tree parent = state.getPath().getParentPath().getLeaf();
- if (parent.getKind() == IF) {
- IfTree parentAsIf = (IfTree) parent;
- matches =
- (parentAsIf.getThenStatement() instanceof EmptyStatementTree)
- && (parentAsIf.getElseStatement() == null);
+ TreePath parentPath = state.getPath().getParentPath();
+ Tree parent = parentPath.getLeaf();
+ if (!(parent instanceof IfTree)) {
+ return NO_MATCH;
}
- if (!matches) {
- return Description.NO_MATCH;
+ IfTree ifTree = (IfTree) parent;
+ if (!(ifTree.getThenStatement() instanceof EmptyStatementTree)
+ || ifTree.getElseStatement() != null) {
+ return NO_MATCH;
}
/*
@@ -75,18 +72,13 @@ public Description matchEmptyStatement(EmptyStatementTree tree, VisitorState sta
* empty then part of the if. If the next statement is not a block, then also
* suggest deleting the empty then part of the if.
*/
- boolean nextStmtIsNull =
- parentNode(nextStatement(Matchers.isSame(null))).matches(tree, state);
-
- assert (state.getPath().getParentPath().getLeaf().getKind() == IF);
- IfTree ifParent = (IfTree) state.getPath().getParentPath().getLeaf();
- if (nextStmtIsNull) {
+ if (isLastStatementInBlock().matches(ifTree, state.withPath(parentPath))) {
// No following statements. Delete whole if.
return describeMatch(parent, SuggestedFix.delete(parent));
} else {
// There are more statements. Delete the empty then part of the if.
return describeMatch(
- ifParent.getThenStatement(), SuggestedFix.delete(ifParent.getThenStatement()));
+ ifTree.getThenStatement(), SuggestedFix.delete(ifTree.getThenStatement()));
}
}
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/FunctionalInterfaceMethodChanged.java b/core/src/main/java/com/google/errorprone/bugpatterns/FunctionalInterfaceMethodChanged.java
index edbe8ecafca..9393de5d7c4 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/FunctionalInterfaceMethodChanged.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/FunctionalInterfaceMethodChanged.java
@@ -64,7 +64,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
ClassTree enclosingClazz = ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class);
if (tree.getModifiers().getFlags().contains(Modifier.DEFAULT)
&& IS_FUNCTIONAL_INTERFACE.matches(enclosingClazz, state)) {
- Types types = Types.instance(state.context);
+ Types types = state.getTypes();
Set functionalSuperInterfaceSams =
enclosingClazz.getImplementsClause().stream()
.filter(t -> IS_FUNCTIONAL_INTERFACE.matches(t, state))
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ImplementAssertionWithChaining.java b/core/src/main/java/com/google/errorprone/bugpatterns/ImplementAssertionWithChaining.java
index b83ecf89718..2d79d4f45b0 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/ImplementAssertionWithChaining.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/ImplementAssertionWithChaining.java
@@ -55,6 +55,7 @@
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.List;
+import java.util.regex.Pattern;
/**
* Migrates Truth subjects from a manual "test and fail" approach to one using {@code
@@ -63,7 +64,7 @@
* {@code
* // Before:
* if (actual().foo() != expected) {
- * fail("has foo", expected);
+ * failWithActual("expected to have foo", expected);
* }
*
* // After:
@@ -177,7 +178,7 @@ private static boolean isEnum(ExpressionTree tree, VisitorState state) {
/**
* Checks that the statement, after unwrapping any braces, consists of a single call to a {@code
- * fail} method.
+ * fail*} method.
*/
private static boolean isCallToFail(StatementTree then, VisitorState state) {
while (then.getKind() == BLOCK) {
@@ -198,10 +199,9 @@ private static boolean isCallToFail(StatementTree then, VisitorState state) {
ExpressionTree methodSelect = thenCall.getMethodSelect();
if (methodSelect.getKind() != IDENTIFIER) {
return false;
- // TODO(cpovirk): Handle "this.fail(...)," etc.
+ // TODO(cpovirk): Handle "this.fail*(...)," etc.
}
- // TODO(cpovirk): Support *all* fail methods.
- return LEGACY_FAIL_METHOD.matches(methodSelect, state);
+ return FAIL_METHOD.matches(methodSelect, state);
}
/**
@@ -257,22 +257,10 @@ private static boolean refersToFieldNamedActual(ExpressionTree tree) {
&& symbol.getSimpleName().contentEquals("actual");
}
- private static final Matcher LEGACY_FAIL_METHOD =
- anyOf(
- instanceMethod().onDescendantOf("com.google.common.truth.Subject").named("fail"),
- instanceMethod()
- .onDescendantOf("com.google.common.truth.Subject")
- .named("failWithBadResults"),
- instanceMethod()
- .onDescendantOf("com.google.common.truth.Subject")
- .named("failWithCustomSubject"),
- instanceMethod()
- .onDescendantOf("com.google.common.truth.Subject")
- .named("failWithoutActual")
- .withParameters("java.lang.String"),
- instanceMethod()
- .onDescendantOf("com.google.common.truth.Subject")
- .named("failWithoutSubject"));
+ private static final Matcher FAIL_METHOD =
+ instanceMethod()
+ .onDescendantOf("com.google.common.truth.Subject")
+ .withNameMatching(Pattern.compile("fail.*"));
private static final Matcher EQUALS_LIKE_METHOD =
anyOf(
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/InconsistentHashCode.java b/core/src/main/java/com/google/errorprone/bugpatterns/InconsistentHashCode.java
index b74a235aa03..e5eb5b4576d 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/InconsistentHashCode.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/InconsistentHashCode.java
@@ -215,7 +215,8 @@ public Void visitIdentifier(IdentifierTree tree, Void unused) {
}
private void handleSymbol(Symbol symbol) {
- if (symbol.getKind() == ElementKind.FIELD
+ if (symbol != null
+ && symbol.getKind() == ElementKind.FIELD
&& !symbol.isStatic()
&& symbol.owner.equals(classSymbol)) {
String name = symbol.name.toString();
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/JavaLangClash.java b/core/src/main/java/com/google/errorprone/bugpatterns/JavaLangClash.java
index 266290102a6..8986f655a8c 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/JavaLangClash.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/JavaLangClash.java
@@ -37,7 +37,6 @@
import com.sun.tools.javac.tree.JCTree.JCClassDecl;
import com.sun.tools.javac.tree.JCTree.JCTypeParameter;
import com.sun.tools.javac.util.Name;
-import com.sun.tools.javac.util.Names;
/** @author cushon@google.com (Liam Miller-Cushon) */
@BugPattern(
@@ -61,8 +60,7 @@ public Description matchTypeParameter(TypeParameterTree tree, VisitorState state
private Description check(Tree tree, Name simpleName, VisitorState state) {
Symtab symtab = state.getSymtab();
- PackageSymbol javaLang =
- symtab.enterPackage(symtab.java_base, Names.instance(state.context).java_lang);
+ PackageSymbol javaLang = symtab.enterPackage(symtab.java_base, state.getNames().java_lang);
Symbol other =
getFirst(
javaLang.members().getSymbolsByName(simpleName, s -> s.getModifiers().contains(PUBLIC)),
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/LiteProtoToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/LiteProtoToString.java
new file mode 100644
index 00000000000..b9f3198cc93
--- /dev/null
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/LiteProtoToString.java
@@ -0,0 +1,99 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.ProvidesFix.NO_FIX;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.predicates.TypePredicates.allOf;
+import static com.google.errorprone.predicates.TypePredicates.isDescendantOf;
+import static com.google.errorprone.predicates.TypePredicates.not;
+import static com.google.errorprone.util.ASTHelpers.getReceiver;
+import static com.google.errorprone.util.ASTHelpers.getSymbol;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Streams;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.fixes.Fix;
+import com.google.errorprone.predicates.TypePredicate;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Type;
+import java.util.Optional;
+
+/** Flags calls to {@code toString} on lite protos. */
+@BugPattern(
+ name = "LiteProtoToString",
+ summary =
+ "#toString on lite protos will not generate a useful representation of the proto from"
+ + " optimized builds. Consider whether using some subset of fields instead would"
+ + " provide useful information.",
+ severity = WARNING,
+ providesFix = NO_FIX)
+public final class LiteProtoToString extends AbstractToString {
+ private static final TypePredicate IS_LITE_PROTO =
+ allOf(
+ isDescendantOf("com.google.protobuf.MessageLite"),
+ not(isDescendantOf("com.google.protobuf.Message")));
+
+ private static final ImmutableSet VERBOSE_LOGGING =
+ ImmutableSet.of("atVerbose", "atFine", "atFinest", "atDebug", "v", "d");
+
+ @Override
+ protected TypePredicate typePredicate() {
+ return LiteProtoToString::matches;
+ }
+
+ private static boolean matches(Type type, VisitorState state) {
+ if (state.errorProneOptions().isTestOnlyTarget()) {
+ return false;
+ }
+ if (isVerboseLogMessage(state)) {
+ return false;
+ }
+ return IS_LITE_PROTO.apply(type, state);
+ }
+
+ private static boolean isVerboseLogMessage(VisitorState state) {
+ return Streams.stream(state.getPath()).anyMatch(LiteProtoToString::isVerboseLogMessage);
+ }
+
+ private static boolean isVerboseLogMessage(Tree tree) {
+ for (; tree instanceof MethodInvocationTree; tree = getReceiver((MethodInvocationTree) tree)) {
+ if (VERBOSE_LOGGING.contains(getSymbol(tree).getSimpleName().toString())) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ protected Optional descriptionMessageForDefaultMatch(Type type, VisitorState state) {
+ return Optional.of(message());
+ }
+
+ @Override
+ protected Optional implicitToStringFix(ExpressionTree tree, VisitorState state) {
+ return Optional.empty();
+ }
+
+ @Override
+ protected Optional toStringFix(Tree parent, ExpressionTree tree, VisitorState state) {
+ return Optional.empty();
+ }
+}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ObjectToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/ObjectToString.java
index d2e0d032b5e..b7df3c59c98 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/ObjectToString.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/ObjectToString.java
@@ -18,7 +18,6 @@
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
-import com.google.common.base.Optional;
import com.google.common.collect.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
@@ -32,6 +31,7 @@
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.util.Names;
+import java.util.Optional;
/**
* Warns against calling toString() on Objects which don't have toString() method overridden and
@@ -58,7 +58,7 @@ private static boolean finalNoOverrides(Type type, VisitorState state) {
return false;
}
Types types = state.getTypes();
- Names names = Names.instance(state.context);
+ Names names = state.getNames();
// find Object.toString
MethodSymbol toString =
(MethodSymbol) state.getSymtab().objectType.tsym.members().findFirst(names.toString);
@@ -94,11 +94,11 @@ protected Optional descriptionMessageForDefaultMatch(Type type, VisitorS
@Override
protected Optional implicitToStringFix(ExpressionTree tree, VisitorState state) {
- return Optional.absent();
+ return Optional.empty();
}
@Override
protected Optional toStringFix(Tree parent, ExpressionTree tree, VisitorState state) {
- return Optional.absent();
+ return Optional.empty();
}
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ProvideDescriptionToCheck.java b/core/src/main/java/com/google/errorprone/bugpatterns/ProvideDescriptionToCheck.java
deleted file mode 100644
index 42c013dad5b..00000000000
--- a/core/src/main/java/com/google/errorprone/bugpatterns/ProvideDescriptionToCheck.java
+++ /dev/null
@@ -1,166 +0,0 @@
-/*
- * Copyright 2019 The Error Prone Authors.
- *
- * 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.errorprone.bugpatterns;
-
-import static com.google.common.collect.Iterables.getOnlyElement;
-import static com.google.errorprone.BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION;
-import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
-import static com.google.errorprone.bugpatterns.ImplementAssertionWithChaining.makeCheckDescription;
-import static com.google.errorprone.fixes.SuggestedFix.replace;
-import static com.google.errorprone.matchers.Description.NO_MATCH;
-import static com.google.errorprone.matchers.Matchers.anyOf;
-import static com.google.errorprone.matchers.Matchers.instanceMethod;
-import static com.sun.source.tree.Tree.Kind.IDENTIFIER;
-import static com.sun.source.tree.Tree.Kind.MEMBER_SELECT;
-import static com.sun.source.tree.Tree.Kind.METHOD_INVOCATION;
-import static java.lang.String.format;
-
-import com.google.errorprone.BugPattern;
-import com.google.errorprone.VisitorState;
-import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
-import com.google.errorprone.matchers.Description;
-import com.google.errorprone.matchers.Matcher;
-import com.sun.source.tree.ExpressionTree;
-import com.sun.source.tree.MemberSelectTree;
-import com.sun.source.tree.MethodInvocationTree;
-import com.sun.source.tree.Tree;
-import com.sun.source.util.TreePath;
-
-/**
- * Migrates Truth subjects from the no-arg overload of {@code Subject.check()} to the overload that
- * accepts a description.
- *
- * {@code
- * // Before:
- * check().that(actual().foo()).isEqualTo(expected);
- *
- * // After:
- * check("foo()").that(actual().foo()).isEqualTo(expected);
- * }
- */
-@BugPattern(
- name = "ProvideDescriptionToCheck",
- summary = "Provide a description of the value to be included in the failure message.",
- severity = SUGGESTION,
- providesFix = REQUIRES_HUMAN_ATTENTION)
-public final class ProvideDescriptionToCheck extends BugChecker
- implements MethodInvocationTreeMatcher {
- @Override
- public Description matchMethodInvocation(
- MethodInvocationTree maybeCheckCall, VisitorState state) {
- if (!NO_ARG_CHECK.matches(maybeCheckCall, state)) {
- return NO_MATCH;
- }
-
- MethodInvocationTree maybeThatCall = findThatCall(state);
- if (maybeThatCall == null || maybeThatCall.getArguments().size() != 1) {
- return NO_MATCH;
- }
-
- ExpressionTree arg = getOnlyElement(maybeThatCall.getArguments());
- String checkDescription = makeCheckDescription(arg, state);
- if (checkDescription == null) {
- /*
- * TODO(cpovirk): Consider alternative ways of guessing the name.
- *
- * Our best bet: Look at the name of the assertion method. If the method is `StringSubject
- * hasUsername()`, then we should probably generate `check("username()")`. Similarly for
- * `StringSubject withUsername()` and `StringSubject username()`.
- *
- * One bit of complexity here is parameters: If the method is `StringSubject
- * hasUsername(String param)`, should we generate `check("username()")` or
- * `check("username(%s)", param)`? The former is right if the assertion is
- * `check().that(actualUsername).isEqualTo(param)`; the latter is right if the assertion is
- * `return check().that(usernameFor(param))`. We could guess based on whether `param` is used
- * before the call to `that` (including in computing the argument to `that`, even though the
- * argument comes textually *after* `that`) and based on whether the assertion method returns
- * a Subject (though note that some methods `return this`, a practice we discourage).
- */
- return NO_MATCH;
- }
-
- String newCheck = format("check(%s)", checkDescription);
- if (maybeCheckCall.getMethodSelect().getKind() == IDENTIFIER) {
- return describeMatch(maybeCheckCall, replace(maybeCheckCall, newCheck));
- } else if (maybeCheckCall.getMethodSelect().getKind() == MEMBER_SELECT) {
- MemberSelectTree methodSelect = (MemberSelectTree) maybeCheckCall.getMethodSelect();
- return describeMatch(
- maybeCheckCall,
- replace(
- state.getEndPosition(methodSelect.getExpression()),
- state.getEndPosition(maybeCheckCall),
- "." + newCheck));
- } else {
- return NO_MATCH;
- }
- }
-
- /**
- * Starting from a {@code VisitorState} pointing at part of a fluent assertion statement (like
- * {@code check()} or {@code assertWithMessage()}, walks up the tree and returns the subsequent
- * call to {@code that(...)}.
- *
- * Often, the call is made directly on the result of the given tree -- like when the input is
- * {@code check()}, which is part of the expression {@code check().that(...)}. But sometimes there
- * is an intervening call to {@code withMessage}, {@code about}, or both.
- */
- static MethodInvocationTree findThatCall(VisitorState state) {
- TreePath path = state.getPath();
- /*
- * Each iteration walks 1 method call up the tree, but it's actually 2 steps in the tree because
- * there's a MethodSelectTree between each pair of MethodInvocationTrees.
- */
- while (true) {
- path = path.getParentPath().getParentPath();
- Tree leaf = path.getLeaf();
- if (leaf.getKind() != METHOD_INVOCATION) {
- return null;
- }
- MethodInvocationTree maybeThatCall = (MethodInvocationTree) leaf;
- if (WITH_MESSAGE_OR_ABOUT.matches(maybeThatCall, state)) {
- continue;
- } else if (SUBJECT_BUILDER_THAT.matches(maybeThatCall, state)) {
- return maybeThatCall;
- } else {
- return null;
- }
- }
- }
-
- private static final Matcher NO_ARG_CHECK =
- instanceMethod()
- .onDescendantOf("com.google.common.truth.Subject")
- .named("check")
- .withParameters();
-
- private static final Matcher SUBJECT_BUILDER_THAT =
- anyOf(
- instanceMethod()
- .onDescendantOf("com.google.common.truth.CustomSubjectBuilder")
- .named("that"),
- instanceMethod()
- .onDescendantOf("com.google.common.truth.SimpleSubjectBuilder")
- .named("that"),
- instanceMethod()
- .onDescendantOf("com.google.common.truth.StandardSubjectBuilder")
- .named("that"));
-
- private static final Matcher WITH_MESSAGE_OR_ABOUT =
- instanceMethod()
- .onDescendantOf("com.google.common.truth.StandardSubjectBuilder")
- .namedAnyOf("withMessage", "about");
-}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/SelfAssignment.java b/core/src/main/java/com/google/errorprone/bugpatterns/SelfAssignment.java
index b6817aea4d5..5813e4c30b4 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/SelfAssignment.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/SelfAssignment.java
@@ -65,8 +65,15 @@ public class SelfAssignment extends BugChecker
implements AssignmentTreeMatcher, VariableTreeMatcher {
private static final Matcher NON_NULL_MATCHER =
anyOf(
+ staticMethod().onClass("java.util.Objects").named("requireNonNull"),
staticMethod().onClass("com.google.common.base.Preconditions").named("checkNotNull"),
- staticMethod().onClass("java.util.Objects").named("requireNonNull"));
+ staticMethod()
+ .onClass("com.google.common.time.Durations")
+ .namedAnyOf("checkNotNegative", "checkPositive"),
+ staticMethod()
+ .onClass("com.google.protobuf.util.Durations")
+ .namedAnyOf("checkNotNegative", "checkPositive", "checkValid"),
+ staticMethod().onClass("com.google.protobuf.util.Timestamps").named("checkValid"));
@Override
public Description matchAssignment(AssignmentTree tree, VisitorState state) {
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StreamToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/StreamToString.java
index b8335c7ae8b..4ebe4fd59af 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/StreamToString.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/StreamToString.java
@@ -17,16 +17,15 @@
package com.google.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.predicates.TypePredicates.isDescendantOf;
-import com.google.common.base.Optional;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.predicates.TypePredicate;
-import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree;
-import com.sun.tools.javac.code.Type;
+import java.util.Optional;
/** @author cushon@google.com (Liam Miller-Cushon) */
@BugPattern(
@@ -35,14 +34,7 @@
severity = ERROR)
public class StreamToString extends AbstractToString {
- static final TypePredicate STREAM =
- new TypePredicate() {
- @Override
- public boolean apply(Type type, VisitorState state) {
- Type stream = state.getTypeFromString("java.util.stream.Stream");
- return ASTHelpers.isSubtype(type, stream, state);
- }
- };
+ private static final TypePredicate STREAM = isDescendantOf("java.util.stream.Stream");
@Override
protected TypePredicate typePredicate() {
@@ -51,11 +43,11 @@ protected TypePredicate typePredicate() {
@Override
protected Optional implicitToStringFix(ExpressionTree tree, VisitorState state) {
- return Optional.absent();
+ return Optional.empty();
}
@Override
protected Optional toStringFix(Tree parent, ExpressionTree tree, VisitorState state) {
- return Optional.absent();
+ return Optional.empty();
}
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TreeToString.java b/core/src/main/java/com/google/errorprone/bugpatterns/TreeToString.java
index 91c7f7993b7..07da8447b84 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/TreeToString.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/TreeToString.java
@@ -21,7 +21,6 @@
import static com.google.errorprone.util.ASTHelpers.getReceiver;
import static com.google.errorprone.util.ASTHelpers.isSubtype;
-import com.google.common.base.Optional;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.fixes.Fix;
@@ -37,6 +36,7 @@
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type;
+import java.util.Optional;
/**
* Flags {@code com.sun.source.tree.Tree#toString} usage in {@link BugChecker}s.
@@ -84,30 +84,26 @@ protected Optional implicitToStringFix(ExpressionTree tree, VisitorState st
@Override
protected Optional toStringFix(Tree parent, ExpressionTree tree, VisitorState state) {
if (!(parent instanceof MethodInvocationTree)) {
- return Optional.absent();
+ return Optional.empty();
}
ExpressionTree receiver = getReceiver((ExpressionTree) parent);
if (receiver == null) {
- return Optional.absent();
+ return Optional.empty();
}
return fix(receiver, parent, state);
}
private static Optional fix(Tree target, Tree replace, VisitorState state) {
- return Optional.fromJavaUtil(
- FindIdentifiers.findAllIdents(state).stream()
- .filter(
- s ->
- isSubtype(
- s.type,
- state.getTypeFromString("com.google.errorprone.VisitorState"),
- state))
- .findFirst()
- .map(
- s ->
- SuggestedFix.replace(
- replace,
- String.format(
- "%s.getSourceForNode(%s)", s, state.getSourceForNode(target)))));
+ return FindIdentifiers.findAllIdents(state).stream()
+ .filter(
+ s ->
+ isSubtype(
+ s.type, state.getTypeFromString("com.google.errorprone.VisitorState"), state))
+ .findFirst()
+ .map(
+ s ->
+ SuggestedFix.replace(
+ replace,
+ String.format("%s.getSourceForNode(%s)", s, state.getSourceForNode(target))));
}
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TruthSelfEquals.java b/core/src/main/java/com/google/errorprone/bugpatterns/TruthSelfEquals.java
index 9471020afa7..d3d2c2258e3 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/TruthSelfEquals.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/TruthSelfEquals.java
@@ -59,17 +59,18 @@ public class TruthSelfEquals extends BugChecker implements MethodInvocationTreeM
*
* - assertThat(a).isEqualTo(a)
*
- assertThat(a).isNotEqualTo(a)
- *
- assertThat(a).isNotSameAs(a)
- *
- assertThat(a).isSameAs(a)
+ *
- assertThat(a).isNotSameInstanceAs(a)
+ *
- assertThat(a).isSameInstanceAs(a)
*
- assertWithMessage(msg).that(a).isEqualTo(a)
*
- assertWithMessage(msg).that(a).isNotEqualTo(a)
- *
- assertWithMessage(msg).that(a).isNotSameAs(a)
- *
- assertWithMessage(msg).that(a).isSameAs(a)
+ *
- assertWithMessage(msg).that(a).isNotSameInstanceAs(a)
+ *
- assertWithMessage(msg).that(a).isSameInstanceAs(a)
*
*/
- private static final Pattern EQUALS_SAME = Pattern.compile("(isEqualTo|isSameAs)");
+ private static final Pattern EQUALS_SAME = Pattern.compile("(isEqualTo|isSameInstanceAs)");
- private static final Pattern NOT_EQUALS_NOT_SAME = Pattern.compile("(isNotEqualTo|isNotSameAs)");
+ private static final Pattern NOT_EQUALS_NOT_SAME =
+ Pattern.compile("(isNotEqualTo|isNotSameInstanceAs)");
private static final Matcher EQUALS_MATCHER =
allOf(
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TypeNameShadowing.java b/core/src/main/java/com/google/errorprone/bugpatterns/TypeNameShadowing.java
index 2a5389363e6..bbde5e7c44b 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/TypeNameShadowing.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/TypeNameShadowing.java
@@ -46,7 +46,6 @@
import com.sun.tools.javac.comp.Enter;
import com.sun.tools.javac.comp.Env;
import com.sun.tools.javac.tree.JCTree.Tag;
-import com.sun.tools.javac.util.Names;
import java.util.List;
import java.util.Objects;
import java.util.Set;
@@ -123,8 +122,7 @@ private Description findShadowedTypes(
.getEnv(ASTHelpers.getSymbol(state.findEnclosing(ClassTree.class)));
Symtab symtab = state.getSymtab();
- PackageSymbol javaLang =
- symtab.enterPackage(symtab.java_base, Names.instance(state.context).java_lang);
+ PackageSymbol javaLang = symtab.enterPackage(symtab.java_base, state.getNames().java_lang);
Iterable enclosingTypes = typesInEnclosingScope(env, javaLang);
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterQualifier.java b/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterQualifier.java
index 326d87926df..397381fc972 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterQualifier.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/TypeParameterQualifier.java
@@ -47,8 +47,7 @@ public Description matchMemberSelect(MemberSelectTree tree, VisitorState state)
return Description.NO_MATCH;
}
TreeMaker make =
- TreeMaker.instance(state.context)
- .forToplevel((JCCompilationUnit) state.getPath().getCompilationUnit());
+ state.getTreeMaker().forToplevel((JCCompilationUnit) state.getPath().getCompilationUnit());
JCExpression qual = make.QualIdent(ASTHelpers.getSymbol(tree));
return describeMatch(tree, SuggestedFix.replace(tree, qual.toString()));
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryAnonymousClass.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryAnonymousClass.java
index 6bdc0b254a3..13f2d782e85 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryAnonymousClass.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryAnonymousClass.java
@@ -44,6 +44,7 @@
import com.sun.source.tree.VariableTree;
import com.sun.source.util.TreePathScanner;
import com.sun.tools.javac.code.Symbol;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.tree.JCTree;
import java.util.Objects;
@@ -99,6 +100,18 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
if (type == null || !state.getTypes().isFunctionalInterface(type)) {
return NO_MATCH;
}
+ MethodSymbol methodSymbol = getSymbol(implementation);
+ if (methodSymbol == null) {
+ return NO_MATCH;
+ }
+ Symbol descriptorSymbol = state.getTypes().findDescriptorSymbol(type.tsym);
+ if (!methodSymbol.getSimpleName().contentEquals(descriptorSymbol.getSimpleName())) {
+ return NO_MATCH;
+ }
+ if (!methodSymbol.overrides(
+ descriptorSymbol, methodSymbol.owner.enclClass(), state.getTypes(), false)) {
+ return NO_MATCH;
+ }
if (state.isAndroidCompatible()) {
return NO_MATCH;
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedException.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedException.java
index 1e3962c2d76..53ecc36546a 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedException.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedException.java
@@ -18,7 +18,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.Iterables.getLast;
-import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.STYLE;
@@ -66,8 +65,6 @@
+ " exception rather than setting it as a cause. This can make debugging harder.",
severity = WARNING,
tags = STYLE,
- linkType = CUSTOM,
- link = "https://google.github.io/styleguide/javaguide.html#s6.2-caught-exceptions",
providesFix = REQUIRES_HUMAN_ATTENTION,
documentSuppression = false)
public final class UnusedException extends BugChecker implements CatchTreeMatcher {
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTests.java b/core/src/main/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTests.java
index 8b4cee7e3e8..1ca57fd1d72 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTests.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTests.java
@@ -66,8 +66,8 @@ public class UseCorrectAssertInTests extends BugChecker implements MethodTreeMat
private static final String ASSERT_WITH_MESSAGE = "assertWithMessage(%s).that(%s).";
private static final String IS_TRUE = "isTrue();";
private static final String IS_FALSE = "isFalse();";
- private static final String IS_SAME_AS = "isSameAs(%s);";
- private static final String IS_NOT_SAME_AS = "isNotSameAs(%s);";
+ private static final String IS_SAME_AS = "isSameInstanceAs(%s);";
+ private static final String IS_NOT_SAME_AS = "isNotSameInstanceAs(%s);";
private static final String IS_EQUAL_TO = "isEqualTo(%s);";
private static final String IS_NULL = "isNull();";
private static final String IS_NOT_NULL = "isNotNull();";
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/package-info.java b/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/package-info.java
index dc9f0139b3a..7bd29ed0f8d 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/package-info.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/inject/dagger/package-info.java
@@ -14,5 +14,5 @@
* limitations under the License.
*/
-/** Bug patterns related to Dagger. */
+/** Bug patterns related to Dagger. */
package com.google.errorprone.bugpatterns.inject.dagger;
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByBinder.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByBinder.java
index 68330781a14..a101ca9bc0f 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByBinder.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByBinder.java
@@ -57,7 +57,7 @@ public static Optional bindExpression(
ALREADY_BOUND_RESOLVER,
ASTHelpers.getSymbol(visitorState.findEnclosing(ClassTree.class)),
visitorState.getTypes(),
- Names.instance(visitorState.context))));
+ visitorState.getNames())));
} catch (IllegalGuardedBy expected) {
return Optional.empty();
}
@@ -72,8 +72,8 @@ static Optional bindString(String string, GuardedBySymbolRe
BinderContext.of(
resolver,
resolver.enclosingClass(),
- Types.instance(resolver.context()),
- Names.instance(resolver.context()))));
+ resolver.visitorState().getTypes(),
+ resolver.visitorState().getNames())));
} catch (IllegalGuardedBy expected) {
return Optional.empty();
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedBySymbolResolver.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedBySymbolResolver.java
index 3b3c1819912..b815df1b691 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedBySymbolResolver.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/GuardedBySymbolResolver.java
@@ -35,8 +35,6 @@
import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
import com.sun.tools.javac.tree.TreeMaker;
import com.sun.tools.javac.util.Context;
-import com.sun.tools.javac.util.Name;
-import com.sun.tools.javac.util.Names;
import javax.lang.model.element.ElementKind;
/**
@@ -50,6 +48,7 @@ public class GuardedBySymbolResolver implements GuardedByBinder.Resolver {
private final Tree decl;
private final JCTree.JCCompilationUnit compilationUnit;
private final Context context;
+ private final VisitorState visitorState;
private final Types types;
public static GuardedBySymbolResolver from(Tree tree, VisitorState visitorState) {
@@ -57,27 +56,41 @@ public static GuardedBySymbolResolver from(Tree tree, VisitorState visitorState)
ASTHelpers.getSymbol(tree).owner.enclClass(),
visitorState.getPath().getCompilationUnit(),
visitorState.context,
- tree);
+ tree,
+ visitorState);
}
public static GuardedBySymbolResolver from(
- ClassSymbol owner, CompilationUnitTree compilationUnit, Context context, Tree leaf) {
- return new GuardedBySymbolResolver(owner, compilationUnit, context, leaf);
+ ClassSymbol owner,
+ CompilationUnitTree compilationUnit,
+ Context context,
+ Tree leaf,
+ VisitorState visitorState) {
+ return new GuardedBySymbolResolver(owner, compilationUnit, context, leaf, visitorState);
}
private GuardedBySymbolResolver(
- ClassSymbol enclosingClass, CompilationUnitTree compilationUnit, Context context, Tree leaf) {
+ ClassSymbol enclosingClass,
+ CompilationUnitTree compilationUnit,
+ Context context,
+ Tree leaf,
+ VisitorState visitorState) {
this.compilationUnit = (JCCompilationUnit) compilationUnit;
this.enclosingClass = requireNonNull(enclosingClass);
this.context = context;
- this.types = Types.instance(context);
+ this.types = visitorState.getTypes();
this.decl = leaf;
+ this.visitorState = visitorState;
}
public Context context() {
return context;
}
+ public VisitorState visitorState() {
+ return visitorState;
+ }
+
public ClassSymbol enclosingClass() {
return enclosingClass;
}
@@ -151,7 +164,7 @@ private T getMember(
}
for (Type t : types.closure(classSymbol.type)) {
Scope scope = t.tsym.members();
- for (Symbol sym : scope.getSymbolsByName(getName(name))) {
+ for (Symbol sym : scope.getSymbolsByName(visitorState.getName(name))) {
if (sym.getKind().equals(kind)) {
return type.cast(sym);
}
@@ -241,12 +254,8 @@ private Symbol getLexicallyEnclosing(ClassSymbol symbol, String name) {
private Symbol attribIdent(String name) {
Attr attr = Attr.instance(context);
- TreeMaker tm = TreeMaker.instance(context);
- return attr.attribIdent(tm.Ident(getName(name)), compilationUnit);
- }
-
- private Name getName(String name) {
- return Names.instance(context).fromString(name);
+ TreeMaker tm = visitorState.getTreeMaker();
+ return attr.attribIdent(tm.Ident(visitorState.getName(name)), compilationUnit);
}
@Override
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/WellKnownMutability.java b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/WellKnownMutability.java
index 42a45d36bcf..5cc35a78c20 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/WellKnownMutability.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/threadsafety/WellKnownMutability.java
@@ -285,7 +285,14 @@ private static ImmutableMap buildImmutableClasses(
.add("org.threeten.bp.zone.ZoneOffsetTransitionRule")
.add("org.threeten.bp.zone.ZoneRules")
.add("org.threeten.bp.zone.ZoneRulesProvider")
+ .add("org.threeten.extra.Days")
+ .add("org.threeten.extra.Hours")
.add("org.threeten.extra.Interval")
+ .add("org.threeten.extra.Minutes")
+ .add("org.threeten.extra.Months")
+ .add("org.threeten.extra.Seconds")
+ .add("org.threeten.extra.Weeks")
+ .add("org.threeten.extra.Years")
.add("org.joda.time.DateTime")
.add("org.joda.time.DateTimeZone")
.add("org.joda.time.Days")
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/DurationGetTemporalUnit.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/DurationGetTemporalUnit.java
index ea53a7f4878..20fb2d84941 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/time/DurationGetTemporalUnit.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/time/DurationGetTemporalUnit.java
@@ -79,7 +79,9 @@ public final class DurationGetTemporalUnit extends BugChecker
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (MATCHER.matches(tree, state)) {
- Optional invalidUnit = getInvalidChronoUnit(tree, INVALID_TEMPORAL_UNITS);
+ Optional invalidUnit =
+ getInvalidChronoUnit(
+ Iterables.getOnlyElement(tree.getArguments()), INVALID_TEMPORAL_UNITS);
if (invalidUnit.isPresent()) {
if (SUGGESTIONS.containsKey(invalidUnit.get())) {
SuggestedFix.Builder builder = SuggestedFix.builder();
@@ -95,10 +97,10 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
return Description.NO_MATCH;
}
- // used by PeriodGetTemporalUnit
+ // used by PeriodGetTemporalUnit and DurationOfLongTemporalUnit
static Optional getInvalidChronoUnit(
- MethodInvocationTree tree, EnumSet invalidUnits) {
- Optional constant = getEnumName(Iterables.getOnlyElement(tree.getArguments()));
+ ExpressionTree tree, Iterable invalidUnits) {
+ Optional constant = getEnumName(tree);
if (constant.isPresent()) {
for (ChronoUnit invalidTemporalUnit : invalidUnits) {
if (constant.get().equals(invalidTemporalUnit.name())) {
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/DurationTemporalUnit.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/DurationTemporalUnit.java
new file mode 100644
index 00000000000..131c9de064d
--- /dev/null
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/time/DurationTemporalUnit.java
@@ -0,0 +1,88 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns.time;
+
+import static com.google.common.collect.Sets.toImmutableEnumSet;
+import static com.google.errorprone.BugPattern.ProvidesFix.NO_FIX;
+import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.bugpatterns.time.DurationGetTemporalUnit.getInvalidChronoUnit;
+import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
+import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.Matchers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import java.time.temporal.ChronoUnit;
+import java.util.Arrays;
+
+/**
+ * Bans calls to {@code Duration} APIs where the {@link java.time.temporal.TemporalUnit} is not
+ * {@link ChronoUnit#DAYS} or it has an estimated duration (which is guaranteed to throw an {@code
+ * DateTimeException}).
+ */
+@BugPattern(
+ name = "DurationTemporalUnit",
+ summary = "Duration APIs only work for DAYS or exact durations.",
+ explanation =
+ "Duration APIs only work for TemporalUnits with exact durations or"
+ + " ChronoUnit.DAYS. E.g., Duration.of(1, ChronoUnit.YEARS) is guaranteed to throw a"
+ + " DateTimeException.",
+ severity = ERROR,
+ providesFix = NO_FIX)
+public final class DurationTemporalUnit extends BugChecker implements MethodInvocationTreeMatcher {
+
+ private static final String DURATION = "java.time.Duration";
+ private static final String TEMPORAL_UNIT = "java.time.temporal.TemporalUnit";
+
+ private static final Matcher DURATION_OF_LONG_TEMPORAL_UNIT =
+ allOf(
+ anyOf(
+ staticMethod().onClass(DURATION).named("of").withParameters("long", TEMPORAL_UNIT),
+ instanceMethod()
+ .onExactClass(DURATION)
+ .named("minus")
+ .withParameters("long", TEMPORAL_UNIT),
+ instanceMethod()
+ .onExactClass(DURATION)
+ .named("plus")
+ .withParameters("long", TEMPORAL_UNIT)),
+ Matchers.not(Matchers.packageStartsWith("java.")));
+
+ private static final ImmutableSet INVALID_TEMPORAL_UNITS =
+ Arrays.stream(ChronoUnit.values())
+ .filter(c -> c.isDurationEstimated())
+ .filter(c -> !c.equals(ChronoUnit.DAYS)) // DAYS is explicitly allowed
+ .collect(toImmutableEnumSet());
+
+ @Override
+ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+ if (DURATION_OF_LONG_TEMPORAL_UNIT.matches(tree, state)) {
+ if (getInvalidChronoUnit(tree.getArguments().get(1), INVALID_TEMPORAL_UNITS).isPresent()) {
+ return describeMatch(tree);
+ }
+ }
+ return Description.NO_MATCH;
+ }
+}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/InstantTemporalUnit.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/InstantTemporalUnit.java
new file mode 100644
index 00000000000..97609ee310b
--- /dev/null
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/time/InstantTemporalUnit.java
@@ -0,0 +1,89 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns.time;
+
+import static com.google.common.collect.Sets.toImmutableEnumSet;
+import static com.google.errorprone.BugPattern.ProvidesFix.NO_FIX;
+import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.bugpatterns.time.DurationGetTemporalUnit.getInvalidChronoUnit;
+import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.Matchers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import java.time.temporal.ChronoUnit;
+import java.util.Arrays;
+
+/**
+ * Bans calls to {@code Instant} APIs where the {@link java.time.temporal.TemporalUnit} is not one
+ * of: {@code NANOS}, {@code MICROS}, {@code MILLIS}, {@code SECONDS}, {@code MINUTES}, {@code
+ * HOURS}, {@code HALF_DAYS}, or {@code DAYS}.
+ */
+@BugPattern(
+ name = "InstantTemporalUnit",
+ summary =
+ "Instant APIs only work for NANOS, MICROS, MILLIS, SECONDS, MINUTES, HOURS, HALF_DAYS and"
+ + " DAYS.",
+ severity = ERROR,
+ providesFix = NO_FIX)
+public final class InstantTemporalUnit extends BugChecker implements MethodInvocationTreeMatcher {
+
+ private static final String INSTANT = "java.time.Instant";
+ private static final String TEMPORAL_UNIT = "java.time.temporal.TemporalUnit";
+
+ private static final Matcher INSTANT_OF_LONG_TEMPORAL_UNIT =
+ allOf(
+ anyOf(
+ instanceMethod()
+ .onExactClass(INSTANT)
+ .named("minus")
+ .withParameters("long", TEMPORAL_UNIT),
+ instanceMethod()
+ .onExactClass(INSTANT)
+ .named("plus")
+ .withParameters("long", TEMPORAL_UNIT),
+ instanceMethod()
+ .onExactClass(INSTANT)
+ .named("until")
+ .withParameters("java.time.temporal.Temporal", TEMPORAL_UNIT)),
+ Matchers.not(Matchers.packageStartsWith("java.")));
+
+ // This definition comes from Instant.isSupported(TemporalUnit)
+ static final ImmutableSet INVALID_TEMPORAL_UNITS =
+ Arrays.stream(ChronoUnit.values())
+ .filter(c -> !c.isTimeBased())
+ .filter(c -> !c.equals(ChronoUnit.DAYS)) // DAYS is explicitly allowed
+ .collect(toImmutableEnumSet());
+
+ @Override
+ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+ if (INSTANT_OF_LONG_TEMPORAL_UNIT.matches(tree, state)) {
+ if (getInvalidChronoUnit(tree.getArguments().get(1), INVALID_TEMPORAL_UNITS).isPresent()) {
+ return describeMatch(tree);
+ }
+ }
+ return Description.NO_MATCH;
+ }
+}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/JavaTimeDefaultTimeZone.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/JavaTimeDefaultTimeZone.java
index 9c60673ea00..879f11868db 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/time/JavaTimeDefaultTimeZone.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/time/JavaTimeDefaultTimeZone.java
@@ -30,8 +30,7 @@
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
-import java.util.ArrayList;
-import java.util.List;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.google.errorprone.fixes.SuggestedFixes;
@@ -80,27 +79,35 @@ public final class JavaTimeDefaultTimeZone extends BugChecker
.named("systemDefaultZone")
.withParameters();
- private static final Matcher MATCHER =
- Matchers.allOf(
- buildMatcher(),
- // Allow usage by java.time itself
- Matchers.not(Matchers.packageStartsWith("java.time")));
+ private static final Matcher IN_JAVA_TIME =
+ Matchers.packageStartsWith("java.time");
- private static Matcher buildMatcher() {
- List> matchers = new ArrayList<>();
- for (String type : NOW_STATIC) {
- matchers.add(Matchers.staticMethod().onClass(type).named("now").withParameters());
+ private static boolean matches(MethodInvocationTree tree) {
+ if (!tree.getArguments().isEmpty()) {
+ return false;
}
- for (String type : DATE_NOW_INSTANCE) {
- matchers.add(Matchers.instanceMethod().onExactClass(type).named("dateNow").withParameters());
+ MethodSymbol symbol = ASTHelpers.getSymbol(tree);
+ if (symbol == null) {
+ return false;
+ }
+
+ switch (symbol.getSimpleName().toString()) {
+ case "now":
+ return symbol.isStatic() && NOW_STATIC.contains(symbol.owner.getQualifiedName().toString());
+ case "dateNow":
+ return !symbol.isStatic()
+ && DATE_NOW_INSTANCE.contains(symbol.owner.getQualifiedName().toString());
+ case "systemDefaultZone":
+ return symbol.isStatic()
+ && symbol.owner.getQualifiedName().contentEquals("java.time.Clock");
+ default:
+ return false;
}
- matchers.add(CLOCK_MATCHER);
- return Matchers.anyOf(matchers);
}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
- if (!MATCHER.matches(tree, state)) {
+ if (!matches(tree) || IN_JAVA_TIME.matches(tree, state)) {
return Description.NO_MATCH;
}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/LocalDateTemporalAmount.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/LocalDateTemporalAmount.java
new file mode 100644
index 00000000000..dba0009e71d
--- /dev/null
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/time/LocalDateTemporalAmount.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns.time;
+
+import static com.google.errorprone.BugPattern.ProvidesFix.NO_FIX;
+import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.argument;
+import static com.google.errorprone.matchers.Matchers.isSameType;
+import static com.google.errorprone.matchers.Matchers.not;
+import static com.google.errorprone.matchers.Matchers.packageStartsWith;
+import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
+
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.MethodInvocationTree;
+
+/**
+ * Bans calls to {@code LocalDate.plus(TemporalAmount)} and {@code LocalDate.minus(TemporalAmount)}
+ * where the {@link java.time.temporal.TemporalAmount} is a non-zero {@link java.time.Duration}.
+ */
+@BugPattern(
+ name = "LocalDateTemporalAmount",
+ summary =
+ "LocalDate.plus() and minus() does not work with Durations. LocalDate represents civil"
+ + " time (years/months/days), so java.time.Period is the appropriate thing to add or"
+ + " subtract instead.",
+ severity = ERROR,
+ providesFix = NO_FIX)
+public final class LocalDateTemporalAmount extends BugChecker
+ implements MethodInvocationTreeMatcher {
+
+ private static final Matcher MATCHER =
+ allOf(
+ instanceMethod()
+ .onExactClass("java.time.LocalDate")
+ .namedAnyOf("plus", "minus")
+ .withParameters("java.time.temporal.TemporalAmount"),
+ // TODO(kak): If the parameter is equal to Duration.ZERO, then technically this call will
+ // work, but that's a very small corner case to worry about.
+ argument(0, isSameType("java.time.Duration")),
+ not(packageStartsWith("java.")));
+
+ @Override
+ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+ return MATCHER.matches(tree, state) ? describeMatch(tree) : Description.NO_MATCH;
+ }
+}
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/time/PeriodGetTemporalUnit.java b/core/src/main/java/com/google/errorprone/bugpatterns/time/PeriodGetTemporalUnit.java
index c13ce96a3cb..e807ee5f040 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/time/PeriodGetTemporalUnit.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/time/PeriodGetTemporalUnit.java
@@ -19,6 +19,7 @@
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.bugpatterns.time.DurationGetTemporalUnit.getInvalidChronoUnit;
+import com.google.common.collect.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
@@ -57,7 +58,9 @@ public final class PeriodGetTemporalUnit extends BugChecker implements MethodInv
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return MATCHER.matches(tree, state)
- && getInvalidChronoUnit(tree, INVALID_TEMPORAL_UNITS).isPresent()
+ && getInvalidChronoUnit(
+ Iterables.getOnlyElement(tree.getArguments()), INVALID_TEMPORAL_UNITS)
+ .isPresent()
? describeMatch(tree)
: Description.NO_MATCH;
}
diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
index 9618fc638a7..bb32a3986aa 100644
--- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
+++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java
@@ -19,6 +19,7 @@
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Streams;
import com.google.errorprone.BugCheckerInfo;
import com.google.errorprone.bugpatterns.AmbiguousMethodReference;
import com.google.errorprone.bugpatterns.AnnotateFormatMethod;
@@ -55,6 +56,7 @@
import com.google.errorprone.bugpatterns.CatchFail;
import com.google.errorprone.bugpatterns.ChainedAssertionLosesContext;
import com.google.errorprone.bugpatterns.ChainingConstructorIgnoresParameter;
+import com.google.errorprone.bugpatterns.CheckNotNullMultipleTimes;
import com.google.errorprone.bugpatterns.CheckReturnValue;
import com.google.errorprone.bugpatterns.ClassCanBeStatic;
import com.google.errorprone.bugpatterns.ClassName;
@@ -155,6 +157,7 @@
import com.google.errorprone.bugpatterns.LambdaFunctionalInterface;
import com.google.errorprone.bugpatterns.LiteByteStringUtf8;
import com.google.errorprone.bugpatterns.LiteEnumValueOf;
+import com.google.errorprone.bugpatterns.LiteProtoToString;
import com.google.errorprone.bugpatterns.LockNotBeforeTry;
import com.google.errorprone.bugpatterns.LogicalAssignment;
import com.google.errorprone.bugpatterns.LongLiteralLowerCaseSuffix;
@@ -229,7 +232,6 @@
import com.google.errorprone.bugpatterns.ProtoTruthMixedDescriptors;
import com.google.errorprone.bugpatterns.ProtocolBufferOrdinal;
import com.google.errorprone.bugpatterns.ProtosAsKeyOfSetOrMap;
-import com.google.errorprone.bugpatterns.ProvideDescriptionToCheck;
import com.google.errorprone.bugpatterns.ProvidesFixChecker;
import com.google.errorprone.bugpatterns.RandomCast;
import com.google.errorprone.bugpatterns.RandomModInteger;
@@ -391,7 +393,9 @@
import com.google.errorprone.bugpatterns.threadsafety.UnlockMethodChecker;
import com.google.errorprone.bugpatterns.time.DurationFrom;
import com.google.errorprone.bugpatterns.time.DurationGetTemporalUnit;
+import com.google.errorprone.bugpatterns.time.DurationTemporalUnit;
import com.google.errorprone.bugpatterns.time.DurationToLongTimeUnit;
+import com.google.errorprone.bugpatterns.time.InstantTemporalUnit;
import com.google.errorprone.bugpatterns.time.JavaDurationGetSecondsGetNano;
import com.google.errorprone.bugpatterns.time.JavaDurationWithNanos;
import com.google.errorprone.bugpatterns.time.JavaDurationWithSeconds;
@@ -405,6 +409,7 @@
import com.google.errorprone.bugpatterns.time.JodaTimeConverterManager;
import com.google.errorprone.bugpatterns.time.JodaToSelf;
import com.google.errorprone.bugpatterns.time.JodaWithDurationAddedLong;
+import com.google.errorprone.bugpatterns.time.LocalDateTemporalAmount;
import com.google.errorprone.bugpatterns.time.PeriodFrom;
import com.google.errorprone.bugpatterns.time.PeriodGetTemporalUnit;
import com.google.errorprone.bugpatterns.time.PeriodTimeMath;
@@ -427,11 +432,9 @@ public static ImmutableSet getSuppliers(Class getSuppliers(
Iterable> checkers) {
- ImmutableSet.Builder result = ImmutableSet.builder();
- for (Class checker : checkers) {
- result.add(BugCheckerInfo.create(checker));
- }
- return result.build();
+ return Streams.stream(checkers)
+ .map(BugCheckerInfo::create)
+ .collect(ImmutableSet.toImmutableSet());
}
/** Returns a {@link ScannerSupplier} with all {@link BugChecker}s in Error Prone. */
@@ -473,6 +476,7 @@ public static ScannerSupplier errorChecks() {
BadShiftAmount.class,
BundleDeserializationCast.class,
ChainingConstructorIgnoresParameter.class,
+ CheckNotNullMultipleTimes.class,
CheckReturnValue.class,
CollectionIncompatibleType.class,
CollectionToArraySafeParameter.class,
@@ -491,6 +495,7 @@ public static ScannerSupplier errorChecks() {
DuplicateMapKeys.class,
DurationFrom.class,
DurationGetTemporalUnit.class,
+ DurationTemporalUnit.class,
DurationToLongTimeUnit.class,
EqualsHashCode.class,
EqualsNaN.class,
@@ -516,6 +521,7 @@ public static ScannerSupplier errorChecks() {
InfiniteRecursion.class,
InjectOnFinalField.class,
InjectOnMemberAndConstructor.class,
+ InstantTemporalUnit.class,
InvalidPatternSyntax.class,
InvalidTimeZoneID.class,
InvalidZoneId.class,
@@ -530,6 +536,7 @@ public static ScannerSupplier errorChecks() {
JUnit4TestNotRun.class,
JUnitAssertSameCheck.class,
LiteByteStringUtf8.class,
+ LocalDateTemporalAmount.class,
LoopConditionChecker.class,
MathRoundIntLong.class,
MislabeledAndroidString.class,
@@ -672,6 +679,7 @@ public static ScannerSupplier errorChecks() {
JUnit4ClassUsedInJUnit3.class,
JUnitAmbiguousTestClass.class,
LiteEnumValueOf.class,
+ LiteProtoToString.class,
LockNotBeforeTry.class,
LogicalAssignment.class,
MathAbsoluteRandom.class,
@@ -827,7 +835,6 @@ public static ScannerSupplier errorChecks() {
PrivateConstructorForNoninstantiableModule.class,
PrivateConstructorForUtilityClass.class,
ProtosAsKeyOfSetOrMap.class,
- ProvideDescriptionToCheck.class,
ProvidesFixChecker.class,
QualifierWithTypeUse.class,
RedundantThrows.class,
diff --git a/core/src/test/java/com/google/errorprone/VisitorStateTest.java b/core/src/test/java/com/google/errorprone/VisitorStateTest.java
new file mode 100644
index 00000000000..827c43e66c3
--- /dev/null
+++ b/core/src/test/java/com/google/errorprone/VisitorStateTest.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link VisitorState}. */
+@RunWith(JUnit4.class)
+public class VisitorStateTest {
+
+ @Test
+ public void symbolFromString_defaultPackage() {
+ assertThat(VisitorState.inferBinaryName("InDefaultPackage")).isEqualTo("InDefaultPackage");
+ }
+
+ @Test
+ public void symbolFromString_nestedTypeInDefaultPackage() {
+ assertThat(VisitorState.inferBinaryName("InDefaultPackage.Nested"))
+ .isEqualTo("InDefaultPackage$Nested");
+ }
+
+ @Test
+ public void symbolFromString_regularClass() {
+ assertThat(VisitorState.inferBinaryName("test.RegularClass")).isEqualTo("test.RegularClass");
+ assertThat(VisitorState.inferBinaryName("com.google.RegularClass"))
+ .isEqualTo("com.google.RegularClass");
+ }
+
+ @Test
+ public void symbolFromString_nestedTypeInRegularPackage() {
+ assertThat(VisitorState.inferBinaryName("test.RegularClass.Nested"))
+ .isEqualTo("test.RegularClass$Nested");
+ assertThat(VisitorState.inferBinaryName("com.google.RegularClass.Nested"))
+ .isEqualTo("com.google.RegularClass$Nested");
+ }
+}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/CheckNotNullMultipleTimesTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/CheckNotNullMultipleTimesTest.java
new file mode 100644
index 00000000000..6fa15e9b512
--- /dev/null
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/CheckNotNullMultipleTimesTest.java
@@ -0,0 +1,113 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link CheckNotNullMultipleTimes}. */
+@RunWith(JUnit4.class)
+public final class CheckNotNullMultipleTimesTest {
+
+ private final CompilationTestHelper helper =
+ CompilationTestHelper.newInstance(CheckNotNullMultipleTimes.class, getClass());
+
+ @Test
+ public void positive() {
+ helper
+ .addSourceLines(
+ "Test.java",
+ "import static com.google.common.base.Preconditions.checkNotNull;",
+ "class Test {",
+ " Test(Integer a, Integer b) {",
+ " checkNotNull(a);",
+ " // BUG: Diagnostic contains:",
+ " checkNotNull(a);",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void negative() {
+ helper
+ .addSourceLines(
+ "Test.java",
+ "import static com.google.common.base.Preconditions.checkNotNull;",
+ "class Test {",
+ " Test(Integer a, Integer b) {",
+ " checkNotNull(a);",
+ " checkNotNull(b);",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void conditional_noError() {
+ helper
+ .addSourceLines(
+ "Test.java",
+ "import static com.google.common.base.Preconditions.checkNotNull;",
+ "class Test {",
+ " Test(Integer a) {",
+ " if (hashCode() > 0) {",
+ " checkNotNull(a);",
+ " } else {",
+ " checkNotNull(a);",
+ " }",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void assignedTwice_noError() {
+ helper
+ .addSourceLines(
+ "Test.java",
+ "import static com.google.common.base.Preconditions.checkNotNull;",
+ "class Test {",
+ " int a;",
+ " Test(Integer a) {",
+ " this.a = checkNotNull(a);",
+ " checkNotNull(a);",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void withinTryAndCatch_noError() {
+ helper
+ .addSourceLines(
+ "Test.java",
+ "import static com.google.common.base.Preconditions.checkNotNull;",
+ "class Test {",
+ " Test(Integer a) {",
+ " try {",
+ // There could be intervening lines here which would throw, leading to the first check
+ // not being reached.
+ " checkNotNull(a);",
+ " } catch (Exception e) {",
+ " checkNotNull(a);",
+ " }",
+ " }",
+ "}")
+ .doTest();
+ }
+}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/InconsistentHashCodeTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/InconsistentHashCodeTest.java
index 24f97f57647..1e1fa7dbcd1 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/InconsistentHashCodeTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/InconsistentHashCodeTest.java
@@ -128,4 +128,31 @@ public void memoizedHashCode() {
"}")
.doTest();
}
+
+ @Test
+ public void breakLabeledBlock() {
+ helper
+ .addSourceLines(
+ "Test.java",
+ "class Test {",
+ " private int a;",
+ " private int b;",
+ " private int hashCode;",
+ " public void accessesLocalWithLabeledBreak() {",
+ " label: {",
+ " switch (a) {",
+ " case 0: break label;",
+ " }",
+ " }",
+ " }",
+ " @Override public boolean equals(Object o) {",
+ " Test that = (Test) o;",
+ " return this.a == that.a && this.b == that.b;",
+ " }",
+ " @Override public int hashCode() {",
+ " return hashCode;",
+ " }",
+ "}")
+ .doTest();
+ }
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/LiteProtoToStringTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/LiteProtoToStringTest.java
new file mode 100644
index 00000000000..bd31e0e845b
--- /dev/null
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/LiteProtoToStringTest.java
@@ -0,0 +1,103 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Ignore;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Tests for {@link LiteProtoToString} bugpattern.
+ *
+ * @author ghm@google.com (Graeme Morgan)
+ */
+@RunWith(JUnit4.class)
+public final class LiteProtoToStringTest {
+ private final CompilationTestHelper compilationHelper =
+ CompilationTestHelper.newInstance(LiteProtoToString.class, getClass());
+
+ @Test
+ public void positive() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.protobuf.GeneratedMessageLite;",
+ "class Test {",
+ " private String test(GeneratedMessageLite message) {",
+ " // BUG: Diagnostic contains:",
+ " return message.toString();",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void negative() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.protobuf.GeneratedMessageLite;",
+ "class Test {",
+ " private void test(GeneratedMessageLite message) {",
+ " atVerbose(message.toString());",
+ " }",
+ " public void atVerbose(String s) {}",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ @Ignore // TODO(b/130683674,ghm): Support checking toString on proto enums.
+ public void enums() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.protobuf.Internal.EnumLite;",
+ "import com.google.protobuf.ProtocolMessageEnum;",
+ "class Test {",
+ " private String test(EnumLite e) {",
+ " return e.toString();",
+ " }",
+ " private String test2(ProtocolMessageEnum e) {",
+ " return e.toString();",
+ " }",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void nestedLogStatement() {
+ compilationHelper
+ .addSourceLines(
+ "Test.java",
+ "import com.google.protobuf.GeneratedMessageLite;",
+ "class Test {",
+ " private void test(GeneratedMessageLite message) {",
+ " atVerbose().log(message.toString());",
+ " }",
+ " public Test atVerbose() {",
+ " return this;",
+ " }",
+ " public Test log(String s) {",
+ " return this;",
+ " }",
+ "}")
+ .doTest();
+ }
+}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ProvideDescriptionToCheckTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ProvideDescriptionToCheckTest.java
deleted file mode 100644
index 24ebb1971b9..00000000000
--- a/core/src/test/java/com/google/errorprone/bugpatterns/ProvideDescriptionToCheckTest.java
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * Copyright 2019 The Error Prone Authors.
- *
- * 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.errorprone.bugpatterns;
-
-import com.google.errorprone.CompilationTestHelper;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-/** @author cpovirk@google.com (Chris Povirk) */
-@RunWith(JUnit4.class)
-public class ProvideDescriptionToCheckTest {
- private CompilationTestHelper compilationHelper;
-
- @Before
- public void setUp() {
- compilationHelper =
- CompilationTestHelper.newInstance(ProvideDescriptionToCheck.class, getClass());
- }
-
- @Test
- public void testPositiveCase() {
- compilationHelper.addSourceFile("ProvideDescriptionToCheckPositiveCases.java").doTest();
- }
-
- @Test
- public void testNegativeCase() {
- compilationHelper.addSourceFile("ProvideDescriptionToCheckNegativeCases.java").doTest();
- }
-}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/TypeParameterNamingTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/TypeParameterNamingTest.java
index 5c87889114c..7311aaecd5e 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/TypeParameterNamingTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/TypeParameterNamingTest.java
@@ -312,7 +312,7 @@ public void classifyTypeName_invalidTypeParameters() {
assertKindOfName("ACanalPanamaT").isEqualTo(NON_CLASS_NAME_WITH_T_SUFFIX);
}
- private static Subject assertKindOfName(String s) {
+ private static Subject assertKindOfName(String s) {
return assertWithMessage(s).that(TypeParameterNamingClassification.classify(s));
}
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryAnonymousClassTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryAnonymousClassTest.java
index e9b89c93886..fe371e8e389 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryAnonymousClassTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryAnonymousClassTest.java
@@ -91,4 +91,29 @@ public void variable_static() {
"}")
.doTest();
}
+
+ @Test
+ public void abstractClass() {
+ testHelper
+ .addInputLines(
+ "Test.java",
+ "import java.util.function.Function;",
+ "class Test {",
+ " static abstract class Impl implements Function {",
+ " public String apply(String input) {",
+ " return input;",
+ " }",
+ " public abstract void f(String input);",
+ " }",
+ " private final Function camelCase = new Impl() {",
+ " public void f(String input) {}",
+ " };",
+ " void g() {",
+ " Function f = camelCase;",
+ " System.err.println(camelCase.apply(\"world\"));",
+ " }",
+ "}")
+ .expectUnchanged()
+ .doTest();
+ }
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTestsTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTestsTest.java
index 27508df802a..a91d5fd1d18 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTestsTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UseCorrectAssertInTestsTest.java
@@ -200,7 +200,7 @@ public void wrongAssertReferenceSameCase() {
.addOutputLines(
OUTPUT,
inputWithExpressionsAndImport(
- "Integer a = 1;", "assertThat(a).isSameAs(1);", ASSERT_THAT_IMPORT))
+ "Integer a = 1;", "assertThat(a).isSameInstanceAs(1);", ASSERT_THAT_IMPORT))
.doTest();
}
@@ -211,7 +211,7 @@ public void wrongAssertReferenceWithParensCase() {
.addOutputLines(
OUTPUT,
inputWithExpressionsAndImport(
- "Integer a = 1;", "assertThat(a).isSameAs(1);", ASSERT_THAT_IMPORT))
+ "Integer a = 1;", "assertThat(a).isSameInstanceAs(1);", ASSERT_THAT_IMPORT))
.doTest();
}
@@ -222,7 +222,7 @@ public void wrongAssertReferenceNotSameCase() {
.addOutputLines(
OUTPUT,
inputWithExpressionsAndImport(
- "Integer a = 1;", "assertThat(a).isNotSameAs(1);", ASSERT_THAT_IMPORT))
+ "Integer a = 1;", "assertThat(a).isNotSameInstanceAs(1);", ASSERT_THAT_IMPORT))
.doTest();
}
@@ -234,7 +234,7 @@ public void wrongAssertReferenceSameCaseWithDetailCase() {
OUTPUT,
inputWithExpressionsAndImport(
"int a = 1;",
- "assertWithMessage(\"detail\").that(a).isSameAs(1);",
+ "assertWithMessage(\"detail\").that(a).isSameInstanceAs(1);",
ASSERT_WITH_MESSAGE_IMPORT))
.doTest();
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ChainedAssertionLosesContextNegativeCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ChainedAssertionLosesContextNegativeCases.java
index d15a4b195ba..7b2f4926e4d 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ChainedAssertionLosesContextNegativeCases.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ChainedAssertionLosesContextNegativeCases.java
@@ -24,7 +24,7 @@
/** @author cpovirk@google.com (Chris Povirk) */
public class ChainedAssertionLosesContextNegativeCases {
- static final class FooSubject extends Subject {
+ static final class FooSubject extends Subject {
private final Foo actual;
private FooSubject(FailureMetadata metadata, Foo actual) {
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ChainedAssertionLosesContextPositiveCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ChainedAssertionLosesContextPositiveCases.java
index 282cda1b6bf..e13eae6ad51 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ChainedAssertionLosesContextPositiveCases.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ChainedAssertionLosesContextPositiveCases.java
@@ -26,7 +26,7 @@
/** @author cpovirk@google.com (Chris Povirk) */
public class ChainedAssertionLosesContextPositiveCases {
- static final class FooSubject extends Subject {
+ static final class FooSubject extends Subject {
private final Foo actual;
static Factory foos() {
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ImplementAssertionWithChainingNegativeCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ImplementAssertionWithChainingNegativeCases.java
index be823584fd3..8f05547c781 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ImplementAssertionWithChainingNegativeCases.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ImplementAssertionWithChainingNegativeCases.java
@@ -21,7 +21,7 @@
/** @author cpovirk@google.com (Chris Povirk) */
public class ImplementAssertionWithChainingNegativeCases {
- static final class FooSubject extends Subject {
+ static final class FooSubject extends Subject {
private final Foo actual;
private FooSubject(FailureMetadata metadata, Foo actual) {
@@ -31,19 +31,19 @@ private FooSubject(FailureMetadata metadata, Foo actual) {
void doesNotHaveString(String other) {
if (actual.string().equals(other)) {
- fail("matched unexpected string");
+ failWithActual("expected not to have string", other);
}
}
void doesNotHaveInteger(int other) {
if (actual.integer() == other) {
- fail("had unexpected integer");
+ failWithActual("expected not to have integer", other);
}
}
void hasBoxedIntegerSameInstance(Integer expected) {
if (actual.boxedInteger() != expected) {
- fail("didn't match expected string instance");
+ failWithActual("expected to have boxed integer", expected);
}
}
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ImplementAssertionWithChainingPositiveCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ImplementAssertionWithChainingPositiveCases.java
index 2cb9e7c6663..d1abadb8d67 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ImplementAssertionWithChainingPositiveCases.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ImplementAssertionWithChainingPositiveCases.java
@@ -21,7 +21,7 @@
/** @author cpovirk@google.com (Chris Povirk) */
public class ImplementAssertionWithChainingPositiveCases {
- static final class FooSubject extends Subject {
+ static final class FooSubject extends Subject {
private final Foo actual;
private FooSubject(FailureMetadata metadata, Foo actual) {
@@ -32,35 +32,35 @@ private FooSubject(FailureMetadata metadata, Foo actual) {
void hasString(String expected) {
// BUG: Diagnostic contains: check("string()").that(actual.string()).isEqualTo(expected)
if (!actual.string().equals(expected)) {
- fail("didn't match expected string");
+ failWithActual("expected to have string", expected);
}
}
void hasStringGuavaObjectsEqual(String expected) {
// BUG: Diagnostic contains: check("string()").that(actual.string()).isEqualTo(expected)
if (!com.google.common.base.Objects.equal(actual.string(), expected)) {
- fail("didn't match expected string");
+ failWithActual("expected to have string", expected);
}
}
void hasStringJavaObjectsEquals(String expected) {
// BUG: Diagnostic contains: check("string()").that(actual.string()).isEqualTo(expected)
if (!java.util.Objects.equals(actual.string(), expected)) {
- fail("didn't match expected string");
+ failWithActual("expected to have string", expected);
}
}
void hasInteger(int expected) {
// BUG: Diagnostic contains: check("integer()").that(actual.integer()).isEqualTo(expected)
if (actual.integer() != expected) {
- fail("has integer %s", expected);
+ failWithActual("expected to have integer", expected);
}
}
void hasKind(Kind expected) {
// BUG: Diagnostic contains: check("kind()").that(actual.kind()).isEqualTo(expected)
if (actual.kind() != expected) {
- fail("has kind %s", expected);
+ failWithActual("expected to have kind", expected);
}
}
@@ -68,7 +68,7 @@ void hasOtherFooInteger(int expected) {
// BUG: Diagnostic contains:
// check("otherFoo().integer()").that(actual.otherFoo().integer()).isEqualTo(expected)
if (actual.otherFoo().integer() != expected) {
- fail("has other foo with integer %s", expected);
+ failWithActual("expected to have other foo with integer", expected);
}
}
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ProvideDescriptionToCheckNegativeCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ProvideDescriptionToCheckNegativeCases.java
deleted file mode 100644
index c0ee1377a6d..00000000000
--- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ProvideDescriptionToCheckNegativeCases.java
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- * Copyright 2019 The Error Prone Authors.
- *
- * 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.errorprone.bugpatterns.testdata;
-
-import com.google.common.truth.FailureMetadata;
-import com.google.common.truth.Subject;
-
-/** @author cpovirk@google.com (Chris Povirk) */
-public class ProvideDescriptionToCheckNegativeCases {
- static final class FooSubject extends Subject {
- private final Foo actual;
-
- private FooSubject(FailureMetadata metadata, Foo actual) {
- super(metadata, actual);
- this.actual = actual;
- }
-
- void alreadyPassesDescription(String expected) {
- check("s()").that(actual.string()).isEqualTo(expected);
- }
- }
-
- private static final class Foo {
- final String string;
- final int integer;
-
- Foo(String string, int integer) {
- this.string = string;
- this.integer = integer;
- }
-
- String string() {
- return string;
- }
-
- int integer() {
- return integer;
- }
-
- Foo otherFoo() {
- return this;
- }
- }
-}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ProvideDescriptionToCheckPositiveCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ProvideDescriptionToCheckPositiveCases.java
deleted file mode 100644
index 9b2ba2ed024..00000000000
--- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/ProvideDescriptionToCheckPositiveCases.java
+++ /dev/null
@@ -1,121 +0,0 @@
-/*
- * Copyright 2019 The Error Prone Authors.
- *
- * 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.errorprone.bugpatterns.testdata;
-
-import static com.google.errorprone.bugpatterns.testdata.ProvideDescriptionToCheckPositiveCases.MyStringSubject.myStrings;
-
-import com.google.common.truth.CustomSubjectBuilder;
-import com.google.common.truth.FailureMetadata;
-import com.google.common.truth.StringSubject;
-import com.google.common.truth.Subject;
-
-/** @author cpovirk@google.com (Chris Povirk) */
-public class ProvideDescriptionToCheckPositiveCases {
- static final class FooSubject extends Subject {
- private final Foo actual;
-
- private FooSubject(FailureMetadata metadata, Foo actual) {
- super(metadata, actual);
- this.actual = actual;
- }
-
- void hasString(String expected) {
- // BUG: Diagnostic contains: check("string()").that(actual.string()).isEqualTo(expected)
- check().that(actual.string()).isEqualTo(expected);
- }
-
- void hasStringThisCheck(String expected) {
- // BUG: Diagnostic contains: this.check("string()").that(actual.string()).isEqualTo(
- this.check().that(actual.string()).isEqualTo(expected);
- }
-
- void hasStringWithMessage(String expected) {
- // BUG: Diagnostic contains:
- // check("string()").withMessage("abc").that(actual.string()).isEqualTo(expected)
- check().withMessage("abc").that(actual.string()).isEqualTo(expected);
- }
-
- void hasStringAbout(String expected) {
- // BUG: Diagnostic contains:
- // check("string()").about(myStrings()).that(actual.string()).isEqualTo(expected)
- check().about(myStrings()).that(actual.string()).isEqualTo(expected);
- }
-
- void hasStringWithMessageAbout(String expected) {
- // BUG: Diagnostic contains: check("string()")
- check().withMessage("abc").about(myStrings()).that(actual.string()).isEqualTo(expected);
- }
-
- void hasOtherFooInteger(int expected) {
- // BUG: Diagnostic contains:
- // check("otherFoo().integer()").that(actual.otherFoo().integer()).isEqualTo(expected)
- check().that(actual.otherFoo().integer()).isEqualTo(expected);
- }
-
- FooSubject hasOtherFooUsingFactory() {
- // BUG: Diagnostic contains:
- // check("otherFoo()").about(foos()).that(actual.otherFoo())
- return check().about(foos()).that(actual.otherFoo());
- }
- }
-
- static final class MyStringSubject extends StringSubject {
- private MyStringSubject(FailureMetadata metadata, String string) {
- super(metadata, string);
- }
-
- static Factory myStrings() {
- return MyStringSubject::new;
- }
- }
-
- static final class FooCustomSubjectBuilder extends CustomSubjectBuilder {
- FooCustomSubjectBuilder(FailureMetadata metadata) {
- super(metadata);
- }
-
- FooSubject that(Foo actual) {
- return new FooSubject(metadata(), actual);
- }
- }
-
- static CustomSubjectBuilder.Factory foos() {
- return FooCustomSubjectBuilder::new;
- }
-
- private static final class Foo {
- final String string;
- final int integer;
-
- Foo(String string, int integer) {
- this.string = string;
- this.integer = integer;
- }
-
- String string() {
- return string;
- }
-
- int integer() {
- return integer;
- }
-
- Foo otherFoo() {
- return this;
- }
- }
-}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/TruthSelfEqualsPositiveCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/TruthSelfEqualsPositiveCases.java
index 4978ebcc9d8..907992b0786 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/TruthSelfEqualsPositiveCases.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/TruthSelfEqualsPositiveCases.java
@@ -41,13 +41,13 @@ public void testAssertWithMessageEq() {
public void testAssertThatSame() {
String test = Boolean.TRUE.toString();
// BUG: Diagnostic contains: new EqualsTester().addEqualityGroup(test).testEquals()
- assertThat(test).isSameAs(test);
+ assertThat(test).isSameInstanceAs(test);
}
public void testAssertWithMessageSame() {
String test = Boolean.TRUE.toString();
// BUG: Diagnostic contains: new EqualsTester().addEqualityGroup(test).testEquals()
- assertWithMessage("msg").that(test).isSameAs(test);
+ assertWithMessage("msg").that(test).isSameInstanceAs(test);
}
public void testAssertThatNeq() {
@@ -58,8 +58,8 @@ public void testAssertThatNeq() {
public void testAssertThatNotSame() {
String test = Boolean.TRUE.toString();
- // BUG: Diagnostic contains: isNotSameAs method are the same object
- assertThat(test).isNotSameAs(test);
+ // BUG: Diagnostic contains: isNotSameInstanceAs method are the same object
+ assertThat(test).isNotSameInstanceAs(test);
}
public void testAssertWithMessageNeq() {
@@ -70,7 +70,7 @@ public void testAssertWithMessageNeq() {
public void testAssertWithMessageNotSame() {
String test = Boolean.TRUE.toString();
- // BUG: Diagnostic contains: isNotSameAs method are the same object
- assertWithMessage("msg").that(test).isNotSameAs(test);
+ // BUG: Diagnostic contains: isNotSameInstanceAs method are the same object
+ assertWithMessage("msg").that(test).isNotSameInstanceAs(test);
}
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByBinderTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByBinderTest.java
index 1799b233c2e..612272fe288 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByBinderTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/threadsafety/GuardedByBinderTest.java
@@ -21,6 +21,7 @@
import static org.junit.Assert.fail;
import com.google.errorprone.ErrorProneInMemoryFileManager;
+import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.CompilationUnitTree;
import com.sun.tools.javac.api.JavacTaskImpl;
@@ -515,7 +516,11 @@ private String bind(String className, String exprString, JavaFileObject fileObje
GuardedByBinder.bindString(
exprString,
GuardedBySymbolResolver.from(
- ASTHelpers.getSymbol(classDecl), compilationUnit, task.getContext(), null));
+ ASTHelpers.getSymbol(classDecl),
+ compilationUnit,
+ task.getContext(),
+ null,
+ VisitorState.createForUtilityPurposes(task.getContext())));
if (!guardExpression.isPresent()) {
throw new IllegalGuardedBy(exprString);
}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/time/DurationTemporalUnitTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/time/DurationTemporalUnitTest.java
new file mode 100644
index 00000000000..38331b89658
--- /dev/null
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/time/DurationTemporalUnitTest.java
@@ -0,0 +1,270 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns.time;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link DurationTemporalUnit}. */
+@RunWith(JUnit4.class)
+public class DurationTemporalUnitTest {
+ private final CompilationTestHelper helper =
+ CompilationTestHelper.newInstance(DurationTemporalUnit.class, getClass());
+
+ @Test
+ public void durationOf_good() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " private static final Duration D0 = Duration.of(1, ChronoUnit.DAYS);",
+ " private static final Duration D1 = Duration.of(1, ChronoUnit.HALF_DAYS);",
+ " private static final Duration D2 = Duration.of(1, ChronoUnit.HOURS);",
+ " private static final Duration D3 = Duration.of(1, ChronoUnit.MICROS);",
+ " private static final Duration D4 = Duration.of(1, ChronoUnit.MILLIS);",
+ " private static final Duration D5 = Duration.of(1, ChronoUnit.MINUTES);",
+ " private static final Duration D6 = Duration.of(1, ChronoUnit.NANOS);",
+ " private static final Duration D7 = Duration.of(1, ChronoUnit.SECONDS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationOf_bad() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D0 = Duration.of(1, ChronoUnit.CENTURIES);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D1 = Duration.of(1, ChronoUnit.DECADES);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D2 = Duration.of(1, ChronoUnit.ERAS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D3 = Duration.of(1, ChronoUnit.FOREVER);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D4 = Duration.of(1, ChronoUnit.MILLENNIA);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D5 = Duration.of(1, ChronoUnit.MONTHS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D6 = Duration.of(1, ChronoUnit.WEEKS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D7 = Duration.of(1, ChronoUnit.YEARS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationPlus_good() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " private static final Duration D0 = Duration.ZERO.plus(1, ChronoUnit.DAYS);",
+ " private static final Duration D1 = Duration.ZERO.plus(1, ChronoUnit.HALF_DAYS);",
+ " private static final Duration D2 = Duration.ZERO.plus(1, ChronoUnit.HOURS);",
+ " private static final Duration D3 = Duration.ZERO.plus(1, ChronoUnit.MICROS);",
+ " private static final Duration D4 = Duration.ZERO.plus(1, ChronoUnit.MILLIS);",
+ " private static final Duration D5 = Duration.ZERO.plus(1, ChronoUnit.MINUTES);",
+ " private static final Duration D6 = Duration.ZERO.plus(1, ChronoUnit.NANOS);",
+ " private static final Duration D7 = Duration.ZERO.plus(1, ChronoUnit.SECONDS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationPlus_bad() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D0 = Duration.ZERO.plus(1, ChronoUnit.CENTURIES);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D1 = Duration.ZERO.plus(1, ChronoUnit.DECADES);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D2 = Duration.ZERO.plus(1, ChronoUnit.ERAS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D3 = Duration.ZERO.plus(1, ChronoUnit.FOREVER);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D4 = Duration.ZERO.plus(1, ChronoUnit.MILLENNIA);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D5 = Duration.ZERO.plus(1, ChronoUnit.MONTHS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D6 = Duration.ZERO.plus(1, ChronoUnit.WEEKS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D7 = Duration.ZERO.plus(1, ChronoUnit.YEARS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationMinus_good() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " private static final Duration D0 = Duration.ZERO.minus(1, ChronoUnit.DAYS);",
+ " private static final Duration D1 = Duration.ZERO.minus(1, ChronoUnit.HALF_DAYS);",
+ " private static final Duration D2 = Duration.ZERO.minus(1, ChronoUnit.HOURS);",
+ " private static final Duration D3 = Duration.ZERO.minus(1, ChronoUnit.MICROS);",
+ " private static final Duration D4 = Duration.ZERO.minus(1, ChronoUnit.MILLIS);",
+ " private static final Duration D5 = Duration.ZERO.minus(1, ChronoUnit.MINUTES);",
+ " private static final Duration D6 = Duration.ZERO.minus(1, ChronoUnit.NANOS);",
+ " private static final Duration D7 = Duration.ZERO.minus(1, ChronoUnit.SECONDS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationMinus_bad() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D0 = Duration.ZERO.minus(1, ChronoUnit.CENTURIES);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D1 = Duration.ZERO.minus(1, ChronoUnit.DECADES);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D2 = Duration.ZERO.minus(1, ChronoUnit.ERAS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D3 = Duration.ZERO.minus(1, ChronoUnit.FOREVER);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D4 = Duration.ZERO.minus(1, ChronoUnit.MILLENNIA);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D5 = Duration.ZERO.minus(1, ChronoUnit.MONTHS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D6 = Duration.ZERO.minus(1, ChronoUnit.WEEKS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D7 = Duration.ZERO.minus(1, ChronoUnit.YEARS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationOfStaticImport() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import static java.time.temporal.ChronoUnit.NANOS;",
+ "import static java.time.temporal.ChronoUnit.SECONDS;",
+ "import static java.time.temporal.ChronoUnit.YEARS;",
+ "import java.time.Duration;",
+ "public class TestClass {",
+ " private static final Duration D1 = Duration.of(1, SECONDS);",
+ " private static final Duration D2 = Duration.of(1, NANOS);",
+ " // BUG: Diagnostic contains: DurationTemporalUnit",
+ " private static final Duration D3 = Duration.of(1, YEARS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationOfWithRandomTemporalUnit() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import static java.time.temporal.ChronoUnit.SECONDS;",
+ "import static java.time.temporal.ChronoUnit.YEARS;",
+ "import java.time.Duration;",
+ "import java.time.temporal.TemporalUnit;",
+ "import java.util.Random;",
+ "public class TestClass {",
+ " private static final TemporalUnit random = ",
+ " new Random().nextBoolean() ? YEARS : SECONDS;",
+ // Since we don't know at compile time what 'random' is, we can't flag this
+ " private static final Duration D1 = Duration.of(1, random);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationOfWithAliasedTemporalUnit() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import static java.time.temporal.ChronoUnit.YEARS;",
+ "import java.time.Duration;",
+ "import java.time.temporal.Temporal;",
+ "import java.time.temporal.TemporalUnit;",
+ "public class TestClass {",
+ " private static final TemporalUnit SECONDS = YEARS;",
+ // This really should be flagged, but isn't :(
+ " private static final Duration D1 = Duration.of(1, SECONDS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void durationOfWithCustomTemporalUnit() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.temporal.Temporal;",
+ "import java.time.temporal.TemporalUnit;",
+ "public class TestClass {",
+ " private static class BadTemporalUnit implements TemporalUnit {",
+ " @Override",
+ " public long between(Temporal t1, Temporal t2) {",
+ " throw new AssertionError();",
+ " }",
+ " @Override",
+ " public R addTo(R temporal, long amount) {",
+ " throw new AssertionError();",
+ " }",
+ " @Override",
+ " public boolean isTimeBased() {",
+ " throw new AssertionError();",
+ " }",
+ " @Override",
+ " public boolean isDateBased() {",
+ " throw new AssertionError();",
+ " }",
+ " @Override",
+ " public boolean isDurationEstimated() {",
+ " throw new AssertionError();",
+ " }",
+ " @Override",
+ " public Duration getDuration() {",
+ " throw new AssertionError();",
+ " }",
+ " }",
+ " private static final TemporalUnit MINUTES = new BadTemporalUnit();",
+ " private static final TemporalUnit SECONDS = new BadTemporalUnit();",
+ // This really should be flagged, but isn't :(
+ " private static final Duration D1 = Duration.of(1, SECONDS);",
+ " private static final Duration D2 = Duration.of(1, MINUTES);",
+ "}")
+ .doTest();
+ }
+}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/time/InstantTemporalUnitTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/time/InstantTemporalUnitTest.java
new file mode 100644
index 00000000000..dfd27d9a27e
--- /dev/null
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/time/InstantTemporalUnitTest.java
@@ -0,0 +1,146 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns.time;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.errorprone.CompilationTestHelper;
+import java.time.temporal.ChronoUnit;
+import java.util.EnumSet;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link InstantTemporalUnit}. */
+@RunWith(JUnit4.class)
+public final class InstantTemporalUnitTest {
+ private final CompilationTestHelper helper =
+ CompilationTestHelper.newInstance(InstantTemporalUnit.class, getClass());
+
+ @Test
+ public void invalidTemporalUnitsMatchesEnumeratedList() throws Exception {
+ // This list comes from the Instant javadocs, but the checker builds the list based on the
+ // definition in Instant.isSupported(): unit.isTimeBased() || unit == DAYS;
+ assertThat(InstantTemporalUnit.INVALID_TEMPORAL_UNITS)
+ .containsExactlyElementsIn(
+ EnumSet.complementOf(
+ EnumSet.of(
+ ChronoUnit.NANOS,
+ ChronoUnit.MICROS,
+ ChronoUnit.MILLIS,
+ ChronoUnit.SECONDS,
+ ChronoUnit.MINUTES,
+ ChronoUnit.HOURS,
+ ChronoUnit.HALF_DAYS,
+ ChronoUnit.DAYS)));
+ }
+
+ @Test
+ public void instantPlus_good() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Instant;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " private static final Instant I0 = Instant.EPOCH.plus(1, ChronoUnit.DAYS);",
+ " private static final Instant I1 = Instant.EPOCH.plus(1, ChronoUnit.HALF_DAYS);",
+ " private static final Instant I2 = Instant.EPOCH.plus(1, ChronoUnit.HOURS);",
+ " private static final Instant I3 = Instant.EPOCH.plus(1, ChronoUnit.MICROS);",
+ " private static final Instant I4 = Instant.EPOCH.plus(1, ChronoUnit.MILLIS);",
+ " private static final Instant I5 = Instant.EPOCH.plus(1, ChronoUnit.MINUTES);",
+ " private static final Instant I6 = Instant.EPOCH.plus(1, ChronoUnit.NANOS);",
+ " private static final Instant I7 = Instant.EPOCH.plus(1, ChronoUnit.SECONDS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void instantPlus_bad() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Instant;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I0 = Instant.EPOCH.plus(1, ChronoUnit.CENTURIES);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I1 = Instant.EPOCH.plus(1, ChronoUnit.DECADES);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I2 = Instant.EPOCH.plus(1, ChronoUnit.ERAS);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I3 = Instant.EPOCH.plus(1, ChronoUnit.FOREVER);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I4 = Instant.EPOCH.plus(1, ChronoUnit.MILLENNIA);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I5 = Instant.EPOCH.plus(1, ChronoUnit.MONTHS);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I6 = Instant.EPOCH.plus(1, ChronoUnit.WEEKS);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I7 = Instant.EPOCH.plus(1, ChronoUnit.YEARS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void instantMinus_good() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Instant;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " private static final Instant I0 = Instant.EPOCH.minus(1, ChronoUnit.DAYS);",
+ " private static final Instant I1 = Instant.EPOCH.minus(1, ChronoUnit.HALF_DAYS);",
+ " private static final Instant I2 = Instant.EPOCH.minus(1, ChronoUnit.HOURS);",
+ " private static final Instant I3 = Instant.EPOCH.minus(1, ChronoUnit.MICROS);",
+ " private static final Instant I4 = Instant.EPOCH.minus(1, ChronoUnit.MILLIS);",
+ " private static final Instant I5 = Instant.EPOCH.minus(1, ChronoUnit.MINUTES);",
+ " private static final Instant I6 = Instant.EPOCH.minus(1, ChronoUnit.NANOS);",
+ " private static final Instant I7 = Instant.EPOCH.minus(1, ChronoUnit.SECONDS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void instantMinus_bad() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Instant;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I0 = Instant.EPOCH.minus(1, ChronoUnit.CENTURIES);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I1 = Instant.EPOCH.minus(1, ChronoUnit.DECADES);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I2 = Instant.EPOCH.minus(1, ChronoUnit.ERAS);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I3 = Instant.EPOCH.minus(1, ChronoUnit.FOREVER);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I4 = Instant.EPOCH.minus(1, ChronoUnit.MILLENNIA);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I5 = Instant.EPOCH.minus(1, ChronoUnit.MONTHS);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I6 = Instant.EPOCH.minus(1, ChronoUnit.WEEKS);",
+ " // BUG: Diagnostic contains: InstantTemporalUnit",
+ " private static final Instant I7 = Instant.EPOCH.minus(1, ChronoUnit.YEARS);",
+ "}")
+ .doTest();
+ }
+}
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/time/LocalDateTemporalAmountTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/time/LocalDateTemporalAmountTest.java
new file mode 100644
index 00000000000..cee4b076372
--- /dev/null
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/time/LocalDateTemporalAmountTest.java
@@ -0,0 +1,126 @@
+/*
+ * Copyright 2019 The Error Prone Authors.
+ *
+ * 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.errorprone.bugpatterns.time;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link LocalDateTemporalAmount}. */
+@RunWith(JUnit4.class)
+public final class LocalDateTemporalAmountTest {
+ private final CompilationTestHelper helper =
+ CompilationTestHelper.newInstance(LocalDateTemporalAmount.class, getClass());
+
+ @Test
+ public void localDatePlus_good() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.LocalDate;",
+ "import java.time.Period;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " private static final LocalDate LD = LocalDate.of(1985, 5, 31);",
+ " private static final Period PERIOD = Period.ofDays(1);",
+ " private static final LocalDate LD1 = LD.plus(PERIOD);",
+ " private static final LocalDate LD2 = LD.plus(1, ChronoUnit.DAYS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void localDatePlus_bad() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.LocalDate;",
+ "public class TestClass {",
+ " private static final LocalDate LD = LocalDate.of(1985, 5, 31);",
+ " private static final Duration DURATION = Duration.ofDays(1);",
+ " // BUG: Diagnostic contains: LocalDateTemporalAmount",
+ " private static final LocalDate LD0 = LD.plus(DURATION);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void localDatePlus_zero() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.LocalDate;",
+ "public class TestClass {",
+ " private static final LocalDate LD = LocalDate.of(1985, 5, 31);",
+ " // BUG: Diagnostic contains: LocalDateTemporalAmount",
+ // This call technically doesn't throw, but we don't currently special case it
+ " private static final LocalDate LD0 = LD.plus(Duration.ZERO);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void localDateMinus_good() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.LocalDate;",
+ "import java.time.Period;",
+ "import java.time.temporal.ChronoUnit;",
+ "public class TestClass {",
+ " private static final LocalDate LD = LocalDate.of(1985, 5, 31);",
+ " private static final Period PERIOD = Period.ofDays(1);",
+ " private static final LocalDate LD1 = LD.minus(PERIOD);",
+ " private static final LocalDate LD2 = LD.minus(1, ChronoUnit.DAYS);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void localDateMinus_bad() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.LocalDate;",
+ "public class TestClass {",
+ " private static final LocalDate LD = LocalDate.of(1985, 5, 31);",
+ " private static final Duration DURATION = Duration.ofDays(1);",
+ " // BUG: Diagnostic contains: LocalDateTemporalAmount",
+ " private static final LocalDate LD0 = LD.minus(DURATION);",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void localDateMinus_zero() {
+ helper
+ .addSourceLines(
+ "TestClass.java",
+ "import java.time.Duration;",
+ "import java.time.LocalDate;",
+ "public class TestClass {",
+ " private static final LocalDate LD = LocalDate.of(1985, 5, 31);",
+ " // BUG: Diagnostic contains: LocalDateTemporalAmount",
+ // This call technically doesn't throw, but we don't currently special case it
+ " private static final LocalDate LD0 = LD.minus(Duration.ZERO);",
+ "}")
+ .doTest();
+ }
+}
diff --git a/core/src/test/java/com/google/errorprone/refaster/UArrayAccessTest.java b/core/src/test/java/com/google/errorprone/refaster/UArrayAccessTest.java
index 3cf2f7656cd..e794ee08a1f 100644
--- a/core/src/test/java/com/google/errorprone/refaster/UArrayAccessTest.java
+++ b/core/src/test/java/com/google/errorprone/refaster/UArrayAccessTest.java
@@ -16,7 +16,7 @@
package com.google.errorprone.refaster;
-import static org.mockito.Matchers.isA;
+import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
diff --git a/core/src/test/java/com/google/errorprone/refaster/UMethodInvocationTest.java b/core/src/test/java/com/google/errorprone/refaster/UMethodInvocationTest.java
index bfa7aa0d206..32bf0adfdae 100644
--- a/core/src/test/java/com/google/errorprone/refaster/UMethodInvocationTest.java
+++ b/core/src/test/java/com/google/errorprone/refaster/UMethodInvocationTest.java
@@ -16,7 +16,7 @@
package com.google.errorprone.refaster;
-import static org.mockito.Matchers.isA;
+import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
diff --git a/core/src/test/java/com/google/errorprone/refaster/UUnaryTest.java b/core/src/test/java/com/google/errorprone/refaster/UUnaryTest.java
index e6ef7cf3c01..e78f17b08f9 100644
--- a/core/src/test/java/com/google/errorprone/refaster/UUnaryTest.java
+++ b/core/src/test/java/com/google/errorprone/refaster/UUnaryTest.java
@@ -17,7 +17,7 @@
package com.google.errorprone.refaster;
import static org.junit.Assert.assertThrows;
-import static org.mockito.Matchers.isA;
+import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
diff --git a/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java
index c67725d1b29..da29ca1d6bc 100644
--- a/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java
+++ b/core/src/test/java/com/google/errorprone/scanner/ScannerSupplierTest.java
@@ -631,8 +631,7 @@ public void disablingPackageLocation_unsuppressible() {
assertThat(exception).hasMessageThat().contains("may not be disabled");
}
- private static class ScannerSupplierSubject
- extends Subject {
+ private static class ScannerSupplierSubject extends Subject {
private final ScannerSupplier actual;
ScannerSupplierSubject(FailureMetadata failureMetadata, ScannerSupplier scannerSupplier) {
diff --git a/core/src/test/java/com/google/errorprone/scanner/ScannerTest.java b/core/src/test/java/com/google/errorprone/scanner/ScannerTest.java
index eb18ce1c769..953669664f8 100644
--- a/core/src/test/java/com/google/errorprone/scanner/ScannerTest.java
+++ b/core/src/test/java/com/google/errorprone/scanner/ScannerTest.java
@@ -27,6 +27,7 @@
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.IdentifierTreeMatcher;
import com.google.errorprone.matchers.Description;
+import com.google.errorprone.util.RuntimeVersion;
import com.sun.source.tree.IdentifierTree;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -35,6 +36,10 @@
/** Tests for {@link Scanner}. */
@RunWith(JUnit4.class)
public class ScannerTest {
+ private static final String IMPORT_GENERATED_ANNOTATION =
+ RuntimeVersion.isAtLeast9()
+ ? "import javax.annotation.processing.Generated;"
+ : "import javax.annotation.Generated;";
private final CompilationTestHelper compilationHelper =
CompilationTestHelper.newInstance(ShouldNotUseFoo.class, getClass());
diff --git a/docs/bugpattern/ChainedAssertionLosesContext.md b/docs/bugpattern/ChainedAssertionLosesContext.md
index e250731186d..dcb2f800ebf 100644
--- a/docs/bugpattern/ChainedAssertionLosesContext.md
+++ b/docs/bugpattern/ChainedAssertionLosesContext.md
@@ -6,7 +6,7 @@ Before:
```
class MyProtoSubject {
public void hasFoo(Foo expected) {
- assertThat(actual().foo()).isEqualTo(expected);
+ assertThat(actual.foo()).isEqualTo(expected);
}
}
```
@@ -16,7 +16,7 @@ After:
```
class MyProtoSubject {
public void hasFoo(Foo expected) {
- check("foo()").that(actual().foo()).isEqualTo(expected);
+ check("foo()").that(actual.foo()).isEqualTo(expected);
}
}
```
diff --git a/docs/bugpattern/EmptySetMultibindingContributions.md b/docs/bugpattern/EmptySetMultibindingContributions.md
index b1000650c2b..8b3cedcbb4b 100644
--- a/docs/bugpattern/EmptySetMultibindingContributions.md
+++ b/docs/bugpattern/EmptySetMultibindingContributions.md
@@ -13,4 +13,4 @@ However, there's a slightly easier way to express this:
@Multibinds abstract Set provideEmptySetOfMyType();
```
-[dmb]: https://google.github.io/dagger/multibindings.html
+[dmb]: https://dagger.dev/multibindings.html
diff --git a/docs/bugpattern/LiteProtoToString.md b/docs/bugpattern/LiteProtoToString.md
new file mode 100644
index 00000000000..f320f6b2a96
--- /dev/null
+++ b/docs/bugpattern/LiteProtoToString.md
@@ -0,0 +1,23 @@
+After bytecode optimization, Lite protos do not generate a meaningful
+`#toString()`. This can be acceptable for debugging, as development builds will
+typically not be optimized, but can pose a problem if the default lite
+`#toString` finds its way into production code as part of an important error
+message.
+
+```java {.bad}
+ public void validate(MyProto myProto) {
+ if (!myProto.hasFrobnicator()) {
+ // MyProto missing frobnicator: asf@6531767b
+ throw new IllegalArgumentException("MyProto missing frobnicator: " + myProto);
+ }
+ }
+```
+
+The fix will be highly dependent on the case in point. There may be an
+identifier associated with the message that would be useful to log, or even some
+serialized version of the entire proto.
+
+NOTE: Logging fields of a proto will force those fields to be retained after
+optimization. This is not an issue if they're already being used, but writing a
+comprehensive `#toString` method for a proto which is otherwise largely unused
+would increase code size.
diff --git a/docs/bugpattern/ProtoFieldNullComparison.md b/docs/bugpattern/ProtoFieldNullComparison.md
index c17bbcd8051..cf3aab467f8 100644
--- a/docs/bugpattern/ProtoFieldNullComparison.md
+++ b/docs/bugpattern/ProtoFieldNullComparison.md
@@ -5,9 +5,9 @@ comparisons like these often indicate a nearby error.
If you need to distinguish between an unset optional value and a default value,
you have two options. In most cases, you can simply use the `hasField()` method.
-proto3 however does not generate `hasField()` methods for scalar fields of type
-`string` or `bytes`. In those cases you will need to wrap your field in
-`google.protobuf.StringValue` or `google.protobuf.BytesValue`, respectively.
+proto3 however does not generate `hasField()` methods for primitive types
+(including `string` and `bytes`). In those cases you will need to wrap your
+field in `google.protobuf.StringValue` or similar.
NOTE: This check applies to normal (server) protos and Lite protos. The
deprecated nano runtime does produce objects which use `null` values to indicate
@@ -40,3 +40,15 @@ void test(MyProto proto) {
}
}
```
+
+If the presence of a field is required information in proto3, the field can be
+wrapped. For example,
+
+```java {.good}
+message MyMessage {
+ google.protobuf.StringValue my_string = 1;
+}
+```
+
+Presence can then be tested using `myMessage.hasMyString()`, and the value
+retrieved using `myMessage.getMyString().getValue()`.
diff --git a/docs/bugpattern/UnsafeFinalization.md b/docs/bugpattern/UnsafeFinalization.md
index 44cb685f726..c2a29b6646b 100644
--- a/docs/bugpattern/UnsafeFinalization.md
+++ b/docs/bugpattern/UnsafeFinalization.md
@@ -27,7 +27,7 @@ public class GameRunner {
}
public void run() {
- GameLibrary.playGame(nativeResourcePointer); // Bug!
+ GameLibrary.playGame(nativeResourcePtr); // Bug!
}
}
diff --git a/pom.xml b/pom.xml
index 129b1006c7e..9aa9c2954fb 100644
--- a/pom.xml
+++ b/pom.xml
@@ -34,13 +34,13 @@
UTF-8
27.0.1-jre
2.8.2
- 0.44
+ 0.45
9+181-r4173-1
1.5.3
4.13-beta-1
- 2.5.3
+ 2.8.1
2.25.0
- 0.16
+ 0.18
diff --git a/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java b/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java
index 1ffd39cbf85..34237817387 100644
--- a/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java
+++ b/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java
@@ -25,6 +25,7 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import com.google.common.io.CharStreams;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.annotations.CheckReturnValue;
import com.google.errorprone.apply.DescriptionBasedDiff;
import com.google.errorprone.apply.ImportOrganizer;
@@ -62,6 +63,7 @@
*
* @author kurs@google.com (Jan Kurs)
*/
+@CheckReturnValue
public class BugCheckerRefactoringTestHelper {
/** Test mode for matching refactored source against expected source. */
@@ -143,44 +145,37 @@ private BugCheckerRefactoringTestHelper(BugChecker refactoringBugChecker, Class<
this.fileManager = new ErrorProneInMemoryFileManager(clazz);
}
- @CheckReturnValue
public static BugCheckerRefactoringTestHelper newInstance(
BugChecker refactoringBugChecker, Class clazz) {
return new BugCheckerRefactoringTestHelper(refactoringBugChecker, clazz);
}
- @CheckReturnValue
public BugCheckerRefactoringTestHelper.ExpectOutput addInput(String inputFilename) {
return new ExpectOutput(fileManager.forResource(inputFilename));
}
- @CheckReturnValue
public BugCheckerRefactoringTestHelper.ExpectOutput addInputLines(String path, String... input) {
String inputPath = getPath("in/", path);
assertThat(fileManager.exists(inputPath)).isFalse();
return new ExpectOutput(fileManager.forSourceLines(inputPath, input));
}
- @CheckReturnValue
public BugCheckerRefactoringTestHelper setFixChooser(FixChooser chooser) {
this.fixChooser = chooser;
return this;
}
- @CheckReturnValue
public BugCheckerRefactoringTestHelper setArgs(String... args) {
this.options = ImmutableList.copyOf(args);
return this;
}
/** If set, fixes that produce output that doesn't compile are allowed. Off by default. */
- @CheckReturnValue
public BugCheckerRefactoringTestHelper allowBreakingChanges() {
allowBreakingChanges = true;
return this;
}
- @CheckReturnValue
public BugCheckerRefactoringTestHelper setImportOrder(String importOrder) {
this.importOrder = importOrder;
return this;
@@ -217,6 +212,7 @@ private void runTestOnPair(JavaFileObject input, JavaFileObject output, TestMode
}
}
+ @CanIgnoreReturnValue
private JCCompilationUnit doCompile(
final JavaFileObject input, Iterable files, Context context)
throws IOException {
@@ -299,11 +295,10 @@ private ErrorProneScannerTransformer transformer(BugChecker bugChecker) {
public class ExpectOutput {
private final JavaFileObject input;
- public ExpectOutput(JavaFileObject input) {
+ private ExpectOutput(JavaFileObject input) {
this.input = input;
}
- @CheckReturnValue
public BugCheckerRefactoringTestHelper addOutputLines(String path, String... output) {
String outputPath = getPath("out/", path);
if (fileManager.exists(outputPath)) {
@@ -312,12 +307,10 @@ public BugCheckerRefactoringTestHelper addOutputLines(String path, String... out
return addInputAndOutput(input, fileManager.forSourceLines(outputPath, output));
}
- @CheckReturnValue
public BugCheckerRefactoringTestHelper addOutput(String outputFilename) {
return addInputAndOutput(input, fileManager.forResource(outputFilename));
}
- @CheckReturnValue
public BugCheckerRefactoringTestHelper expectUnchanged() {
return addInputAndOutput(input, input);
}
diff --git a/test_helpers/src/main/java/com/google/errorprone/CompilationTestHelper.java b/test_helpers/src/main/java/com/google/errorprone/CompilationTestHelper.java
index d742c3d0769..2c3be76fb4e 100644
--- a/test_helpers/src/main/java/com/google/errorprone/CompilationTestHelper.java
+++ b/test_helpers/src/main/java/com/google/errorprone/CompilationTestHelper.java
@@ -57,6 +57,7 @@
import javax.tools.StandardLocation;
/** Helps test Error Prone bug checkers and compilations. */
+@CheckReturnValue
public class CompilationTestHelper {
private static final ImmutableList DEFAULT_ARGS =
ImmutableList.of(
@@ -98,7 +99,6 @@ private CompilationTestHelper(ScannerSupplier scannerSupplier, String checkName,
* @param scannerSupplier the {@link ScannerSupplier} to test
* @param clazz the class to use to locate file resources
*/
- @CheckReturnValue
public static CompilationTestHelper newInstance(ScannerSupplier scannerSupplier, Class clazz) {
return new CompilationTestHelper(scannerSupplier, null, clazz);
}
@@ -109,7 +109,6 @@ public static CompilationTestHelper newInstance(ScannerSupplier scannerSupplier,
* @param checker the {@link BugChecker} to test
* @param clazz the class to use to locate file resources
*/
- @CheckReturnValue
public static CompilationTestHelper newInstance(
Class checker, Class clazz) {
ScannerSupplier scannerSupplier = ScannerSupplier.fromBugCheckerClasses(checker);
@@ -179,7 +178,6 @@ private static Optional getOverrideClasspath(@Nullable List> over
*/
// TODO(eaftan): We could eliminate this path parameter and just infer the path from the
// package and class name
- @CheckReturnValue
public CompilationTestHelper addSourceLines(String path, String... lines) {
this.sources.add(fileManager.forSourceLines(path, lines));
return this;
@@ -192,7 +190,6 @@ public CompilationTestHelper addSourceLines(String path, String... lines) {
*
* @param path the path to the source file
*/
- @CheckReturnValue
public CompilationTestHelper addSourceFile(String path) {
this.sources.add(fileManager.forResource(path));
return this;
@@ -205,7 +202,6 @@ public CompilationTestHelper addSourceFile(String path) {
*
* @param classes the class(es) to use as the classpath
*/
- @CheckReturnValue
public CompilationTestHelper withClasspath(Class... classes) {
this.overrideClasspath = ImmutableList.copyOf(classes);
return this;
@@ -215,7 +211,6 @@ public CompilationTestHelper withClasspath(Class... classes) {
* Sets custom command-line arguments for the compilation. These will be appended to the default
* compilation arguments.
*/
- @CheckReturnValue
public CompilationTestHelper setArgs(List args) {
this.extraArgs = ImmutableList.copyOf(args);
return this;
@@ -226,7 +221,6 @@ public CompilationTestHelper setArgs(List args) {
* source file contains bug markers. Useful for testing that a check is actually disabled when the
* proper command-line argument is passed.
*/
- @CheckReturnValue
public CompilationTestHelper expectNoDiagnostics() {
this.expectNoDiagnostics = true;
return this;
@@ -237,7 +231,6 @@ public CompilationTestHelper expectNoDiagnostics() {
* javac errors. This behaviour can be disabled to test the interaction between Error Prone checks
* and javac diagnostics.
*/
- @CheckReturnValue
public CompilationTestHelper ignoreJavacErrors() {
this.checkWellFormed = false;
return this;
@@ -248,7 +241,6 @@ public CompilationTestHelper ignoreJavacErrors() {
* tested. This behaviour can be disabled to test the interaction between Error Prone checks and
* javac diagnostics.
*/
- @CheckReturnValue
public CompilationTestHelper matchAllDiagnostics() {
this.lookForCheckNameInDiagnostic = LookForCheckNameInDiagnostic.NO;
return this;
@@ -258,7 +250,6 @@ public CompilationTestHelper matchAllDiagnostics() {
* Tells the compilation helper to expect a specific result from the compilation, e.g. success or
* failure.
*/
- @CheckReturnValue
public CompilationTestHelper expectResult(Result result) {
expectedResult = Optional.of(result);
return this;
@@ -278,14 +269,12 @@ public CompilationTestHelper expectResult(Result result) {
*
* Error message keys that don't match any diagnostics will cause test to fail.
*/
- @CheckReturnValue
public CompilationTestHelper expectErrorMessage(String key, Predicate matcher) {
diagnosticHelper.expectErrorMessage(key, matcher);
return this;
}
/** Performs a compilation and checks that the diagnostics and result match the expectations. */
- // TODO(eaftan): any way to ensure that this is actually called?
public void doTest() {
Preconditions.checkState(!sources.isEmpty(), "No source files to compile");
Result result = compile();
@@ -326,17 +315,17 @@ public void doTest() {
.isTrue();
}
- if (expectedResult.isPresent()) {
- assertWithMessage(
- String.format(
- "Expected compilation result %s, but was %s\n%s\n%s",
- expectedResult.get(),
- result,
- Joiner.on('\n').join(diagnosticHelper.getDiagnostics()),
- outputStream))
- .that(result)
- .isEqualTo(expectedResult.get());
- }
+ expectedResult.ifPresent(
+ expected ->
+ assertWithMessage(
+ String.format(
+ "Expected compilation result %s, but was %s\n%s\n%s",
+ expected,
+ result,
+ Joiner.on('\n').join(diagnosticHelper.getDiagnostics()),
+ outputStream))
+ .that(result)
+ .isEqualTo(expected));
}
private Result compile() {