Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configuring linters separate from formatter #1925

Merged
merged 4 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading