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

Handle Sorbet levels to prevent some feature duplication #2322

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions lib/ruby_lsp/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ class LanguageId < T::Enum
end
end

class SorbetLevel < T::Enum
vinistock marked this conversation as resolved.
Show resolved Hide resolved
enums do
None = new("none")
Ignore = new("ignore")
False = new("false")
True = new("true")
Strict = new("strict")
end
end

extend T::Sig
extend T::Helpers

Expand Down Expand Up @@ -213,10 +223,23 @@ def locate(node, char_position, node_types: [])
NodeContext.new(closest, parent, nesting_nodes, call_node)
end

sig { returns(T::Boolean) }
def sorbet_sigil_is_true_or_higher
parse_result.magic_comments.any? do |comment|
comment.key == "typed" && ["true", "strict", "strong"].include?(comment.value)
sig { returns(SorbetLevel) }
def sorbet_level
sigil = parse_result.magic_comments.find do |comment|
comment.key == "typed"
end&.value

case sigil
when "ignore"
SorbetLevel::Ignore
when "false"
SorbetLevel::False
when "true"
SorbetLevel::True
when "strict", "strong"
vinistock marked this conversation as resolved.
Show resolved Hide resolved
SorbetLevel::Strict
else
SorbetLevel::None
end
end

Expand Down
76 changes: 46 additions & 30 deletions lib/ruby_lsp/listeners/completion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class Completion
response_builder: ResponseBuilders::CollectionResponseBuilder[Interface::CompletionItem],
global_state: GlobalState,
node_context: NodeContext,
typechecker_enabled: T::Boolean,
sorbet_level: Document::SorbetLevel,
dispatcher: Prism::Dispatcher,
uri: URI::Generic,
trigger_character: T.nilable(String),
Expand All @@ -66,7 +66,7 @@ def initialize( # rubocop:disable Metrics/ParameterLists
response_builder,
global_state,
node_context,
typechecker_enabled,
sorbet_level,
dispatcher,
uri,
trigger_character
Expand All @@ -76,7 +76,7 @@ def initialize( # rubocop:disable Metrics/ParameterLists
@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
@sorbet_level = sorbet_level
@uri = uri
@trigger_character = trigger_character

Expand All @@ -97,7 +97,9 @@ def initialize( # rubocop:disable Metrics/ParameterLists
# Handle completion on regular constant references (e.g. `Bar`)
sig { params(node: Prism::ConstantReadNode).void }
def on_constant_read_node_enter(node)
return if @global_state.has_type_checker
# The only scenario where Sorbet doesn't provide constant completion is on ignored files. Even if the file has
# no sigil, Sorbet will still provide completion for constants
return if @sorbet_level != Document::SorbetLevel::Ignore

name = constant_name(node)
return if name.nil?
Expand All @@ -118,7 +120,9 @@ def on_constant_read_node_enter(node)
# Handle completion on namespaced constant references (e.g. `Foo::Bar`)
sig { params(node: Prism::ConstantPathNode).void }
def on_constant_path_node_enter(node)
return if @global_state.has_type_checker
# The only scenario where Sorbet doesn't provide constant completion is on ignored files. Even if the file has
# no sigil, Sorbet will still provide completion for constants
return if @sorbet_level != Document::SorbetLevel::Ignore

name = constant_name(node)
return if name.nil?
Expand All @@ -128,28 +132,32 @@ def on_constant_path_node_enter(node)

sig { params(node: Prism::CallNode).void }
def on_call_node_enter(node)
receiver = node.receiver

# When writing `Foo::`, the AST assigns a method call node (because you can use that syntax to invoke singleton
# methods). However, in addition to providing method completion, we also need to show possible constant
# completions
if (receiver.is_a?(Prism::ConstantReadNode) || receiver.is_a?(Prism::ConstantPathNode)) &&
node.call_operator == "::"

name = constant_name(receiver)

if name
start_loc = node.location
end_loc = T.must(node.call_operator_loc)

constant_path_completion(
"#{name}::",
Interface::Range.new(
start: Interface::Position.new(line: start_loc.start_line - 1, character: start_loc.start_column),
end: Interface::Position.new(line: end_loc.end_line - 1, character: end_loc.end_column),
),
)
return
# The only scenario where Sorbet doesn't provide constant completion is on ignored files. Even if the file has
# no sigil, Sorbet will still provide completion for constants
if @sorbet_level == Document::SorbetLevel::Ignore
receiver = node.receiver

# When writing `Foo::`, the AST assigns a method call node (because you can use that syntax to invoke
# singleton methods). However, in addition to providing method completion, we also need to show possible
# constant completions
if (receiver.is_a?(Prism::ConstantReadNode) || receiver.is_a?(Prism::ConstantPathNode)) &&
node.call_operator == "::"

name = constant_name(receiver)

if name
start_loc = node.location
end_loc = T.must(node.call_operator_loc)

constant_path_completion(
"#{name}::",
Interface::Range.new(
start: Interface::Position.new(line: start_loc.start_line - 1, character: start_loc.start_column),
end: Interface::Position.new(line: end_loc.end_line - 1, character: end_loc.end_column),
),
)
return
end
end
end

Expand All @@ -162,7 +170,7 @@ def on_call_node_enter(node)
when "require_relative"
complete_require_relative(node)
else
complete_methods(node, name) unless @typechecker_enabled
complete_methods(node, name)
end
end

Expand Down Expand Up @@ -247,6 +255,10 @@ def constant_path_completion(name, range)

sig { params(name: String, location: Prism::Location).void }
def handle_instance_variable_completion(name, location)
# Sorbet enforces that all instance variables be declared on typed strict or higher, which means it will be able
# to provide all features for them
return if @sorbet_level == Document::SorbetLevel::Strict

type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

Expand Down Expand Up @@ -321,12 +333,16 @@ def complete_require_relative(node)

sig { params(node: Prism::CallNode, name: String).void }
def complete_methods(node, name)
# If the node has a receiver, then we don't need to provide local nor keyword completions
if !@global_state.has_type_checker && !node.receiver
# If the node has a receiver, then we don't need to provide local nor keyword completions. Sorbet can provide
# local and keyword completion for any file with a Sorbet level of true or higher
if !sorbet_level_true_or_higher?(@sorbet_level) && !node.receiver
add_local_completions(node, name)
add_keyword_completions(node, name)
end

# Sorbet can provide completion for methods invoked on self on typed true or higher files
return if sorbet_level_true_or_higher?(@sorbet_level) && self_receiver?(node)

type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

Expand Down
26 changes: 19 additions & 7 deletions lib/ruby_lsp/listeners/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@ class Definition
uri: URI::Generic,
node_context: NodeContext,
dispatcher: Prism::Dispatcher,
typechecker_enabled: T::Boolean,
sorbet_level: Document::SorbetLevel,
).void
end
def initialize(response_builder, global_state, language_id, uri, node_context, dispatcher, typechecker_enabled) # rubocop:disable Metrics/ParameterLists
def initialize(response_builder, global_state, language_id, uri, node_context, dispatcher, sorbet_level) # rubocop:disable Metrics/ParameterLists
@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)
@language_id = language_id
@uri = uri
@node_context = node_context
@typechecker_enabled = typechecker_enabled
@sorbet_level = sorbet_level

dispatcher.register(
self,
Expand All @@ -53,6 +53,11 @@ def initialize(response_builder, global_state, language_id, uri, node_context, d

sig { params(node: Prism::CallNode).void }
def on_call_node_enter(node)
# Sorbet can handle go to definition for methods invoked on self on typed true or higher
if (@sorbet_level == Document::SorbetLevel::True || @sorbet_level == Document::SorbetLevel::Strict) &&
self_receiver?(node)
end

message = node.message
return unless message

Expand Down Expand Up @@ -149,6 +154,9 @@ def on_forwarding_super_node_enter(node)

sig { void }
def handle_super_node_definition
# Sorbet can handle super hover on typed true or higher
return if sorbet_level_true_or_higher?(@sorbet_level)

surrounding_method = @node_context.surrounding_method
return unless surrounding_method

Expand All @@ -161,6 +169,10 @@ def handle_super_node_definition

sig { params(name: String).void }
def handle_instance_variable_definition(name)
# Sorbet enforces that all instance variables be declared on typed strict or higher, which means it will be able
# to provide all features for them
return if @sorbet_level == Document::SorbetLevel::Strict
st0012 marked this conversation as resolved.
Show resolved Hide resolved

type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

Expand Down Expand Up @@ -196,7 +208,7 @@ def handle_method_definition(message, receiver_type, inherited_only: false)

methods.each do |target_method|
file_path = target_method.file_path
next if @typechecker_enabled && not_in_dependencies?(file_path)
next if sorbet_level_true_or_higher?(@sorbet_level) && not_in_dependencies?(file_path)

@response_builder << Interface::LocationLink.new(
target_uri: URI::Generic.from_path(path: file_path).to_s,
Expand Down Expand Up @@ -253,10 +265,10 @@ def find_in_index(value)

entries.each do |entry|
# If the project has Sorbet, then we only want to handle go to definition for constants defined in gems, as an
# additional behavior on top of jumping to RBIs. Sorbet can already handle go to definition for all constants
# in the project, even if the files are typed false
# additional behavior on top of jumping to RBIs. The only sigil where Sorbet cannot handle constants is typed
# ignore
file_path = entry.file_path
next if @typechecker_enabled && not_in_dependencies?(file_path)
next if @sorbet_level != Document::SorbetLevel::Ignore && not_in_dependencies?(file_path)

@response_builder << Interface::LocationLink.new(
target_uri: URI::Generic.from_path(path: file_path).to_s,
Expand Down
21 changes: 14 additions & 7 deletions lib/ruby_lsp/listeners/hover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,17 @@ class Hover
uri: URI::Generic,
node_context: NodeContext,
dispatcher: Prism::Dispatcher,
typechecker_enabled: T::Boolean,
sorbet_level: Document::SorbetLevel,
).void
end
def initialize(response_builder, global_state, uri, node_context, dispatcher, typechecker_enabled) # rubocop:disable Metrics/ParameterLists
def initialize(response_builder, global_state, uri, node_context, dispatcher, sorbet_level) # rubocop:disable Metrics/ParameterLists
@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)
@path = T.let(uri.to_standardized_path, T.nilable(String))
@node_context = node_context
@typechecker_enabled = typechecker_enabled
@sorbet_level = sorbet_level

dispatcher.register(
self,
Expand All @@ -73,7 +73,7 @@ def initialize(response_builder, global_state, uri, node_context, dispatcher, ty

sig { params(node: Prism::ConstantReadNode).void }
def on_constant_read_node_enter(node)
return if @typechecker_enabled
return if @sorbet_level != Document::SorbetLevel::Ignore

name = constant_name(node)
return if name.nil?
Expand All @@ -83,14 +83,14 @@ def on_constant_read_node_enter(node)

sig { params(node: Prism::ConstantWriteNode).void }
def on_constant_write_node_enter(node)
return if @global_state.has_type_checker
return if @sorbet_level != Document::SorbetLevel::Ignore

generate_hover(node.name.to_s, node.name_loc)
end

sig { params(node: Prism::ConstantPathNode).void }
def on_constant_path_node_enter(node)
return if @global_state.has_type_checker
return if @sorbet_level != Document::SorbetLevel::Ignore

name = constant_name(node)
return if name.nil?
Expand All @@ -105,7 +105,7 @@ def on_call_node_enter(node)
return
end

return if @typechecker_enabled
return if sorbet_level_true_or_higher?(@sorbet_level) && self_receiver?(node)

message = node.message
return unless message
Expand Down Expand Up @@ -157,6 +157,9 @@ def on_forwarding_super_node_enter(node)

sig { void }
def handle_super_node_hover
# Sorbet can handle super hover on typed true or higher
return if sorbet_level_true_or_higher?(@sorbet_level)

surrounding_method = @node_context.surrounding_method
return unless surrounding_method

Expand All @@ -180,6 +183,10 @@ def handle_method_hover(message, inherited_only: false)

sig { params(name: String).void }
def handle_instance_variable_hover(name)
# Sorbet enforces that all instance variables be declared on typed strict or higher, which means it will be able
# to provide all features for them
return if @sorbet_level == Document::SorbetLevel::Strict
st0012 marked this conversation as resolved.
Show resolved Hide resolved

type = @type_inferrer.infer_receiver_type(@node_context)
return unless type

Expand Down
8 changes: 4 additions & 4 deletions lib/ruby_lsp/listeners/signature_help.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ class SignatureHelp
global_state: GlobalState,
node_context: NodeContext,
dispatcher: Prism::Dispatcher,
typechecker_enabled: T::Boolean,
sorbet_level: Document::SorbetLevel,
).void
end
def initialize(response_builder, global_state, node_context, dispatcher, typechecker_enabled)
@typechecker_enabled = typechecker_enabled
def initialize(response_builder, global_state, node_context, dispatcher, sorbet_level)
@sorbet_level = sorbet_level
@response_builder = response_builder
@global_state = global_state
@index = T.let(global_state.index, RubyIndexer::Index)
Expand All @@ -28,7 +28,7 @@ def initialize(response_builder, global_state, node_context, dispatcher, typeche

sig { params(node: Prism::CallNode).void }
def on_call_node_enter(node)
return if @typechecker_enabled
return if sorbet_level_true_or_higher?(@sorbet_level)

message = node.message
return unless message
Expand Down
6 changes: 3 additions & 3 deletions lib/ruby_lsp/requests/completion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ def provider
document: Document,
global_state: GlobalState,
params: T::Hash[Symbol, T.untyped],
typechecker_enabled: T::Boolean,
sorbet_level: Document::SorbetLevel,
dispatcher: Prism::Dispatcher,
).void
end
def initialize(document, global_state, params, typechecker_enabled, dispatcher)
def initialize(document, global_state, params, sorbet_level, dispatcher)
super()
@target = T.let(nil, T.nilable(Prism::Node))
@dispatcher = dispatcher
Expand Down Expand Up @@ -84,7 +84,7 @@ def initialize(document, global_state, params, typechecker_enabled, dispatcher)
@response_builder,
global_state,
node_context,
typechecker_enabled,
sorbet_level,
dispatcher,
document.uri,
params.dig(:context, :triggerCharacter),
Expand Down
Loading
Loading