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

Fix top level namespace declaration indexing #2297

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

vinistock
Copy link
Member

Motivation

Closes #2287

The problem surfaced in the issue is actually unrelated to linearizing ancestors. We were incorrectly handling top level namespace declarations and accidentally inserting entries in the index that included a leading ::.

To match Ruby's behaviour, we need to account that a top level reference may be found at any level of nesting and it fully removes all of the preceding namespaces.

Implementation

Created a method to compute the correct nesting. The method searches for the first top level reference in reverse. If a constant with the :: prefix is found, we break out of the loop and return the nesting only up to that point.

Otherwise, we just keep prepending the names into the array and it just results in the same nesting.

Automated Tests

Added two examples that demonstrate the scenarios we did not support.

@vinistock vinistock added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels Jul 12, 2024
@vinistock vinistock self-assigned this Jul 12, 2024
@vinistock vinistock requested a review from a team as a code owner July 12, 2024 16:31
@vinistock vinistock requested review from andyw8 and st0012 July 12, 2024 16:31
@vinistock vinistock merged commit 29c03ac into main Jul 12, 2024
35 checks passed
@vinistock vinistock deleted the vs/fix_top_level_reference_indexing branch July 12, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug 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.

Indexing error in existing_or_new_singleton_class
2 participants