[`flake8-todos`] Allow words starting with todo by autinerd · Pull Request #13640 · astral-sh/ruff

autinerd · 2024-10-05T09:51:08Z

Summary

Allow words which start with "todo" or other todo-directives
Fixes #13638

Test Plan

Updated snapshots

github-actions · 2024-10-05T10:04:56Z

`ruff-ecosystem` results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+0 -1693 violations, +0 -0 fixes in 5 projects; 49 projects unchanged)

apache/airflow (+0 -81 violations, +0 -0 fixes)

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

- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:114:6: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:122:6: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:138:16: DTZ001 `datetime.datetime()` called without a `tzinfo` argument
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:143:107: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:152:78: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:162:83: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:168:82: COM812 [*] Trailing comma missing
... 10 additional changes omitted for rule COM812
- tests/www/views/test_views_dataset.py:103:40: PLR2004 Magic value used in comparison, consider replacing `400` with a constant variable
- tests/www/views/test_views_dataset.py:108:40: PLR2004 Magic value used in comparison, consider replacing `400` with a constant variable
- tests/www/views/test_views_dataset.py:111:40: ANN001 Missing type annotation for function argument `admin_client`
... 71 additional changes omitted for project

apache/superset (+0 -246 violations, +0 -0 fixes)

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

- superset/config.py:1004:21: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
- superset/config.py:1015:9: ERA001 Found commented-out code
- superset/config.py:1016:9: ERA001 Found commented-out code
- superset/config.py:1017:9: ERA001 Found commented-out code
- superset/config.py:1018:9: ERA001 Found commented-out code
- superset/config.py:1026:1: ERA001 Found commented-out code
- superset/config.py:105:1: ERA001 Found commented-out code
... 167 additional changes omitted for rule ERA001
- superset/config.py:1068:89: E501 Line too long (90 > 88)
- superset/config.py:1089:64: COM812 [*] Trailing comma missing
- superset/config.py:1139:5: D103 Missing docstring in public function
... 236 additional changes omitted for project

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

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

- src/bokeh/colors/color.py:116:9: D210 [*] No whitespaces allowed surrounding docstring text
- src/bokeh/colors/color.py:116:9: D300 [*] Use triple double quotes `"""`
- src/bokeh/colors/color.py:116:9: Q002 [*] Single quote docstring found but double quotes preferred
- src/bokeh/colors/color.py:133:9: D210 [*] No whitespaces allowed surrounding docstring text
- src/bokeh/colors/color.py:133:9: D300 [*] Use triple double quotes `"""`
- src/bokeh/colors/color.py:133:9: Q002 [*] Single quote docstring found but double quotes preferred
- src/bokeh/colors/color.py:148:9: D210 [*] No whitespaces allowed surrounding docstring text
- src/bokeh/colors/color.py:148:9: D300 [*] Use triple double quotes `"""`
- src/bokeh/colors/color.py:148:9: Q002 [*] Single quote docstring found but double quotes preferred
- src/bokeh/colors/color.py:161:12: E741 Ambiguous variable name: `l`
... 123 additional changes omitted for project

pandas-dev/pandas (+0 -0 violations, +0 -0 fixes)

zulip/zulip (+0 -1233 violations, +0 -0 fixes)

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

- corporate/lib/stripe.py:1000:57: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1005:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1009:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1013:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1017:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1021:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1037:9: D102 Missing docstring in public method
... 120 additional changes omitted for rule D102
- corporate/lib/stripe.py:1038:9: SIM103 Return the condition directly
- corporate/lib/stripe.py:1043:75: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1046:101: E501 Line too long (116 > 100)
- corporate/lib/stripe.py:1057:75: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1063:9: S101 Use of `assert` detected
- corporate/lib/stripe.py:1068:64: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1074:9: S101 Use of `assert` detected
- corporate/lib/stripe.py:1099:16: RET504 Unnecessary assignment to `customer` before `return` statement
- corporate/lib/stripe.py:1103:61: FBT001 Boolean-typed positional argument in function definition
- corporate/lib/stripe.py:1103:61: FBT002 Boolean default positional argument in function definition
- corporate/lib/stripe.py:1103:87: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1106:92: COM812 [*] Trailing comma missing
... 197 additional changes omitted for rule COM812
- corporate/lib/stripe.py:111:5: D103 Missing docstring in public function
- corporate/lib/stripe.py:1128:101: E501 Line too long (104 > 100)
- corporate/lib/stripe.py:1130:13: S101 Use of `assert` detected
- corporate/lib/stripe.py:1137:86: FBT003 Boolean positional value in function call
- corporate/lib/stripe.py:1144:9: D205 1 blank line required between summary line and description
- corporate/lib/stripe.py:1144:9: D212 [*] Multi-line docstring summary should start at the first line
- corporate/lib/stripe.py:1145:101: E501 Line too long (101 > 100)
- corporate/lib/stripe.py:114:5: SIM108 Use ternary operator `precision = 0 if cents % 100 == 0 else 2` instead of `if`-`else`-block
- corporate/lib/stripe.py:1150:9: PT018 Assertion should be broken down into multiple parts
- corporate/lib/stripe.py:1150:9: S101 Use of `assert` detected
- corporate/lib/stripe.py:1157:19: TRY003 Avoid specifying long messages outside the exception class
- corporate/lib/stripe.py:1158:17: EM101 Exception must not use a string literal, assign to variable first
- corporate/lib/stripe.py:1163:13: S101 Use of `assert` detected
- corporate/lib/stripe.py:1164:13: S101 Use of `assert` detected
... 207 additional changes omitted for rule S101
- corporate/lib/stripe.py:1206:17: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling
- corporate/lib/stripe.py:1206:23: TRY003 Avoid specifying long messages outside the exception class
- corporate/lib/stripe.py:1206:39: EM101 Exception must not use a string literal, assign to variable first
... 1197 additional changes omitted for project

Changes by rule (89 rules affected)

code	total	- violation
COM812	228	228
S101	212	212
ERA001	173	173
D102	126	126
E501	90	90
D103	80	80
D101	61	61
FBT001	44	44
TRY003	43	43
D300	32	32
PLR2004	31	31
D210	31	31
ANN001	29	29
Q002	29	29
D107	28	28
FBT002	27	27
EM101	27	27
EM102	19	19
RET505	18	18
C901	18	18
FIX002	17	17
TD002	17	17
TD003	17	17
D205	15	15
D212	14	14
RET504	13	13
PLR0912	13	13
ANN201	11	11
N806	11	11
ARG001	10	10
D400	10	10
D415	10	10
B904	10	10
PLR0913	9	9
PLR0915	9	9
FIX004	9	9
RUF012	8	8
E402	7	7
SIM103	6	6
FBT003	6	6
PT018	6	6
A001	6	6
PTH118	5	5
S105	5	5
TCH002	5	5
D200	5	5
PLR0911	5	5
B008	5	5
D401	5	5
D202	5	5
BLE001	4	4
TID252	4	4
SIM114	4	4
N802	3	3
ANN401	3	3
TCH001	3	3
E741	3	3
SIM108	3	3
RET506	3	3
D209	3	3
B007	3	3
PLW0603	3	3
PT006	2	2
PTH123	2	2
Q000	2	2
ARG002	2	2
TRY301	2	2
D100	2	2
TRY300	2	2
DTZ001	1	1
ANN202	1	1
RUF100	1	1
TCH003	1	1
UP035	1	1
ARG005	1	1
PTH111	1	1
D105	1	1
TRY201	1	1
SLF001	1	1
RUF001	1	1
PT017	1	1
TRY400	1	1
TD004	1	1
PLR5501	1	1
D413	1	1
PLW2901	1	1
D104	1	1
D404	1	1
RET508	1	1

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+0 -1857 violations, +0 -0 fixes in 4 projects; 50 projects unchanged)

apache/airflow (+0 -95 violations, +0 -0 fixes)

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

- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:114:6: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:122:6: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:138:16: DTZ001 `datetime.datetime()` called without a `tzinfo` argument
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:143:107: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:152:78: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:162:83: COM812 [*] Trailing comma missing
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:168:82: COM812 [*] Trailing comma missing
... 10 additional changes omitted for rule COM812
- providers/tests/system/google/cloud/cloud_sql/example_cloud_sql.py:1:1: CPY001 Missing copyright notice at top of file
- tests/www/views/test_views_dataset.py:103:40: PLR2004 Magic value used in comparison, consider replacing `400` with a constant variable
- tests/www/views/test_views_dataset.py:108:40: PLR2004 Magic value used in comparison, consider replacing `400` with a constant variable
... 85 additional changes omitted for project

apache/superset (+0 -250 violations, +0 -0 fixes)

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

- superset/config.py:1004:21: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
- superset/config.py:1015:9: ERA001 Found commented-out code
- superset/config.py:1016:9: ERA001 Found commented-out code
- superset/config.py:1017:9: ERA001 Found commented-out code
- superset/config.py:1018:9: ERA001 Found commented-out code
- superset/config.py:1026:1: ERA001 Found commented-out code
- superset/config.py:105:1: ERA001 Found commented-out code
... 167 additional changes omitted for rule ERA001
- superset/config.py:1068:89: E501 Line too long (90 > 88)
- superset/config.py:1089:64: COM812 [*] Trailing comma missing
- superset/config.py:1139:5: D103 Missing docstring in public function
... 240 additional changes omitted for project

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

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

- src/bokeh/colors/color.py:116:9: D210 [*] No whitespaces allowed surrounding docstring text
- src/bokeh/colors/color.py:116:9: D300 [*] Use triple double quotes `"""`
- src/bokeh/colors/color.py:116:9: Q002 [*] Single quote docstring found but double quotes preferred
- src/bokeh/colors/color.py:11:1: E265 [*] Block comment should start with `# `
- src/bokeh/colors/color.py:133:9: D210 [*] No whitespaces allowed surrounding docstring text
- src/bokeh/colors/color.py:133:9: D300 [*] Use triple double quotes `"""`
- src/bokeh/colors/color.py:133:9: Q002 [*] Single quote docstring found but double quotes preferred
- src/bokeh/colors/color.py:13:1: E265 [*] Block comment should start with `# `
- src/bokeh/colors/color.py:148:9: D210 [*] No whitespaces allowed surrounding docstring text
- src/bokeh/colors/color.py:148:9: D300 [*] Use triple double quotes `"""`
... 170 additional changes omitted for project

zulip/zulip (+0 -1332 violations, +0 -0 fixes)

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

- corporate/lib/stripe.py:1000:57: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1005:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1009:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1013:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1017:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1021:9: D102 Missing docstring in public method
- corporate/lib/stripe.py:1037:9: D102 Missing docstring in public method
... 120 additional changes omitted for rule D102
- corporate/lib/stripe.py:1038:9: SIM103 Return the condition directly
- corporate/lib/stripe.py:1042:9: PLR6301 Method `get_remote_server_legacy_plan` could be a function, class method, or static method
- corporate/lib/stripe.py:1043:75: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1046:101: E501 Line too long (116 > 100)
- corporate/lib/stripe.py:1057:75: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1063:9: S101 Use of `assert` detected
- corporate/lib/stripe.py:1068:64: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1074:9: S101 Use of `assert` detected
- corporate/lib/stripe.py:1099:16: RET504 Unnecessary assignment to `customer` before `return` statement
- corporate/lib/stripe.py:1103:61: FBT001 Boolean-typed positional argument in function definition
- corporate/lib/stripe.py:1103:61: FBT002 Boolean default positional argument in function definition
- corporate/lib/stripe.py:1103:87: COM812 [*] Trailing comma missing
- corporate/lib/stripe.py:1106:92: COM812 [*] Trailing comma missing
... 197 additional changes omitted for rule COM812
- corporate/lib/stripe.py:111:5: D103 Missing docstring in public function
- corporate/lib/stripe.py:1128:101: E501 Line too long (104 > 100)
- corporate/lib/stripe.py:1130:13: S101 Use of `assert` detected
- corporate/lib/stripe.py:1137:86: FBT003 Boolean positional value in function call
- corporate/lib/stripe.py:1144:9: D205 1 blank line required between summary line and description
- corporate/lib/stripe.py:1144:9: D212 [*] Multi-line docstring summary should start at the first line
- corporate/lib/stripe.py:1145:101: E501 Line too long (101 > 100)
- corporate/lib/stripe.py:114:5: SIM108 Use ternary operator `precision = 0 if cents % 100 == 0 else 2` instead of `if`-`else`-block
- corporate/lib/stripe.py:1150:9: PT018 Assertion should be broken down into multiple parts
- corporate/lib/stripe.py:1150:9: S101 Use of `assert` detected
- corporate/lib/stripe.py:1157:19: DOC501 Raised exception `BillingError` missing from docstring
- corporate/lib/stripe.py:1157:19: TRY003 Avoid specifying long messages outside the exception class
- corporate/lib/stripe.py:1158:17: EM101 Exception must not use a string literal, assign to variable first
- corporate/lib/stripe.py:1163:13: S101 Use of `assert` detected
- corporate/lib/stripe.py:1164:13: S101 Use of `assert` detected
... 207 additional changes omitted for rule S101
... 1297 additional changes omitted for project

