Skip to content

Commit

Permalink
Allow configuring linters separate from formatter (#1925)
Browse files Browse the repository at this point in the history
* Remember active linters in global state

* Use active linters to run diagnostics

* Add linters setting and pass it to the server

* Apply PR suggestion
  • Loading branch information
vinistock authored Apr 22, 2024
1 parent 800cd7e commit 70cb714
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 8 deletions.
18 changes: 18 additions & 0 deletions lib/ruby_lsp/global_state.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def initialize
@encoding = T.let(Encoding::UTF_8, Encoding)

@formatter = T.let("auto", String)
@linters = T.let([], T::Array[String])
@test_library = T.let(detect_test_library, String)
@typechecker = T.let(detect_typechecker, T::Boolean)
@index = T.let(RubyIndexer::Index.new, RubyIndexer::Index)
Expand All @@ -46,6 +47,11 @@ def active_formatter
@supported_formatters[@formatter]
end

sig { returns(T::Array[Requests::Support::Formatter]) }
def active_linters
@linters.filter_map { |name| @supported_formatters[name] }
end

sig { params(options: T::Hash[Symbol, T.untyped]).void }
def apply_options(options)
workspace_uri = options.dig(:workspaceFolders, 0, :uri)
Expand All @@ -55,6 +61,9 @@ def apply_options(options)
@formatter = specified_formatter if specified_formatter
@formatter = detect_formatter if @formatter == "auto"

specified_linters = options.dig(:initializationOptions, :linters)
@linters = specified_linters || detect_linters

encodings = options.dig(:capabilities, :general, :positionEncodings)
@encoding = if !encodings || encodings.empty?
Encoding::UTF_16LE
Expand Down Expand Up @@ -108,6 +117,15 @@ def detect_formatter
end
end

# Try to detect if there are linters in the project's dependencies. For auto-detection, we always only consider a
# single linter. To have multiple linters running, the user must configure them manually
sig { returns(T::Array[String]) }
def detect_linters
linters = []
linters << "rubocop" if direct_dependency?(/^rubocop/)
linters
end

sig { returns(String) }
def detect_test_library
if direct_dependency?(/^rspec/)
Expand Down
11 changes: 7 additions & 4 deletions lib/ruby_lsp/requests/diagnostics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def provider
sig { params(global_state: GlobalState, document: Document).void }
def initialize(global_state, document)
super()
@active_formatter = T.let(global_state.active_formatter, T.nilable(Support::Formatter))
@active_linters = T.let(global_state.active_linters, T::Array[Support::Formatter])
@document = document
@uri = T.let(document.uri, URI::Generic)
end
Expand All @@ -45,10 +45,13 @@ def perform
diagnostics.concat(syntax_error_diagnostics, syntax_warning_diagnostics)

# Running RuboCop is slow, so to avoid excessive runs we only do so if the file is syntactically valid
return diagnostics if @document.syntax_error? || !@active_formatter
return diagnostics if @document.syntax_error? || @active_linters.empty?

@active_linters.each do |linter|
linter_diagnostics = linter.run_diagnostic(@uri, @document)
diagnostics.concat(linter_diagnostics) if linter_diagnostics
end

formatter_diagnostics = @active_formatter.run_diagnostic(@uri, @document)
diagnostics.concat(formatter_diagnostics) if formatter_diagnostics
diagnostics
end

Expand Down
27 changes: 27 additions & 0 deletions test/global_state_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,33 @@ def test_watching_files_if_not_reported
refute(state.supports_watching_files)
end

def test_linter_specification
state = GlobalState.new
state.apply_options({
initializationOptions: { linters: ["rubocop", "brakeman"] },
})

assert_equal(["rubocop", "brakeman"], state.instance_variable_get(:@linters))
end

def test_linter_auto_detection
stub_dependencies("rubocop" => "1.2.3")
state = GlobalState.new
state.apply_options({})

assert_equal(["rubocop"], state.instance_variable_get(:@linters))
end

def test_specifying_empty_linters
stub_dependencies("rubocop" => "1.2.3")
state = GlobalState.new
state.apply_options({
initializationOptions: { linters: [] },
})

assert_empty(state.instance_variable_get(:@linters))
end

private

def stub_dependencies(dependencies)
Expand Down
4 changes: 3 additions & 1 deletion test/requests/code_actions_formatting_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ def assert_corrects_to_expected(diagnostic_code, code_action_title, source, expe
)

global_state = RubyLsp::GlobalState.new
global_state.formatter = "rubocop"
global_state.apply_options({
initializationOptions: { linters: ["rubocop"] },
})
global_state.register_formatter(
"rubocop",
RubyLsp::Requests::Support::RuboCopFormatter.new,
Expand Down
4 changes: 3 additions & 1 deletion test/requests/diagnostics_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ def run_expectations(source)
document = RubyLsp::RubyDocument.new(source: source, version: 1, uri: URI::Generic.from_path(path: __FILE__))
result = T.let(nil, T.nilable(T::Array[RubyLsp::Interface::Diagnostic]))
global_state = RubyLsp::GlobalState.new
global_state.formatter = "rubocop"
global_state.apply_options({
initializationOptions: { linters: ["rubocop"] },
})
global_state.register_formatter(
"rubocop",
RubyLsp::Requests::Support::RuboCopFormatter.new,
Expand Down
8 changes: 6 additions & 2 deletions test/requests/diagnostics_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
class DiagnosticsTest < Minitest::Test
def setup
@global_state = RubyLsp::GlobalState.new
@global_state.formatter = "rubocop"
@global_state.apply_options({
initializationOptions: { linters: ["rubocop"] },
})
@global_state.register_formatter(
"rubocop",
RubyLsp::Requests::Support::RuboCopFormatter.new,
Expand Down Expand Up @@ -96,7 +98,9 @@ def run_diagnostic(uri, document)
end

@global_state.register_formatter("my-custom-formatter", T.unsafe(formatter_class).new)
@global_state.formatter = "my-custom-formatter"
@global_state.apply_options({
initializationOptions: { linters: ["my-custom-formatter"] },
})

diagnostics = T.must(RubyLsp::Requests::Diagnostics.new(@global_state, document).perform)
assert(diagnostics.find { |d| d.message == "Hello from custom formatter" })
Expand Down
10 changes: 10 additions & 0 deletions vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,16 @@
],
"default": "auto"
},
"rubyLsp.linters": {
"description": "List of linter tools that the Ruby LSP should use for diagnostics",
"type": "array",
"examples": [
[
"rubocop"
]
],
"default": null
},
"rubyLsp.bundleGemfile": {
"description": "Relative or absolute path to the Gemfile to use for bundling the Ruby LSP server. Only necessary when using a separate Gemfile for the Ruby LSP",
"type": "string",
Expand Down
1 change: 1 addition & 0 deletions vscode/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ function collectClientOptions(
),
featuresConfiguration: configuration.get("featuresConfiguration"),
formatter: configuration.get("formatter"),
linters: configuration.get("linters"),
},
};
}
Expand Down

0 comments on commit 70cb714

Please sign in to comment.