-
-
Notifications
You must be signed in to change notification settings - Fork 55
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.
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
Add Node#loc?
to determine if a node has a given location
#346
Conversation
e853b25
to
6c19dd4
Compare
TBH, I wonder why we never thought of simply adding the missing locations. The "difference" between not responding to a loc and having that loc as |
So the problem is that the parser gem defines quite a number of different For example, an #<Parser::Source::Map::Condition:0x000000013877fba8
@begin=nil,
@else=#<Parser::Source::Range /src/rubocop/test.rb 44...48>,
@end=#<Parser::Source::Range /src/rubocop/test.rb 55...58>,
@expression=#<Parser::Source::Range /src/rubocop/test.rb 31...58>,
@keyword=#<Parser::Source::Range /src/rubocop/test.rb 31...33>,
@node=s(:if,
s(:send, nil, :foo),
s(:send, nil, :bar),
s(:send, nil, :baz))> but its ternary form uses #<Parser::Source::Map::Ternary:0x000000012295e228
@colon=#<Parser::Source::Range /src/rubocop/test.rb 41...42>,
@expression=#<Parser::Source::Range /src/rubocop/test.rb 31...46>,
@node=s(:if,
s(:send, nil, :foo),
s(:send, nil, :bar),
s(:send, nil, :baz)),
@question=#<Parser::Source::Range /src/rubocop/test.rb 35...36>> As well, there are a lot of places where we act without knowing exactly what node type is being evaluated, which is another reason why the locations can differ. So it's probably not straightforward to have a consistent interface across the board. |
6c19dd4
to
73c5264
Compare
@marcandre when you have time can you take another look please? 🙌 |
Cool, thanks |
Thanks @marcandre! |
Thank you @dvandersluis ❤️ Released as 1.38.0 |
Following #345, it'd be helpful to have a helper method for
loc.respond_to?(:foo) && loc.foo
, which happens in a number of places in rubocop. I've addedNode#loc?(:foo)
to accomplish this, and redefinedloc_is?
to useloc?
.There were a couple places in rubocop-ast that just used
loc.respond_to?
without the and. I updated the one forternary?
because it should always be present in a ternary. I did not updateMethodDispatchNode#selector
because there's a lot of node types that include that andselector
is not well tested for that (and I'm not sure if it's possible to have akeyword
location in these cases without a value).