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
Gh1940/alias support #1951
base: main
Are you sure you want to change the base?
Gh1940/alias support #1951
Conversation
Co-authored-by: Vinicius Stock <vinistock@users.noreply.github.com>
Co-authored-by: Vinicius Stock <vinistock@users.noreply.github.com>
Co-authored-by: Vinicius Stock <vinistock@users.noreply.github.com>
20b01c9
to
0a55ab4
Compare
@@ -17,6 +17,7 @@ def index(source) | |||
|
|||
def assert_entry(expected_name, type, expected_location) | |||
entries = @index[expected_name] | |||
refute_nil(entries, "Expected #{expected_name} to be indexed") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introduced a refute_nil
before we refute empty for better error messages
assert_entry("whatever", Entry::UnresolvedMethodAlias, "/fake/path/foo.rb:1-8:1-16") | ||
assert_entry("foo", Entry::UnresolvedMethodAlias, "/fake/path/foo.rb:2-15:2-19") | ||
assert_entry("bar", Entry::UnresolvedMethodAlias, "/fake/path/foo.rb:3-15:3-20") | ||
assert_equal(4, @index.instance_variable_get(:@entries).length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be worth a short explanation of where the 4
comes from.
return unless new_name && old_name | ||
|
||
new_name_value = case new_name | ||
when Prism::StringNode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would unescaped
allow us to avoid this conditional?
ruby-lsp(main):028> Prism.parse("alias foo").value.statements.body.first.new_name.unescaped
=> "foo"
ruby-lsp(main):029> Prism.parse("alias :foo").value.statements.body.first.new_name.unescaped
=> "foo"
old_name: String, | ||
owner: T.nilable(Entry::Namespace), | ||
file_path: String, | ||
location: T.any(Prism::Location, RubyIndexer::Location), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is RubyIndexer::Location
needed here? It typechecks without it, so if we do need it we may be missing a test.
Motivation
first step for #1940
We need to keep track of aliases but they cannot be resolved until we know if the old method is inherited. Blocked on resolving aliases until #1333 is complete.
Implementation
Created new class for UnresolvedMethodAlias and started handling AliasNode and AliasMethod
Automated Tests
Added test in method_test