-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flake8-todos
] Allow words starting with todo
#13640
Conversation
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
COM812 | 228 | 0 | 228 | 0 | 0 |
S101 | 212 | 0 | 212 | 0 | 0 |
ERA001 | 173 | 0 | 173 | 0 | 0 |
D102 | 126 | 0 | 126 | 0 | 0 |
E501 | 90 | 0 | 90 | 0 | 0 |
D103 | 80 | 0 | 80 | 0 | 0 |
D101 | 61 | 0 | 61 | 0 | 0 |
FBT001 | 44 | 0 | 44 | 0 | 0 |
TRY003 | 43 | 0 | 43 | 0 | 0 |
D300 | 32 | 0 | 32 | 0 | 0 |
PLR2004 | 31 | 0 | 31 | 0 | 0 |
D210 | 31 | 0 | 31 | 0 | 0 |
ANN001 | 29 | 0 | 29 | 0 | 0 |
Q002 | 29 | 0 | 29 | 0 | 0 |
D107 | 28 | 0 | 28 | 0 | 0 |
FBT002 | 27 | 0 | 27 | 0 | 0 |
EM101 | 27 | 0 | 27 | 0 | 0 |
EM102 | 19 | 0 | 19 | 0 | 0 |
RET505 | 18 | 0 | 18 | 0 | 0 |
C901 | 18 | 0 | 18 | 0 | 0 |
FIX002 | 17 | 0 | 17 | 0 | 0 |
TD002 | 17 | 0 | 17 | 0 | 0 |
TD003 | 17 | 0 | 17 | 0 | 0 |
D205 | 15 | 0 | 15 | 0 | 0 |
D212 | 14 | 0 | 14 | 0 | 0 |
RET504 | 13 | 0 | 13 | 0 | 0 |
PLR0912 | 13 | 0 | 13 | 0 | 0 |
ANN201 | 11 | 0 | 11 | 0 | 0 |
N806 | 11 | 0 | 11 | 0 | 0 |
ARG001 | 10 | 0 | 10 | 0 | 0 |
D400 | 10 | 0 | 10 | 0 | 0 |
D415 | 10 | 0 | 10 | 0 | 0 |
B904 | 10 | 0 | 10 | 0 | 0 |
PLR0913 | 9 | 0 | 9 | 0 | 0 |
PLR0915 | 9 | 0 | 9 | 0 | 0 |
FIX004 | 9 | 0 | 9 | 0 | 0 |
RUF012 | 8 | 0 | 8 | 0 | 0 |
E402 | 7 | 0 | 7 | 0 | 0 |
SIM103 | 6 | 0 | 6 | 0 | 0 |
FBT003 | 6 | 0 | 6 | 0 | 0 |
PT018 | 6 | 0 | 6 | 0 | 0 |
A001 | 6 | 0 | 6 | 0 | 0 |
PTH118 | 5 | 0 | 5 | 0 | 0 |
S105 | 5 | 0 | 5 | 0 | 0 |
TCH002 | 5 | 0 | 5 | 0 | 0 |
D200 | 5 | 0 | 5 | 0 | 0 |
PLR0911 | 5 | 0 | 5 | 0 | 0 |
B008 | 5 | 0 | 5 | 0 | 0 |
D401 | 5 | 0 | 5 | 0 | 0 |
D202 | 5 | 0 | 5 | 0 | 0 |
BLE001 | 4 | 0 | 4 | 0 | 0 |
TID252 | 4 | 0 | 4 | 0 | 0 |
SIM114 | 4 | 0 | 4 | 0 | 0 |
N802 | 3 | 0 | 3 | 0 | 0 |
ANN401 | 3 | 0 | 3 | 0 | 0 |
TCH001 | 3 | 0 | 3 | 0 | 0 |
E741 | 3 | 0 | 3 | 0 | 0 |
SIM108 | 3 | 0 | 3 | 0 | 0 |
RET506 | 3 | 0 | 3 | 0 | 0 |
D209 | 3 | 0 | 3 | 0 | 0 |
B007 | 3 | 0 | 3 | 0 | 0 |
PLW0603 | 3 | 0 | 3 | 0 | 0 |
PT006 | 2 | 0 | 2 | 0 | 0 |
PTH123 | 2 | 0 | 2 | 0 | 0 |
Q000 | 2 | 0 | 2 | 0 | 0 |
ARG002 | 2 | 0 | 2 | 0 | 0 |
TRY301 | 2 | 0 | 2 | 0 | 0 |
D100 | 2 | 0 | 2 | 0 | 0 |
TRY300 | 2 | 0 | 2 | 0 | 0 |
DTZ001 | 1 | 0 | 1 | 0 | 0 |
ANN202 | 1 | 0 | 1 | 0 | 0 |
RUF100 | 1 | 0 | 1 | 0 | 0 |
TCH003 | 1 | 0 | 1 | 0 | 0 |
UP035 | 1 | 0 | 1 | 0 | 0 |
ARG005 | 1 | 0 | 1 | 0 | 0 |
PTH111 | 1 | 0 | 1 | 0 | 0 |
D105 | 1 | 0 | 1 | 0 | 0 |
TRY201 | 1 | 0 | 1 | 0 | 0 |
SLF001 | 1 | 0 | 1 | 0 | 0 |
RUF001 | 1 | 0 | 1 | 0 | 0 |
PT017 | 1 | 0 | 1 | 0 | 0 |
TRY400 | 1 | 0 | 1 | 0 | 0 |
TD004 | 1 | 0 | 1 | 0 | 0 |
PLR5501 | 1 | 0 | 1 | 0 | 0 |
D413 | 1 | 0 | 1 | 0 | 0 |
PLW2901 | 1 | 0 | 1 | 0 | 0 |
D104 | 1 | 0 | 1 | 0 | 0 |
D404 | 1 | 0 | 1 | 0 | 0 |
RET508 | 1 | 0 | 1 | 0 | 0 |
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 | - violation | + fix | - fix |
---|---|---|---|---|---|
COM812 | 228 | 0 | 228 | 0 | 0 |
S101 | 212 | 0 | 212 | 0 | 0 |
ERA001 | 173 | 0 | 173 | 0 | 0 |
D102 | 126 | 0 | 126 | 0 | 0 |
E501 | 90 | 0 | 90 | 0 | 0 |
D103 | 80 | 0 | 80 | 0 | 0 |
D101 | 61 | 0 | 61 | 0 | 0 |
FBT001 | 44 | 0 | 44 | 0 | 0 |
TRY003 | 43 | 0 | 43 | 0 | 0 |
D300 | 32 | 0 | 32 | 0 | 0 |
PLR2004 | 31 | 0 | 31 | 0 | 0 |
D210 | 31 | 0 | 31 | 0 | 0 |
ANN001 | 29 | 0 | 29 | 0 | 0 |
Q002 | 29 | 0 | 29 | 0 | 0 |
D107 | 28 | 0 | 28 | 0 | 0 |
FBT002 | 27 | 0 | 27 | 0 | 0 |
EM101 | 27 | 0 | 27 | 0 | 0 |
PLR6301 | 26 | 0 | 26 | 0 | 0 |
E226 | 24 | 0 | 24 | 0 | 0 |
EM102 | 19 | 0 | 19 | 0 | 0 |
PLR6201 | 19 | 0 | 19 | 0 | 0 |
RET505 | 18 | 0 | 18 | 0 | 0 |
C901 | 18 | 0 | 18 | 0 | 0 |
FIX002 | 17 | 0 | 17 | 0 | 0 |
TD002 | 17 | 0 | 17 | 0 | 0 |
TD003 | 17 | 0 | 17 | 0 | 0 |
E265 | 16 | 0 | 16 | 0 | 0 |
D205 | 15 | 0 | 15 | 0 | 0 |
D212 | 14 | 0 | 14 | 0 | 0 |
RET504 | 13 | 0 | 13 | 0 | 0 |
PLR0912 | 13 | 0 | 13 | 0 | 0 |
DOC201 | 12 | 0 | 12 | 0 | 0 |
ANN201 | 11 | 0 | 11 | 0 | 0 |
N806 | 11 | 0 | 11 | 0 | 0 |
ARG001 | 10 | 0 | 10 | 0 | 0 |
D400 | 10 | 0 | 10 | 0 | 0 |
D415 | 10 | 0 | 10 | 0 | 0 |
B904 | 10 | 0 | 10 | 0 | 0 |
PLR0914 | 10 | 0 | 10 | 0 | 0 |
PLR0913 | 9 | 0 | 9 | 0 | 0 |
PLR0917 | 9 | 0 | 9 | 0 | 0 |
PLR0915 | 9 | 0 | 9 | 0 | 0 |
FIX004 | 9 | 0 | 9 | 0 | 0 |
PLC1901 | 9 | 0 | 9 | 0 | 0 |
RUF012 | 8 | 0 | 8 | 0 | 0 |
E402 | 7 | 0 | 7 | 0 | 0 |
CPY001 | 6 | 0 | 6 | 0 | 0 |
SIM103 | 6 | 0 | 6 | 0 | 0 |
FBT003 | 6 | 0 | 6 | 0 | 0 |
PT018 | 6 | 0 | 6 | 0 | 0 |
F841 | 6 | 0 | 6 | 0 | 0 |
A001 | 6 | 0 | 6 | 0 | 0 |
PTH118 | 5 | 0 | 5 | 0 | 0 |
S105 | 5 | 0 | 5 | 0 | 0 |
TCH002 | 5 | 0 | 5 | 0 | 0 |
D200 | 5 | 0 | 5 | 0 | 0 |
PLR0911 | 5 | 0 | 5 | 0 | 0 |
PLC0415 | 5 | 0 | 5 | 0 | 0 |
B008 | 5 | 0 | 5 | 0 | 0 |
PLR0904 | 5 | 0 | 5 | 0 | 0 |
D401 | 5 | 0 | 5 | 0 | 0 |
D202 | 5 | 0 | 5 | 0 | 0 |
BLE001 | 4 | 0 | 4 | 0 | 0 |
DOC501 | 4 | 0 | 4 | 0 | 0 |
TID252 | 4 | 0 | 4 | 0 | 0 |
SIM114 | 4 | 0 | 4 | 0 | 0 |
N802 | 3 | 0 | 3 | 0 | 0 |
ANN401 | 3 | 0 | 3 | 0 | 0 |
TCH001 | 3 | 0 | 3 | 0 | 0 |
E741 | 3 | 0 | 3 | 0 | 0 |
E302 | 3 | 0 | 3 | 0 | 0 |
SIM108 | 3 | 0 | 3 | 0 | 0 |
RET506 | 3 | 0 | 3 | 0 | 0 |
D209 | 3 | 0 | 3 | 0 | 0 |
B007 | 3 | 0 | 3 | 0 | 0 |
PLW0603 | 3 | 0 | 3 | 0 | 0 |
PT006 | 2 | 0 | 2 | 0 | 0 |
PLW1514 | 2 | 0 | 2 | 0 | 0 |
PTH123 | 2 | 0 | 2 | 0 | 0 |
Q000 | 2 | 0 | 2 | 0 | 0 |
ARG002 | 2 | 0 | 2 | 0 | 0 |
TRY301 | 2 | 0 | 2 | 0 | 0 |
D100 | 2 | 0 | 2 | 0 | 0 |
TRY300 | 2 | 0 | 2 | 0 | 0 |
DTZ001 | 1 | 0 | 1 | 0 | 0 |
ANN202 | 1 | 0 | 1 | 0 | 0 |
RUF100 | 1 | 0 | 1 | 0 | 0 |
TCH003 | 1 | 0 | 1 | 0 | 0 |
UP035 | 1 | 0 | 1 | 0 | 0 |
PLC2701 | 1 | 0 | 1 | 0 | 0 |
ARG005 | 1 | 0 | 1 | 0 | 0 |
PTH111 | 1 | 0 | 1 | 0 | 0 |
E261 | 1 | 0 | 1 | 0 | 0 |
FURB180 | 1 | 0 | 1 | 0 | 0 |
D105 | 1 | 0 | 1 | 0 | 0 |
TRY201 | 1 | 0 | 1 | 0 | 0 |
SLF001 | 1 | 0 | 1 | 0 | 0 |
RUF001 | 1 | 0 | 1 | 0 | 0 |
PT017 | 1 | 0 | 1 | 0 | 0 |
TRY400 | 1 | 0 | 1 | 0 | 0 |
TD004 | 1 | 0 | 1 | 0 | 0 |
PLR5501 | 1 | 0 | 1 | 0 | 0 |
D413 | 1 | 0 | 1 | 0 | 0 |
PLW2901 | 1 | 0 | 1 | 0 | 0 |
FURB145 | 1 | 0 | 1 | 0 | 0 |
S405 | 1 | 0 | 1 | 0 | 0 |
PLR0916 | 1 | 0 | 1 | 0 | 0 |
D104 | 1 | 0 | 1 | 0 | 0 |
D404 | 1 | 0 | 1 | 0 | 0 |
PLR1702 | 1 | 0 | 1 | 0 | 0 |
RET508 | 1 | 0 | 1 | 0 | 0 |
FURB118 | 1 | 0 | 1 | 0 | 0 |
The ecosystem changes have a mix of new false negatives: and fixed false positives: |
Hmm, the question is if "Hacky" is used as a tag, or just being a word. In that regard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
I think we can simplify the implementation, see my inline comment.
crates/ruff_linter/src/directives.rs
Outdated
for length in [3, 4, 5] { | ||
let Some(substr) = s.get(..length) else { | ||
break; | ||
}; | ||
|
||
if s.len() > length { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand, sorry (Maybe my Rust knowledge is not good enough)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have applied your suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Summary
Allow words which start with "todo" or other todo-directives
Fixes #13638
Test Plan
Updated snapshots