r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.16k stars 184 forks source link

Allow linters to provide fix #784

Open renkun-ken opened 3 years ago

renkun-ken commented 3 years ago

Some code editors (e.g. vscode) supports code actions to allow language server to provide code refactoring or fixes via the language server protocol.

In languageserver, I'm trying to support code actions like this to provide quick fixes of the lint results for many linters. However, lintr might be the best place to provide the fix along with the problem as each linter has the best knowledge of the problem it finds and how it should be resolved if possible.

Does it make sense to allow an optional a field fix in the lint result which contains a list of edits (each has a range and a new text) so that languageserver could directly transform these fixes into code actions at downstream?

The code issues found by many linters could be easy to fix and the fix is optional rather than required so that implementing fixes could be done in a progressive way.

Also see https://github.com/REditorSupport/languageserver/issues/396.

AshesITR commented 3 years ago

This sounds promising. Maybe we could upgrade the ranges attribute somehow? IINM it's possible to specify multiple ranges for a single lint and they already contain start and end markers. The current interface assumes a list of length-2 integer vectors.

We could add optional attributes to each vector (e.g. attr(., "fix")) or add a new attribute to Lint(). Using the names() of ranges would also be thinkable, but that will lead to issues when the source code is in a non-system encoding as R names must be encoded in the system encoding.

renkun-ken commented 3 years ago

A simple way is to add fixes to Lint structure like the following:

  structure(
    list(
      filename = filename,
      line_number = as.integer(line_number),
      column_number = as.integer(column_number),
      type = type,
      message = message,
      line = line,
      ranges = ranges,
      fixes = fixes,
      linter = NA_character_
    ),
    class = "lint")

where fixes is a character vector of replacement strings for each range in ranges. This assumes that the ranges to emphasize must be the ranges to replace text as fixes.

In most cases, a Lint only has one range, then fixes is a character vector of one string.

renkun-ken commented 3 years ago

Here's a simple demo with #785 and https://github.com/REditorSupport/languageserver/pull/397:

Kapture 2021-03-21 at 23 35 50

MichaelChirico commented 3 years ago

FWIW this is exactly the sort of thing I had in mind for #710