[go: nahoru, domu]

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

Provide features on singletons #2188

Merged
merged 6 commits into from
Jun 18, 2024
Merged

Conversation

vinistock
Copy link
Member
@vinistock vinistock commented Jun 14, 2024

Motivation

Second to last step for #1938. The only part missing is proper linearization of singleton ancestors, which I'm working on.

This PR uses the type checker introduced in #2187 to provide go to definition, hover, completion and signature help inside singleton contexts.

Implementation

I split each feature by commit:

  1. Definition
  2. Hover
  3. Completion
  4. Signature help

The only notable part of the implementation is completion. When invoking methods on a singleton, we know the receiver type, which means we can show all available methods immediately when the user types the dot.

To support that, we needed a few extra changes:

  1. Started considering dot a trigger character
  2. When we type in the dot, Prism will consider whatever is the next node to be name of the incomplete method call. We need to know that completion is being triggered by a dot character so that we can search for all available methods instead of trying to extract the name from the AST
  3. Finally, when providing completion with the dot, the range that we're substituting in the text edit is slightly different, which we need to ensure is correct or else the editor rejects all completion candidates

Automated Tests

Added tests for all the features.

Manual Tests

Launch the LSP on this branch and start testing features for things invoked directly on singletons (remember that ancestors are still missing for now!).

singleton_demo.mov

@vinistock vinistock added enhancement New feature or request server This pull request should be included in the server gem's release notes labels Jun 14, 2024
@vinistock vinistock self-assigned this Jun 14, 2024
@vinistock vinistock requested a review from a team as a code owner June 14, 2024 21:00
@vinistock vinistock requested review from andyw8 and st0012 June 14, 2024 21:00
@vinistock vinistock force-pushed the vs/provide_features_on_singletons branch from 6e85194 to f0c87e5 Compare June 14, 2024 21:20
@vinistock vinistock force-pushed the vs/introduce_type_checker branch 2 times, most recently from afa3ff3 to c4a8708 Compare June 17, 2024 19:15
Base automatically changed from vs/introduce_type_checker to main June 17, 2024 21:40
@vinistock vinistock force-pushed the vs/provide_features_on_singletons branch from 89a063f to ea7a450 Compare June 18, 2024 13:47
@response_builder = response_builder
@global_state = global_state
@index = T.let(global_state.index, RubyIndexer::Index)
@type_inferrer = T.let(global_state.type_inferrer, TypeInferrer)
@node_context = node_context
@typechecker_enabled = typechecker_enabled
Copy link
Contributor
@andyw8 andyw8 Jun 18, 2024

Choose a reason for hiding this comment

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

Not for this PR, but I wonder if we can avoid the need to pass typechecker_enabled by instead determining this from type_inferrer, i.e. if type_inferrer is nil that means a typechecker is in use?

Copy link
Member Author

Choose a reason for hiding this comment

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

For now, that would work. However, long term we would like to integrate more deeply with Sorbet and do something like:

  1. First ask Sorbet if it can find the most accurate definition
  2. If a definition was found, use that and do nothing else
  3. If not found, fallback to the Ruby LSP behaviour (we'd need the inferrer here)

@vinistock vinistock merged commit 6172892 into main Jun 18, 2024
37 checks passed
@vinistock vinistock deleted the vs/provide_features_on_singletons branch June 18, 2024 20:33
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.

None yet

3 participants