Skip to content

Commit

Permalink
Turn on Var and Immutable checks, and fix many reported errors
Browse files Browse the repository at this point in the history
  • Loading branch information
jbduncan committed Feb 22, 2018
1 parent 8ec27fe commit 0eb0f65
Show file tree
Hide file tree
Showing 39 changed files with 146 additions and 35 deletions.
16 changes: 4 additions & 12 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,6 @@ subprojects { subproj ->
// adds a lot of churn for not a lot of gain
'-Xep:InconsistentOverloads:OFF',

// TODO: turning this check on and applying @com.google.errorprone.annotations.Immutable
// to immutable classes causes JDK 9 to fail to compile; investigate and consider
// turning it on when error-prone supports JDK 9
//'-Xep:Immutable:OFF',

// adds a lot of churn for not a lot of gain
'-Xep:MethodCanBeStatic:OFF',

Expand All @@ -602,11 +597,6 @@ subprojects { subproj ->

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

// TODO: turning this check on and applying @com.google.errorprone.annotations.Var to
// non-final variables causes JDK 9 to fail to compile; investigate and consider turning
// it on when error-prone supports JDK 9
//'-Xep:Var:OFF'
]

def errorproneTestFlags = errorproneBaseFlags + [
Expand All @@ -620,6 +610,7 @@ subprojects { subproj ->
]

task compileJavaWithErrorProne(type: JavaCompile) {
dependsOn compileJava, compileKotlin
group = 'verification'
toolChain = net.ltgt.gradle.errorprone.ErrorProneToolChain.create(project)
classpath = compileJava.classpath + compileKotlin.classpath
Expand All @@ -630,10 +621,11 @@ subprojects { subproj ->
check.dependsOn(compileJavaWithErrorProne)

task compileTestJavaWithErrorProne(type: JavaCompile) {
dependsOn compileJava, compileKotlin, compileTestJava, compileTestKotlin
group = 'verification'
toolChain = net.ltgt.gradle.errorprone.ErrorProneToolChain.create(project)
classpath = compileTestJava.classpath + compileTestKotlin.classpath
source = compileTestJava.source + compileTestKotlin.source
classpath = compileJava.classpath + compileKotlin.classpath + compileTestJava.classpath + compileTestKotlin.classpath
source = compileJava.source + compileKotlin.source + compileTestJava.source + compileTestKotlin.source
options.compilerArgs += compileTestJava.options.compilerArgs + errorproneTestFlags
destinationDir = file("$buildDir/classes/error-prone-java/test")
}
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 @@ -22,6 +22,8 @@
import java.util.regex.PatternSyntaxException;
import java.util.stream.IntStream;

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 @@ -70,6 +72,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 @@ -147,7 +150,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 @@ -75,6 +77,7 @@ static <T> T assertTimeout(Duration timeout, ThrowingSupplier<T> supplier, Suppl
private static <T> T assertTimeout(Duration timeout, ThrowingSupplier<T> supplier, Object messageOrSupplier) {
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 @@ -14,6 +14,8 @@

import java.util.Locale;

import com.google.errorprone.annotations.Var;

import org.apiguardian.api.API;
import org.junit.platform.commons.logging.Logger;
import org.junit.platform.commons.logging.LoggerFactory;
Expand Down Expand Up @@ -65,6 +67,7 @@ public enum OS {
private static final OS CURRENT_OS = determineCurrentOs();

private static OS determineCurrentOs() {
@Var
String osName = System.getProperty("os.name");

if (StringUtils.isBlank(osName)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

import java.util.Optional;

import com.google.errorprone.annotations.Immutable;

import org.apiguardian.api.API;
import org.junit.jupiter.engine.descriptor.JupiterEngineDescriptor;
import org.junit.jupiter.engine.discovery.DiscoverySelectorResolver;
Expand All @@ -30,6 +32,7 @@
* @since 5.0
*/
@API(status = INTERNAL, since = "5.0")
@Immutable
public final class JupiterTestEngine extends HierarchicalTestEngine<JupiterEngineExecutionContext> {

public static final String ENGINE_ID = "junit-jupiter";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,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 @@ -61,6 +63,7 @@ abstract class AbstractExtensionContext<T extends TestDescriptor> implements Ext
}

private ExtensionValuesStore createStore(ExtensionContext parent) {
@Var
ExtensionValuesStore parentStore = null;
if (parent != null) {
parentStore = ((AbstractExtensionContext<?>) parent).valuesStore;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import java.util.Set;
import java.util.function.Function;

import com.google.errorprone.annotations.Var;

import org.apiguardian.api.API;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.junit.jupiter.api.extension.AfterAllCallback;
Expand Down Expand Up @@ -295,6 +297,7 @@ private AfterEachMethodAdapter synthesizeAfterEachMethodAdapter(Method method) {
}

private 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 @@ -18,6 +18,8 @@
import java.util.function.Supplier;
import java.util.stream.Stream;

import com.google.errorprone.annotations.Var;

import org.apiguardian.api.API;
import org.junit.jupiter.api.DynamicContainer;
import org.junit.jupiter.api.DynamicNode;
Expand Down Expand Up @@ -83,6 +85,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 @@ -14,6 +14,8 @@

import java.util.Optional;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.junit.platform.commons.logging.Logger;
Expand Down Expand Up @@ -55,6 +57,7 @@ static TestInstance.Lifecycle getDefaultTestInstanceLifecycle(ConfigurationParam
String propertyName = DEFAULT_TEST_INSTANCE_LIFECYCLE_PROPERTY_NAME;

Optional<String> optional = configParams.get(propertyName);
@Var
String constantName = null;
if (optional.isPresent()) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import java.util.Optional;
import java.util.Set;

import com.google.errorprone.annotations.Var;

import org.junit.jupiter.engine.JupiterTestEngine;
import org.junit.jupiter.engine.descriptor.ClassTestDescriptor;
import org.junit.jupiter.engine.descriptor.Filterable;
Expand Down Expand Up @@ -107,6 +109,7 @@ private Deque<TestDescriptor> resolveAllSegments(UniqueId uniqueId) {
TestDescriptor parent = resolvedDescriptors.getLast();
UniqueId partialUniqueId = parent.getUniqueId().append(segment);

@Var
Optional<TestDescriptor> resolvedDescriptor = findTestDescriptorByUniqueId(partialUniqueId);
if (!resolvedDescriptor.isPresent()) {
// @formatter:off
Expand Down Expand Up @@ -137,6 +140,7 @@ else if (numSegmentsResolved != numSegmentsToResolve) {
}
else {
logger.warn(() -> {
@Var
List<Segment> unresolved = segments.subList(1, segments.size()); // Remove engine ID
unresolved = unresolved.subList(numSegmentsResolved, unresolved.size()); // Remove resolved segments
return format("Unique ID '%s' could only be partially resolved. "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import com.google.errorprone.annotations.Var;

import org.apiguardian.api.API;
import org.junit.jupiter.api.extension.ConditionEvaluationResult;
import org.junit.jupiter.api.extension.ExecutionCondition;
Expand Down Expand Up @@ -116,7 +118,7 @@ private String getDeactivatePatternString(ConfigurationParameters configurationP
* See {@link Constants#DEACTIVATE_CONDITIONS_PATTERN_PROPERTY_NAME} for
* details on the pattern matching syntax.
*/
private String convertToRegEx(String pattern) {
private 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 @@ -22,6 +22,8 @@
import java.util.List;
import java.util.Optional;

import com.google.errorprone.annotations.Var;

import org.apiguardian.api.API;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ParameterContext;
Expand Down Expand Up @@ -159,6 +161,7 @@ private Object[] resolveParameters(Executable executable, Optional<Object> targe

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 @@ -22,6 +22,8 @@
import java.util.function.Function;
import java.util.function.Supplier;

import com.google.errorprone.annotations.Var;

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

<K, V> Object getOrComputeIfAbsent(Namespace namespace, K key, Function<K, V> defaultCreator) {
CompositeKey compositeKey = new CompositeKey(namespace, key);
@Var
Supplier<Object> storedValue = getStoredValue(compositeKey);
if (storedValue == null) {
storedValue = new MemoizingSupplier(() -> defaultCreator.apply(key));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import java.lang.annotation.Annotation;
import java.util.Objects;

import com.google.errorprone.annotations.Immutable;

import org.apiguardian.api.API;
import org.junit.platform.commons.util.Preconditions;

Expand All @@ -27,6 +29,7 @@
* @see ScriptExecutionManager
*/
@API(status = INTERNAL, since = "5.1")
@Immutable
public final class Script {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import javax.script.ScriptEngineManager;
import javax.script.ScriptException;

import com.google.errorprone.annotations.Var;

import org.apiguardian.api.API;
import org.junit.platform.commons.util.Preconditions;

Expand Down Expand Up @@ -54,6 +56,7 @@ public class ScriptExecutionManager {
*/
public Object evaluate(Script script, Bindings bindings) throws ScriptException {
// Always look for a compiled script in our cache.
@Var
CompiledScript compiledScript = compiledScripts.get(script);

// No compiled script found?
Expand All @@ -73,6 +76,7 @@ public Object evaluate(Script script, Bindings bindings) throws ScriptException
}

ScriptEngine createScriptEngine(String engine) {
@Var
ScriptEngine scriptEngine = scriptEngineManager.getEngineByName(engine);
if (scriptEngine == null) {
scriptEngine = scriptEngineManager.getEngineByExtension(engine);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import java.util.Arrays;
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 @@ -37,6 +39,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 @@ -31,6 +31,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import com.google.errorprone.annotations.Var;

import org.apiguardian.api.API;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.logging.Logger;
Expand Down Expand Up @@ -129,6 +131,7 @@ public static List<Class<?>> findAllClassesInModule(String moduleName, ClassFilt
*/
private static Stream<ResolvedModule> streamResolvedModules(Predicate<String> moduleNamePredicate) {
Module module = ModuleUtils.class.getModule();
@Var
ModuleLayer layer = module.getLayer();
if (layer == null) {
logger.config(() -> ModuleUtils.class + " is a member of " + module
Expand Down Expand Up @@ -203,7 +206,7 @@ List<Class<?>> scan(ModuleReference reference) {
/**
* Convert resource name to binary class name.
*/
private String className(String resourceName) {
private String className(@Var String resourceName) {
resourceName = resourceName.substring(0, resourceName.length() - 6); // 6 = ".class".length()
resourceName = resourceName.replace('/', '.');
return resourceName;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.util.logging.Level;
import java.util.logging.LogRecord;

import com.google.errorprone.annotations.Var;

import org.apiguardian.api.API;
import org.junit.platform.commons.JUnitException;

Expand Down Expand Up @@ -155,8 +157,11 @@ private void log(Level level, Throwable throwable, Supplier<String> messageSuppl

private LogRecord createLogRecord(Level level, Throwable throwable, Supplier<String> messageSupplier) {
StackTraceElement[] stack = new Throwable().getStackTrace();
@Var
String sourceClassName = null;
@Var
String sourceMethodName = null;
@Var
boolean found = false;
for (StackTraceElement element : stack) {
String className = element.getClassName();
Expand Down
Loading

0 comments on commit 0eb0f65

Please sign in to comment.