Skip to content

Commit

Permalink
Refactor RuboCop diagnostics for better encapsulation
Browse files Browse the repository at this point in the history
The code description addition for RuboCop diagnostics has added a cop lookup to the diagnostics runner, but I feel like that was an abstraction added at the wrong layer.

What we really want to cache is the registry as a hash lookup but we want to do that as late as possible to ensure that we have loaded the full registry. For that reason, I moved the actual cop lookup by cop name to the RuboCop runner class as a class method, which uses a memoized registry hash under the hood.

This way, we can keep the code of the RuboCop diagnostics class clean and with less conditional code.

I've also taken the liberty to extract other calculation logic from inside the `to_lsp_diagnostic` method to separate methods, to make the intent more clear.
  • Loading branch information
paracycle committed Oct 18, 2023
1 parent 51593f4 commit 8e1bc9e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 16 deletions.
34 changes: 18 additions & 16 deletions lib/ruby_lsp/requests/support/rubocop_diagnostic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ class RuboCopDiagnostic
T::Hash[Symbol, Integer],
)

# Cache cops to attach URLs to diagnostics. Only built-in cops for now.
COP_TO_DOC_URL = T.let(
RuboCop::Cop::Registry.global.to_h,
T::Hash[String, [T.class_of(RuboCop::Cop::Base)]],
)

sig { params(offense: RuboCop::Cop::Offense, uri: URI::Generic).void }
def initialize(offense, uri)
@offense = offense
Expand Down Expand Up @@ -53,16 +47,6 @@ def to_lsp_code_action

sig { returns(Interface::Diagnostic) }
def to_lsp_diagnostic
severity = RUBOCOP_TO_LSP_SEVERITY[@offense.severity.name]
message = @offense.message

message += "\n\nThis offense is not auto-correctable.\n" unless @offense.correctable?

cop = COP_TO_DOC_URL[@offense.cop_name]&.first
if cop&.documentation_url
code_description = { href: cop.documentation_url }
end

Interface::Diagnostic.new(
message: message,
source: "RuboCop",
Expand All @@ -88,6 +72,24 @@ def to_lsp_diagnostic

private

sig { returns(String) }
def message
message = @offense.message
message += "\n\nThis offense is not auto-correctable.\n" unless @offense.correctable?
message
end

sig { returns(T.nilable(Integer)) }
def severity
RUBOCOP_TO_LSP_SEVERITY[@offense.severity.name]
end

sig { returns(T.nilable(Interface::CodeDescription)) }
def code_description
doc_url = RuboCopRunner.find_cop_by_name(@offense.cop_name)&.documentation_url
Interface::CodeDescription.new(href: doc_url) if doc_url
end

sig { returns(T::Array[Interface::TextEdit]) }
def offense_replacements
@offense.corrector.as_replacements.map do |range, replacement|
Expand Down
19 changes: 19 additions & 0 deletions lib/ruby_lsp/requests/support/rubocop_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,25 @@ def formatted_source
@options[:stdin]
end

class << self
extend T::Sig

sig { params(cop_name: String).returns(T.nilable(T.class_of(RuboCop::Cop::Base))) }
def find_cop_by_name(cop_name)
cop_registry[cop_name]&.first
end

private

sig { returns(T::Hash[String, [T.class_of(RuboCop::Cop::Base)]]) }
def cop_registry
@cop_registry ||= T.let(
RuboCop::Cop::Registry.global.to_h,
T.nilable(T::Hash[String, [T.class_of(RuboCop::Cop::Base)]]),
)
end
end

private

sig { params(_file: String, offenses: T::Array[RuboCop::Cop::Offense]).void }
Expand Down

0 comments on commit 8e1bc9e

Please sign in to comment.