Skip to content

Modernize java.time API usages #35861

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.

Sign up for GitHub

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Modernize java.time API usages #35861

wants to merge 1 commit into from

Conversation

@Pankraz76
Copy link

@Pankraz76 Pankraz76 commented Nov 20, 2025 •
edited
Loading

I’d kindly like to suggest considering Error Prone integration featuring the TimeRules findings:

As discussed in:

PS:

Congratulations on the recent major release — a lot of work has clearly gone into it.
I'm sure Spring Boot will continue to succeed as well.

Changes have been made to spring-context/src/main/java/org/springframework/scheduling/support/CronTrigger.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$InstantAtZoneRecipe
Changes have been made to spring-context/src/main/java/org/springframework/format/datetime/standard/DateTimeConverters.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$InstantAtZoneRecipe
Changes have been made to spring-context/src/test/java/org/springframework/scheduling/concurrent/DefaultManagedTaskSchedulerTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$DurationOfMinutesRecipe
Changes have been made to spring-test/src/main/java/org/springframework/test/http/HttpHeadersAssert.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$InstantAtZoneRecipe
Changes have been made to spring-test/src/test/java/org/springframework/test/web/servlet/result/HeaderResultMatchersTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
Changes have been made to spring-test/src/test/java/org/springframework/test/web/servlet/client/RestTestClientTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
Changes have been made to spring-test/src/test/java/org/springframework/test/web/support/HeaderAssertionTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
Changes have been made to spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcClockRecipe
Changes have been made to spring-web/src/main/java/org/springframework/http/HttpHeaders.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$InstantAtZoneRecipe
Changes have been made to spring-web/src/test/java/org/springframework/web/server/i18n/FixedLocaleContextResolverTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
Changes have been made to spring-web/src/test/java/org/springframework/http/HttpHeadersTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
Changes have been made to spring-web/src/test/java/org/springframework/http/client/reactive/ClientHttpConnectorTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
Changes have been made to spring-webflux/src/test/java/org/springframework/web/reactive/function/server/DefaultRenderingResponseTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$DurationOfMillisRecipe
Changes have been made to spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java by:
    tech.picnic.errorprone.refasterrules.TimeRulesRecipes
        tech.picnic.errorprone.refasterrules.TimeRulesRecipes$UtcConstantRecipe
Please review and commit the results.
Estimate time saved: 1h 10m

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 20, 2025
Pankraz76
Pankraz76 commented Nov 20, 2025
View reviewed changes
mdeinum
mdeinum reviewed Nov 20, 2025
View reviewed changes
Copy link
Contributor

@mdeinum mdeinum left a comment

Choose a reason for hiding this comment

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

Looking at existing rules and deliberate care in dependencies I would expect some changes. Which would mean that the rewrite rules aren't applicable.

@Pankraz76 Pankraz76 changed the title [prone] Add StreamRulesRecipes [prone] Add TimeRulesRecipes Nov 21, 2025
@Pankraz76 Pankraz76 force-pushed the fix-StreamRulesRecipes branch from a8e5b97 to def2188 Compare November 21, 2025 10:03
@Pankraz76
Copy link
Author

Pankraz76 commented Nov 21, 2025

Looking at existing rules and deliberate care in dependencies I would expect some changes. Which would mean that the rewrite rules aren't applicable.

Thank you for the kind feedback.

I’ve reworked the scope to focus on the TimeRules. Hopefully this is something easier to consider.

the rewrite rules aren't applicable.

I think it’s difficult to judge something universally or generically; each scope has its own individual context.

This is the first project that passed the included GradleBestPractices checks without any findings.
That suggests the codebase is already very up-to-date regarding this scope — well done.

Considering the Java8toJava11 migration, as explored in

the code compiles on Java 21+ while still largely using Java 8–era and Java 17–era constructs.
That’s something to consider — similar to how Checkstyle, Google Error Prone, and Spotless have already done clean up the entire codebase in a glance.

Pankraz76
Pankraz76 commented Nov 21, 2025
View reviewed changes
...g-web/src/test/java/org/springframework/web/server/i18n/FixedLocaleContextResolverTests.java
@Test
void resolveCustomizedAndTimeZoneLocale() {
TimeZone timeZone = TimeZone.getTimeZone(ZoneId.of("UTC"));
TimeZone timeZone = TimeZone.getTimeZone(ZoneOffset.UTC);
Copy link
Author

@Pankraz76 Pankraz76 Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

this seems a good thing to consider, being straight up instead of "error prone".

Copy link

@rickie rickie Nov 21, 2025

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Author

@Pankraz76 Pankraz76 Nov 21, 2025

Choose a reason for hiding this comment

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

sorry i meant this is a good thing and seems easy consider suggest, as it avoids error prone (random) string.

@Pankraz76 Pankraz76 requested a review from mdeinum November 21, 2025 13:28
@Pankraz76 Pankraz76 mentioned this pull request Nov 22, 2025
Merged
3 tasks
@sdeleuze
Copy link
Contributor

sdeleuze commented Nov 25, 2025 •
edited
Loading

We prefer to not introduce new tooling infrastructure in our build, please refine this PR to only include the .*java changes.

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Nov 25, 2025
@Pankraz76
Copy link
Author

Pankraz76 commented Nov 25, 2025

yes of course, thanks for the kind feedback.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 25, 2025
@Pankraz76 Pankraz76 changed the title [prone] Add TimeRulesRecipes [picnic] apply TimeRules Nov 25, 2025
[picnic] apply TimeRules
d87c456 
Signed-off-by: Vincent Potucek <[email protected]>
@Pankraz76 Pankraz76 force-pushed the fix-StreamRulesRecipes branch from f3e25e5 to d87c456 Compare November 25, 2025 13:42
@Pankraz76 Pankraz76 marked this pull request as ready for review November 25, 2025 13:43
@Pankraz76 Pankraz76 requested a review from rickie November 25, 2025 13:45
@sdeleuze sdeleuze self-assigned this Nov 25, 2025
rickie
rickie approved these changes Nov 25, 2025
View reviewed changes
Copy link

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for applying @Pankraz76

@sdeleuze sdeleuze added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Nov 25, 2025
@sdeleuze sdeleuze added this to the 7.0.2 milestone Nov 25, 2025
@sdeleuze sdeleuze changed the title [picnic] apply TimeRules Modernize java.time API usages Nov 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mdeinum mdeinum Awaiting requested review from mdeinum

1 more reviewer

@rickie rickie rickie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@sdeleuze sdeleuze

Labels

type: task A general task

Projects

None yet

Milestone

7.0.2

Development

Successfully merging this pull request may close these issues.

5 participants

@Pankraz76 @sdeleuze @mdeinum @rickie @spring-projects-issues