Ensure that tab characters aren't in multi-line strings before throwing a violation #3837

evanrittenhouse · 2023-04-01T01:55:48Z

github-actions · 2023-04-01T02:26:19Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.04     21.8±1.10ms  1909.2 KB/sec    1.00     21.0±0.53ms  1982.5 KB/sec
linter/all-rules/numpy/ctypeslib.py        1.04      5.5±0.20ms     3.0 MB/sec    1.00      5.3±0.22ms     3.2 MB/sec
linter/all-rules/numpy/globals.py          1.00   593.5±17.81µs     5.0 MB/sec    1.03   609.0±46.49µs     4.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      9.0±0.39ms     2.8 MB/sec    1.00      9.0±0.36ms     2.8 MB/sec
linter/default-rules/large/dataset.py      1.00     10.2±0.44ms     4.0 MB/sec    1.00     10.2±0.34ms     4.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00      2.1±0.11ms     7.9 MB/sec    1.08      2.3±0.22ms     7.4 MB/sec
linter/default-rules/numpy/globals.py      1.00   244.0±11.22µs    12.1 MB/sec    1.08   264.6±19.40µs    11.1 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.6±0.17ms     5.5 MB/sec    1.05      4.8±0.21ms     5.3 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.02     20.0±0.70ms     2.0 MB/sec    1.00     19.6±0.62ms     2.1 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      5.1±0.19ms     3.3 MB/sec    1.01      5.1±0.22ms     3.3 MB/sec
linter/all-rules/numpy/globals.py          1.01   598.3±32.18µs     4.9 MB/sec    1.00   590.3±20.67µs     5.0 MB/sec
linter/all-rules/pydantic/types.py         1.01      8.6±0.36ms     3.0 MB/sec    1.00      8.4±0.39ms     3.0 MB/sec
linter/default-rules/large/dataset.py      1.00      9.8±0.34ms     4.2 MB/sec    1.01      9.9±0.34ms     4.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00      2.1±0.07ms     7.9 MB/sec    1.08      2.3±0.10ms     7.3 MB/sec
linter/default-rules/numpy/globals.py      1.00   236.6±46.68µs    12.5 MB/sec    1.06   250.4±18.24µs    11.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.4±0.11ms     5.9 MB/sec    1.07      4.6±0.18ms     5.5 MB/sec

crates/ruff/src/checkers/physical_lines.rs

crates/ruff_python_ast/src/source_code/indexer.rs

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

…d row

charliermarsh · 2023-04-06T22:56:30Z

(Doing some testing, then will merge tonight!)

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

charliermarsh · 2023-04-06T23:40:04Z

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

+            // Tab on last line of the quoted range, preceding the quote.
+            if lineno == end.row() && tab_column < end.column() {
+                return None;
+            }


I think it's sufficient to just check these, and not check the "Is it on the start and end line" condition, since if it's on the start and end line, it'll be caught by the first condition... But hoping I'm not thinking about it incorrectly.

Nah, I think that's right. IIRC I included a test case which contained a triple-quoted string on one line, so if that passes I think we should be good.

charliermarsh · 2023-04-06T23:40:16Z

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

+                return None;
+            }
+
+            let tab_column = line[..tab_index].chars().count();


(I think we need to count characters here, since .column() is based on characters.)

How do you mean?

indent.find('\t') returns the index of the starting byte for the tab character, but some characters can require multiple bytes (think unicode) -- so str.len() in Rust doesn't always return what you might expect if coming from another programming language.

Oh interesting, I didn't know that. I'll have to read up on it, thanks!

evanrittenhouse marked this pull request as ready for review April 1, 2023 15:26

evanrittenhouse force-pushed the 3674_multiline_tabs branch from 74eb245 to 6b67f9e Compare April 1, 2023 15:28

charliermarsh reviewed Apr 1, 2023

View reviewed changes

crates/ruff/src/checkers/physical_lines.rs Outdated Show resolved Hide resolved

evanrittenhouse marked this pull request as draft April 1, 2023 21:42

evanrittenhouse added 4 commits April 3, 2023 08:48

Add tests with different quote lines

9fe6762

Check in_quote with hard-coded delimiter

1e3231c

Remove hard-coded delimiters

d9ebdb4

Finish

1885a44

