Skip to content

Find references from constant definitions - #4218

Draft
SlavaEremenko wants to merge 2 commits into
Shopify:mainfrom
SlavaEremenko:se/find-references-from-constant-definitions
Draft

SlavaEremenko wants to merge 2 commits into
Shopify:mainfrom
SlavaEremenko:se/find-references-from-constant-definitions

Conversation

@SlavaEremenko

Copy link
Copy Markdown

Motivation

Find references returns nothing when invoked on a constant's definition, for example with the cursor on STATUSES here:

class Invoice
  STATUSES = %w[open paid].freeze
end

Invoking it on any usage (STATUSES, Invoice::STATUSES) works and even lists the definition, so only the declaration itself is a dead end. Class and module names are unaffected because class Foo holds a ConstantReadNode.

Implementation

The request only located ConstantReadNode, ConstantPathNode and ConstantPathTargetNode. On STATUSES = ..., a ConstantWriteNode, locate fell back to the program node and the request returned early.

ReferenceFinder already collects ConstantWriteNode, ConstantOrWriteNode, ConstantAndWriteNode, ConstantOperatorWriteNode and multi-write ConstantTargetNodes, so the request now accepts the same nodes as starting points and resolves node.name through the current nesting. The resolve-to-ConstTarget step is extracted so both constant branches share it.

While testing the ||= case I found ReferenceFinder registered on_constant_or_write_node_enter with the dispatcher twice, which returned every CONST ||= value as two identical locations no matter where the request started. That's fixed in its own commit.

Two things I noticed but left out to keep this focused:

  • Foo::BAR = 1 is also reported twice, because both on_constant_path_write_node_enter and on_constant_path_node_enter fire for the write's target. Those handlers came with rename support, so I didn't want to change them without knowing the intent.
  • Rename locates the same three node types, so renaming from a constant's definition has the same gap.

Happy to follow up on either.

Automated Tests

Added request tests for plain, ||=, += and multi-write definitions, plus a ReferenceFinder regression test for the duplicate or-write. bundle exec rake, bundle exec srb tc and bin/rubocop pass locally.

Manual Tests

  1. In a workspace, add class Foo; BAR = 1; end to one file and Foo::BAR to another.
  2. Right-click BAR on its definition line and pick Find All References.
  3. Before: "No references found". After: both the definition and Foo::BAR are listed.

`ReferenceFinder` registered `on_constant_or_write_node_enter` with the
dispatcher twice, so every `CONST ||= value` was collected as two
identical references.
Invoking find references on the name in `CONST = value` returned
nothing, because the request only located constant reads and paths.
`ReferenceFinder` already collects constant writes and multi-write
targets as references, so the request now accepts the same node types
as starting points and resolves them through the current nesting.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant