Skip to content

Conversation

@tginsberg
Copy link
Contributor

@tginsberg tginsberg commented Jan 2, 2026

What's changed?

This is a failing test case against issue #800

What's your motivation?

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

- Failing test case for ReplaceWeekYearWithYear when used with week-year
  and week of year
- When `w` (week in year) is detected and we are not inside quotes, abandon replacement and return the original `input`.
- Also simplified `replaceY`, which had a redundant else-if branch.
@timtebeek timtebeek self-requested a review January 3, 2026 11:14
@timtebeek
Copy link
Member

Great to see, thanks! Sharing the notes you left on the commit here as well to make those easier to find:

  • When w (week in year) is detected and we are not inside quotes, abandon replacement and return the original input.
  • Also simplified replaceY, which had a redundant else-if branch.

@timtebeek timtebeek added the bug Something isn't working label Jan 3, 2026
@timtebeek timtebeek changed the title 800: Failing test case ReplaceWeekYearWithYear should not replace valid week-year usages Jan 3, 2026
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Jan 3, 2026
Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Thanks for the help here @tginsberg ; Nice simplification & bug fix. Guess this was the best week of the year for discovery & fix. All the best there!

@timtebeek timtebeek marked this pull request as ready for review January 3, 2026 12:23
@timtebeek timtebeek merged commit 2a88748 into openrewrite:main Jan 3, 2026
2 checks passed
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Jan 3, 2026
@tginsberg
Copy link
Contributor Author

Thanks for making contributing so easy @timtebeek! I was writing a blog post on week year and was covering how to fix it and noticed this aspect of the rule. I figured it was a good chance to contribute, in a small way, to a project I admire so much. Thanks for accepting my fix! :)

@tginsberg tginsberg deleted the fix-800-replaceweekyearwithyear-should-ignore-some-cases branch January 3, 2026 13:42
@timtebeek
Copy link
Member

timtebeek commented Jan 3, 2026

Thanks for the kind words & help! Linking your post here for others interested as well:

We'll put out a new release in the coming days that contains your fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

ReplaceWeekYearWithYear incorrectly replaces valid week-year usages

2 participants