Pull #34816: Add checkstyle rule UnusedLocalVariable, UnusedLocalMethodCheck

Signed-off-by: Vincent Potucek <[email protected]>

Author: Vincent Potucek

buildSrc/config/checkstyle/checkstyle.xml

@@ -16,7 +16,10 @@
 
 		<!-- Imports -->
 		<module name="AvoidStarImport"/>
+		<!-- Unused -->
 		<module name="UnusedImports"/>
+		<module name="UnusedLocalVariable"/>
+		<!-- <module name="UnusedLocalMethodCheck"/> https://github.com/checkstyle/checkstyle/pull/16894 -->
 		<module name="RedundantImport"/>
 		<!-- Modifiers -->
 		<module name="com.puppycrawl.tools.checkstyle.checks.modifier.ModifierOrderCheck"/>

buildSrc/src/test/java/org/springframework/build/multirelease/MultiReleaseJarPluginTests.java

@@ -104,7 +104,7 @@ void packageInJar() throws IOException {
 		writeClass("src/main/java17", "Main.java", """
 				public class Main {}
 				""");
-		BuildResult buildResult = runGradle("assemble");
+		runGradle("assemble");
 		File file = new File(this.projectDir, "/build/libs/" + this.projectDir.getName() + "-1.2.3.jar");
 		assertThat(file).exists();
 		try (JarFile jar = new JarFile(file)) {

framework-docs/src/main/java/org/springframework/docs/core/aot/hints/testing/SampleReflectionRuntimeHintsTests.java

@@ -44,8 +44,7 @@ void shouldRegisterReflectionHints() {
 
 		// Invoke the relevant piece of code we want to test within a recording lambda
 		RuntimeHintsInvocations invocations = org.springframework.aot.test.agent.RuntimeHintsRecorder.record(() -> {
-			SampleReflection sample = new SampleReflection();
-			sample.performReflection();
+			new SampleReflection().performReflection();
 		});
 		// assert that the recorded invocations are covered by the contributed hints
 		assertThat(invocations).match(runtimeHints);

integration-tests/src/test/java/org/springframework/aot/test/ReflectionInvocationsTests.java

@@ -20,7 +20,6 @@
 
 import org.springframework.aot.hint.RuntimeHints;
 import org.springframework.aot.test.agent.EnabledIfRuntimeHintsAgent;
-import org.springframework.aot.test.agent.RuntimeHintsInvocations;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -33,24 +32,17 @@ class ReflectionInvocationsTests {
 	void sampleTest() {
 		RuntimeHints hints = new RuntimeHints();
 		hints.reflection().registerType(String.class);
-
-		RuntimeHintsInvocations invocations = org.springframework.aot.test.agent.RuntimeHintsRecorder.record(() -> {
-			SampleReflection sample = new SampleReflection();
-			sample.sample(); // does Method[] methods = String.class.getMethods();
-		});
-		assertThat(invocations).match(hints);
+		// does Method[] methods = String.class.getMethods();
+		assertThat(org.springframework.aot.test.agent.RuntimeHintsRecorder.record(new SampleReflection()::sample)).match(hints);
 	}
 
 	@Test
 	void multipleCallsTest() {
 		RuntimeHints hints = new RuntimeHints();
 		hints.reflection().registerType(String.class);
 		hints.reflection().registerType(Integer.class);
-		RuntimeHintsInvocations invocations = org.springframework.aot.test.agent.RuntimeHintsRecorder.record(() -> {
-			SampleReflection sample = new SampleReflection();
-			sample.multipleCalls(); // does Method[] methods = String.class.getMethods(); methods = Integer.class.getMethods();
-		});
-		assertThat(invocations).match(hints);
+		// does String.class.getMethods(); Integer.class.getMethods();
+		assertThat(org.springframework.aot.test.agent.RuntimeHintsRecorder.record(new SampleReflection()::multipleCalls)).match(hints);
 	}
 
 }

integration-tests/src/test/java/org/springframework/aot/test/SampleReflection.java

@@ -16,8 +16,6 @@
 
 package org.springframework.aot.test;
 
-import java.lang.reflect.Method;
-
 /**
  * @author Brian Clozel
  * @since 6.0
@@ -26,15 +24,13 @@ public class SampleReflection {
 
 	@SuppressWarnings("unused")
 	public void sample() {
-		String value = "Sample";
-		Method[] methods = String.class.getMethods();
+		String.class.getMethods();
 	}
 
 	@SuppressWarnings("unused")
 	public void multipleCalls() {
-		String value = "Sample";
-		Method[] methods = String.class.getMethods();
-		methods = Integer.class.getMethods();
+		String.class.getMethods();
+		Integer.class.getMethods();
 	}
 
 }

spring-beans/src/test/java/org/springframework/beans/factory/config/ServiceLocatorFactoryBeanTests.java

@@ -122,15 +122,9 @@ void testStringArgGetter() throws Exception {
 				genericBeanDefinition(ServiceLocatorFactoryBean.class)
 				.addPropertyValue("serviceLocatorInterface", TestServiceLocator2.class)
 				.getBeanDefinition());
-
-		// test string-arg getter with null id
-		TestServiceLocator2 factory = (TestServiceLocator2) bf.getBean("factory");
-
-		@SuppressWarnings("unused")
-		TestService testBean = factory.getTestService(null);
-		// now test with explicit id
-		testBean = factory.getTestService("testService");
-		// now verify failure on bad id
+		TestServiceLocator2 factory = (TestServiceLocator2) bf.getBean("factory"); // test string-arg getter with null id
+		factory.getTestService(null); // now test with explicit id
+		factory.getTestService("testService"); // now verify failure on bad id
 		assertThatExceptionOfType(NoSuchBeanDefinitionException.class).isThrownBy(() ->
 				factory.getTestService("bogusTestService"));
 	}

spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java

@@ -7145,8 +7145,7 @@ public TestClass9(int i) {
 	static class HttpServlet3RequestFactory {
 
 		static Servlet3SecurityContextHolderAwareRequestWrapper getOne() {
-			HttpServlet3RequestFactory outer = new HttpServlet3RequestFactory();
-			return outer.new Servlet3SecurityContextHolderAwareRequestWrapper();
+			return new HttpServlet3RequestFactory().new Servlet3SecurityContextHolderAwareRequestWrapper();
 		}
 
 		// private class Servlet3SecurityContextHolderAwareRequestWrapper extends SecurityContextHolderAwareRequestWrapper

spring-test/src/test/java/org/springframework/test/web/client/samples/SampleTests.java

@@ -67,13 +67,15 @@ public void setup() {
 	@Test
 	public void performGet() {
 
-		String responseBody = "{\"name\" : \"Ludwig van Beethoven\", \"someDouble\" : \"1.6035\"}";
+		String responseBody = """
+				{"name" : "Ludwig van Beethoven", "someDouble" : "1.6035"}""";
 
 		this.mockServer.expect(requestTo("/composers/42")).andExpect(method(HttpMethod.GET))
 			.andRespond(withSuccess(responseBody, MediaType.APPLICATION_JSON));
 
-		@SuppressWarnings("unused")
 		Person ludwig = this.restTemplate.getForObject("/composers/{id}", Person.class, 42);
+		assertThat(ludwig.getName()).isEqualTo("Ludwig van Beethoven");
+		assertThat(ludwig.getSomeDouble()).isEqualTo(1.6035);
 
 		// We are only validating the request. The response is mocked out.
 		// hotel.getId() == 42
@@ -90,8 +92,9 @@ public void performGetManyTimes() {
 		this.mockServer.expect(manyTimes(), requestTo("/composers/42")).andExpect(method(HttpMethod.GET))
 				.andRespond(withSuccess(responseBody, MediaType.APPLICATION_JSON));
 
-		@SuppressWarnings("unused")
 		Person ludwig = this.restTemplate.getForObject("/composers/{id}", Person.class, 42);
+		assertThat(ludwig.getName()).isEqualTo("Ludwig van Beethoven");
+		assertThat(ludwig.getSomeDouble()).isEqualTo(1.6035);
 
 		// We are only validating the request. The response is mocked out.
 		// hotel.getId() == 42
@@ -142,11 +145,9 @@ public void performGetWithResponseBodyFromFile() {
 		this.mockServer.expect(requestTo("/composers/42")).andExpect(method(HttpMethod.GET))
 			.andRespond(withSuccess(responseBody, MediaType.APPLICATION_JSON));
 
-		@SuppressWarnings("unused")
 		Person ludwig = this.restTemplate.getForObject("/composers/{id}", Person.class, 42);
-
-		// hotel.getId() == 42
-		// hotel.getName().equals("Holiday Inn")
+		assertThat(ludwig.getName()).isEqualTo("Ludwig van Beethoven");
+		assertThat(ludwig.getSomeDouble()).isEqualTo(1.6035);
 
 		this.mockServer.verify();
 	}
@@ -166,13 +167,8 @@ public void verify() {
 		this.mockServer.expect(requestTo("/number")).andExpect(method(HttpMethod.GET))
 			.andRespond(withSuccess("8", MediaType.TEXT_PLAIN));
 
-		@SuppressWarnings("unused")
-		String result1 = this.restTemplate.getForObject("/number", String.class);
-		// result1 == "1"
-
-		@SuppressWarnings("unused")
-		String result2 = this.restTemplate.getForObject("/number", String.class);
-		// result == "2"
+		assertThat(this.restTemplate.getForObject("/number", String.class)).isEqualTo("1");
+		assertThat(this.restTemplate.getForObject("/number", String.class)).isEqualTo("2");
 
 		try {
 			this.mockServer.verify();

spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java

@@ -291,7 +291,7 @@ void getCorsConfigWithBeanNameHandler() throws Exception {
 
 		this.mapping.setApplicationContext(context);
 		this.mapping.registerMapping(key, beanName, this.method1);
-		HandlerMethod handlerMethod = this.mapping.getHandlerInternal(new MockHttpServletRequest("GET", key));
+		this.mapping.getHandlerInternal(new MockHttpServletRequest("GET", key));
 	}
 
 	@Test

src/checkstyle/checkstyle.xml

@@ -90,7 +90,10 @@
 
 		<!-- Imports -->
 		<module name="AvoidStarImport"/>
+		<!-- Unused -->
 		<module name="UnusedImports"/>
+		<module name="UnusedLocalVariable"/>
+		<!-- <module name="UnusedLocalMethodCheck"/> https://github.com/checkstyle/checkstyle/pull/16894 -->
 		<module name="RedundantImport"/>
 		<module name="com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck">
 			<property name="groups" value="java,javax,*,org.springframework"/>