Current dependencies on/for this PR:

• main
  • PR Extract message emitters from Printer #3895
    • PR Cheap cloneable LineIndex  #3896
      • PR Store source code on message #3897 👈
        • PR Render code frame with context #3901
          • PR Introduce SourceFile to avoid cloning the message filename #3904
            • PR Order Edits by Locations #3905
              • PR Pretty print Diagnostics in snapshot tests #3906

This comment was auto-generated by Graphite.

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     14.2±0.05ms     2.9 MB/sec    1.01     14.2±0.08ms     2.9 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.6±0.01ms     4.7 MB/sec    1.00      3.6±0.01ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.01    456.7±1.29µs     6.5 MB/sec    1.00    451.8±1.49µs     6.5 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.0±0.02ms     4.2 MB/sec    1.00      6.0±0.01ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.2±0.02ms     5.7 MB/sec    1.01      7.2±0.02ms     5.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1616.1±5.75µs    10.3 MB/sec    1.00   1620.4±5.21µs    10.3 MB/sec
linter/default-rules/numpy/globals.py      1.01    180.5±0.44µs    16.4 MB/sec    1.00    178.9±0.50µs    16.5 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.3±0.01ms     7.7 MB/sec    1.00      3.3±0.00ms     7.7 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     18.4±0.56ms     2.2 MB/sec    1.02     18.9±0.59ms     2.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.06      4.9±0.17ms     3.4 MB/sec    1.00      4.6±0.18ms     3.6 MB/sec
linter/all-rules/numpy/globals.py          1.02   583.5±21.62µs     5.1 MB/sec    1.00   569.6±26.61µs     5.2 MB/sec
linter/all-rules/pydantic/types.py         1.03      8.1±0.17ms     3.2 MB/sec    1.00      7.8±0.25ms     3.3 MB/sec
linter/default-rules/large/dataset.py      1.00      9.6±0.24ms     4.3 MB/sec    1.00      9.5±0.27ms     4.3 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01      2.1±0.10ms     7.9 MB/sec    1.00      2.1±0.09ms     8.0 MB/sec
linter/default-rules/numpy/globals.py      1.00   220.8±10.68µs    13.4 MB/sec    1.03   226.3±12.58µs    13.0 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.2±0.16ms     6.1 MB/sec    1.03      4.4±0.39ms     5.9 MB/sec

@MichaReiser Am I understanding correctly that this will also make it possible to use the errors cached on disk and displaying the source code without loading the file in memory at all?
Maybe that's the primary intent of this PR, not sure I get the whole picture.

@MichaReiser Am I understanding correctly that this will also make it possible to use the errors cached on disk and displaying the source code without loading the file in memory at all?

Yes, depending on your definition of loading the file in memory. But you could argue that the old implementation did this more efficiently.

Previously, Message only stored the source of the violating line. This has the advantage that a Message uses less space than storing the source code of the whole file. But it has a few disadvantages:

• Ruff supports different formats to output the Message: JSON, text, grouped, etc. Only serializing the violating line limits what the formatter can output. E.g. they can't render additional context lines before/after the violation because the source of these lines is unavailable.
• It limits formatters from rendering an inline diff with the code fix because the fix isn't necessary on the same line. We could include all the lines for every edit, but that means we're already getting closer to just including the source code.

But our main reason for storing the whole source code on Message is that we want to transition to using byte offsets rather than row/col locations. But this imposes a problem when rendering Messages: We need to compute the line and column information because who wants to see byte offsets in diagnostics? To compute the row/column location we either need an index or... just use the source and recompute the position. That's why Message now stores the whole source code and we serialize it as part of the diagnostics. I don't love this... because it means that users we'll have two versions of every file with violations on their disk: The original, and once in our cache (same file as the diagnostics). I considered loading the file instead of duplicating the content into our cache but decided against it because:

• It's more complicated
• It requires reading another file
• We transform Jupyter notebooks before linting (concatenate all cells), meaning we would need to do that too, in a generic way
• Ideally, we would support other resources as source other than files. Maybe a file that you opened in your editor but has no disk representation yet, a file that you provided over STDIN, or the command line arguments...
• We only store the content for files with violations... there should only be a few files with violations for projects that have adopted ruff.

So in a sense, yes, this architecture avoids opening the source file and loading its content. But only because it loads the content from the cache ;)

How does elmreview cache diagnostics? Does it re-read the content from the original file when rendering the diagnostic?