evanrittenhouse force-pushed the 3674_multiline_tabs branch from 1792ddd to 3ce6308 Compare April 3, 2023 13:49

evanrittenhouse added 6 commits April 3, 2023 08:54

Clear clippy errors

f6bcf1f

Add test for multiple delimiters on the same line

4acd0aa

WIP: Adding string_lines to Indexer, binary search implementation

44a4ca1

Change indexer.string_list to Ranges

af1eab7

Binary search implementation and tests

e02e9ec

Change binary search to linear search (temporary)

dca4605

evanrittenhouse force-pushed the 3674_multiline_tabs branch from 6776335 to 442379b Compare April 3, 2023 13:56

Fix physical_lines unit tests to take Indexer, reset cargo insta

2c3ad96

evanrittenhouse force-pushed the 3674_multiline_tabs branch from 442379b to 2c3ad96 Compare April 3, 2023 13:56

evanrittenhouse added 8 commits April 3, 2023 08:57

Remove new W191 snap

b75a505

Remove slow code

caac12f

Fix linear search

76bb5fa

Move tests to bottom

45cc51c

Reset insta snapshots

3626f45

Implement binary search

63a54ef

Clear clippy errors

b2271e5

Implement binary search

0ca6b04

MichaReiser approved these changes Apr 4, 2023

View reviewed changes

evanrittenhouse added 2 commits April 4, 2023 19:55

Change lineno indexing

940720c

Ensure that tabs are outside string range if on string range start/en…

4b6a96b

…d row

evanrittenhouse added 2 commits April 4, 2023 21:16

Change cargo insta

015d6c3

Change tests

23becc3

evanrittenhouse marked this pull request as ready for review April 5, 2023 02:56

evanrittenhouse requested review from charliermarsh and MichaReiser April 5, 2023 02:56

Merge branch 'main' into 3674_multiline_tabs

95a803e

Rename to string_ranges; use binary_search_by

3c90f56

charliermarsh reviewed Apr 6, 2023

View reviewed changes

crates/ruff/src/rules/pycodestyle/rules/tab_indentation.rs Show resolved Hide resolved

charliermarsh reviewed Apr 6, 2023

View reviewed changes

Sort imports

b602d2a

charliermarsh merged commit abaf0a1 into astral-sh:main Apr 7, 2023

charliermarsh mentioned this pull request Apr 7, 2023

W191 triggers on tabs in multi-line strings #3438

Closed

p-linnane mentioned this pull request Apr 20, 2023

ruff 0.0.262 Homebrew/homebrew-core#128839

Merged

evanrittenhouse deleted the 3674_multiline_tabs branch June 15, 2023 21:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure that tab characters aren't in multi-line strings before throwing a violation #3837

Ensure that tab characters aren't in multi-line strings before throwing a violation #3837

evanrittenhouse commented Apr 1, 2023 •

edited

Loading

github-actions bot commented Apr 1, 2023 •

edited

Loading

charliermarsh commented Apr 6, 2023

charliermarsh Apr 6, 2023

evanrittenhouse Apr 6, 2023

charliermarsh Apr 6, 2023

evanrittenhouse Apr 6, 2023

charliermarsh Apr 7, 2023

evanrittenhouse Apr 7, 2023

Ensure that tab characters aren't in multi-line strings before throwing a violation #3837

Ensure that tab characters aren't in multi-line strings before throwing a violation #3837

Conversation

evanrittenhouse commented Apr 1, 2023 • edited Loading

github-actions bot commented Apr 1, 2023 • edited Loading

PR Check Results

Ecosystem

Benchmark

Linux

Windows

charliermarsh commented Apr 6, 2023

charliermarsh Apr 6, 2023

Choose a reason for hiding this comment

evanrittenhouse Apr 6, 2023

Choose a reason for hiding this comment

charliermarsh Apr 6, 2023

Choose a reason for hiding this comment

evanrittenhouse Apr 6, 2023

Choose a reason for hiding this comment

charliermarsh Apr 7, 2023

Choose a reason for hiding this comment

evanrittenhouse Apr 7, 2023

Choose a reason for hiding this comment

evanrittenhouse commented Apr 1, 2023 •

edited

Loading

github-actions bot commented Apr 1, 2023 •

edited

Loading