Introduce ErrorProne, fix compiler warnings

.mvn/jvm.config

@@ -0,0 +1,10 @@
+--add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
+--add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
+--add-exports jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED
+--add-exports jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED
+--add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
+--add-exports jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED
+--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
+--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+--add-opens jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED
+--add-opens jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED

pom.xml

@@ -180,7 +180,34 @@
 					<release>${java.version}</release>
 					<compilerArgs>
 						<compilerArg>-parameters</compilerArg>
+						<!-- https://errorprone.info/docs/installation#maven -->
+						<compilerArg>-XDcompilePolicy=simple</compilerArg>
+						<compilerArg>--should-stop=ifError=FLOW</compilerArg>
+						<compilerArg>
+							-Xplugin:ErrorProne
+							-Xep:DefaultCharset:OFF
+							-Xep:EqualsGetClass:OFF
+							-Xep:Finally:OFF
+							-Xep:HidingField:OFF
+							-Xep:JavaTimeDefaultTimeZone:OFF
+							-Xep:JdkObsolete:OFF
+							-Xep:MixedMutabilityReturnType:OFF
+							-Xep:MutablePublicArray:OFF
+							-Xep:NonAtomicVolatileUpdate:OFF
+							-Xep:ReferenceEquality:OFF
+							-Xep:StringCaseLocaleUsage:OFF
+							-Xep:StringSplitter:OFF
+							-Xep:SynchronizeOnNonFinalField:OFF
+						</compilerArg>
 					</compilerArgs>
+					<failOnWarning>true</failOnWarning>
+					<annotationProcessorPaths>
+						<path>
+							<groupId>com.google.errorprone</groupId>
+							<artifactId>error_prone_core</artifactId>
+							<version>2.37.0</version>
+						</path>
+					</annotationProcessorPaths>
 				</configuration>
 			</plugin>
 			<plugin>

spring-batch-core/src/main/java/org/springframework/batch/core/Entity.java

