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

@@ -181,7 +181,37 @@
 					<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:DirectInvocationOnMock:OFF
+							-Xep:EqualsGetClass:OFF
+							-Xep:Finally:OFF
+							-Xep:HidingField:OFF
+							-Xep:JavaTimeDefaultTimeZone:OFF
+							-Xep:JdkObsolete:OFF
+							-Xep:MissingSummary:OFF
+							-Xep:MixedMutabilityReturnType:OFF
+							-Xep:MutablePublicArray:OFF
+							-Xep:NonAtomicVolatileUpdate:OFF
+							-Xep:ReferenceEquality:OFF
+							-Xep:StringCaseLocaleUsage:OFF
+							-Xep:StringSplitter:OFF
+							-Xep:SynchronizeOnNonFinalField:OFF
+							-Xep:UnnecessaryStringBuilder:OFF
+						</compilerArg>
 					</compilerArgs>
+					<failOnWarning>true</failOnWarning>
+					<annotationProcessorPaths>
+						<path>
+							<groupId>com.google.errorprone</groupId>
+							<artifactId>error_prone_core</artifactId>
+							<version>2.38.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/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/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

@@ -118,16 +118,13 @@ protected AbstractBeanDefinition parseStep(Element stepElement, ParserContext pa
 					new TaskletParser().parseTasklet(stepElement, nestedElement, bd, parserContext, stepUnderspecified);
 				}
 				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 +196,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 +255,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 +281,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/configuration/xml/CoreNamespacePostProcessor.java

@@ -115,15 +115,13 @@ public Object postProcessBeforeInitialization(Object bean, String beanName) thro
 	 * @return the bean with default collaborators injected into it
 	 */
 	private Object injectDefaults(Object bean) {
-		if (bean instanceof JobParserJobFactoryBean) {
-			JobParserJobFactoryBean fb = (JobParserJobFactoryBean) bean;
+		if (bean instanceof JobParserJobFactoryBean fb) {
 			JobRepository jobRepository = fb.getJobRepository();
 			if (jobRepository == null) {
 				fb.setJobRepository((JobRepository) applicationContext.getBean(DEFAULT_JOB_REPOSITORY_NAME));
 			}
 		}
-		else if (bean instanceof StepParserStepFactoryBean) {
-			StepParserStepFactoryBean<?, ?> fb = (StepParserStepFactoryBean<?, ?>) bean;
+		else if (bean instanceof StepParserStepFactoryBean<?, ?> fb) {
 			JobRepository jobRepository = fb.getJobRepository();
 			if (jobRepository == null) {
 				fb.setJobRepository((JobRepository) applicationContext.getBean(DEFAULT_JOB_REPOSITORY_NAME));

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

@@ -32,8 +32,8 @@ public ManagedMap<TypedStringValue, Boolean> parse(Element element, ParserContex
 		if (children.size() == 1) {
 			ManagedMap<TypedStringValue, Boolean> map = new ManagedMap<>();
 			Element exceptionClassesElement = children.get(0);
-			addExceptionClasses("include", true, exceptionClassesElement, map, parserContext);
-			addExceptionClasses("exclude", false, exceptionClassesElement, map, parserContext);
+			addExceptionClasses("include", true, exceptionClassesElement, map);
+			addExceptionClasses("exclude", false, exceptionClassesElement, map);
 			map.put(new TypedStringValue(ForceRollbackForWriteSkipException.class.getName(), Class.class), true);
 			return map;
 		}
@@ -46,7 +46,7 @@ else if (children.size() > 1) {
 	}
 
 	private void addExceptionClasses(String elementName, boolean include, Element exceptionClassesElement,
-			ManagedMap<TypedStringValue, Boolean> map, ParserContext parserContext) {
+			ManagedMap<TypedStringValue, Boolean> map) {
 		for (Element child : DomUtils.getChildElementsByTagName(exceptionClassesElement, elementName)) {
 			String className = child.getAttribute("class");
 			map.put(new TypedStringValue(className, Class.class), include);

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

@@ -103,7 +103,7 @@ protected void doParse(Element element, ParserContext parserContext, BeanDefinit
 			builder.addPropertyValue("restartable", restartableAttribute);
 		}
 
-		String incrementer = (element.getAttribute("incrementer"));
+		String incrementer = element.getAttribute("incrementer");
 		if (StringUtils.hasText(incrementer)) {
 			builder.addPropertyReference("jobParametersIncrementer", incrementer);
 		}

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

@@ -205,7 +205,7 @@ public FlowExecutionStatus handle(FlowExecutor executor) throws Exception {
 
 		@Override
 		public Collection<Flow> getFlows() {
-			return (state instanceof FlowHolder) ? ((FlowHolder) state).getFlows() : Collections.<Flow>emptyList();
+			return (state instanceof FlowHolder flowHolder) ? flowHolder.getFlows() : Collections.emptyList();
 		}
 
 	}

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

@@ -273,8 +273,8 @@ protected void enhanceCommonStep(StepBuilderHelper<?> builder) {
 			builder.startLimit(startLimit);
 		}
 		for (Object listener : stepExecutionListeners) {
-			if (listener instanceof StepExecutionListener) {
-				builder.listener((StepExecutionListener) listener);
+			if (listener instanceof StepExecutionListener stepExecutionListener) {
+				builder.listener(stepExecutionListener);
 			}
 		}
 	}
@@ -566,16 +566,16 @@ private void validateDependency(String dependentName, Object dependentValue, Str
 	 * @return {@code true} if the object has a value
 	 */
 	private boolean isPresent(Object o) {
-		if (o instanceof Integer) {
-			return isPositive((Integer) o);
+		if (o instanceof Integer i) {
+			return isPositive(i);
 		}
-		if (o instanceof Collection) {
-			return !((Collection<?>) o).isEmpty();
+		if (o instanceof Collection<?> collection) {
+			return !collection.isEmpty();
 		}
-		if (o instanceof Map) {
-			return !((Map<?, ?>) o).isEmpty();
+		if (o instanceof Map<?, ?> map) {
+			return !map.isEmpty();
 		}
-		return o != null;
+		return true;
 	}
 
 	/**

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

@@ -213,7 +213,7 @@ private void handleExceptionElement(Element element, ParserContext parserContext
 			ManagedList<TypedStringValue> list = new ManagedList<>();
 			list.setMergeEnabled(exceptionClassesElement.hasAttribute(MERGE_ATTR)
 					&& Boolean.parseBoolean(exceptionClassesElement.getAttribute(MERGE_ATTR)));
-			addExceptionClasses("include", exceptionClassesElement, list, parserContext);
+			addExceptionClasses("include", exceptionClassesElement, list);
 			propertyValues.addPropertyValue(propertyName, list);
 		}
 		else if (children.size() > 1) {
@@ -224,7 +224,7 @@ else if (children.size() > 1) {
 	}
 
 	private void addExceptionClasses(String elementName, Element exceptionClassesElement,
-			ManagedList<TypedStringValue> list, ParserContext parserContext) {
+			ManagedList<TypedStringValue> list) {
 		for (Element child : DomUtils.getChildElementsByTagName(exceptionClassesElement, elementName)) {
 			String className = child.getAttribute("class");
 			list.add(new TypedStringValue(className, Class.class));

spring-batch-core/src/main/java/org/springframework/batch/core/job/SimpleJob.java

@@ -79,8 +79,8 @@ public Collection<String> getStepNames() {
 		for (Step step : steps) {
 			names.add(step.getName());
 
-			if (step instanceof StepLocator) {
-				names.addAll(((StepLocator) step).getStepNames());
+			if (step instanceof StepLocator stepLocator) {
+				names.addAll(stepLocator.getStepNames());
 			}
 		}
 		return names;
@@ -100,8 +100,8 @@ public Step getStep(String stepName) {
 			if (step.getName().equals(stepName)) {
 				return step;
 			}
-			else if (step instanceof StepLocator) {
-				Step result = ((StepLocator) step).getStep(stepName);
+			else if (step instanceof StepLocator stepLocator) {
+				Step result = stepLocator.getStep(stepName);
 				if (result != null) {
 					return result;
 				}