Enable error prone's UnusedVariable check (#6217)

* Enable error prone's UnusedVariable check

* Spotless

conventions/src/main/kotlin/otel.errorprone-conventions.gradle.kts

@@ -86,10 +86,6 @@ tasks {
         disable("JdkObsolete")
         disable("JavaUtilDate")
 
-        // Storing into a variable in onEnter triggers this unfortunately.
-        // TODO(anuraaga): Only disable for auto instrumentation project.
-        disable("UnusedVariable")
-
         // TODO(anuraaga): Remove this, we use this pattern in several tests and it will mean
         // some moving.
         disable("DefaultPackage")

instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/AttributeBindingFactory.java

@@ -134,11 +134,11 @@ class AttributeBindingFactory {
     }
     if (componentType == Integer.class) {
       AttributeKey<List<Long>> key = AttributeKey.longArrayKey(name);
-      return mappedListBinding(Integer.class, key, Integer::longValue);
+      return mappedListBinding(key, Integer::longValue);
     }
     if (componentType == Float.class) {
       AttributeKey<List<Double>> key = AttributeKey.doubleArrayKey(name);
-      return mappedListBinding(Float.class, key, Float::doubleValue);
+      return mappedListBinding(key, Float::doubleValue);
     }
 
     return defaultListBinding(name);
@@ -308,7 +308,7 @@ class AttributeBindingFactory {
   }
 
   private static <T, U> AttributeBinding mappedListBinding(
-      Class<T> fromClass, AttributeKey<List<U>> key, Function<T, U> mapping) {
+      AttributeKey<List<U>> key, Function<T, U> mapping) {
     return (setter, arg) -> {
       @SuppressWarnings("unchecked")
       List<T> list = (List<T>) arg;
@@ -331,7 +331,7 @@ class AttributeBindingFactory {
 
   private static AttributeBinding defaultListBinding(String name) {
     AttributeKey<List<String>> key = AttributeKey.stringArrayKey(name);
-    return mappedListBinding(Object.class, key, Object::toString);
+    return mappedListBinding(key, Object::toString);
   }
 
   private static AttributeBinding defaultBinding(String name) {

instrumentation/grails-3.0/javaagent/src/test/java/test/GrailsTest.java

@@ -22,7 +22,6 @@ import io.opentelemetry.instrumentation.testing.junit.http.HttpServerTestOptions
 import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
 import io.opentelemetry.sdk.testing.assertj.SpanDataAssert;
 import io.opentelemetry.sdk.trace.data.StatusData;
-import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -67,21 +66,24 @@ public class GrailsTest extends AbstractHttpServerTest<ConfigurableApplicationCo
   static class TestApplication extends GrailsAutoConfiguration {
     static ConfigurableApplicationContext start(int port, String contextPath) {
       GrailsApp grailsApp = new GrailsApp(TestApplication.class);
-      // context path configuration property name changes between spring boot versions
-      String contextPathKey;
-      try {
-        Method method = ServerProperties.class.getDeclaredMethod("getServlet");
-        contextPathKey = "server.servlet.contextPath";
-      } catch (NoSuchMethodException ignore) {
-        contextPathKey = "server.context-path";
-      }
       Map<String, Object> properties = new HashMap<>();
       properties.put("server.port", port);
-      properties.put(contextPathKey, contextPath);
+      properties.put(getContextPathKey(), contextPath);
       grailsApp.setDefaultProperties(properties);
       return grailsApp.run();
     }
 
+    @SuppressWarnings("ReturnValueIgnored")
+    private static String getContextPathKey() {
+      // context path configuration property name changes between spring boot versions
+      try {
+        ServerProperties.class.getDeclaredMethod("getServlet");
+        return "server.servlet.contextPath";
+      } catch (NoSuchMethodException ignore) {
+        return "server.context-path";
+      }
+    }
+
     @SuppressWarnings("rawtypes")
     @Override
     public Collection<Class> classes() {

instrumentation/grpc-1.6/testing/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/AbstractGrpcTest.java

@@ -1192,7 +1192,6 @@ public abstract class AbstractGrpcTest {
     GreeterGrpc.GreeterStub client = GreeterGrpc.newStub(channel);
 
     IllegalStateException thrown = new IllegalStateException("illegal");
-    AtomicReference<Helloworld.Response> response = new AtomicReference<>();
     AtomicReference<Throwable> error = new AtomicReference<>();
     CountDownLatch latch = new CountDownLatch(1);
     testing()

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParser.java

@@ -76,7 +76,6 @@ public enum JdbcConnectionUrlParser {
     @Override
     DbInfo.Builder doParse(String jdbcUrl, DbInfo.Builder builder) {
       String serverName = "";
-      Integer port = null;
 
       int hostIndex = jdbcUrl.indexOf("jtds:sqlserver://");
       if (hostIndex < 0) {

instrumentation/jsp-2.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jsp/JspCompilationContextInstrumentation.java

@@ -34,6 +34,7 @@ public class JspCompilationContextInstrumentation implements TypeInstrumentation
         JspCompilationContextInstrumentation.class.getName() + "$JasperJspCompilationContext");
   }
 
+  @SuppressWarnings("unused")
   public static class JasperJspCompilationContext {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)

instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/BootstrapInstrumentation.java

@@ -37,6 +37,7 @@ public class BootstrapInstrumentation implements TypeInstrumentation {
         BootstrapInstrumentation.class.getName() + "$ConnectAdvice");
   }
 
+  @SuppressWarnings("unused")
   public static class ConnectAdvice {
     @Advice.OnMethodEnter
     public static void startConnect(

instrumentation/reactor/reactor-3.1/library/src/main/java/io/opentelemetry/instrumentation/reactor/ContextPropagationOperator.java

@@ -162,7 +162,7 @@ public final class ContextPropagationOperator {
       implements BiFunction<Scannable, CoreSubscriber<? super T>, CoreSubscriber<? super T>> {
 
     /** Holds reference to strategy to prevent it from being collected. */
-    @SuppressWarnings("FieldCanBeLocal")
+    @SuppressWarnings({"FieldCanBeLocal", "UnusedVariable"})
     private final ReactorAsyncOperationEndStrategy asyncOperationEndStrategy;
 
     public Lifter(ReactorAsyncOperationEndStrategy asyncOperationEndStrategy) {

instrumentation/tapestry-5.4/javaagent/src/test/java/test/tapestry/components/Layout.java

@@ -11,6 +11,7 @@ import org.apache.tapestry5.annotations.Parameter;
 import org.apache.tapestry5.annotations.Property;
 import org.apache.tapestry5.ioc.annotations.Inject;
 
+@SuppressWarnings("unused")
 public class Layout {
   @Inject private ComponentResources resources;
 

javaagent-tooling/src/testExceptionHandler/java/io/opentelemetry/javaagent/tooling/bytebuddy/BadAdvice.java

@@ -7,7 +7,9 @@ package io.opentelemetry.javaagent.tooling.bytebuddy;
 
 import net.bytebuddy.asm.Advice;
 
+@SuppressWarnings("unused")
 public class BadAdvice {
+
   @Advice.OnMethodExit(suppress = Throwable.class)
   public static void throwAnException(@Advice.Return(readOnly = false) boolean returnVal) {
     returnVal = true;

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java

@@ -169,7 +169,7 @@ public class HelperInjector implements Transformer {
       ClassLoader classLoader,
       JavaModule module) {
     if (!helperClassNames.isEmpty()) {
-      injectHelperClasses(typeDescription, classLoader, module);
+      injectHelperClasses(typeDescription, classLoader);
     }
 
     if (classLoader != null && helpersSource != null && !helperResources.isEmpty()) {
@@ -226,8 +226,7 @@ public class HelperInjector implements Transformer {
         });
   }
 
-  private void injectHelperClasses(
+  private void injectHelperClasses(TypeDescription typeDescription, ClassLoader classLoader) {
-      TypeDescription typeDescription, ClassLoader classLoader, JavaModule module) {
     classLoader = maskNullClassLoader(classLoader);
     if (classLoader == BOOTSTRAP_CLASSLOADER_PLACEHOLDER && instrumentation == null) {
       logger.log(

muzzle/src/test/java/io/opentelemetry/javaagent/tooling/muzzle/DeclaredFieldTestClass.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.tooling.muzzle;
 
+@SuppressWarnings("unused")
 public class DeclaredFieldTestClass {
   public static class Advice {
     public void instrument() {

muzzle/src/test/java/muzzle/HelperReferenceWrapperTestClasses.java

@@ -5,7 +5,7 @@
 
 package muzzle;
 
-@SuppressWarnings({"UnusedMethod", "MethodCanBeStatic"})
+@SuppressWarnings("unused")
 public class HelperReferenceWrapperTestClasses {
   interface Interface1 {
     void foo();
@@ -21,6 +21,7 @@ public class HelperReferenceWrapperTestClasses {
 
     static void staticMethodsAreIgnored() {}
 
+    @SuppressWarnings("MethodCanBeStatic")
     private void privateMethodsToo() {}
   }
 }

muzzle/src/test/java/muzzle/TestClasses.java

@@ -10,9 +10,10 @@ import io.opentelemetry.instrumentation.OtherTestHelperClasses;
 import io.opentelemetry.instrumentation.TestHelperClasses.Helper;
 import net.bytebuddy.asm.Advice;
 
-@SuppressWarnings("ClassNamedLikeTypeParameter")
+@SuppressWarnings("unused")
 public class TestClasses {
 
+  @SuppressWarnings("ClassNamedLikeTypeParameter")
   public static class MethodBodyAdvice {
     @SuppressWarnings("ReturnValueIgnored")
     @Advice.OnMethodEnter
@@ -46,7 +47,7 @@ public class TestClasses {
         return s;
       }
 
-      @SuppressWarnings({"UnusedMethod", "MethodCanBeStatic"})
+      @SuppressWarnings("MethodCanBeStatic")
       private void privateStuff() {}
 
       protected void protectedMethod() {}

smoke-tests/src/test/java/io/opentelemetry/smoketest/windows/WindowsTestContainerManager.java

@@ -244,7 +244,7 @@ public class WindowsTestContainerManager extends AbstractTestContainerManager {
     }
   }
 
-  private ContainerLogHandler consumeLogs(String containerId, Waiter waiter, Logger logger) {
+  private void registerLogListener(String containerId, Waiter waiter, Logger logger) {
     ContainerLogFrameConsumer consumer = new ContainerLogFrameConsumer();
     waiter.configureLogger(consumer);
 
@@ -257,7 +257,6 @@ public class WindowsTestContainerManager extends AbstractTestContainerManager {
         .exec(consumer);
 
     consumer.addListener(new Slf4jDockerLogLineListener(logger));
-    return consumer;
   }
 
   private static int extractMappedPort(Container container, int internalPort) {
@@ -295,11 +294,11 @@ public class WindowsTestContainerManager extends AbstractTestContainerManager {
     prepareAction.accept(containerId);
 
     client.startContainerCmd(containerId).exec();
-    ContainerLogHandler logHandler = consumeLogs(containerId, waiter, logger);
+    registerLogListener(containerId, waiter, logger);
 
     InspectContainerResponse inspectResponse =
         inspect ? client.inspectContainerCmd(containerId).exec() : null;
-    Container container = new Container(imageName, containerId, logHandler, inspectResponse);
+    Container container = new Container(imageName, containerId, inspectResponse);
 
     waiter.waitFor(container);
     return container;
@@ -318,17 +317,12 @@ public class WindowsTestContainerManager extends AbstractTestContainerManager {
   private static class Container {
     public final String imageName;
     public final String containerId;
-    public final ContainerLogHandler logConsumer;
     public final InspectContainerResponse inspectResponse;
 
     private Container(
-        String imageName,
+        String imageName, String containerId, InspectContainerResponse inspectResponse) {
-        String containerId,
-        ContainerLogHandler logConsumer,
-        InspectContainerResponse inspectResponse) {
       this.imageName = imageName;
       this.containerId = containerId;
-      this.logConsumer = logConsumer;
       this.inspectResponse = inspectResponse;
     }
   }

testing-common/integration-tests/src/main/java/ResourceLevelInstrumentation.java

@@ -23,6 +23,7 @@ public class ResourceLevelInstrumentation implements TypeInstrumentation {
         named("toString"), this.getClass().getName() + "$ToStringAdvice");
   }
 
+  @SuppressWarnings("unused")
   public static class ToStringAdvice {
     @Advice.OnMethodExit(suppress = Throwable.class)
     static void toStringReplace(@Advice.Return(readOnly = false) String ret) {

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java

@@ -315,7 +315,6 @@ public abstract class AbstractHttpClientTest<REQUEST> {
 
     testing.waitAndAssertTraces(
         trace -> {
-          List<List<SpanData>> traces = testing.traces();
           trace.hasSpansSatisfyingExactly(
               span -> assertClientSpan(span, uri, method, 200).hasNoParent(),
               span -> assertServerSpan(span).hasParent(trace.getSpan(0)));