Skip to content

Search for closest namespace when connecting definition to declaration#575

Merged
vinistock merged 1 commit intomainfrom
02-09-fix_definition_id_to_declaration_id_corner_cases
Feb 10, 2026
Merged

Search for closest namespace when connecting definition to declaration#575
vinistock merged 1 commit intomainfrom
02-09-fix_definition_id_to_declaration_id_corner_cases

Conversation

@vinistock
Copy link
Member

@vinistock vinistock commented Feb 9, 2026

Finding the declaration for a definition that was nested inside something that isn't a namespace was crashing. For example, this scenario:

class Foo
  def bar
    # This ivar is nested under the `bar` method. To find its declaration, we need
    # to go all the way to `Foo`
    @something = 1
  end
end

The fix is to keep searching for the closest namespace and only default to Object if the definition is not nested under anything.

Copy link
Member Author

vinistock commented Feb 9, 2026

@vinistock vinistock self-assigned this Feb 9, 2026
@vinistock vinistock marked this pull request as ready for review February 9, 2026 18:31
@vinistock vinistock requested a review from a team as a code owner February 9, 2026 18:31
@vinistock vinistock added the bugfix A change that fixes an existing bug label Feb 9, 2026
@vinistock vinistock force-pushed the 02-09-fix_definition_id_to_declaration_id_corner_cases branch from 1009dfc to 8839b3a Compare February 10, 2026 14:25
@vinistock vinistock force-pushed the 02-09-fix_definition_id_to_declaration_id_corner_cases branch from 8839b3a to 964ffd5 Compare February 10, 2026 14:29
@vinistock vinistock requested a review from alexcrocha February 10, 2026 14:29
}

/// Finds the closest namespace name ID to connect a definition to its declaration
fn find_enclosing_namespace_name_id(&self, starting_id: Option<&DefinitionId>) -> Option<&NameId> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming between lexical_nesting and namespace is getting confusing, we may want to revisit it at some point

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed. What indexing collects isn't really the lexical nesting since methods don't produce lexical scopes. We'll need to review the nomenclature.

Copy link
Member Author

vinistock commented Feb 10, 2026

Merge activity

  • Feb 10, 3:31 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Feb 10, 3:31 PM UTC: @vinistock merged this pull request with Graphite.

@vinistock vinistock merged commit 9a8aa40 into main Feb 10, 2026
27 of 28 checks passed
@vinistock vinistock deleted the 02-09-fix_definition_id_to_declaration_id_corner_cases branch February 10, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix A change that fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants