Skip to content

diagnostic rendering for import lint I001 is sub-optimal in some cases #15504

@BurntSushi

Description

@BurntSushi

On current main, here's what one example of a I001 diagnostic looks like:

lines_after_imports.pyi:1:1: I001 [*] Import block is un-sorted or un-formatted
   |
 1 | / from __future__ import annotations
 2 | |
 3 | | from typing import Any
 4 | |
 5 | | from requests import Session
 6 | |
 7 | | from my_first_party import my_first_party_object
 8 | |
 9 | | from . import my_local_folder_object
10 | |
11 | |
12 | |
13 | | class Thing(object):
   | |_^ I001
14 |     name: str
15 |     def __init__(self, name: str):
   |
   = help: Organize imports

In particular, the above comes from this snapshot:

https://github.com/astral-sh/ruff/blob/55a7f72035390cf1093e4da0cf2462e793bfbce1/crates/ruff_linter/src/rules/isort/snapshots/ruff_linter__rules__isort__tests__lines_after_imports.pyi.snap

In the above rendering, the ^ is pointing to a class definition, but it should be pointing to an import statement. After #15359 is merged, the rendering will improve somewhat to this:

lines_after_imports.pyi:1:1: I001 [*] Import block is un-sorted or un-formatted
   |
 1 | / from __future__ import annotations
 2 | |
 3 | | from typing import Any
 4 | |
 5 | | from requests import Session
 6 | |
 7 | | from my_first_party import my_first_party_object
 8 | |
 9 | | from . import my_local_folder_object
10 | |
11 | |
12 | |
   | |__^ I001
13 |   class Thing(object):
14 |     name: str
15 |     def __init__(self, name: str):
   |
   = help: Organize imports

This is better, but still not ideal. However, this isn't a problem in the diagnostic rendering itself (which is what #15359 didn't address it), but rather, in the spans generated by the I001 lint. To demonstrate, this is a minimal reproducer I made by debug printing the annotate-snippets::Message when running the above test:

use annotate_snippets::{Level, Renderer, Snippet};

fn main() {
    let source = "from __future__ import annotations\n\nfrom typing import Any\n\nfrom requests import Session\n\nfrom my_first_party import my_first_party_object\n\nfrom . import my_local_folder_object\n\n\n\nclass Thing(object):\n  name: str\n  def __init__(self, name: str):\n";
    let src_annotation = Level::Error.span(0..180).label("I001");
    let snippet =
        Snippet::source(source).line_start(1).annotation(src_annotation);
    let message = Level::Error.title("").snippet(snippet);

    println!("{}", Renderer::plain().render(message));
}

This outputs the same rendering as above. And the span, 0..180, corresponds to this substring:

from __future__ import annotations\n\nfrom typing import Any\n\nfrom requests import Session\n\nfrom my_first_party import my_first_party
_object\n\nfrom . import my_local_folder_object\n\n\n\n

So whatever is generating the spans here is including those empty lines at the end. We should fix the span generation to be tighter. (It's perhaps as simple as trimming the trailing lines from the range.)

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdiagnosticsRelated to reporting of diagnostics.help wantedContributions especially welcome

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions