Skip to content

Commit

Permalink
Add Prism warnings in diagnostics (#1437)
Browse files Browse the repository at this point in the history
* feat: surface prism warnings as diagnostics

* clean: shorter location usage
  • Loading branch information
snutij committed Mar 1, 2024
1 parent b1cf3ad commit 00c42a2
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 11 deletions.
50 changes: 42 additions & 8 deletions lib/ruby_lsp/requests/diagnostics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,27 +42,61 @@ def initialize(document)

sig { override.returns(T.nilable(T.all(T::Array[Interface::Diagnostic], Object))) }
def perform
diagnostics = []
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 syntax_error_diagnostics if @document.syntax_error?
return [] unless defined?(Support::RuboCopDiagnosticsRunner)
return diagnostics if @document.syntax_error?

diagnostics.concat(
Support::RuboCopDiagnosticsRunner.instance.run(
@uri,
@document,
).map!(&:to_lsp_diagnostic),
) if defined?(Support::RuboCopDiagnosticsRunner)

Support::RuboCopDiagnosticsRunner.instance.run(@uri, @document).map!(&:to_lsp_diagnostic)
diagnostics
end

private

sig { returns(T.nilable(T::Array[Interface::Diagnostic])) }
sig { returns(T::Array[Interface::Diagnostic]) }
def syntax_warning_diagnostics
@document.parse_result.warnings.map do |warning|
location = warning.location

Interface::Diagnostic.new(
source: "Prism",
message: warning.message,
severity: Constant::DiagnosticSeverity::WARNING,
range: Interface::Range.new(
start: Interface::Position.new(
line: location.start_line - 1,
character: location.start_column,
),
end: Interface::Position.new(
line: location.end_line - 1,
character: location.end_column,
),
),
)
end
end

sig { returns(T::Array[Interface::Diagnostic]) }
def syntax_error_diagnostics
@document.parse_result.errors.map do |error|
location = error.location

Interface::Diagnostic.new(
range: Interface::Range.new(
start: Interface::Position.new(
line: error.location.start_line - 1,
character: error.location.start_column,
line: location.start_line - 1,
character: location.start_column,
),
end: Interface::Position.new(
line: error.location.end_line - 1,
character: error.location.end_column,
line: location.end_line - 1,
character: location.end_column,
),
),
message: error.message,
Expand Down
110 changes: 110 additions & 0 deletions test/expectations/diagnostics/syntax_diagnostics.exp.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
{
"result": [
{
"range": {
"start": {
"line": 7,
"character": 0
},
"end": {
"line": 7,
"character": 0
}
},
"severity": 1,
"source": "Prism",
"message": "unexpected end of file, assuming it is closing the parent top level context"
},
{
"range": {
"start": {
"line": 7,
"character": 0
},
"end": {
"line": 7,
"character": 0
}
},
"severity": 1,
"source": "Prism",
"message": "expected an `end` to close the `def` statement"
},
{
"range": {
"start": {
"line": 0,
"character": 2
},
"end": {
"line": 0,
"character": 3
}
},
"severity": 2,
"source": "Prism",
"message": "ambiguous first argument; put parentheses or a space even after `+` operator"
},
{
"range": {
"start": {
"line": 1,
"character": 2
},
"end": {
"line": 1,
"character": 3
}
},
"severity": 2,
"source": "Prism",
"message": "ambiguous first argument; put parentheses or a space even after `-` operator"
},
{
"range": {
"start": {
"line": 2,
"character": 2
},
"end": {
"line": 2,
"character": 3
}
},
"severity": 2,
"source": "Prism",
"message": "ambiguous `*` has been interpreted as an argument prefix"
},
{
"range": {
"start": {
"line": 3,
"character": 2
},
"end": {
"line": 3,
"character": 3
}
},
"severity": 2,
"source": "Prism",
"message": "ambiguous `/`; wrap regexp in parentheses or add a space after `/` operator"
},
{
"range": {
"start": {
"line": 4,
"character": 7
},
"end": {
"line": 4,
"character": 10
}
},
"severity": 2,
"source": "Prism",
"message": "END in method; use at_exit"
}
],
"params": []
}
7 changes: 7 additions & 0 deletions test/fixtures/syntax_diagnostics.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
b +a
b -a
b *a
b /a/
def m; END{}; end

def foo
6 changes: 3 additions & 3 deletions test/requests/diagnostics_expectations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def run_expectations(source)
assert_empty(stdout)

# On Windows, RuboCop will complain that the file is missing a carriage return at the end. We need to ignore these
T.must(result).reject { |diagnostic| diagnostic.code == "Layout/EndOfLine" }
T.must(result).reject { |diagnostic| diagnostic.source == "RuboCop" && diagnostic.code == "Layout/EndOfLine" }
end

def assert_expectations(source, expected)
Expand All @@ -32,7 +32,7 @@ def assert_expectations(source, expected)
actual.each do |diagnostic|
attributes = diagnostic.attributes

attributes[:data][:code_actions].each do |code_action|
attributes.fetch(:data, {}).fetch(:code_actions, []).each do |code_action|
code_action_changes = code_action.attributes[:edit].attributes[:documentChanges]
code_action_changes.each do |code_action_change|
code_action_change
Expand All @@ -51,7 +51,7 @@ def map_diagnostics(diagnostics)
diagnostics.map do |diagnostic|
LanguageServer::Protocol::Interface::Diagnostic.new(
message: diagnostic["message"],
source: "RuboCop",
source: diagnostic["source"],
code: diagnostic["code"],
severity: diagnostic["severity"],
code_description: diagnostic["codeDescription"],
Expand Down

0 comments on commit 00c42a2

Please sign in to comment.