Fix (doc-)line-too-long start location by MichaReiser · Pull Request #4006 · astral-sh/ruff

MichaReiser · 2023-04-18T06:30:29Z

I noticed in the byte offset refactor that the (doc-)line-too-long start range can point past the end of the line if the line contains characters with a width greater or equal to two. This is because we set the start location's column to limit instead of computing the character at which the line width is exceeded.

The current range can cause panics when printing the diagnostics because location can be greater than end_location which panics when slicing into the text string (the byte offset refactor will catch this earlier because it uses TextRange that enforces this invariant).

MichaReiser · 2023-04-18T06:30:41Z

Current dependencies on/for this PR:

main
- PR Fix (doc-)line-too-long start location #4006 👈

This comment was auto-generated by Graphite.

MichaReiser · 2023-04-18T06:32:45Z

crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E501_E501.py.snap

  |

-E501.py:16:89: E501 Line too long (95 > 88 characters)
+E501.py:16:85: E501 Line too long (95 > 88 characters)


We could use this as a chance to improve the diagnostic. Any recommendations for a replacement term for characters that captures that we compare the width and not characters count?

I noticed in the byte offset refactor that the (doc-)line-too-long start range can point past the end of the line if the line contains characters with a width greater or equal to two. This is because we set the start locations' column to `limit` instead of computing the character at which the line width is exceeded.

github-actions · 2023-04-18T06:47:59Z

PR Check Results

Ecosystem

ℹ️ ecosystem check detected changes. (+6, -6, 0 error(s))

zulip (+6, -6)

- zerver/tests/test_message_fetch.py:2275:101: E501 Line too long (123 > 100 characters)
+ zerver/tests/test_message_fetch.py:2275:92: E501 Line too long (123 > 100 characters)
- zerver/tests/test_message_fetch.py:2443:101: E501 Line too long (110 > 100 characters)
+ zerver/tests/test_message_fetch.py:2443:89: E501 Line too long (110 > 100 characters)
- zerver/webhooks/slack_incoming/tests.py:149:101: E501 Line too long (191 > 100 characters)
+ zerver/webhooks/slack_incoming/tests.py:149:103: E501 Line too long (191 > 100 characters)
- zerver/webhooks/slack_incoming/tests.py:151:101: E501 Line too long (217 > 100 characters)
+ zerver/webhooks/slack_incoming/tests.py:151:103: E501 Line too long (217 > 100 characters)
+ zerver/webhooks/slack_incoming/tests.py:155:100: E501 Line too long (211 > 100 characters)
- zerver/webhooks/slack_incoming/tests.py:155:101: E501 Line too long (211 > 100 characters)
- zilencer/management/commands/populate_db.py:872:101: E501 Line too long (106 > 100 characters)
+ zilencer/management/commands/populate_db.py:872:95: E501 Line too long (106 > 100 characters)

Benchmark

Linux

