KEMBAR78
Fix incorrect autocorrect for Style/AccessModifierDeclarations by girasquid · Pull Request #14354 · rubocop/rubocop · GitHub
Skip to content

Conversation

@girasquid
Copy link
Contributor

@girasquid girasquid commented Jul 9, 2025

I noticed that when running autocorrect for Style/AccessModifierDeclarations, the method above the group declaration was being deleted. So this:

class MyThing
  def other_method; end

  def a_method_that_is_public; end

  private

  def my_other_private_method; end
end

After going through autocorrect would become:

class MyThing
  def other_method; end

  private

  private def my_other_private_method; end
end

This PR adds a spec demonstrating the behavior with as small a code sample as I could get down to, and then fixes the issue - when trying to change the code it was always using index 1, which wasn't correct if the AST had the entire class body for what needed to be processed.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@girasquid girasquid force-pushed the fix-private-auto-correct branch 3 times, most recently from 093ad5e to 6586212 Compare July 9, 2025 21:28
@girasquid girasquid changed the title Fix incorrect autocorrect for Style/AccessModifierDeclaration Fix incorrect autocorrect for Style/AccessModifierDeclarations Jul 9, 2025
@girasquid girasquid force-pushed the fix-private-auto-correct branch from 6586212 to 1e2d281 Compare July 9, 2025 21:56
@@ -0,0 +1 @@
* [#14354](https://github.com/rubocop/rubocop/pull/14354): Fix incorrect autocorrect for `Style/AccessModifierDeclarations`. ([@girasquid][])
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a bit more context? e.g.,

Suggested change
* [#14354](https://github.com/rubocop/rubocop/pull/14354): Fix incorrect autocorrect for `Style/AccessModifierDeclarations`. ([@girasquid][])
* [#14354](https://github.com/rubocop/rubocop/pull/14354): Fix incorrect autocorrect for `Style/AccessModifierDeclarations` when using a grouped access modifier declaration. ([@girasquid][])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 👍🏻

RUBY
end

it 'does not delete methods before the group declaration' do
Copy link
Member

Choose a reason for hiding this comment

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

Can you update it? The description in it should only state the condition under which an offense is registered.

Suggested change
it 'does not delete methods before the group declaration' do
it 'registers an offense when using a grouped access modifier declaration' do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a great improvement, I couldn't figure out what to describe this with - thanks for the suggestion, done 👍🏻

@girasquid girasquid force-pushed the fix-private-auto-correct branch from 1e2d281 to fe4196d Compare July 10, 2025 15:05
@girasquid girasquid force-pushed the fix-private-auto-correct branch from fe4196d to 1cfebee Compare July 10, 2025 15:06
@koic koic merged commit 52b7ef3 into rubocop:master Jul 10, 2025
24 checks passed
@koic
Copy link
Member

koic commented Jul 10, 2025

Thanks!

@girasquid girasquid deleted the fix-private-auto-correct branch July 10, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants