From f7e6d4e21ef042a1cdfe7af3c3298e6a963356ed Mon Sep 17 00:00:00 2001 From: Vincent Potucek Date: Fri, 25 Apr 2025 12:04:33 +0200 Subject: [PATCH] Pull #34816: Add checkstyle rule UnusedLocalVariable, UnusedLocalMethodCheck Signed-off-by: Vincent Potucek --- buildSrc/config/checkstyle/checkstyle.xml | 3 +++ .../MultiReleaseJarPluginTests.java | 2 +- .../SampleReflectionRuntimeHintsTests.java | 3 +-- .../aot/test/ReflectionInvocationsTests.java | 16 ++++--------- .../aot/test/SampleReflection.java | 10 +++----- .../ServiceLocatorFactoryBeanTests.java | 12 +++------- .../spel/SpelCompilationCoverageTests.java | 3 +-- .../test/web/client/samples/SampleTests.java | 24 ++++++++----------- .../handler/HandlerMethodMappingTests.java | 2 +- src/checkstyle/checkstyle.xml | 3 +++ 10 files changed, 30 insertions(+), 48 deletions(-) diff --git a/buildSrc/config/checkstyle/checkstyle.xml b/buildSrc/config/checkstyle/checkstyle.xml index b75fed0d6583..5d0e537bfe92 100644 --- a/buildSrc/config/checkstyle/checkstyle.xml +++ b/buildSrc/config/checkstyle/checkstyle.xml @@ -16,7 +16,10 @@ + + + diff --git a/buildSrc/src/test/java/org/springframework/build/multirelease/MultiReleaseJarPluginTests.java b/buildSrc/src/test/java/org/springframework/build/multirelease/MultiReleaseJarPluginTests.java index b4d2f618803f..a2fd3616d236 100644 --- a/buildSrc/src/test/java/org/springframework/build/multirelease/MultiReleaseJarPluginTests.java +++ b/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)) { diff --git a/framework-docs/src/main/java/org/springframework/docs/core/aot/hints/testing/SampleReflectionRuntimeHintsTests.java b/framework-docs/src/main/java/org/springframework/docs/core/aot/hints/testing/SampleReflectionRuntimeHintsTests.java index 44bb21e30cd9..7350bf637c07 100644 --- a/framework-docs/src/main/java/org/springframework/docs/core/aot/hints/testing/SampleReflectionRuntimeHintsTests.java +++ b/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); diff --git a/integration-tests/src/test/java/org/springframework/aot/test/ReflectionInvocationsTests.java b/integration-tests/src/test/java/org/springframework/aot/test/ReflectionInvocationsTests.java index 1a1123c22670..a93aefe1a2bf 100644 --- a/integration-tests/src/test/java/org/springframework/aot/test/ReflectionInvocationsTests.java +++ b/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,12 +32,8 @@ 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 @@ -46,11 +41,8 @@ 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); } } diff --git a/integration-tests/src/test/java/org/springframework/aot/test/SampleReflection.java b/integration-tests/src/test/java/org/springframework/aot/test/SampleReflection.java index 3f8cbadffc61..9ddca1cbf921 100644 --- a/integration-tests/src/test/java/org/springframework/aot/test/SampleReflection.java +++ b/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(); } } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/config/ServiceLocatorFactoryBeanTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/config/ServiceLocatorFactoryBeanTests.java index 45956f2c1f0b..3a926d3f5e17 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/config/ServiceLocatorFactoryBeanTests.java +++ b/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")); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 0bae62c0dc84..c0bab0b5f512 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/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 diff --git a/spring-test/src/test/java/org/springframework/test/web/client/samples/SampleTests.java b/spring-test/src/test/java/org/springframework/test/web/client/samples/SampleTests.java index 82c1d5a0dc42..59198b059acd 100644 --- a/spring-test/src/test/java/org/springframework/test/web/client/samples/SampleTests.java +++ b/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(); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java index 865c209e3605..4ef8ef824b58 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java +++ b/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 diff --git a/src/checkstyle/checkstyle.xml b/src/checkstyle/checkstyle.xml index 940446e9f85b..957a46766217 100644 --- a/src/checkstyle/checkstyle.xml +++ b/src/checkstyle/checkstyle.xml @@ -90,7 +90,10 @@ + + +