Skip to content

Commit

Permalink
Language Server: Add autofix code action.
Browse files Browse the repository at this point in the history
This reacts to the textDocument/codeAction JSON RPC
request and responds with possible fixes in the requested
region. Adds one more translation function in
verible-lsp-adapter that converts verible internal
structs to LSP external structs.
Tested in verible-verilog-ls_test by requesting and
edit for the (autofix available) no-newline-at-EOF lint rule.

(also tested with a bunch of editors (emacs, sublime, kate)
that it behaves as expected)

Signed-off-by: Henner Zeller <[email protected]>
  • Loading branch information
hzeller committed Oct 19, 2021
1 parent e8a29ca commit 264a454
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 0 deletions.
21 changes: 21 additions & 0 deletions common/lsp/lsp-protocol.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,24 @@ Diagnostic:
PublishDiagnosticsParams:
uri: string
diagnostics+: Diagnostic

# -- textDocument/codeAction
CodeActionParams:
textDocument: TextDocumentIdentifier
range: Range

TextEdit:
range: Range
newText: string

WorkspaceEdit:
#changes?: string->TextEdit+ # Map of file URI to associated edits...
changes: object # ...not yet supporte by jcxxgen. Let's object

# textDocument/codeAction will return an array of these.
CodeAction:
title: string
kind: string # Kind of change e.g quickfix, refactor, ...
diagnostics+: Diagnostic # What diagnostic is this referring to ?
isPreferred: boolean = false
edit: WorkspaceEdit
6 changes: 6 additions & 0 deletions verilog/tools/ls/lsp-parse-buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,10 @@ BufferTracker *BufferTrackerContainer::Update(
return inserted.first->second.get();
}

