Skip to content

Commit

Permalink
Turn on @var check and fixup other error-prone warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
jbduncan committed Jul 22, 2017
1 parent 7e596cf commit 9d5c483
Show file tree
Hide file tree
Showing 33 changed files with 103 additions and 67 deletions.
17 changes: 8 additions & 9 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -437,27 +437,26 @@ subprojects { subproj ->
'-Xep:ThrowsUncheckedException:OFF',

// we do not follow Google Java Style Guide
'-Xep:UngroupedOverloads:OFF',

'-Xep:Var:OFF'
'-Xep:UngroupedOverloads:OFF'
]
}
compileTestJava {
options.compilerArgs += [
'-XepAllDisabledChecksAsWarnings',

// many nested test classes are non-static for testing reasons
// these checks are often violated on purpose for testing reasons
'-Xep:ClassCanBeStatic:OFF',

'-Xep:ConstantField:OFF',
'-Xep:ConstructorInvokesOverridable:OFF',
'-Xep:ConstructorLeaksThis:OFF',

// produces many warnings, none critical
// TODO: investigate if this check is worth having on for tests
'-Xep:MethodCanBeStatic:OFF',

'-Xep:PrivateConstructorForUtilityClass:OFF',
'-Xep:StaticOrDefaultInterfaceMethod:OFF',
'-Xep:ThrowsUncheckedException:OFF',
'-Xep:UngroupedOverloads:OFF',

// produces lots of warnings, none critical but potentially useful
// TODO: investigate if its worth turning this check on for tests
'-Xep:Var:OFF'
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;

import com.google.errorprone.annotations.Var;

/**
* {@code AssertIterable} is a collection of utility methods that support asserting
* Iterable equality in tests.
Expand Down Expand Up @@ -60,6 +62,7 @@ private static void assertIterableEquals(Iterable<?> expected, Iterable<?> actua
Iterator<?> expectedIterator = expected.iterator();
Iterator<?> actualIterator = actual.iterator();

@Var
int processed = 0;
while (expectedIterator.hasNext() && actualIterator.hasNext()) {
processed++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import java.util.List;
import java.util.regex.PatternSyntaxException;

import com.google.errorprone.annotations.Var;

/**
* {@code AssertLinesMatch} is a collection of utility methods that support asserting
* lines of {@link String} equality or {@link java.util.regex.Pattern}-match in tests.
Expand Down Expand Up @@ -55,6 +57,7 @@ static void assertLinesMatch(List<String> expectedLines, List<String> actualLine

// simple case: both list are equally sized, compare them line-by-line
if (expectedSize == actualSize) {
@Var
boolean allOk = true;
for (int i = 0; i < expectedSize; i++) {
if (matches(expectedLines.get(i), actualLines.get(i))) {
Expand All @@ -76,6 +79,7 @@ private static void assertLinesMatchWithFastForward(List<String> expectedLines,
Deque<String> actualDeque = new ArrayDeque<>(actualLines);

main: while (!expectedDeque.isEmpty()) {
@Var
String expectedLine = expectedDeque.pop();
String actualLine = actualDeque.peek();

Expand Down Expand Up @@ -153,7 +157,7 @@ private static void fail(List<String> expectedLines, List<String> actualLines, S
assertEquals(expected, actual, format(format, args));
}

static boolean isFastForwardLine(String line) {
static boolean isFastForwardLine(@Var String line) {
line = line.trim();
return line.length() >= 4 && line.startsWith(">>") && line.endsWith(">>");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.concurrent.TimeoutException;
import java.util.function.Supplier;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.api.function.Executable;
import org.junit.jupiter.api.function.ThrowingSupplier;
import org.junit.platform.commons.util.ExceptionUtils;
Expand Down Expand Up @@ -68,6 +70,7 @@ static <T> T assertTimeout(Duration timeout, ThrowingSupplier<T> supplier, Strin
static <T> T assertTimeout(Duration timeout, ThrowingSupplier<T> supplier, Supplier<String> messageSupplier) {
long timeoutInMillis = timeout.toMillis();
long start = System.currentTimeMillis();
@Var
T result = null;
try {
result = supplier.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import java.util.Optional;
import java.util.Set;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.engine.execution.ExtensionValuesStore;
import org.junit.jupiter.engine.execution.NamespaceAwareStore;
Expand Down Expand Up @@ -45,6 +47,7 @@ abstract class AbstractExtensionContext<T extends TestDescriptor> implements Ext
}

private static ExtensionValuesStore createStore(ExtensionContext parent) {
@Var
ExtensionValuesStore parentStore = null;
if (parent != null) {
parentStore = ((AbstractExtensionContext<?>) parent).valuesStore;
Expand Down Expand Up @@ -74,6 +77,7 @@ public Optional<ExtensionContext> getParent() {

@Override
public ExtensionContext getRoot() {
@Var
ExtensionContext root = this;
while (parent != null && parent != root) {
root = parent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import java.util.Set;
import java.util.function.Function;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.junit.jupiter.api.extension.AfterAllCallback;
Expand Down Expand Up @@ -286,6 +288,7 @@ private static AfterEachMethodAdapter synthesizeAfterEachMethodAdapter(Method me
private static void invokeMethodInExtensionContext(Method method, ExtensionContext context,
ExtensionRegistry registry) {

@Var
Object testInstance = context.getRequiredTestInstance();
testInstance = ReflectionUtils.getOutermostInstance(testInstance, method.getDeclaringClass()).orElseThrow(
() -> new JUnitException("Failed to find instance for method: " + method.toGenericString()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import java.util.Iterator;
import java.util.stream.Stream;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.api.DynamicContainer;
import org.junit.jupiter.api.DynamicNode;
import org.junit.jupiter.api.DynamicTest;
Expand Down Expand Up @@ -72,6 +74,7 @@ protected void invokeTestMethod(JupiterEngineExecutionContext context, DynamicTe
TestSource source = getSource().orElseThrow(
() -> new JUnitException("Illegal state: TestSource must be present"));
try (Stream<DynamicNode> dynamicNodeStream = toDynamicNodeStream(testFactoryMethodResult)) {
@Var
int index = 1;
Iterator<DynamicNode> iterator = dynamicNodeStream.iterator();
while (iterator.hasNext()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.api.extension.ConditionEvaluationResult;
import org.junit.jupiter.api.extension.ExecutionCondition;
import org.junit.jupiter.api.extension.ExtensionContext;
Expand Down Expand Up @@ -115,7 +117,7 @@ private static String getDeactivatePatternString(ConfigurationParameters configu
* See {@link Constants#DEACTIVATE_CONDITIONS_PATTERN_PROPERTY_NAME} for
* details on the pattern matching syntax.
*/
private static String convertToRegEx(String pattern) {
private static String convertToRegEx(@Var String pattern) {
pattern = Matcher.quoteReplacement(pattern);

// Match "." against "." and "$" since users may declare a "." instead of a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.Optional;
import java.util.logging.Logger;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ParameterContext;
import org.junit.jupiter.api.extension.ParameterResolutionException;
Expand Down Expand Up @@ -157,6 +159,7 @@ private static Object[] resolveParameters(Executable executable, Optional<Object

Parameter[] parameters = executable.getParameters();
Object[] values = new Object[parameters.length];
@Var
int start = 0;

// Ensure that the outer instance is resolved as the first parameter if
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.util.Objects;
import java.util.function.Function;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.junit.jupiter.api.extension.ExtensionContextException;
Expand Down Expand Up @@ -68,6 +70,7 @@ <T> T get(Namespace namespace, Object key, Class<T> requiredType) {

<K, V> Object getOrComputeIfAbsent(Namespace namespace, K key, Function<K, V> defaultCreator) {
synchronized (this.monitor) {
@Var
StoredValue storedValue = getStoredValue(namespace, key);
if (storedValue == null) {
if (this.parentStore != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@

class AssertionTestUtils {

///CLOVER:OFF
private AssertionTestUtils() {
/* no-op */
}
///CLOVER:ON

static void expectAssertionFailedError() {
throw new AssertionError("Should have thrown an " + AssertionFailedError.class.getName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@

final class IterableFactory {

///CLOVER:OFF
private IterableFactory() {
/* no-op */
}
///CLOVER:ON

static List<Object> listOf(Object... objects) {
return Arrays.asList(objects);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@

class FailAfterAllHelper {

///CLOVER:OFF
private FailAfterAllHelper() {
/* no-op */
}
///CLOVER:ON

static void fail() {
// hack: use this blacklisted exception to fail the build, all others would be swallowed...
throw new OutOfMemoryError("a postcondition was violated");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import java.text.MessageFormat;
import java.util.stream.IntStream;

import com.google.errorprone.annotations.Var;

import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.util.StringUtils;

Expand All @@ -36,6 +38,7 @@ String format(int invocationIndex, Object... arguments) {
}

private String prepareMessageFormatPattern(int invocationIndex, Object[] arguments) {
@Var
String result = namePattern.replace("{index}", String.valueOf(invocationIndex));
if (result.contains("{arguments}")) {
// @formatter:off
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import com.google.errorprone.annotations.Var;

import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.meta.API;
import org.junit.platform.commons.util.ReflectionUtils.HierarchyTraversalMode;
Expand Down Expand Up @@ -83,6 +85,7 @@ public static <T> Optional<T> getDefaultValue(Annotation annotation, String attr
Preconditions.notNull(attributeType, "attributeType must not be null");

Class<? extends Annotation> annotationType = annotation.annotationType();
@Var
Method attribute = null;
try {
attribute = annotationType.getDeclaredMethod(attributeName);
Expand Down Expand Up @@ -165,6 +168,7 @@ private static <A extends Annotation> Optional<A> findAnnotation(AnnotatedElemen

// Cached?
AnnotationCacheKey key = new AnnotationCacheKey(element, annotationType);
@Var
A annotation = (A) annotationCache.get(key);
if (annotation != null) {
return Optional.of(annotation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import java.security.CodeSource;
import java.util.Optional;

import com.google.errorprone.annotations.Var;

import org.junit.platform.commons.meta.API;

/**
Expand Down Expand Up @@ -61,6 +63,7 @@ public static ClassLoader getDefaultClassLoader() {
public static Optional<URL> getLocation(Object object) {
Preconditions.notNull(object, "object must not be null");
// determine class loader
@Var
ClassLoader loader = object.getClass().getClassLoader();
if (loader == null) {
loader = ClassLoader.getSystemClassLoader();
Expand All @@ -70,8 +73,7 @@ public static Optional<URL> getLocation(Object object) {
}
// try finding resource by name
if (loader != null) {
String name = object.getClass().getName();
name = name.replace(".", "/") + ".class";
String name = object.getClass().getName().replace(".", "/") + ".class";
try {
return Optional.ofNullable(loader.getResource(name));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
import java.util.logging.Logger;
import java.util.stream.Stream;

import com.google.errorprone.annotations.Var;

import org.junit.platform.commons.meta.API;

/**
Expand Down Expand Up @@ -70,7 +72,7 @@ class ClasspathScanner {
this.loadClass = loadClass;
}

List<Class<?>> scanForClassesInPackage(String basePackageName, Predicate<Class<?>> classFilter,
List<Class<?>> scanForClassesInPackage(@Var String basePackageName, Predicate<Class<?>> classFilter,
Predicate<String> classNameFilter) {

PackageUtils.assertPackageNameIsValid(basePackageName);
Expand Down Expand Up @@ -137,6 +139,7 @@ private List<Class<?>> findClassesForPath(Path baseDir, String basePackageName,

private void processClassFileSafely(Path baseDir, String basePackageName, Predicate<Class<?>> classFilter,
Predicate<String> classNameFilter, Path classFile, Consumer<Class<?>> classConsumer) {
@Var
Optional<Class<?>> clazz = Optional.empty();
try {
String fullyQualifiedClassName = determineFullyQualifiedClassName(baseDir, basePackageName, classFile);
Expand Down Expand Up @@ -173,6 +176,7 @@ private static String determineSimpleClassName(Path classFile) {
private static String determineSubpackageName(Path baseDir, Path classFile) {
Path relativePath = baseDir.relativize(classFile.getParent());
String pathSeparator = baseDir.getFileSystem().getSeparator();
@Var
String subpackageName = relativePath.toString().replace(pathSeparator, PACKAGE_SEPARATOR_STRING);
if (subpackageName.endsWith(pathSeparator)) {
// Workaround for JDK bug: https://bugs.openjdk.java.net/browse/JDK-8153248
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import com.google.errorprone.annotations.Var;

import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.meta.API;

Expand Down Expand Up @@ -383,7 +385,7 @@ public static Optional<Class<?>> loadClass(String name) {
* @param classLoader the {@code ClassLoader} to use; never {@code null}
* @see #loadClass(String)
*/
public static Optional<Class<?>> loadClass(String name, ClassLoader classLoader) {
public static Optional<Class<?>> loadClass(@Var String name, ClassLoader classLoader) {
Preconditions.notBlank(name, "Class name must not be null or blank");
Preconditions.notNull(classLoader, "ClassLoader must not be null");
name = name.trim();
Expand All @@ -393,6 +395,7 @@ public static Optional<Class<?>> loadClass(String name, ClassLoader classLoader)
}

try {
@Var
Matcher matcher;

// Primitive arrays such as "[I", "[[[[D", etc.
Expand Down Expand Up @@ -795,6 +798,7 @@ private static List<Method> findAllMethodsInHierarchy(Class<?> clazz, HierarchyT
private static int defaultMethodSorter(Method method1, Method method2) {
String name1 = method1.getName();
String name2 = method2.getName();
@Var
int comparison = Integer.compare(name1.hashCode(), name2.hashCode());
if (comparison == 0) {
comparison = name1.compareTo(name2);
Expand Down
Loading

0 comments on commit 9d5c483

Please sign in to comment.