Skip to content

Conversation

dongwooooooo
Copy link
Contributor

@dongwooooooo dongwooooooo commented Aug 12, 2025

What's changed?

Fixed whitespace and comment handling when changing method access level to package-private. The removed access modifier's Space (whitespace and comments) is now properly merged with the next modifier to maintain consistent formatting.

What's your motivation?

Resolves spacing issues where the auto-formatter doesn't influence spacing when comments are involved. This was causing inconsistent formatting when access modifiers were removed, particularly when line comments were present.
Referenced issue pattern can be seen in rewrite-spring's BeanMethodsNotPublicTest where dataSource3 bean formatting was problematic.

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

  • The Space merging logic in lines 118-143 that handles whitespace and comment preservation
  • The test case publicToPackagePrivateMovesLineComment() which covers the specific scenario where line comments need to be relocated
  • Validation that the fix doesn't break existing formatting behavior

Have you considered any alternatives or workarounds?

Considered relying solely on auto-formatting, but as noted in the issue, auto-formatter doesn't handle spacing when comments are involved. The current approach manually preserves the spacing structure by merging Space objects.

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

Properly merge whitespace and comments from removed access modifier
with the next modifier when changing to package-private access level.

- Add Space merging logic for removed modifiers
- Handle comment relocation during modifier removal
- Add test for line comment movement
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Aug 12, 2025
@dongwooooooo dongwooooooo changed the title Fix whitespace preservation when removing access modifiers #5902 Fix whitespace preservation when removing access modifiers Aug 12, 2025
@timtebeek timtebeek self-requested a review August 12, 2025 19:31
@timtebeek timtebeek self-assigned this Aug 12, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • rewrite-maven/src/main/java/org/openrewrite/maven/tree/Dependency.java
    • lines 26-26

timtebeek and others added 2 commits August 12, 2025 22:24
…hodAccessLevelTest.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@timtebeek timtebeek assigned dongwooooooo and unassigned timtebeek Aug 12, 2025
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Aug 12, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • rewrite-java/src/test/java/org/openrewrite/java/search/HasBuildToolVersionTest.java
    • lines 21-21
    • lines 36-36

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 helpful fix here @dongwooooooo !

@timtebeek timtebeek merged commit 8e7dc18 into openrewrite:main Aug 13, 2025
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Aug 13, 2025
@dongwooooooo dongwooooooo deleted the package-private-modifier-spacing branch August 13, 2025 15:07
dcsekar pushed a commit to dcsekar/rewrite that referenced this pull request Aug 18, 2025
…te#5905)

* fix: preserve spacing when removing access modifiers

Properly merge whitespace and comments from removed access modifier
with the next modifier when changing to package-private access level.

- Add Space merging logic for removed modifiers
- Handle comment relocation during modifier removal
- Add test for line comment movement

* Update rewrite-java-test/src/test/java/org/openrewrite/java/ChangeMethodAccessLevelTest.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Slight polish

---------

Co-authored-by: Tim te Beek <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Tim te Beek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants