Move `Q001-3` to AST based checker #10312

dhruvmanila · 2024-03-09T12:03:49Z

Summary

Continuing with #7595, this PR moves the Q001, Q002, Q003 rules to the AST based checker.

Test Plan

Make sure all of the existing test cases pass and verify there are no ecosystem changes.

github-actions · 2024-03-09T12:20:24Z

`ruff-ecosystem` results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+115 -115 violations, +0 -0 fixes in 1 projects; 43 projects unchanged)

bokeh/bokeh (+115 -115 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ examples/basic/annotations/arrow.py:1:1: INP001 File `examples/basic/annotations/arrow.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/arrow.py:1:1: INP001 File `examples/basic/annotations/arrow.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/arrowheads.py:1:1: INP001 File `examples/basic/annotations/arrowheads.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/arrowheads.py:1:1: INP001 File `examples/basic/annotations/arrowheads.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/band.py:1:1: INP001 File `examples/basic/annotations/band.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/band.py:1:1: INP001 File `examples/basic/annotations/band.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/box_annotation.py:1:1: INP001 File `examples/basic/annotations/box_annotation.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/box_annotation.py:1:1: INP001 File `examples/basic/annotations/box_annotation.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/colorbar_log.py:1:1: INP001 File `examples/basic/annotations/colorbar_log.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/colorbar_log.py:1:1: INP001 File `examples/basic/annotations/colorbar_log.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/label.py:1:1: INP001 File `examples/basic/annotations/label.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/label.py:1:1: INP001 File `examples/basic/annotations/label.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/legend.py:1:1: INP001 File `examples/basic/annotations/legend.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/legend.py:1:1: INP001 File `examples/basic/annotations/legend.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/legend_two_dimensions.py:1:1: INP001 File `examples/basic/annotations/legend_two_dimensions.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/legend_two_dimensions.py:1:1: INP001 File `examples/basic/annotations/legend_two_dimensions.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/legends_item_visibility.py:1:1: INP001 File `examples/basic/annotations/legends_item_visibility.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/legends_item_visibility.py:1:1: INP001 File `examples/basic/annotations/legends_item_visibility.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/slope.py:1:1: INP001 File `examples/basic/annotations/slope.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/slope.py:1:1: INP001 File `examples/basic/annotations/slope.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/whisker.py:1:1: INP001 File `examples/basic/annotations/whisker.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/whisker.py:1:1: INP001 File `examples/basic/annotations/whisker.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/areas/stacked_area.py:1:1: INP001 File `examples/basic/areas/stacked_area.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/areas/stacked_area.py:1:1: INP001 File `examples/basic/areas/stacked_area.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/axes/logplot.py:1:1: INP001 File `examples/basic/axes/logplot.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/axes/logplot.py:1:1: INP001 File `examples/basic/axes/logplot.py` is part of an implicit namespace package. Add an `__init__.py`.
... 199 additional changes omitted for rule INP001
+ tests/unit/bokeh/command/subcommands/test_serve.py:540:20: S104 Possible binding to all interfaces
- tests/unit/bokeh/command/subcommands/test_serve.py:540:20: S104 Possible binding to all interfaces
+ tests/unit/bokeh/test_server.py:51:48: S104 Possible binding to all interfaces
- tests/unit/bokeh/test_server.py:51:48: S104 Possible binding to all interfaces
+ tests/unit/bokeh/test_server.py:52:34: S104 Possible binding to all interfaces
- tests/unit/bokeh/test_server.py:52:34: S104 Possible binding to all interfaces
... 198 additional changes omitted for project

Changes by rule (2 rules affected)

code	total	+ violation	- violation	+ fix	- fix
INP001	224	112	112	0	0
S104	6	3	3	0	0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+115 -115 violations, +0 -0 fixes in 1 projects; 43 projects unchanged)

bokeh/bokeh (+115 -115 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ examples/basic/annotations/arrow.py:1:1: INP001 File `examples/basic/annotations/arrow.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/arrow.py:1:1: INP001 File `examples/basic/annotations/arrow.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/arrowheads.py:1:1: INP001 File `examples/basic/annotations/arrowheads.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/arrowheads.py:1:1: INP001 File `examples/basic/annotations/arrowheads.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/band.py:1:1: INP001 File `examples/basic/annotations/band.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/band.py:1:1: INP001 File `examples/basic/annotations/band.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/box_annotation.py:1:1: INP001 File `examples/basic/annotations/box_annotation.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/box_annotation.py:1:1: INP001 File `examples/basic/annotations/box_annotation.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/colorbar_log.py:1:1: INP001 File `examples/basic/annotations/colorbar_log.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/colorbar_log.py:1:1: INP001 File `examples/basic/annotations/colorbar_log.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/label.py:1:1: INP001 File `examples/basic/annotations/label.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/label.py:1:1: INP001 File `examples/basic/annotations/label.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/legend.py:1:1: INP001 File `examples/basic/annotations/legend.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/legend.py:1:1: INP001 File `examples/basic/annotations/legend.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/legend_two_dimensions.py:1:1: INP001 File `examples/basic/annotations/legend_two_dimensions.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/legend_two_dimensions.py:1:1: INP001 File `examples/basic/annotations/legend_two_dimensions.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/legends_item_visibility.py:1:1: INP001 File `examples/basic/annotations/legends_item_visibility.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/legends_item_visibility.py:1:1: INP001 File `examples/basic/annotations/legends_item_visibility.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/slope.py:1:1: INP001 File `examples/basic/annotations/slope.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/slope.py:1:1: INP001 File `examples/basic/annotations/slope.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/annotations/whisker.py:1:1: INP001 File `examples/basic/annotations/whisker.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/annotations/whisker.py:1:1: INP001 File `examples/basic/annotations/whisker.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/areas/stacked_area.py:1:1: INP001 File `examples/basic/areas/stacked_area.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/areas/stacked_area.py:1:1: INP001 File `examples/basic/areas/stacked_area.py` is part of an implicit namespace package. Add an `__init__.py`.
+ examples/basic/axes/logplot.py:1:1: INP001 File `examples/basic/axes/logplot.py` is part of an implicit namespace package. Add an `__init__.py`.
- examples/basic/axes/logplot.py:1:1: INP001 File `examples/basic/axes/logplot.py` is part of an implicit namespace package. Add an `__init__.py`.
... 199 additional changes omitted for rule INP001
+ tests/unit/bokeh/command/subcommands/test_serve.py:540:20: S104 Possible binding to all interfaces
- tests/unit/bokeh/command/subcommands/test_serve.py:540:20: S104 Possible binding to all interfaces
+ tests/unit/bokeh/test_server.py:51:48: S104 Possible binding to all interfaces
- tests/unit/bokeh/test_server.py:51:48: S104 Possible binding to all interfaces
+ tests/unit/bokeh/test_server.py:52:34: S104 Possible binding to all interfaces
- tests/unit/bokeh/test_server.py:52:34: S104 Possible binding to all interfaces
... 198 additional changes omitted for project

Changes by rule (2 rules affected)

code	total	+ violation	- violation	+ fix	- fix
INP001	224	112	112	0	0
S104	6	3	3	0	0

dhruvmanila · 2024-03-09T12:23:39Z

Oh right. We will visit the strings inside a f-string and try to change that. I'll fix this later.

charliermarsh · 2024-03-09T12:54:58Z

Oops, approved before I saw the ecosystem changes.

codspeed-hq · 2024-03-10T05:01:01Z

CodSpeed Performance Report

Merging #10312 will not alter performance

_{Comparing dhruv/move-flake8-quote-rules-to-ast-checker (743c772) with main (0c194f5)}

Summary

✅ 30 untouched benchmarks

crates/ruff_linter/src/rules/flake8_quotes/rules/check_string_quotes.rs

dhruvmanila · 2024-03-14T08:02:31Z

crates/ruff_linter/src/rules/flake8_quotes/rules/check_string_quotes.rs

+        .indexer()
+        .fstring_ranges()
+        .outermost(string_like.start())
+        .is_some_and(|outer| outer.start() < string_like.start() && string_like.end() < outer.end())


We only want to ignore strings which are strictly inside another f-string which is not what TextRange::contains_range does.

dhruvmanila · 2024-03-14T08:31:01Z

Weird, this PR doesn't include any change to the rules detected in the ecosystem checks INP001 and S104. I tried running them locally as well and it gave me no difference between running ecosystem on main and this branch.

MichaReiser · 2024-03-18T13:15:43Z

crates/ruff_linter/src/rules/flake8_quotes/rules/check_string_quotes.rs

-            continue;
-        }
+pub(crate) fn check_string_quotes(checker: &mut Checker, string_like: StringLike) {
+    // If the string is part of a f-string, ignore it.


Nit: I noticed that this rule performs quiet many allocations even in the happy path where the quotes match (collects the text ranges, collects the trivia, etc).

I wonder if we could add a fast path (you may need to wait for @AlexWaygood's changes) where we first test if:

If it's a docstring, the quote matches the configured docstring quotes and if so, exits.

For "regular strings", early exit if the configured inline or multiline quotes are the same and the string quotes match.

Yeah, I had something similar in mind which is to completely rewrite this rule by only using the string flags which I think should be possible. I'll open a tracking issue for it. Thanks for the suggestion!

Edit: #10456

dhruvmanila added the internal An internal refactor or improvement label Mar 9, 2024

dhruvmanila force-pushed the dhruv/string-like-f-string branch from dad0e35 to ab4b600 Compare March 9, 2024 12:06

dhruvmanila requested a review from AlexWaygood as a code owner March 9, 2024 12:06

dhruvmanila force-pushed the dhruv/move-flake8-quote-rules-to-ast-checker branch from 7b77530 to 1e24c01 Compare March 9, 2024 12:06

dhruvmanila requested a review from charliermarsh March 9, 2024 12:07

charliermarsh approved these changes Mar 9, 2024

View reviewed changes

dhruvmanila force-pushed the dhruv/move-flake8-quote-rules-to-ast-checker branch from 42bf089 to d30dc29 Compare March 10, 2024 04:56

dhruvmanila commented Mar 11, 2024

View reviewed changes

crates/ruff_linter/src/rules/flake8_quotes/rules/check_string_quotes.rs Outdated Show resolved Hide resolved

dhruvmanila force-pushed the dhruv/string-like-f-string branch from ab4b600 to 67810b6 Compare March 13, 2024 08:51

dhruvmanila requested a review from MichaReiser as a code owner March 13, 2024 08:51

dhruvmanila force-pushed the dhruv/move-flake8-quote-rules-to-ast-checker branch from d30dc29 to b096569 Compare March 13, 2024 08:51

dhruvmanila mentioned this pull request Mar 13, 2024

Use ExprFString for StringLike::FString variant #10311

Merged

dhruvmanila force-pushed the dhruv/string-like-f-string branch from 67810b6 to 1ef5e95 Compare March 14, 2024 07:43

dhruvmanila force-pushed the dhruv/move-flake8-quote-rules-to-ast-checker branch from b096569 to e2ae5a0 Compare March 14, 2024 07:58

AlexWaygood removed their request for review March 14, 2024 07:59

Base automatically changed from dhruv/string-like-f-string to main March 14, 2024 08:00

dhruvmanila force-pushed the dhruv/move-flake8-quote-rules-to-ast-checker branch from e2ae5a0 to 677178a Compare March 14, 2024 08:01

dhruvmanila commented Mar 14, 2024

View reviewed changes

dhruvmanila requested a review from charliermarsh March 14, 2024 08:31

MichaReiser reviewed Mar 18, 2024

View reviewed changes

MichaReiser approved these changes Mar 18, 2024

View reviewed changes

dhruvmanila added 3 commits March 23, 2024 22:45

Move Q001-3 to AST based checker

61e5f67

Ignore string if it is inside a f-string

b4bed1d

Check if string is completely inside f-string

743c772

dhruvmanila force-pushed the dhruv/move-flake8-quote-rules-to-ast-checker branch from 677178a to 743c772 Compare March 23, 2024 17:15

dhruvmanila merged commit 895d9df into main Mar 23, 2024
17 checks passed

dhruvmanila deleted the dhruv/move-flake8-quote-rules-to-ast-checker branch March 23, 2024 17:29

qarmin mentioned this pull request Mar 24, 2024

Rule Q000-Q002 started to cause panic #10546

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move `Q001-3` to AST based checker #10312

Move `Q001-3` to AST based checker #10312

dhruvmanila commented Mar 9, 2024

github-actions bot commented Mar 9, 2024 •

edited

Loading

dhruvmanila commented Mar 9, 2024

charliermarsh commented Mar 9, 2024

codspeed-hq bot commented Mar 10, 2024 •

edited

Loading

dhruvmanila Mar 14, 2024

dhruvmanila commented Mar 14, 2024

MichaReiser Mar 18, 2024

dhruvmanila Mar 18, 2024 •

edited

Loading

Move Q001-3 to AST based checker #10312

Move Q001-3 to AST based checker #10312

Conversation

dhruvmanila commented Mar 9, 2024

Summary

Test Plan

github-actions bot commented Mar 9, 2024 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

dhruvmanila commented Mar 9, 2024

charliermarsh commented Mar 9, 2024

codspeed-hq bot commented Mar 10, 2024 • edited Loading

CodSpeed Performance Report

Merging #10312 will not alter performance

Summary

dhruvmanila Mar 14, 2024

Choose a reason for hiding this comment

dhruvmanila commented Mar 14, 2024

MichaReiser Mar 18, 2024

Choose a reason for hiding this comment

dhruvmanila Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Move `Q001-3` to AST based checker #10312

Move `Q001-3` to AST based checker #10312

github-actions bot commented Mar 9, 2024 •

edited

Loading

`ruff-ecosystem` results

codspeed-hq bot commented Mar 10, 2024 •

edited

Loading

dhruvmanila Mar 18, 2024 •

edited

Loading