@@ -146,7 +146,7 @@ public boolean equals(Object other) {
 	@Override
 	public int hashCode() {
 		if (id == null) {
-			return super.hashCode();
+			return System.identityHashCode(this);
 		}
 		return 39 + 87 * id.hashCode();
 	}

spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java

@@ -18,6 +18,7 @@
 
 import java.io.IOException;
 import java.io.ObjectInputStream;
+import java.time.Clock;
 import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -203,7 +204,7 @@ public void upgradeStatus(BatchStatus status) {
 	/**
 	 * Convenience getter for the {@code id} of the enclosing job. Useful for DAO
 	 * implementations.
-	 * @return the @{code id} of the enclosing job.
+	 * @return the {@code id} of the enclosing job.
 	 */
 	public Long getJobId() {
 		if (jobInstance != null) {

spring-batch-core/src/main/java/org/springframework/batch/core/JobParameters.java

@@ -373,7 +373,7 @@ public String toString() {
 		for (Map.Entry<String, JobParameter<?>> entry : this.parameters.entrySet()) {
 			parameters.add(String.format("'%s':'%s'", entry.getKey(), entry.getValue()));
 		}
-		return new StringBuilder("{").append(String.join(",", parameters)).append("}").toString();
+		return "{" + String.join(",", parameters) + "}";
 	}
 
 }

spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java

@@ -492,7 +492,7 @@ public boolean equals(Object obj) {
 			return super.equals(obj);
 		}
 
-		return stepName.equals(other.getStepName()) && (jobExecutionId.equals(other.getJobExecutionId()))
+		return stepName.equals(other.getStepName()) && jobExecutionId.equals(other.getJobExecutionId())
 				&& getId().equals(other.getId());
 	}
 

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/JobFactory.java

@@ -32,7 +32,7 @@ public interface JobFactory {
 	Job createJob();
 
 	/**
-	 * @return The {@link String} that contains the {@link Job} name.
+	 * Return the {@link String} that contains the {@link Job} name.
 	 */
 	String getJobName();
 

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/annotation/BatchObservabilityBeanPostProcessor.java

@@ -50,11 +50,11 @@ public Object postProcessAfterInitialization(Object bean, String beanName) throw
 		try {
 			if (bean instanceof AbstractJob || bean instanceof AbstractStep) {
 				ObservationRegistry observationRegistry = this.beanFactory.getBean(ObservationRegistry.class);
-				if (bean instanceof AbstractJob) {
-					((AbstractJob) bean).setObservationRegistry(observationRegistry);
+				if (bean instanceof AbstractJob job) {
+					job.setObservationRegistry(observationRegistry);
 				}
-				if (bean instanceof AbstractStep) {
-					((AbstractStep) bean).setObservationRegistry(observationRegistry);
+				if (bean instanceof AbstractStep step) {
+					step.setObservationRegistry(observationRegistry);
 				}
 			}
 		}

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/annotation/BatchRegistrar.java

@@ -49,8 +49,6 @@ class BatchRegistrar implements ImportBeanDefinitionRegistrar {
 
 	private static final Log LOGGER = LogFactory.getLog(BatchRegistrar.class);
 
-	private static final String MISSING_ANNOTATION_ERROR_MESSAGE = "EnableBatchProcessing is not present on importing class '%s' as expected";
-
 	private static final String JOB_REPOSITORY = "jobRepository";
 
 	private static final String JOB_EXPLORER = "jobExplorer";
@@ -84,7 +82,8 @@ public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, B
 	private void validateState(AnnotationMetadata importingClassMetadata) {
 		if (!importingClassMetadata.isAnnotated(EnableBatchProcessing.class.getName())) {
 			String className = importingClassMetadata.getClassName();
-			String errorMessage = String.format(MISSING_ANNOTATION_ERROR_MESSAGE, className);
+			String errorMessage = "EnableBatchProcessing is not present on importing class '%s' as expected"
+				.formatted(className);
 			throw new IllegalStateException(errorMessage);
 		}
 	}

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/AbstractApplicationContextFactory.java

@@ -19,6 +19,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 import org.apache.commons.logging.Log;
@@ -196,13 +197,11 @@ protected void prepareContext(ConfigurableApplicationContext parent, Configurabl
 	protected void prepareBeanFactory(ConfigurableListableBeanFactory parent,
 			ConfigurableListableBeanFactory beanFactory) {
 		if (copyConfiguration && parent != null) {
-			List<BeanPostProcessor> parentPostProcessors = new ArrayList<>();
-			List<BeanPostProcessor> childPostProcessors = new ArrayList<>();
-
-			childPostProcessors.addAll(beanFactory instanceof AbstractBeanFactory
-					? ((AbstractBeanFactory) beanFactory).getBeanPostProcessors() : new ArrayList<>());
-			parentPostProcessors.addAll(parent instanceof AbstractBeanFactory
-					? ((AbstractBeanFactory) parent).getBeanPostProcessors() : new ArrayList<>());
+			List<BeanPostProcessor> childPostProcessors = new ArrayList<>(
+					beanFactory instanceof AbstractBeanFactory factory ? factory.getBeanPostProcessors()
+							: new ArrayList<>());
+			List<BeanPostProcessor> parentPostProcessors = new ArrayList<>(parent instanceof AbstractBeanFactory factory
+					? factory.getBeanPostProcessors() : new ArrayList<>());
 
 			try {
 				Class<?> applicationContextAwareProcessorClass = ClassUtils.forName(
@@ -237,8 +236,8 @@ protected void prepareBeanFactory(ConfigurableListableBeanFactory parent,
 
 			beanFactory.copyConfigurationFrom(parent);
 
-			List<BeanPostProcessor> beanPostProcessors = beanFactory instanceof AbstractBeanFactory
-					? ((AbstractBeanFactory) beanFactory).getBeanPostProcessors() : new ArrayList<>();
+			List<BeanPostProcessor> beanPostProcessors = beanFactory instanceof AbstractBeanFactory abstractBeanFactory
+					? abstractBeanFactory.getBeanPostProcessors() : new ArrayList<>();
 
 			beanPostProcessors.clear();
 			beanPostProcessors.addAll(aggregatedPostProcessors);

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/AutomaticJobRegistrar.java

@@ -79,8 +79,8 @@ public void setApplicationContext(ApplicationContext applicationContext) {
 	 * use
 	 */
 	public void addApplicationContextFactory(ApplicationContextFactory applicationContextFactory) {
-		if (applicationContextFactory instanceof ApplicationContextAware) {
-			((ApplicationContextAware) applicationContextFactory).setApplicationContext(applicationContext);
+		if (applicationContextFactory instanceof ApplicationContextAware applicationContextAware) {
+			applicationContextAware.setApplicationContext(applicationContext);
 		}
 		this.applicationContextFactories.add(applicationContextFactory);
 	}

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/DefaultJobLoader.java

@@ -255,11 +255,11 @@ private void doRegister(ConfigurableApplicationContext context, Job job) throws
 		jobRegistry.register(jobFactory);
 
 		if (stepRegistry != null) {
-			if (!(job instanceof StepLocator)) {
+			if (!(job instanceof StepLocator stepLocator)) {
 				throw new UnsupportedOperationException("Cannot locate steps from a Job that is not a StepLocator: job="
 						+ job.getName() + " does not implement StepLocator");
 			}
-			stepRegistry.register(job.getName(), getSteps((StepLocator) job, context));
+			stepRegistry.register(job.getName(), getSteps(stepLocator, context));
 		}
 	}
 

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/GenericApplicationContextFactory.java

@@ -126,7 +126,7 @@ protected void prepareBeanFactory(ConfigurableListableBeanFactory beanFactory) {
 				GenericApplicationContextFactory.this.prepareBeanFactory(parentBeanFactory, beanFactory);
 				for (Class<? extends BeanFactoryPostProcessor> cls : getBeanFactoryPostProcessorClasses()) {
 					for (String name : parent.getBeanNamesForType(cls)) {
-						beanFactory.registerSingleton(name, (parent.getBean(name)));
+						beanFactory.registerSingleton(name, parent.getBean(name));
 					}
 				}
 			}

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/GroupAwareJob.java

@@ -99,8 +99,8 @@ public JobParametersValidator getJobParametersValidator() {
 
 	@Override
 	public boolean equals(Object obj) {
-		if (obj instanceof GroupAwareJob) {
-			return ((GroupAwareJob) obj).delegate.equals(delegate);
+		if (obj instanceof GroupAwareJob groupAwareJob) {
+			return groupAwareJob.delegate.equals(delegate);
 		}
 		return false;
 	}

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/support/JobRegistryBeanPostProcessor.java

@@ -87,8 +87,8 @@ public void setJobRegistry(JobRegistry jobRegistry) {
 
 	@Override
 	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
-		if (beanFactory instanceof DefaultListableBeanFactory) {
-			this.beanFactory = (DefaultListableBeanFactory) beanFactory;
+		if (beanFactory instanceof DefaultListableBeanFactory defaultListableBeanFactory) {
+			this.beanFactory = defaultListableBeanFactory;
 		}
 	}
 

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractFlowParser.java

@@ -404,7 +404,7 @@ protected static Collection<BeanDefinition> createTransition(FlowExecutionStatus
 			endBuilder.addConstructorArgValue(exitCodeExists ? exitCode : status.getName());
 
 			String endName = (status == FlowExecutionStatus.STOPPED ? STOP_ELE
-					: status == FlowExecutionStatus.FAILED ? FAIL_ELE : END_ELE) + (endCounter++);
+					: status == FlowExecutionStatus.FAILED ? FAIL_ELE : END_ELE) + endCounter++;
 			endBuilder.addConstructorArgValue(endName);
 
 			endBuilder.addConstructorArgValue(abandon);

spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java

@@ -119,15 +119,15 @@ protected AbstractBeanDefinition parseStep(Element stepElement, ParserContext pa
 				}
 				else if (FLOW_ELE.equals(name)) {
 					boolean stepUnderspecified = CoreNamespaceUtils.isUnderspecified(stepElement);
-					parseFlow(stepElement, nestedElement, bd, parserContext, stepUnderspecified);
+					parseFlow(stepElement, nestedElement, bd);
 				}
 				else if (PARTITION_ELE.equals(name)) {
 					boolean stepUnderspecified = CoreNamespaceUtils.isUnderspecified(stepElement);
-					parsePartition(stepElement, nestedElement, bd, parserContext, stepUnderspecified, jobFactoryRef);
+					parsePartition(stepElement, nestedElement, bd, parserContext, jobFactoryRef);
 				}
 				else if (JOB_ELE.equals(name)) {
 					boolean stepUnderspecified = CoreNamespaceUtils.isUnderspecified(stepElement);
-					parseJob(stepElement, nestedElement, bd, parserContext, stepUnderspecified);
+					parseJob(nestedElement, bd, parserContext);
 				}
 				else if ("description".equals(name)) {
 					bd.setDescription(nestedElement.getTextContent());
@@ -199,7 +199,7 @@ else if (ns.equals("http://www.springframework.org/schema/batch")) {
 	}
 
 	private void parsePartition(Element stepElement, Element partitionElement, AbstractBeanDefinition bd,
-			ParserContext parserContext, boolean stepUnderspecified, String jobFactoryRef) {
+			ParserContext parserContext, String jobFactoryRef) {
 
 		bd.setBeanClass(StepParserStepFactoryBean.class);
 		bd.setAttribute("isNamespaceStep", true);
@@ -258,8 +258,7 @@ else if (inlineStepElement != null) {
 
 	}
 
-	private void parseJob(Element stepElement, Element jobElement, AbstractBeanDefinition bd,
-			ParserContext parserContext, boolean stepUnderspecified) {
+	private void parseJob(Element jobElement, AbstractBeanDefinition bd, ParserContext parserContext) {
 
 		bd.setBeanClass(StepParserStepFactoryBean.class);
 		bd.setAttribute("isNamespaceStep", true);
@@ -285,8 +284,7 @@ private void parseJob(Element stepElement, Element jobElement, AbstractBeanDefin
 
 	}
 
-	private void parseFlow(Element stepElement, Element flowElement, AbstractBeanDefinition bd,
-			ParserContext parserContext, boolean stepUnderspecified) {
+	private void parseFlow(Element stepElement, Element flowElement, AbstractBeanDefinition bd) {
 
 		bd.setBeanClass(StepParserStepFactoryBean.class);
 		bd.setAttribute("isNamespaceStep", true);

spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerFactoryBean.java

@@ -150,8 +150,8 @@ public Object getObject() {
 		// create a proxy listener for only the interfaces that have methods to
 		// be called
 		ProxyFactory proxyFactory = new ProxyFactory();
-		if (delegate instanceof Advised) {
-			proxyFactory.setTargetSource(((Advised) delegate).getTargetSource());
+		if (delegate instanceof Advised advised) {
+			proxyFactory.setTargetSource(advised.getTargetSource());
 		}
 		else {
 			proxyFactory.setTarget(delegate);

spring-batch-core/src/main/java/org/springframework/batch/core/scope/BatchScopeSupport.java

@@ -185,8 +185,8 @@ protected Object resolveValue(Object value) {
 
 			BeanDefinition definition = null;
 			String beanName = null;
-			if (value instanceof BeanDefinition) {
-				definition = (BeanDefinition) value;
+			if (value instanceof BeanDefinition beanDefinition) {
+				definition = beanDefinition;
 				beanName = BeanDefinitionReaderUtils.generateBeanName(definition, registry);
 			}
 			else if (value instanceof BeanDefinitionHolder holder) {

spring-batch-core/src/main/java/org/springframework/batch/core/scope/context/JobContext.java

@@ -161,8 +161,8 @@ public void close() {
 		}
 
 		Exception error = errors.get(0);
-		if (error instanceof RuntimeException) {
-			throw (RuntimeException) error;
+		if (error instanceof RuntimeException runtimeException) {
+			throw runtimeException;
 		}
 		else {
 			throw new UnexpectedJobExecutionException(

spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeItemReadListenerTests.java

@@ -17,7 +17,7 @@
 
 import static org.mockito.Mockito.mock;
 
-import java.util.ArrayList;
+import java.util.List;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -67,11 +67,7 @@ void testOnReadError() {
 
 	@Test
 	void testSetListeners() {
-		compositeListener.setListeners(new ArrayList<>() {
-			{
-				add(listener);
-			}
-		});
+		compositeListener.setListeners(List.of(listener));
 		listener.beforeRead();
 		compositeListener.beforeRead();
 	}

spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeItemWriteListenerTests.java

@@ -15,7 +15,7 @@
  */
 package org.springframework.batch.core.listener;
 
-import java.util.ArrayList;
+import java.util.List;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -69,11 +69,7 @@ void testOnWriteError() {
 
 	@Test
 	void testSetListeners() {
-		compositeListener.setListeners(new ArrayList<>() {
-			{
-				add(listener);
-			}
-		});
+		compositeListener.setListeners(List.of(listener));
 		Chunk<Object> item = Chunk.of(new Object());
 		listener.beforeWrite(item);
 		compositeListener.beforeWrite(item);

spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/ConfigurableSystemProcessExitCodeMapperTests.java

@@ -36,16 +36,13 @@ class ConfigurableSystemProcessExitCodeMapperTests {
 	 */
 	@Test
 	void testMapping() {
-		Map<Object, ExitStatus> mappings = new HashMap<>() {
-			{
-				put(0, ExitStatus.COMPLETED);
-				put(1, ExitStatus.FAILED);
-				put(2, ExitStatus.EXECUTING);
-				put(3, ExitStatus.NOOP);
-				put(4, ExitStatus.UNKNOWN);
-				put(ConfigurableSystemProcessExitCodeMapper.ELSE_KEY, ExitStatus.UNKNOWN);
-			}
-		};
+		Map<Object, ExitStatus> mappings = Map.of( //
+				0, ExitStatus.COMPLETED, //
+				1, ExitStatus.FAILED, //
+				2, ExitStatus.EXECUTING, //
+				3, ExitStatus.NOOP, //
+				4, ExitStatus.UNKNOWN, //
+				ConfigurableSystemProcessExitCodeMapper.ELSE_KEY, ExitStatus.UNKNOWN);
 
 		mapper.setMappings(mappings);