ParsedConstantDefinitions doesn’t support fully qualified constants
Opened this issue · 1 comments
RuboCop::AST::Node#const_name
doesn’t reveal whether a constant is fully qualified (e.g. ::HELLO
) or not:
>> RuboCop::ProcessedSource.new('::HELLO', 2.6).ast.const_name
=> "HELLO"
>> RuboCop::ProcessedSource.new('HELLO', 2.6).ast.const_name
=> "HELLO"
As a result, the implementation of ParsedConstantDefinitions#collect_local_definitions
is unaware of whether a constant is fully qualified, so it’s unable to put it in the correct namespace:
>> definitions = ParsedConstantDefinitions.new(
root_node: parse_code('module Sales; ::HELLO = "World"; end')
)
=> #<Packwerk::ParsedConstantDefinitions
@local_definitions=
{"::Sales"=>#<Parser::Source::Range (string) 7...12>,
"::Sales::HELLO"=>#<Parser::Source::Range (string) 16...21>}>
>> definitions.local_reference?('HELLO')
=> false # should be true
>> definitions.local_reference?('Sales::HELLO')
=> true # should be false
I’m not going to fix this right now because I’m trying to make progress on a different task, but I think it’s a bug that’s going to affect users.
cc: @tomstuart
Things have changed since I opened the original issue in June.
The good news is that Packwerk::Node.constant_name
(née #const_name
) is now aware of fully-qualified constant names:
>> Packwerk::Node.constant_name(Parser::CurrentRuby.parse('::HELLO'))
=> "::HELLO"
>> Packwerk::Node.constant_name(Parser::CurrentRuby.parse('HELLO'))
=> "HELLO"
The bad news is that ParsedConstantDefinitions#collect_local_definitions
doesn’t know what to do with this:
>> definitions = Packwerk::ParsedConstantDefinitions.new(
root_node: Parser::CurrentRuby.parse('module Sales; ::HELLO = "World"; end')
)
=> #<Packwerk::ParsedConstantDefinitions
@local_definitions=
{"::Sales"=>#<struct Packwerk::Node::Location line=1, column=7>,
"::Sales::::HELLO"=>#<struct Packwerk::Node::Location line=1, column=16>}>
>> definitions.local_reference?('HELLO')
=> false # should be true
>> definitions.local_reference?('Sales::HELLO')
=> false # correct, but for the wrong reason
You can see that @local_definitions
now contains an erroneous entry for ::Sales::::HELLO
, formed by naively joining 'Sales'
and '::HELLO'
with '::'
. We still have no test coverage for this so it’s not affecting the build, but it’s obviously wrong behaviour that will bite anyone who tries to use fully-qualified constants.