Skip to content
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

Implement complete constant resolution algorithm #2136

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

vinistock
Copy link
Member

@vinistock vinistock commented Jun 5, 2024

Motivation

Now that we have ancestor linearization, we can finally resolve constants properly. This PR implements the algorithm for constant resolution in Ruby - with a few twists due to the nature of lazy static analysis.

Implementation

We tried to document the code as much as possible. I'll describe the algorithm, which has essentially 3 distinct paths.

Top level references

module First
  module Second
    ::CONST
  end
end

Top level references are the simplest ones. The nesting is completely ignored. All we have to do is try to find the name in the index. We do need to account for aliases too since the top level reference can include a portion of the name that points to an alias.

Qualified references

module First
  module Second
    Third::CONST
  end
end

For qualified references, this is the order of resolution:

  1. First, try to find the declaration within the exact namespace where we found the reference First::Second::Third::CONST
  2. Then, try to find the declaration in ancestors of the namespace where we found the reference. We search all ancestors of First::Second
  3. Then, try to find it in enclosing lexical scopes excluding the top level (which is always considered only as a fallback). We search First::Third::CONST
  4. Finally, since we didn't find anything, search for Third::CONST in the top level

Unqualified references

module First
  module Second
    CONST
  end
end

Unqualified references have the order of operations swapped:

  1. First, try to find the declaration within the exact namespace where we found the reference First::Second::CONST
  2. Then, try to find it in enclosing lexical scopes before searching ancestors. This also excludes the top level. We search First::CONST
  3. Then, try to find the declaration in ancestors of the namespace where we found the reference. We search all ancestors of First::Second
  4. Finally, since we didn't find anything, search for CONST in the top level

Notes

  1. Ruby never searches the ancestors of any enclosing lexical scope except the most specific one
  2. We may want to create a cache for resolutions. Maybe a hash that maps the concatenated name + nesting and points to the resolved name. However, knowing when to invalidate the cache would be quite tricky. I recommend we avoid caching for now
  3. Fun fact: thanks to the lazy nature of our static analysis, we are actually able to "properly" resolve circular references despite them being invalid Ruby code. See IndexTest#test_resolving_circular_alias and IndexTesttest_resolving_circular_alias_three_levels. This is why we need that seen_names array, which not only protects the index from infinite recursion, but also allows us to resolve these cases

Automated Tests

Added a bunch of tests for various constant reference scenarios, but if you can think of anything that we missed, please propose more tests.

@vinistock vinistock added enhancement New feature or request server This pull request should be included in the server gem's release notes labels Jun 5, 2024
@vinistock vinistock self-assigned this Jun 5, 2024
@vinistock vinistock force-pushed the vs/properly_resolve_inherited_constants branch 2 times, most recently from e190e0b to b16d8c7 Compare June 6, 2024 16:09
@vinistock vinistock marked this pull request as ready for review June 6, 2024 16:26
@vinistock vinistock requested a review from a team as a code owner June 6, 2024 16:26
@vinistock vinistock requested review from andyw8 and st0012 June 6, 2024 16:26
lib/ruby_indexer/test/index_test.rb Outdated Show resolved Hide resolved
lib/ruby_indexer/lib/ruby_indexer/index.rb Show resolved Hide resolved
lib/ruby_indexer/lib/ruby_indexer/index.rb Outdated Show resolved Hide resolved
lib/ruby_indexer/test/index_test.rb Show resolved Hide resolved
@vinistock vinistock force-pushed the vs/properly_resolve_inherited_constants branch from b16d8c7 to 601bde9 Compare June 6, 2024 18:19
@vinistock vinistock requested a review from Morriar June 6, 2024 18:19
Co-authored-by: Alexandre Terrasa <Morriar@users.noreply.github.com>
@vinistock vinistock force-pushed the vs/properly_resolve_inherited_constants branch from 601bde9 to d0ff4c9 Compare June 7, 2024 13:26
@vinistock vinistock requested a review from Morriar June 7, 2024 13:29
@vinistock vinistock merged commit f44da30 into main Jun 10, 2024
35 checks passed
@vinistock vinistock deleted the vs/properly_resolve_inherited_constants branch June 10, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants