Introduce ErrorProne, fix compiler warnings #4807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Closed

scordio wants to merge 2 commits into spring-projects:main from scordio:cosmetic

Closed

Introduce ErrorProne, fix compiler warnings #4807

scordio wants to merge 2 commits into spring-projects:main from scordio:cosmetic

Conversation

scordio

Copy link

Contributor

@scordio scordio commented Apr 5, 2025 •

edited

Loading

This PR introduces ErrorProne, sets the maven-compiler-plugin to fail in case of compiler warnings, and fixes all the warnings.

This is a preparation for:

Migrate to JSpecify annotations for nullability constraints #4673

@scordio

This comment was marked as outdated.

@scordio scordio marked this pull request as draft

April 6, 2025 08:05

@scordio scordio force-pushed the cosmetic branch from daf0f98 to 18dd907 Compare

April 7, 2025 07:17

@scordio scordio changed the title ~~(削除) Favor factory methods instead of double braces initialization (削除ここまで)~~ (追記) Introduce ErrorProne, fix compiler warnings (追記ここまで)

Apr 7, 2025

@scordio scordio force-pushed the cosmetic branch 2 times, most recently from 8eb3acc to 8a6ddaf Compare

April 7, 2025 22:27

@fmbenhassine

Copy link

Contributor

fmbenhassine commented Apr 8, 2025

This is fantastic! Thank you for doing that, @scordio !

I have a plan to re-work/optimise the project's build/release process for the next major release (see #4319 as well), and this goes in that same direction.

When this is not a draft PR anymore, please let me know and it should be welcome to merge.

@scordio scordio force-pushed the cosmetic branch 5 times, most recently from f0efc64 to 459933c Compare

April 13, 2025 15:02

@scordio scordio force-pushed the cosmetic branch 17 times, most recently from 6349088 to 48ae732 Compare

May 7, 2025 06:38

@scordio scordio mentioned this pull request

May 9, 2025

Replace spotbugs with errorprone jqno/equalsverifier#1073

Merged

@scordio scordio force-pushed the cosmetic branch from 48ae732 to 13488b1 Compare

May 9, 2025 22:52

@scordio scordio closed this

May 9, 2025

@scordio scordio deleted the cosmetic branch

May 9, 2025 23:28

@scordio scordio restored the cosmetic branch

May 9, 2025 23:29

@scordio scordio reopened this

May 9, 2025

@scordio scordio force-pushed the cosmetic branch 2 times, most recently from 4f4dbfa to ec33b20 Compare

May 28, 2025 14:33

scordio

scordio commented

May 28, 2025

View reviewed changes

Copy link

Contributor Author

@scordio scordio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fmbenhassine, I am finally ready with the changes!

I think there are a few flaky tests like:

SimpleStepFactoryBeanTests.testSimpleConcurrentJob
JdbcPagingItemReaderCommonTests

I couldn't figure out the root cause, but the failing ones are green with single executions in the IDE or when retrying a complete Maven build... if you have any hints, please let me know!

...tch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcJobInstanceDao.java Outdated

Comment on lines 59 to 63

@SuppressWarnings("unused")

private static final String STAR_WILDCARD = "*";

@SuppressWarnings("unused")

private static final String SQL_WILDCARD = "%";

Copy link

Contributor Author

@scordio scordio May 6, 2025 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppressed the warnings here, but it might make sense to delete them instead.

...ructure/src/main/java/org/springframework/batch/item/file/transform/ConversionException.java

Comment on lines +23 to 24

@SuppressWarnings("unused") // FIXME no usage - should it be deprecated for removal?

