Flag B018 for strings and f-strings which aren't docstrings
#11302
+640
−18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| B018 | 3446 | 3446 | 0 | 0 | 0 |
|
Going through the ecosystem changes, it seems like most of the violations is because the string is used as a docstring for a variable or a class attribute. While for |
|
Oh, I just understood what Charlie meant by #11292 (comment), I'll see on to avoid flagging these. I think one way would be to add a |
|
Yeah we should not flag those. Thanks Dhruv! |
dhruvmanila
force-pushed
the
dhruv/b018-string
branch
from
1563582
to
fa69064
Compare
dhruvmanila
force-pushed
the
dhruv/b018-string
branch
2 times, most recently
from
49ecbb8
to
f7ee407
Compare
dhruvmanila
force-pushed
the
dhruv/attribute-docstring
branch
from
43c3e98
to
1e806cf
Compare
dhruvmanila
force-pushed
the
dhruv/b018-string
branch
from
f7ee407
to
170c58e
Compare
|
Still quite a lot of usages of triple-quoted strings which could be comments instead. This makes me wonder if this should be behind preview? |
dhruvmanila
force-pushed
the
dhruv/b018-string
branch
from
170c58e
to
727c3df
Compare
dhruvmanila
force-pushed
the
dhruv/attribute-docstring
branch
from
d4b5002
to
eb62b74
Compare
dhruvmanila
force-pushed
the
dhruv/b018-string
branch
2 times, most recently
from
bb04bc8
to
e606aee
Compare
dhruvmanila
added a commit
that referenced
this pull request
May 10, 2024
## Summary This PR adds updates the semantic model to detect attribute docstring. Refer to [PEP 258](https://peps.python.org/pep-0258/#attribute-docstrings) for the definition of an attribute docstring. This PR doesn't add full support for it but only considers string literals as attribute docstring for the following cases: 1. A string literal following an assignment statement in the **global scope**. 2. A global class attribute For an assignment statement, it's considered an attribute docstring only if the target expression is a name expression (`x = 1`). So, chained assignment, multiple assignment or unpacking, and starred expression, which are all valid in the target position, aren't considered here. In `__init__` method, an assignment to the `self` variable like `self.x = 1` is also a candidate for an attribute docstring. **This PR does not support this position.** ## Test Plan I used the following source code along with a print statement to verify that the attribute docstring detection is correct. Refer to the PR description for the code snippet. I'll add this in the follow-up PR (#11302) which uses this method.
dhruvmanila
force-pushed
the
dhruv/b018-string
branch
from
e606aee
to
b4b4c93
Compare
|
I do think it should probably be in preview. |
| @@ -69,15 +69,11 @@ impl Violation for UselessExpression { | |||
| /// B018 | |||
| pub(crate) fn useless_expression(checker: &mut Checker, value: &Expr) { | |||
| // Ignore comparisons, as they're handled by `useless_comparison`. | |||
| if value.is_compare_expr() { | |||
| if matches!(value, Expr::Compare(_) | Expr::EllipsisLiteral(_)) { | |||
There was a problem hiding this comment.
By the way, we could flag EllipsisLiteral I think? As long as we ignore stub methods...
There was a problem hiding this comment.
But lets ignore that for this PR.
dhruvmanila
force-pushed
the
dhruv/b018-string
branch
from
b4b4c93
to
24536f3
Compare
dhruvmanila
force-pushed
the
dhruv/b018-string
branch
from
3acfcbf
to
9cb191c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR updates the
B018heuristic to consider standalone string literals, except for those used as docstring and attribute docstring, as useless expression.fixes: #11292
Test Plan
Add a test file full of attribute docstring and verified the snapshot.