-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[flake8-pyi
] Avoid syntax error from conflict with PIE790
(PYI021
)
#20010
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
Changes from 201 commits
db6fe35
a50b389
3264d63
8af4168
7800db2
f849dd7
d068e13
d5a2e80
7888e0c
84db505
ebcf493
be4823f
cfff126
91b4241
5354fac
8abb95d
94040a9
34b218e
e44bb15
a363bb8
6c5719e
15a3739
6876231
5d95dcd
43d7463
6e68f91
c2a80b9
d12f57c
df57afa
0a35b03
82eb353
ac8784e
31de0dd
6768657
1ecabd1
789f38a
2dbc45a
03be2ed
02044a5
a722c40
f378db7
7e84da5
de2fc0a
883cc23
218684c
85dea0e
e2a218a
4c19e05
adf3e7b
5cece85
7a142ad
e322e6e
ef666dc
46dbabd
a7e1390
d4cb5dd
e8ca459
d11d9cc
b6d0865
51e8324
a4b9b18
ddacc6f
a136e54
6bc40d0
6dc4233
16b34a9
153d3c6
a3fc78a
81c0f9d
711de3c
ef10020
3cc1e24
a8d4758
fc140ce
db1f50e
61eabcc
e5651ed
abb215b
6dcca17
09fb798
a867935
8eb7394
178b902
eb59daf
8e9da9a
a75a589
7de87bb
c740403
8922255
cb1db5e
d2ffe00
6f67b0e
b439986
b0856d6
eb32ba3
696784b
fe4114e
d0b7055
e835b45
dafd6ef
2bcbc3e
ba446a4
5a94fd7
859f968
b2ec901
1ea3e2b
cc101f6
c6bbb33
b44952b
8438cc2
e682390
08f3f22
f272eb8
e592973
3bf43d9
3db8edd
6768220
34a6862
e2085a6
b12bfda
d2822e1
83261a0
2c857c3
1da9e47
5fdbc0c
a0da738
9e4370f
df02b42
06bf0b9
3eee9d3
eed6a1f
135d464
60f149d
c85a1a1
aa5beb3
18ceec5
1b6d00c
8f5d225
5ac5305
a2ec8a9
9def095
d000a3d
2091d18
8030713
1c99d8a
1497f09
e4d4307
3250f2e
2414829
de3714a
2fb4499
40a1e4b
5d048d4
e26c701
da56213
f1298b2
969ce80
c5db2a4
6db395d
98edce4
4a8e2cd
e65f868
c6ebb93
06f38c0
a078253
c71e05d
3ea99e8
1e3bbd8
7437a77
b0ac987
f56e08c
b73ffac
29b0bae
f583551
6061f3a
39b3b9f
577c180
1be758b
0c385a1
8d70991
51fc1ea
0bc2112
7c926d1
e2b4e25
4318f79
850f35c
464edf2
55d7c47
0df3741
1211217
c76b9aa
3dcbad7
897e05f
65f9f72
2ea22da
87738ff
c4d0eb6
1af1457
1dc7435
2150937
73e2ccd
0ecb545
3f51950
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
def check_isolation_level(mode: int) -> None: | ||
"""Shouldn't try to fix either.""" # ERROR PYI021 | ||
... # ERROR PIE790 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,14 +2,17 @@ use ruff_python_ast::Stmt; | |
|
||
use crate::checkers::ast::Checker; | ||
use crate::codes::Rule; | ||
use crate::rules::flake8_pie; | ||
use crate::rules::refurb; | ||
use crate::rules::{flake8_pie, flake8_pyi}; | ||
|
||
/// Run lint rules over a suite of [`Stmt`] syntax nodes. | ||
pub(crate) fn suite(suite: &[Stmt], checker: &Checker) { | ||
if checker.is_rule_enabled(Rule::UnnecessaryPlaceholder) { | ||
flake8_pie::rules::unnecessary_placeholder(checker, suite); | ||
} | ||
if checker.source_type.is_stub() && checker.is_rule_enabled(Rule::DocstringInStub) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing I didn't realize about moving this here is that it now gets called for non-function (or -class or -module) suites, such as context managers or with foo():
"""docstring"""
pass
if True:
"""docstring"""
pass I tested these locally, and we flag both of these. Fortunately, it looks like we already track a There's an existing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey thanks for this, I think I've addressed all of the comments from the review, please let me know if I've missed anything! |
||
flake8_pyi::rules::docstring_in_stubs(checker, suite); | ||
} | ||
if checker.is_rule_enabled(Rule::RepeatedGlobal) { | ||
refurb::rules::repeated_global(checker, suite); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
--- | ||
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs | ||
--- | ||
PYI021 [*] Docstrings should not be included in stubs | ||
--> PYI021_1.pyi:2:5 | ||
| | ||
1 | def check_isolation_level(mode: int) -> None: | ||
2 | """Shouldn't try to fix either.""" # ERROR PYI021 | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
3 | ... # ERROR PIE790 | ||
| | ||
help: Remove docstring | ||
1 | def check_isolation_level(mode: int) -> None: | ||
- """Shouldn't try to fix either.""" # ERROR PYI021 | ||
2 + # ERROR PYI021 | ||
3 | ... # ERROR PIE790 | ||
note: This is an unsafe fix and may change runtime behavior | ||
|
||
PIE790 [*] Unnecessary `...` literal | ||
--> PYI021_1.pyi:3:5 | ||
| | ||
1 | def check_isolation_level(mode: int) -> None: | ||
2 | """Shouldn't try to fix either.""" # ERROR PYI021 | ||
3 | ... # ERROR PIE790 | ||
| ^^^ | ||
| | ||
help: Remove unnecessary `...` | ||
1 | def check_isolation_level(mode: int) -> None: | ||
2 | """Shouldn't try to fix either.""" # ERROR PYI021 | ||
- ... # ERROR PIE790 | ||
3 + # ERROR PIE790 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,13 @@ use std::path::Path; | |
use bitflags::bitflags; | ||
use rustc_hash::FxHashMap; | ||
|
||
use ruff_python_ast::helpers::from_relative_import; | ||
use ruff_python_ast::helpers::{from_relative_import, map_subscript}; | ||
use ruff_python_ast::name::{QualifiedName, UnqualifiedName}; | ||
use ruff_python_ast::{self as ast, Expr, ExprContext, PySourceType, Stmt}; | ||
use ruff_text_size::{Ranged, TextRange, TextSize}; | ||
|
||
use crate::Imported; | ||
use crate::analyze::visibility; | ||
use crate::binding::{ | ||
Binding, BindingFlags, BindingId, BindingKind, Bindings, Exceptions, FromImport, Import, | ||
SubmoduleImport, | ||
|
@@ -1406,6 +1407,17 @@ impl<'a> SemanticModel<'a> { | |
.expect("No statement found") | ||
} | ||
|
||
/// Return the [`NodeId`] for the given [`Stmt`]. | ||
pub fn statement_id(&self, stmt: &ast::Stmt) -> Option<NodeId> { | ||
|
||
match self.node_id { | ||
Some(node_id) => self | ||
.nodes | ||
.ancestor_ids(node_id) | ||
.find(|node| self.statement(*node) == stmt), | ||
None => None, | ||
} | ||
} | ||
|
||
/// Returns an [`Iterator`] over the statements, starting from the given [`NodeId`]. | ||
/// through to any parents. | ||
pub fn statements(&self, node_id: NodeId) -> impl Iterator<Item = &'a Stmt> + '_ { | ||
|
@@ -1654,6 +1666,21 @@ impl<'a> SemanticModel<'a> { | |
} | ||
} | ||
|
||
/// Return `true` if the model is in a `typing.Protocol` subclass or an abstract | ||
/// method. | ||
pub fn in_protocol_or_abstract_method(&self) -> bool { | ||
|
||
self.current_scopes().any(|scope| match scope.kind { | ||
ScopeKind::Class(class_def) => class_def | ||
.bases() | ||
.iter() | ||
.any(|base| self.match_typing_expr(map_subscript(base), "Protocol")), | ||
ScopeKind::Function(function_def) => { | ||
visibility::is_abstract(&function_def.decorator_list, self) | ||
} | ||
_ => false, | ||
}) | ||
} | ||
|
||
/// Returns `true` if `left` and `right` are in the same branches of an `if`, `match`, or | ||
/// `try` statement. | ||
/// | ||
|
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.
This docstring seems slightly misleading since the snapshot shows it trying to fix both diagnostics 😄
In testing this locally in the CLI, the net effect is that we still emit both diagnostics when checking, but the isolation level means only the first diagnostic actually gets fixed. Since we sort by diagnostic range, that means PYI021 gets fixed and PIE790 does not. You don't need to write all of that in the docstring, to be clear, just recording my observations!