Skip to content

Commit

Permalink
Rework standardizer to use --stdin instead of creating temp files (te…
Browse files Browse the repository at this point in the history
…st is almost twice as fast)

Running lsp_test.rb after this commit: 0.075s - 0.085s
Running before this commit: 0.129s - 0.141s
  • Loading branch information
searls committed Feb 11, 2023
1 parent acfaaab commit aa0b078
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 29 deletions.
9 changes: 7 additions & 2 deletions lib/standard/lsp/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,18 @@ def handle_method_missing(request)

private

def uri_to_path(uri)
# TODO: make this real
uri.sub(%r{^file:///}, "")
end

def format_file(file_uri)
text = @text_cache[file_uri]
if text.nil?
@logger.puts "Format request arrived before text synchonized; skipping: `#{file_uri}'"
[]
else
new_text = @standardizer.format(text)
new_text = @standardizer.format(uri_to_path(file_uri), text)
if new_text == text
[]
else
Expand All @@ -153,7 +158,7 @@ def format_file(file_uri)

def diagnostic(file_uri, text)
@text_cache[file_uri] = text
offenses = @standardizer.offenses(text)
offenses = @standardizer.offenses(uri_to_path(file_uri), text)

lsp_diagnostics = offenses.map { |o|
code = o[:cop_name]
Expand Down
48 changes: 25 additions & 23 deletions lib/standard/lsp/standardizer.rb
Original file line number Diff line number Diff line change
@@ -1,49 +1,51 @@
require_relative "../runners/rubocop"
require "tempfile"

module Standard
module Lsp
class Standardizer
def initialize(config)
@template_options = config
@runner = Standard::Runners::Rubocop.new
@config = config
@rubocop_runner = Standard::Runners::Rubocop.new
end

def format(text)
run_standard(text, format: 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]
end

def offenses(text)
results = run_standard(text, format: false)
def offenses(path, text)
results = capture_rubocop_stdout(fork_config(path, text, format: false))
JSON.parse(results, symbolize_names: true).dig(:files, 0, :offenses)
end

private

BASENAME = ["source", ".rb"].freeze
def run_standard(text, format:)
Tempfile.open(BASENAME) do |temp|
temp.write(text)
temp.flush
stdout = capture_rubocop_stdout(make_config(temp.path, format))
format ? File.read(temp.path) : stdout
end
end

def make_config(file, format)
# Can't make frozen versions of this hash because RuboCop mutates it
# Can't make frozen versions of this hash because RuboCop mutates it
def fork_config(path, text, format:)
o = if format
{autocorrect: true, formatters: [["Standard::Formatter", nil]], parallel: true, todo_file: nil, todo_ignore_files: [], safe_autocorrect: true}
{stdin: text, autocorrect: true, formatters: [["Standard::Formatter", nil]], parallel: false, todo_file: nil, todo_ignore_files: [], safe_autocorrect: true}
else
{autocorrect: false, formatters: [["json"]], parallel: true, todo_file: nil, todo_ignore_files: [], format: "json"}
{stdin: text, autocorrect: false, formatters: [["json"]], parallel: false, todo_file: nil, todo_ignore_files: [], format: "json"}
end
Standard::Config.new(@template_options.runner, [file], o, @template_options.rubocop_config_store)
Standard::Config.new(@config.runner, [path], o, @config.rubocop_config_store)
end

def capture_rubocop_stdout(config)
redir = StringIO.new
$stdout = redir
@runner.call(config)
@rubocop_runner.call(config)
redir.string
ensure
$stdout = STDOUT
Expand Down
8 changes: 4 additions & 4 deletions test/standard/runners/lsp_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def test_format
params: {
textDocument: {
languageId: "ruby",
text: "def hi\n [1, 2,\n 3 ]\nend\n",
text: "puts 'hi'",
uri: "file:///path/to/file.rb",
version: 0
}
Expand All @@ -127,7 +127,7 @@ def test_format
method: "textDocument/didChange",
jsonrpc: "2.0",
params: {
contentChanges: [{text: "def goodbye\n [3, 2,\n 1 ]\n\nend\n"}],
contentChanges: [{text: "puts 'bye'"}],
textDocument: {
uri: "file:///path/to/file.rb",
version: 10
Expand All @@ -151,10 +151,10 @@ def test_format
{
id: 20,
result: [
{newText: "def goodbye\n [3, 2,\n 1]\nend\n",
{newText: "puts \"bye\"\n",
range: {
start: {line: 0, character: 0},
end: {line: 6, character: 0}
end: {line: 1, character: 0}
}}
],
jsonrpc: "2.0"
Expand Down

0 comments on commit aa0b078

Please sign in to comment.