group                                      main                                    pr
-----                                      ----                                    --
linter/all-rules/large/dataset.py          1.00     17.4±1.07ms     2.3 MB/sec     1.06     18.5±1.04ms     2.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.2±0.23ms     4.0 MB/sec     1.05      4.4±0.27ms     3.8 MB/sec
linter/all-rules/numpy/globals.py          1.03   574.4±38.69µs     5.1 MB/sec     1.00   555.3±33.52µs     5.3 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.1±0.38ms     3.6 MB/sec     1.06      7.6±0.39ms     3.4 MB/sec
linter/default-rules/large/dataset.py      1.00      8.7±0.43ms     4.7 MB/sec     1.15     10.0±0.62ms     4.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1990.1±137.15µs     8.4 MB/sec    1.05      2.1±0.12ms     7.9 MB/sec
linter/default-rules/numpy/globals.py      1.00   242.5±13.53µs    12.2 MB/sec     1.02   246.8±17.48µs    12.0 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.1±0.28ms     6.2 MB/sec     1.08      4.5±0.29ms     5.7 MB/sec
parser/large/dataset.py                    1.00      6.2±0.32ms     6.6 MB/sec     1.06      6.6±0.41ms     6.2 MB/sec
parser/numpy/ctypeslib.py                  1.04  1381.8±103.98µs    12.1 MB/sec    1.00  1322.7±85.90µs    12.6 MB/sec
parser/numpy/globals.py                    1.09    143.3±7.18µs    20.6 MB/sec     1.00   131.7±11.99µs    22.4 MB/sec
parser/pydantic/types.py                   1.12      3.1±0.25ms     8.1 MB/sec     1.00      2.8±0.18ms     9.1 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     16.6±0.13ms     2.5 MB/sec    1.01     16.7±0.16ms     2.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.3±0.06ms     3.8 MB/sec    1.01      4.4±0.07ms     3.8 MB/sec
linter/all-rules/numpy/globals.py          1.00   450.8±11.22µs     6.5 MB/sec    1.01   454.7±28.86µs     6.5 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.1±0.09ms     3.6 MB/sec    1.01      7.2±0.12ms     3.5 MB/sec
linter/default-rules/large/dataset.py      1.00      8.4±0.07ms     4.8 MB/sec    1.01      8.5±0.09ms     4.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1819.8±16.32µs     9.2 MB/sec    1.02  1857.6±24.53µs     9.0 MB/sec
linter/default-rules/numpy/globals.py      1.00    190.0±3.38µs    15.5 MB/sec    1.02    194.4±5.79µs    15.2 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.8±0.05ms     6.7 MB/sec    1.02      3.9±0.06ms     6.5 MB/sec
parser/large/dataset.py                    1.00      6.7±0.05ms     6.1 MB/sec    1.00      6.7±0.06ms     6.1 MB/sec
parser/numpy/ctypeslib.py                  1.00  1286.4±15.52µs    12.9 MB/sec    1.00  1292.2±18.08µs    12.9 MB/sec
parser/numpy/globals.py                    1.00    128.2±1.98µs    23.0 MB/sec    1.00    128.5±1.75µs    23.0 MB/sec
parser/pydantic/types.py                   1.00      2.9±0.03ms     8.9 MB/sec    1.01      2.9±0.04ms     8.9 MB/sec

charliermarsh · 2023-04-18T23:07:08Z

crates/ruff/src/rules/pycodestyle/rules/doc_line_too_long.rs

+            } else {
+                length += 1;
+            }
+        }


I wonder if we should move this computation into is_overlong somehow, so that it's guaranteed to yield a consistent result with line_width?

Done. is_overlong now does all the calculations and returns an Option<Overlong> to give the caller access to the computed range and width.

MichaReiser commented Apr 18, 2023

View reviewed changes

MichaReiser force-pushed the fix-line-too-long-range branch from de4bf26 to 5d90734 Compare April 18, 2023 06:33

MichaReiser requested a review from charliermarsh April 18, 2023 06:36

MichaReiser mentioned this pull request Apr 18, 2023

Replace row/column based Location with byte-offsets. #3931

Merged

6 tasks

MichaReiser added the bug Something isn't working label Apr 18, 2023

charliermarsh approved these changes Apr 18, 2023

View reviewed changes

Move width and start/end column calculation into is_overlong

f9e2197

MichaReiser force-pushed the fix-line-too-long-range branch from 3d24a21 to f9e2197 Compare April 19, 2023 06:24

MichaReiser merged commit a3146ab into main Apr 19, 2023

MichaReiser deleted the fix-line-too-long-range branch April 19, 2023 06:42

p-linnane mentioned this pull request Apr 20, 2023

ruff 0.0.262 Homebrew/homebrew-core#128839

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Fix (doc-)line-too-long start location#4006

Fix (doc-)line-too-long start location#4006
MichaReiser merged 2 commits intomainfrom
fix-line-too-long-range

MichaReiser commented Apr 18, 2023 •

edited

Loading

Uh oh!

MichaReiser commented Apr 18, 2023

Uh oh!

MichaReiser Apr 18, 2023

Uh oh!

github-actions bot commented Apr 18, 2023 •

edited

Loading

Uh oh!

charliermarsh Apr 18, 2023

Uh oh!

MichaReiser Apr 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

MichaReiser commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaReiser commented Apr 18, 2023

Uh oh!

MichaReiser Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Check Results

Ecosystem

Benchmark

Linux

Windows

Uh oh!

charliermarsh Apr 18, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MichaReiser commented Apr 18, 2023 •

edited

Loading

github-actions bot commented Apr 18, 2023 •

edited

Loading