Changes by rule (112 rules affected)

code	total	- violation
COM812	228	228
S101	212	212
ERA001	173	173
D102	126	126
E501	90	90
D103	80	80
D101	61	61
FBT001	44	44
TRY003	43	43
D300	32	32
PLR2004	31	31
D210	31	31
ANN001	29	29
Q002	29	29
D107	28	28
FBT002	27	27
EM101	27	27
PLR6301	26	26
E226	24	24
EM102	19	19
PLR6201	19	19
RET505	18	18
C901	18	18
FIX002	17	17
TD002	17	17
TD003	17	17
E265	16	16
D205	15	15
D212	14	14
RET504	13	13
PLR0912	13	13
DOC201	12	12
ANN201	11	11
N806	11	11
ARG001	10	10
D400	10	10
D415	10	10
B904	10	10
PLR0914	10	10
PLR0913	9	9
PLR0917	9	9
PLR0915	9	9
FIX004	9	9
PLC1901	9	9
RUF012	8	8
E402	7	7
CPY001	6	6
SIM103	6	6
FBT003	6	6
PT018	6	6
F841	6	6
A001	6	6
PTH118	5	5
S105	5	5
TCH002	5	5
D200	5	5
PLR0911	5	5
PLC0415	5	5
B008	5	5
PLR0904	5	5
D401	5	5
D202	5	5
BLE001	4	4
DOC501	4	4
TID252	4	4
SIM114	4	4
N802	3	3
ANN401	3	3
TCH001	3	3
E741	3	3
E302	3	3
SIM108	3	3
RET506	3	3
D209	3	3
B007	3	3
PLW0603	3	3
PT006	2	2
PLW1514	2	2
PTH123	2	2
Q000	2	2
ARG002	2	2
TRY301	2	2
D100	2	2
TRY300	2	2
DTZ001	1	1
ANN202	1	1
RUF100	1	1
TCH003	1	1
UP035	1	1
PLC2701	1	1
ARG005	1	1
PTH111	1	1
E261	1	1
FURB180	1	1
D105	1	1
TRY201	1	1
SLF001	1	1
RUF001	1	1
PT017	1	1
TRY400	1	1
TD004	1	1
PLR5501	1	1
D413	1	1
PLW2901	1	1
FURB145	1	1
S405	1	1
PLR0916	1	1
D104	1	1
D404	1	1
PLR1702	1	1
RET508	1	1
FURB118	1	1

zanieb · 2024-10-05T15:33:15Z

The ecosystem changes have a mix of new false negatives:

https://github.com/zulip/zulip/blob/4b4b6c5ebe0880c7e8f79c659d23ebdce0dab309/zerver/actions/create_realm.py#L239

and fixed false positives:

https://github.com/zulip/zulip/blob/4b4b6c5ebe0880c7e8f79c659d23ebdce0dab309/zerver/views/realm_logo.py#L68

autinerd · 2024-10-05T19:18:03Z

Hmm, the question is if "Hacky" is used as a tag, or just being a word. In that regard # Hackers could compromise it would be a todo-tag as well.