const BufferTracker *BufferTrackerContainer::FindBufferTrackerOrNull(
const std::string &uri) const {
auto found = buffers_.find(uri);
if (found == buffers_.end()) return nullptr;
return found->second.get();
}
} // namespace verilog
3 changes: 3 additions & 0 deletions verilog/tools/ls/lsp-parse-buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ class BufferTrackerContainer {
std::function<void(const std::string &uri, const BufferTracker &tracker)>;
void SetChangeListener(const ChangeCallback &cb) { change_listener_ = cb; }

// Given the URI, find the associated parse buffer if it exists.
const BufferTracker *FindBufferTrackerOrNull(const std::string &uri) const;

private:
// Update internal state of the given "uri" with the content of the text
// buffer. Return the buffer tracker.
Expand Down
62 changes: 62 additions & 0 deletions verilog/tools/ls/verible-lsp-adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,66 @@ std::vector<verible::lsp::Diagnostic> CreateDiagnostics(
return result;
}

static std::vector<verible::lsp::TextEdit> AutofixToTextEdits(
const verible::AutoFix &fix, absl::string_view base,
const verible::LineColumnMap &lc_map) {
std::vector<verible::lsp::TextEdit> result;
// TODO(hzeller): figure out if edits are stacking or are all based
// on the same start status.
for (const verible::ReplacementEdit &edit : fix.Edits()) {
verible::LineColumn start = lc_map(edit.fragment.begin() - base.begin());
verible::LineColumn end = lc_map(edit.fragment.end() - base.begin());
result.emplace_back(verible::lsp::TextEdit{
.range =
{
.start = {.line = start.line, .character = start.column},
.end = {.line = end.line, .character = end.column},
},
.newText = edit.replacement,
});
}
return result;
}

std::vector<verible::lsp::CodeAction> GenerateLinterCodeActions(
const BufferTracker *tracker, const verible::lsp::CodeActionParams &p) {
std::vector<verible::lsp::CodeAction> result;
if (!tracker) return result;
const ParsedBuffer *const current = tracker->current();
if (!current) return result;

auto const &lint_violations =
verilog::GetSortedViolations(current->lint_result());
if (lint_violations.empty()) return result;

const absl::string_view base = current->parser().Data().Contents();
verible::LineColumnMap line_column_map(base);

for (const auto &v : lint_violations) {
const verible::LintViolation &violation = *v.violation;
if (violation.autofixes.empty()) continue;
auto diagnostic = ViolationToDiagnostic(v, base, line_column_map);

// The editor usually has the cursor on a line or word, so we
// only want to output edits that are relevant.
if (!rangeOverlap(diagnostic.range, p.range)) continue;

bool preferred_fix = true;
for (const auto &fix : violation.autofixes) {
result.emplace_back(verible::lsp::CodeAction{
.title = fix.Description(),
.kind = "quickfix",
.diagnostics = {diagnostic},
.isPreferred = preferred_fix,
// The following is translated from json, map uri -> edits.
// We're only sending changes for one document, the current one.
.edit = {.changes = {{p.textDocument.uri,
AutofixToTextEdits(fix, base,
line_column_map)}}},
});
preferred_fix = false; // only the first is preferred.
}
}
return result;
}
} // namespace verilog
3 changes: 3 additions & 0 deletions verilog/tools/ls/verible-lsp-adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,8 @@ namespace verilog {
// output to be sent in textDocument/publishDiagnostics notification.
std::vector<verible::lsp::Diagnostic> CreateDiagnostics(const BufferTracker &);

// Generate code actions from autofixes provided by the linter.
std::vector<verible::lsp::CodeAction> GenerateLinterCodeActions(
const BufferTracker *tracker, const verible::lsp::CodeActionParams &p);
} // namespace verilog
#endif // VERILOG_TOOLS_LS_VERIBLE_LSP_ADAPTER_H
9 changes: 9 additions & 0 deletions verilog/tools/ls/verible-verilog-ls_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ awk '{printf("Content-Length: %d\r\n\r\n%s", length($0), $0)}' > ${TMP_IN} <<EOF
{"jsonrpc":"2.0", "id":1, "method":"initialize","params":null}
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://syntaxerror.sv","text":"brokenfile\n"}}}
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://mini.sv","text":"module mini();\nendmodule"}}}
{"jsonrpc":"2.0", "id":2, "method":"textDocument/codeAction","params":{"textDocument":{"uri":"file://mini.sv"},"range":{"start":{"line":0,"character":0},"end":{"line":2,"character":0}}}}
{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file://mini.sv"},"contentChanges":[{"range":{"start":{"character":9,"line":1},"end":{"character":9,"line":1}},"text":"\n"}]}}
{"jsonrpc":"2.0","method":"textDocument/didClose","params":{"textDocument":{"uri":"file://mini.sv"}}}
{"jsonrpc":"2.0", "id":100, "method":"shutdown","params":{}}
Expand Down Expand Up @@ -76,6 +77,14 @@ cat > "${JSON_EXPECTED}" <<EOF
}
}
},
{
"json_contains": {
"id":2,
"result": [
{"edit": {"changes": {"file://mini.sv":[{"newText":"\n"}]}}}
]
}
},
{
"json_contains": {
"method":"textDocument/publishDiagnostics",
Expand Down
9 changes: 9 additions & 0 deletions verilog/tools/ls/verilog_ls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ static InitializeResult InitializeServer(const nlohmann::json &params) {
{"change", 2}, // Incremental updates
},
},
{"codeActionProvider", true}, // Autofixes for lint errors
};

return result;
Expand Down Expand Up @@ -133,6 +134,14 @@ int main(int argc, char *argv[]) {
// Exchange of capabilities.
dispatcher.AddRequestHandler("initialize", InitializeServer);

// Provide autofixes
dispatcher.AddRequestHandler(
"textDocument/codeAction",
[&parsed_buffers](const verible::lsp::CodeActionParams &p) {
return verilog::GenerateLinterCodeActions(
parsed_buffers.FindBufferTrackerOrNull(p.textDocument.uri), p);
});

// The client sends a request to shut down. Use that to exit our loop.
bool shutdown_requested = false;
dispatcher.AddRequestHandler("shutdown",
Expand Down

0 comments on commit 264a454

Please sign in to comment.