search

rubocop / rubocop-ast

RuboCop's AST extensions and NodePattern functionality
https://docs.rubocop.org/rubocop-ast
MIT License
112 stars 53 forks source link

Allow `non_bare_access_modifier_declaration?` to handle modifiers with multiple arguments #325

Closed dvandersluis closed 4 weeks ago

dvandersluis commented 1 month ago

I noticed this when testing Style/AccessModifierDeclarations. When an access modifier has multiple arguments, non_bare_access_modifier_declaration? erroneously (IMO) returns false, which causes the cop to not recognize all cases it should.

The cop does not properly recognize (as a positive or negative, depends on the AllowModifiersOnSymbols configuration) this despite the examples for the cop including it.

private :foo, :bar

I have a follow up PR to Style/AccessModifierDeclarations coming once this can be merged.

dvandersluis commented 1 month ago

Hmm there's a Style/AccessModifierDeclarations failure in internal_investigation now because of this change but it's a catch 22 since it needs the cop to be updated which needs AST to be updated... 😅

dvandersluis commented 1 month ago

I added that offense to .rubocop_todo.yml for now. I'll clean it up after everything is fixed.

(I didn't want to regenerate the whole file because it changes significantly and adds offenses to other files to remove # rubocop:disable directives -- we should maybe do this later though?)

marcandre commented 4 weeks ago

Thanks for fixing this bug.

marcandre commented 4 weeks ago

Released 1.33.1

dvandersluis commented 4 weeks ago

Thanks @marcandre! ❤️