MichaReiser

The change makes sense to me. Although our settings aren't very explicit about what "starting" means:

Comments starting with these tags will be ignored by commented-out code detection (ERA), and skipped by line-length rules (E501) if ignore-overlong-task-comments is set to true.

https://docs.astral.sh/ruff/settings/#lint_task-tags

What surprised me is that this not only changes the behavior of flake8-todos but also flake8-fixme

Flake8-fixme only tests for the use of task tags at word boundaries.

https://github.com/tommilligan/flake8-fixme/blob/master/flake8_fixme/__init__.py

I think we can simplify the implementation, see my inline comment.

MichaReiser · 2024-10-07T09:09:54Z

crates/ruff_linter/src/directives.rs

                break;
            };

+            if s.len() > length {


Nit: I would change the from_str implementation to do a direct match on the string (instead of selecting the substring) and instead split by word boundaries here:

ruff/crates/ruff_linter/src/directives.rs

Line 302 in 8f40928

if let Ok(directive_kind) = trimmed.parse::<TodoDirectiveKind>() {

That should simplify the implementation

I don't quite understand, sorry (Maybe my Rust knowledge is not good enough)

I'm proposing a small rewrite because I think it will simplify the overall code now that we make the change here.

What I propose is that FromStr does a full match (matching the entire string and not just a prefix). So from_str becomes something like this:

match s.to_lowercase().as_str() { "fixme" => { Ok(TodoDirectiveKind::Fixme) } "hack" => { Ok(TodoDirectiveKind::Hack) } "todo" => { Ok(TodoDirectiveKind::Todo) } "xxx" => { Ok(TodoDirectiveKind::Xxx) } _ =>Err(()), }

This does require that we change the one call site to implement the: split by word boundaries (whitespace) and we only pass the first word to FromStr

I have applied your suggestion.

MichaReiser

Thanks

[flake8-todos] Allow words starting with todo

9145cb2

MichaReiser added the rule Implementing or modifying a lint rule label Oct 7, 2024

MichaReiser requested changes Oct 7, 2024

View reviewed changes

autinerd and others added 2 commits October 12, 2024 21:33

Apply review

b699d13

Add comment explaining why we aren't using split_whitespace

2f50b77

MichaReiser approved these changes Oct 14, 2024

View reviewed changes

MichaReiser enabled auto-merge (squash) October 14, 2024 09:36

MichaReiser disabled auto-merge October 14, 2024 09:37

Clippy

457f2ec

MichaReiser enabled auto-merge (squash) October 14, 2024 10:17

MichaReiser merged commit 9bb4722 into astral-sh:main Oct 14, 2024

MichaReiser mentioned this pull request Oct 15, 2024

Fix TODO directive out of bounds acccess #13756

Merged

BrewTestBot mentioned this pull request Oct 17, 2024

ruff 0.7.0 Homebrew/homebrew-core#194818

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`flake8-todos`] Allow words starting with todo#13640

[`flake8-todos`] Allow words starting with todo#13640
MichaReiser merged 4 commits intoastral-sh:mainfrom
autinerd:todo-words

autinerd commented Oct 5, 2024

Uh oh!

github-actions bot commented Oct 5, 2024 •

edited

Loading

Uh oh!

zanieb commented Oct 5, 2024

Uh oh!

autinerd commented Oct 5, 2024

Uh oh!

MichaReiser left a comment

Uh oh!

MichaReiser Oct 7, 2024

Uh oh!

autinerd Oct 11, 2024

Uh oh!

MichaReiser Oct 11, 2024

Uh oh!

autinerd Oct 12, 2024 •

edited

Loading

Uh oh!

MichaReiser left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

autinerd commented Oct 5, 2024

Summary

Test Plan

Uh oh!

github-actions bot commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

zanieb commented Oct 5, 2024

Uh oh!

autinerd commented Oct 5, 2024

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

MichaReiser Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

autinerd Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

MichaReiser Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

autinerd Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Oct 5, 2024 •

edited

Loading

`ruff-ecosystem` results

autinerd Oct 12, 2024 •

edited

Loading