public class ConversionException extends RuntimeException {

Copy link

Contributor Author

@scordio scordio May 6, 2025 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the comment mentions, ConversionException is currently unused in the codebase.

Should this be deprecated for removal?

Copy link

Contributor

@fmbenhassine fmbenhassine Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will take care of that separately. Thank you for adding that "FIXME" for reference 👍

pom.xml Outdated

Comment on lines 189 to 227

-Xep:AlmostJavadoc:OFF

-Xep:BadInstanceof:OFF

-Xep:ByteBufferBackingArray:OFF

-Xep:ClassCanBeStatic:OFF

-Xep:CollectionUndefinedEquality:OFF

-Xep:DefaultCharset:OFF

-Xep:DirectInvocationOnMock:OFF

-Xep:DoNotCallSuggester:OFF

-Xep:EmptyCatch:OFF

-Xep:EqualsGetClass:OFF

-Xep:Finally:OFF

-Xep:FutureReturnValueIgnored:OFF

-Xep:HidingField:OFF

-Xep:ImmutableEnumChecker:OFF

-Xep:InlineMeSuggester:OFF

-Xep:InputStreamSlowMultibyteRead:OFF

-Xep:JavaTimeDefaultTimeZone:OFF

-Xep:JavaUtilDate:OFF

-Xep:JdkObsolete:OFF

-Xep:MissingSummary:OFF

-Xep:MixedMutabilityReturnType:OFF

-Xep:MutablePublicArray:OFF

-Xep:NonAtomicVolatileUpdate:OFF

-Xep:RedundantControlFlow:OFF

-Xep:ReferenceEquality:OFF

-Xep:StaticAssignmentInConstructor:OFF

-Xep:StaticAssignmentOfThrowable:OFF

-Xep:StaticMockMember:OFF

-Xep:StreamResourceLeak:OFF

-Xep:StringCaseLocaleUsage:OFF

-Xep:StringSplitter:OFF

-Xep:SynchronizeOnNonFinalField:OFF

-Xep:ThreadLocalUsage:OFF

-Xep:ThreadPriorityCheck:OFF

-Xep:TypeParameterUnusedInFormals:OFF

-Xep:UndefinedEquals:OFF

-Xep:UnnecessaryStringBuilder:OFF

-Xep:UnusedMethod:OFF

-Xep:UnusedVariable:OFF

-Xep:WaitNotInLoop:OFF

Copy link

Contributor Author

@scordio scordio May 28, 2025 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I excluded all the ErrorProne warnings that didn't have a straightforward solution.

I would suggest addressing them in separate PRs.

Copy link

Contributor

@fmbenhassine fmbenhassine Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's fine, we can address those in separate PRs.

@scordio scordio marked this pull request as ready for review

May 28, 2025 18:13

scordio

scordio commented

May 28, 2025

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

@scordio scordio force-pushed the cosmetic branch 2 times, most recently from e96e01d to aaf4582 Compare

June 2, 2025 16:53

@scordio

Copy link

Contributor Author

scordio commented Jun 2, 2025

... and now the build is green! 😅

@scordio scordio force-pushed the cosmetic branch from aaf4582 to e867fe6 Compare

June 3, 2025 22:27

scordio added 2 commits

June 4, 2025 00:44

@scordio


 Introduce ErrorProne, fix compiler warnings

5d7b731

Signed-off-by: Stefano Cordio <stefano.cordio@gmail.com>

@scordio


 Remove BadInstanceof exclusion

7dffbd7

Signed-off-by: Stefano Cordio <stefano.cordio@gmail.com>

@scordio scordio force-pushed the cosmetic branch from e867fe6 to 7dffbd7 Compare

June 3, 2025 22:47

@fmbenhassine

Copy link

Contributor

fmbenhassine commented Jun 3, 2025

This is a huge amount of work! Thank you very much for doing that, @scordio 👍

I know you had to rebase this several times due to the latest changes in the code base, really appreciated

Rebased, squashed and merged as 74975c5.

@fmbenhassine fmbenhassine closed this

Jun 3, 2025

@fmbenhassine fmbenhassine added pr-for: enhancement in: build labels

Jun 3, 2025

@fmbenhassine fmbenhassine added this to the 6.0.0-M1 milestone

Jun 3, 2025

@scordio

Copy link

Contributor Author

scordio commented Jun 3, 2025

Thanks! I'll continue with #4673 in the upcoming days 🙂

@scordio scordio deleted the cosmetic branch

June 3, 2025 23:14

@cppwfs cppwfs mentioned this pull request

Jun 4, 2025

Remove unused imports #4862

Closed

scordio referenced this pull request

Jun 9, 2025

@fmbenhassine


 Remove build failure on warnings

72694e6

Labels

in: build pr-for: enhancement

2 participants

@scordio @fmbenhassine

Introduce ErrorProne, fix compiler warnings #4807

Introduce ErrorProne, fix compiler warnings #4807

Uh oh!

Conversation

@scordio scordio commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

fmbenhassine commented Apr 8, 2025

Uh oh!

@scordio scordio left a comment

Choose a reason for hiding this comment

Uh oh!

@scordio scordio May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

@scordio scordio May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

@fmbenhassine fmbenhassine Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

@scordio scordio May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

@fmbenhassine fmbenhassine Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scordio commented Jun 2, 2025

Uh oh!

fmbenhassine commented Jun 3, 2025

Uh oh!

scordio commented Jun 3, 2025

Uh oh!

Uh oh!

@scordio scordio commented Apr 5, 2025 •

edited

Loading

@scordio scordio May 6, 2025 •

edited

Loading

@scordio scordio May 6, 2025 •

edited

Loading

@scordio scordio May 28, 2025 •

edited

Loading