zls
zls copied to clipboard
Code action to organize imports
Organize Imports Diagnostic and Code Action
This PR adds a diagnostic and a code action to organize imports in the current Zig file. The code action is available in the command palette as organize @import
.
It is designed to be fairly customizable, so that we can arrive at a acceptable sorting style, like different grouping or case sensitivity. All decisions, especially those around diagnostics are very much open to critique as I do not have experience with the LSP.
Closes #1637, continues work from #881.
Design
An import is a top level declaration of a variable that uses the @import
builtin.
A top level declaration that uses import variables (like const print = std.debug.print;
) is not considered an import and stays in place.
The sorting splits imports into three separated groups: builtin packages, packages and project files.
The builtin packages are sorted in the order: std
, builtin
, build_options
.
The sorting inside other groups is done by comparing the declaration names (the x
in const x = @import("file.zig");
).
The feature is tested both in unit tests and in neovim. The unit tests required a new helper function.
Diagnostic
The diagnostic is enabled under the warn_style
zls config option, which is disabled by default.
The diagnostic's range is the entire line of the import which would be moved by organizing imports.
That means one diagnostic per wrongly placed import line.
The diagnostic does not consider the correctness of separators between imports.
Code Action
The code action is available based on the diagnostic.
This means that users with the warn_style
option disabled will not be offered the code action.
The code action always sorts. The previous PR had optimization using isSorted
to avoid sorting when it was not necessary.
This, however, did not work well in some corner cases.
The documentation comments (///
) are moved with the import they are attached to.
It also respects Top-Level Doc Comments
(//!
), which must appear before any expression. This is handled using the commments token location.
This feature required running generateCodeAction
for each diagnostic provided in request.
Previously, it ran the function only on diagnostics from getAstCheckDiagnostics
.
Considerations and notes
- Top-level declarations (that includes imports) are order-independent (Source and tested)
-
zig fmt
andtextDocument/formatting
does not interfere with the sorting. - Only root declarations are considered for sorting (no import inside functions).
- Will not work with
@import(variable);
(we don't do comptime) - Resolving one diagnostic will resolve all diagnostics in the file (since all imports are moved).
- I removed the part dealing with intersection with action's range (
builder.range
field which no longer exists). If it was relevant, I can study it further. - The code action works without
warn_style
as seen in tests. It is just that user cannot call it without the diagnostic. -
std.sort.sort
does not exist anymore. I pickedstd.sort.heap
as a replacement. My understanding is that we do not need a stable sort here, but feel free to correct me and suggest a different sort from thestd
library.
Unresolved questions
- [ ] Allow for
const print = std.debug.print;
to be sorted as well in some way? - [ ] How to treat
@import("root")
? (Currently a package, which does not seem right) - [ ] The
getSortSlice
function ported from previous PR seems weird. I would just doreturn self.name
and not look in the import path. Any reasoning behind this? - [ ] What about grouping by directory for the file imports? Perhaps if there are > 1 file imports from the same directory, they could be in a group of their own.
- [ ] I think
DiagnosticKind
could be parsed from the diagnostic code, which would be cleaner and would easily allow more versions of the diagnostic message. Or use thedata
field of the diagnostic message. - [ ] What about
@embedFile
? - [ ] How to implement ignoring the diagnostic?
Ast.zig
, for example, imports extra stuff near end of the file for tests.