Skip to content

Commit

Permalink
Simplify and allow to configure rubocop formatter (#403)
Browse files Browse the repository at this point in the history
* fix(formatter): avoid temporary files when formatting with RuboCop

By supplying the file content to RubyCop as a in-memory string the same
way as RuboCop's own CLI tool handles STDIN, we completely avoid having
to write any temporary files to disk, and formatting can occur purely
in-memory.

This also has the benefit of Solargraph no longer needing to be
responsible for finding and passing along any config file paths. RuboCop
will correctly find any relevant .rubocop.yml files for the file being
formatted, as RuboCop behaves exactly as if it was running against the
real file in it's original location on disk, except with the content it
is formatting coming from the in-memory string rather than reading it
from the file on disk.

A new blank formatter is also passed to RuboCop, just to make it print
as little as possible to STDOUT, since we don't need and ignore the
STDOUT produced.

* feat(formatter): add "formatter" option to .solargraph.yml

Allows customizing if format command should run safe, or all cops when
formatting files.

Also includes a `extra_args` option to pass custom CLI arguments to
RuboCop. If you primarily only format via solargraph interactively from
an editor, it might be useful to disable certain cops that causes
annoyances in interactive use. For example:

    formatter:
      cops: safe # use "all" to include unsafe cops
      extra_args:
        - --except
        - Lint/Debugger,Lint/UnusedBlockArgument,Lint/UnusedMethodArgument,Style/EmptyMethod

* chore(diagnostics): don't pass .rubocop.yml file path to RuboCop

RuboCop will itself find and use any relevant configuration files based
on the file path of the file it is analyzing. Hence there is no need for
Solargraph to have it's own implementation of this logic.

* chore(formatter): future proof rubocop formatter configuration structure

Solargraph will gain support for multiple formatters soon, so let's
ensure configuration for the rubocop formatter is future proof.

New nested configuration:

    formatter:
      rubocop:
        cops: all
        extra_args: []

Previously:

    formatter:
      cops: all
      extra_args: []

* feat(formatter): add "except" and "only" rubocop formatter options

They take a list or comma separated string of cops, which will be passed
to rubocop as --except and/or --only.

The "except" option is useful to disable cops which can cause annoyance
when using solargraph's formatter as a before save hook in text
editors.

I'm less aware of what the "only" option might be useful for, but might
as well add it so both are covered.
  • Loading branch information
jimeh committed Jan 28, 2021
1 parent 3dda2df commit d9d2abf
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 45 deletions.
21 changes: 1 addition & 20 deletions lib/solargraph/diagnostics/rubocop_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,13 @@ module RubocopHelpers
# @param code [String]
# @return [Array(Array<String>, Array<String>)]
def generate_options filename, code
args = ['-f', 'j']
rubocop_file = find_rubocop_file(filename)
args.push('-c', fix_drive_letter(rubocop_file)) unless rubocop_file.nil?
args.push filename
args = ['-f', 'j', filename]
base_options = RuboCop::Options.new
options, paths = base_options.parse(args)
options[:stdin] = code
[options, paths]
end

# Find a RuboCop configuration file in a file's directory tree.
#
# @param filename [String]
# @return [String, nil]
def find_rubocop_file filename
return nil unless File.exist?(filename)
filename = File.realpath(filename)
dir = File.dirname(filename)
until File.dirname(dir) == dir
here = File.join(dir, '.rubocop.yml')
return here if File.exist?(here)
dir = File.dirname(dir)
end
nil
end

# RuboCop internally uses capitalized drive letters for Windows paths,
# so we need to convert the paths provided to the command.
#
Expand Down
5 changes: 5 additions & 0 deletions lib/solargraph/language_server/host.rb
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,11 @@ def read_text uri
library.read_text(filename)
end

def formatter_config uri
library = library_for(uri)
library.workspace.config.formatter
end

# @param uri [String]
# @param line [Integer]
# @param column [Integer]
Expand Down
63 changes: 46 additions & 17 deletions lib/solargraph/language_server/message/text_document/formatting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,58 @@ module TextDocument
class Formatting < Base
include Solargraph::Diagnostics::RubocopHelpers

class BlankRubocopFormatter < ::RuboCop::Formatter::BaseFormatter; end

def process
filename = uri_to_file(params['textDocument']['uri'])
Dir.mktmpdir do |tempdir|
tempfile = File.join(tempdir, File.basename(filename))
rubocop_file = Diagnostics::RubocopHelpers.find_rubocop_file(filename)
original = host.read_text(params['textDocument']['uri'])
File.write tempfile, original
begin
args = ['-a', '-f', 'fi', tempfile]
args.unshift('-c', fix_drive_letter(rubocop_file)) unless rubocop_file.nil?
options, paths = RuboCop::Options.new.parse(args)
store = RuboCop::ConfigStore.new
redirect_stdout { RuboCop::Runner.new(options, store).run(paths) }
result = File.read(tempfile)
format original, result
rescue RuboCop::ValidationError, RuboCop::ConfigNotFoundError => e
set_error(Solargraph::LanguageServer::ErrorCodes::INTERNAL_ERROR, "[#{e.class}] #{e.message}")
end
file_uri = params['textDocument']['uri']
config = config_for(file_uri)
original = host.read_text(file_uri)
args = cli_args(file_uri, config)

options, paths = RuboCop::Options.new.parse(args)
options[:stdin] = original
redirect_stdout do
RuboCop::Runner.new(options, RuboCop::ConfigStore.new).run(paths)
end
result = options[:stdin]

format original, result
rescue RuboCop::ValidationError, RuboCop::ConfigNotFoundError => e
set_error(Solargraph::LanguageServer::ErrorCodes::INTERNAL_ERROR, "[#{e.class}] #{e.message}")
end

private

def config_for(file_uri)
conf = host.formatter_config(file_uri)
return {} unless conf.is_a?(Hash)

conf['rubocop'] || {}
end

def cli_args file, config
args = [
config['cops'] == 'all' ? '--auto-correct-all' : '--auto-correct',
'--cache', 'false',
'--format', 'Solargraph::LanguageServer::Message::' \
'TextDocument::Formatting::BlankRubocopFormatter',
]

['except', 'only'].each do |arg|
cops = cop_list(config[arg])
args += ["--#{arg}", cops] if cops
end

args += config['extra_args'] if config['extra_args']
args + [file]
end

def cop_list(value)
value = value.join(',') if value.respond_to?(:join)
return nil if value == '' || !value.is_a?(String)
value
end

# @param original [String]
# @param result [String]
# @return [void]
Expand Down
15 changes: 15 additions & 0 deletions lib/solargraph/workspace/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ def reporters
raw_data['reporters']
end

# A hash of options supported by the formatter
#
# @return [Hash]
def formatter
raw_data['formatter']
end

# An array of plugins to require.
#
# @return [Array<String>]
Expand Down Expand Up @@ -144,6 +151,14 @@ def default_config
'require' => [],
'domains' => [],
'reporters' => %w[rubocop require_not_found],
'formatter' => {
'rubocop' => {
'cops' => 'safe',
'except' => [],
'only' => [],
'extra_args' =>[]
}
},
'require_paths' => [],
'plugins' => [],
'max_files' => MAX_FILES
Expand Down
7 changes: 0 additions & 7 deletions spec/diagnostics/rubocop_helpers_spec.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
describe Solargraph::Diagnostics::RubocopHelpers do
it "finds a .rubocop.yml file in a parent directory" do
file = File.realpath(File.join 'spec', 'fixtures', 'rubocop-subfolder-configuration', 'folder1', 'folder2', 'test.rb')
conf = File.realpath(File.join 'spec', 'fixtures', 'rubocop-subfolder-configuration', '.rubocop.yml')
found = Solargraph::Diagnostics::RubocopHelpers.find_rubocop_file(file)
expect(found).to eq(conf)
end

it "converts lower-case drive letters to upper-case" do
input = 'c:/one/two'
output = Solargraph::Diagnostics::RubocopHelpers.fix_drive_letter(input)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
describe Solargraph::LanguageServer::Message::TextDocument::Formatting do
it 'gracefully handles empty files' do
host = double(:Host, read_text: '')
host = double(:Host, read_text: '', formatter_config: {})
request = {
'params' => {
'textDocument' => {
Expand Down

0 comments on commit d9d2abf

Please sign in to comment.