From e4fe4d1e7dcef28cdb80f732092dc84dded31bb5 Mon Sep 17 00:00:00 2001 From: Justin Searls Date: Sun, 23 Jun 2024 21:58:01 -0400 Subject: [PATCH] Spike out getting code actions working under Ruby LSP Confirm this works locally --- lib/ruby_lsp/standard/addon.rb | 1 + .../wraps_built_in_lsp_standardizer.rb | 63 +------ lib/standard/lsp/diagnostic.rb | 168 ++++++++++++++++++ lib/standard/lsp/server.rb | 1 + lib/standard/lsp/standardizer.rb | 73 +++----- lib/standard/lsp/stdin_rubocop_runner.rb | 61 +++++++ test/ruby_lsp_addon_test.rb | 2 +- 7 files changed, 254 insertions(+), 115 deletions(-) create mode 100644 lib/standard/lsp/diagnostic.rb create mode 100644 lib/standard/lsp/stdin_rubocop_runner.rb diff --git a/lib/ruby_lsp/standard/addon.rb b/lib/ruby_lsp/standard/addon.rb index 031bfe29..92883b8a 100644 --- a/lib/ruby_lsp/standard/addon.rb +++ b/lib/ruby_lsp/standard/addon.rb @@ -14,6 +14,7 @@ def name def activate(global_state, message_queue) warn "Activating Standard Ruby LSP addon v#{::Standard::VERSION}" + RuboCop::LSP.enable @wraps_built_in_lsp_standardizer = WrapsBuiltinLspStandardizer.new global_state.register_formatter("standard", @wraps_built_in_lsp_standardizer) register_additional_file_watchers(global_state, message_queue) diff --git a/lib/ruby_lsp/standard/wraps_built_in_lsp_standardizer.rb b/lib/ruby_lsp/standard/wraps_built_in_lsp_standardizer.rb index 60012a2a..075d922b 100644 --- a/lib/ruby_lsp/standard/wraps_built_in_lsp_standardizer.rb +++ b/lib/ruby_lsp/standard/wraps_built_in_lsp_standardizer.rb @@ -8,12 +8,11 @@ def initialize def init! @config = ::Standard::BuildsConfig.new.call([]) + @standardizer = ::Standard::Lsp::Standardizer.new( @config, ::Standard::Lsp::Logger.new ) - @rubocop_config = @config.rubocop_config_store.for_pwd - @cop_registry = RuboCop::Cop::Registry.global.to_h end def run_formatting(uri, document) @@ -21,55 +20,7 @@ def run_formatting(uri, document) end def run_diagnostic(uri, document) - offenses = @standardizer.offenses(uri_to_path(uri), document.source) - - offenses.map { |o| - cop_name = o[:cop_name] - - msg = o[:message].delete_prefix(cop_name) - loc = o[:location] - - severity = case o[:severity] - when "error", "fatal" - RubyLsp::Constant::DiagnosticSeverity::ERROR - when "warning" - RubyLsp::Constant::DiagnosticSeverity::WARNING - when "convention" - RubyLsp::Constant::DiagnosticSeverity::INFORMATION - when "refactor", "info" - RubyLsp::Constant::DiagnosticSeverity::HINT - else # the above cases fully cover what RuboCop sends at this time - logger.puts "Unknown severity: #{severity.inspect}" - RubyLsp::Constant::DiagnosticSeverity::HINT - end - - RubyLsp::Interface::Diagnostic.new( - code: cop_name, - code_description: code_description(cop_name), - message: msg, - source: "Standard Ruby", - severity: severity, - range: RubyLsp::Interface::Range.new( - start: RubyLsp::Interface::Position.new(line: loc[:start_line] - 1, character: loc[:start_column] - 1), - end: RubyLsp::Interface::Position.new(line: loc[:last_line] - 1, character: loc[:last_column]) - ) - # TODO: We need to do something like to support quickfixes thru code actions - # See: https://github.com/Shopify/ruby-lsp/blob/4c1906172add4d5c39c35d3396aa29c768bfb898/lib/ruby_lsp/requests/support/rubocop_diagnostic.rb#L62 - # data: { - # correctable: correctable?(offense), - # code_actions: to_lsp_code_actions - # } - # - # Right now, our offenses are all just JSON parsed from stdout shelling to RuboCop, so - # it seems we don't have the corrector available to us. - # - # Lifted from: - # https://github.com/Shopify/ruby-lsp/blob/8d4c17efce4e8ecc8e7c557ab2981db6b22c0b6d/lib/ruby_lsp/requests/support/rubocop_diagnostic.rb#L201 - # def correctable?(offense) - # !offense.corrector.nil? - # end - ) - } + @standardizer.offenses(uri_to_path(uri), document.source, document.encoding) end private @@ -83,16 +34,6 @@ def uri_to_path(uri) uri.to_s.sub(%r{^file://}, "") end end - - # lifted from: - # https://github.com/Shopify/ruby-lsp/blob/4c1906172add4d5c39c35d3396aa29c768bfb898/lib/ruby_lsp/requests/support/rubocop_diagnostic.rb#L84 - def code_description(cop_name) - if (cop_class = @cop_registry[cop_name]&.first) - if (doc_url = cop_class.documentation_url(@rubocop_config)) - Interface::CodeDescription.new(href: doc_url) - end - end - end end end end diff --git a/lib/standard/lsp/diagnostic.rb b/lib/standard/lsp/diagnostic.rb new file mode 100644 index 00000000..77f86387 --- /dev/null +++ b/lib/standard/lsp/diagnostic.rb @@ -0,0 +1,168 @@ +module Standard + module Lsp + class Diagnostic + Constant = LanguageServer::Protocol::Constant + Interface = LanguageServer::Protocol::Interface + + RUBOCOP_TO_LSP_SEVERITY = { + info: Constant::DiagnosticSeverity::HINT, + refactor: Constant::DiagnosticSeverity::INFORMATION, + convention: Constant::DiagnosticSeverity::INFORMATION, + warning: Constant::DiagnosticSeverity::WARNING, + error: Constant::DiagnosticSeverity::ERROR, + fatal: Constant::DiagnosticSeverity::ERROR + }.freeze + + def initialize(document_encoding, offense, uri, cop_class) + @document_encoding = document_encoding + @offense = offense + @uri = uri + @cop_class = cop_class + end + + def to_lsp_code_actions + code_actions = [] + + code_actions << autocorrect_action if correctable? + code_actions << disable_line_action + + code_actions + end + + def to_lsp_diagnostic(config) + highlighted = @offense.highlighted_area + Interface::Diagnostic.new( + message: message, + source: "Standard Ruby", + code: @offense.cop_name, + code_description: code_description(config), + severity: severity, + range: Interface::Range.new( + start: Interface::Position.new( + line: @offense.line - 1, + character: highlighted.begin_pos + ), + end: Interface::Position.new( + line: @offense.line - 1, + character: highlighted.end_pos + ) + ), + data: { + correctable: correctable?, + code_actions: to_lsp_code_actions + } + ) + end + + private + + def message + message = @offense.message + message += "\n\nThis offense is not auto-correctable.\n" unless correctable? + message + end + + def severity + RUBOCOP_TO_LSP_SEVERITY[@offense.severity.name] + end + + def code_description(config) + return unless @cop_class + + if (doc_url = @cop_class.documentation_url(config)) + Interface::CodeDescription.new(href: doc_url) + end + end + + def autocorrect_action + Interface::CodeAction.new( + title: "Autocorrect #{@offense.cop_name}", + kind: Constant::CodeActionKind::QUICK_FIX, + edit: Interface::WorkspaceEdit.new( + document_changes: [ + Interface::TextDocumentEdit.new( + text_document: Interface::OptionalVersionedTextDocumentIdentifier.new( + uri: @uri.to_s, + version: nil + ), + edits: correctable? ? offense_replacements : [] + ) + ] + ), + is_preferred: true + ) + end + + def offense_replacements + @offense.corrector.as_replacements.map do |range, replacement| + Interface::TextEdit.new( + range: Interface::Range.new( + start: Interface::Position.new(line: range.line - 1, character: range.column), + end: Interface::Position.new(line: range.last_line - 1, character: range.last_column) + ), + new_text: replacement + ) + end + end + + def disable_line_action + Interface::CodeAction.new( + title: "Disable #{@offense.cop_name} for this line", + kind: Constant::CodeActionKind::QUICK_FIX, + edit: Interface::WorkspaceEdit.new( + document_changes: [ + Interface::TextDocumentEdit.new( + text_document: Interface::OptionalVersionedTextDocumentIdentifier.new( + uri: @uri.to_s, + version: nil + ), + edits: line_disable_comment + ) + ] + ) + ) + end + + def line_disable_comment + new_text = if @offense.source_line.include?(" # standard:disable ") + ",#{@offense.cop_name}" + else + " # standard:disable #{@offense.cop_name}" + end + + eol = Interface::Position.new( + line: @offense.line - 1, + character: length_of_line(@offense.source_line) + ) + + # TODO: fails for multiline strings - may be preferable to use block + # comments to disable some offenses + inline_comment = Interface::TextEdit.new( + range: Interface::Range.new(start: eol, end: eol), + new_text: new_text + ) + + [inline_comment] + end + + def length_of_line(line) + if @document_encoding == Encoding::UTF_16LE + line_length = 0 + line.codepoints.each do |codepoint| + line_length += 1 + if codepoint > RubyLsp::Document::Scanner::SURROGATE_PAIR_START + line_length += 1 + end + end + line_length + else + line.length + end + end + + def correctable? + !@offense.corrector.nil? + end + end + end +end diff --git a/lib/standard/lsp/server.rb b/lib/standard/lsp/server.rb index 0e049bd5..828f00c5 100644 --- a/lib/standard/lsp/server.rb +++ b/lib/standard/lsp/server.rb @@ -18,6 +18,7 @@ def initialize(config) end def start + RuboCop::LSP.enable @reader.read do |request| if !request.key?(:method) @routes.handle_method_missing(request) diff --git a/lib/standard/lsp/standardizer.rb b/lib/standard/lsp/standardizer.rb index 16bb85b2..30e1713a 100644 --- a/lib/standard/lsp/standardizer.rb +++ b/lib/standard/lsp/standardizer.rb @@ -1,68 +1,35 @@ -require_relative "../runners/rubocop" +require_relative "stdin_rubocop_runner" +require_relative "diagnostic" module Standard module Lsp class Standardizer def initialize(config, logger) - @config = config - @logger = logger - @rubocop_runner = Standard::Runners::Rubocop.new - end + @diagnostic_runner = ::Standard::Lsp::StdinRubocopRunner.new(config) + @format_runner = ::Standard::Lsp::StdinRubocopRunner.new(config.dup.tap { |c| + c.rubocop_options[:autocorrect] = true + }) - # This abuses the --stdin option of rubocop and reads the formatted text - # from the options[:stdin] that rubocop mutates. This depends on - # parallel: false as well as the fact that rubocop doesn't otherwise dup - # or reassign that options object. Risky business! - # - # Reassigning options[:stdin] is done here: - # https://github.com/rubocop/rubocop/blob/master/lib/rubocop/cop/team.rb#L131 - # Printing options[:stdin] - # https://github.com/rubocop/rubocop/blob/master/lib/rubocop/cli/command/execute_runner.rb#L95 - # Setting `parallel: true` would break this here: - # https://github.com/rubocop/rubocop/blob/master/lib/rubocop/runner.rb#L72 - def format(path, text) - ad_hoc_config = fork_config(path, text, format: true) - capture_rubocop_stdout(ad_hoc_config) - ad_hoc_config.rubocop_options[:stdin] + @logger = logger # TODO: delete if no longer needed for anything. + @cop_registry = RuboCop::Cop::Registry.global.to_h end - def offenses(path, text) - results = JSON.parse( - capture_rubocop_stdout(fork_config(path, text, format: false)), - symbolize_names: true - ) - if results[:files].empty? - @logger.puts_once "Ignoring file, per configuration: #{path}" - [] - else - results.dig(:files, 0, :offenses) - end + def format(path, text) + @format_runner.run(path, text) + @format_runner.formatted_source end - private + def offenses(path, text, document_encoding = nil) + @diagnostic_runner.run(path, text) - BASE_OPTIONS = { - force_exclusion: true, - parallel: false, - todo_file: nil, - todo_ignore_files: [] - } - def fork_config(path, text, format:) - options = if format - {stdin: text, autocorrect: true, safe_autocorrect: true, formatters: []} - else - {stdin: text, autocorrect: false, safe_autocorrect: false, formatters: [["json"]], format: "json"} + @diagnostic_runner.offenses.map do |offense| + Diagnostic.new( + document_encoding, + offense, + path, + @cop_registry[offense.cop_name]&.first + ).to_lsp_diagnostic(@diagnostic_runner.config_for_working_directory) end - Standard::Config.new(@config.runner, [path], BASE_OPTIONS.merge(options), @config.rubocop_config_store) - end - - def capture_rubocop_stdout(config) - redir = StringIO.new - $stdout = redir - @rubocop_runner.call(config) - redir.string - ensure - $stdout = STDOUT end end end diff --git a/lib/standard/lsp/stdin_rubocop_runner.rb b/lib/standard/lsp/stdin_rubocop_runner.rb new file mode 100644 index 00000000..34bd989c --- /dev/null +++ b/lib/standard/lsp/stdin_rubocop_runner.rb @@ -0,0 +1,61 @@ +module Standard + module Lsp + # Originally lifted from: + # https://github.com/Shopify/ruby-lsp/blob/8d4c17efce4e8ecc8e7c557ab2981db6b22c0b6d/lib/ruby_lsp/requests/support/rubocop_runner.rb#L20 + class StdinRubocopRunner < ::RuboCop::Runner + class ConfigurationError < StandardError; end + + attr_reader :offenses + + attr_reader :config_for_working_directory + + DEFAULT_RUBOCOP_OPTIONS = { + stderr: true, + force_exclusion: true, + format: "RuboCop::Formatter::BaseFormatter", + raise_cop_error: true + }.freeze + + def initialize(config) + @options = {} + @offenses = [] + @errors = [] + @warnings = [] + + @config_for_working_directory = config.rubocop_config_store.for_pwd + + super( + config.rubocop_options.merge(DEFAULT_RUBOCOP_OPTIONS), + config.rubocop_config_store + ) + end + + def run(path, contents) + @errors = [] + @warnings = [] + @offenses = [] + @options[:stdin] = contents + + super([path]) + + raise Interrupt if aborting? + rescue ::RuboCop::Runner::InfiniteCorrectionLoop => error + raise RubyLsp::Requests::Formatting::Erro, error.message + rescue ::RuboCop::ValidationError => error + raise ConfigurationError, error.message + rescue => error + raise ::RubyLsp::Requests::Support::InternalRuboCopError, error + end + + def formatted_source + @options[:stdin] + end + + private + + def file_finished(_file, offenses) + @offenses = offenses + end + end + end +end diff --git a/test/ruby_lsp_addon_test.rb b/test/ruby_lsp_addon_test.rb index 8c1f12d3..557fcd4f 100644 --- a/test/ruby_lsp_addon_test.rb +++ b/test/ruby_lsp_addon_test.rb @@ -56,7 +56,7 @@ def test_diagnostic assert_equal "Style/StringLiterals", item.code assert_equal "https://docs.rubocop.org/rubocop/cops_style.html#stylestringliterals", item.code_description.href assert_equal "Standard Ruby", item.source - assert_equal "Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.", item.message + assert_equal "Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.", item.message end end