mirror of https://github.com/astral-sh/ruff
[`flake8-pyi`]: add rules for unrecognized platform check (PYI007, PYI008) (#2805)
Add two [flake8-pyi](https://github.com/PyCQA/flake8-pyi) rules (Y007, Y008). ref: #848 The specifications are described in [PEP 484 - Version and platform checking](https://peps.python.org/pep-0484/#version-and-platform-checking) The original implementation in flake8-pyi is shown below. - Implemention:66f28a4407/pyi.py (L1429-L1443)- Tests:66f28a4407/tests/sysplatform.pyi
This commit is contained in:
parent
ca8a122889
commit
fc465cc2af
|
|
@ -1181,6 +1181,8 @@ For more, see [flake8-pyi](https://pypi.org/project/flake8-pyi/) on PyPI.
|
||||||
| Code | Name | Message | Fix |
|
| Code | Name | Message | Fix |
|
||||||
| ---- | ---- | ------- | --- |
|
| ---- | ---- | ------- | --- |
|
||||||
| PYI001 | [prefix-type-params](https://github.com/charliermarsh/ruff/blob/main/docs/rules/prefix-type-params.md) | Name of private `{kind}` must start with _ | |
|
| PYI001 | [prefix-type-params](https://github.com/charliermarsh/ruff/blob/main/docs/rules/prefix-type-params.md) | Name of private `{kind}` must start with _ | |
|
||||||
|
| PYI007 | [unrecognized-platform-check](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unrecognized-platform-check.md) | Unrecognized sys.platform check | |
|
||||||
|
| PYI008 | [unrecognized-platform-name](https://github.com/charliermarsh/ruff/blob/main/docs/rules/unrecognized-platform-name.md) | Unrecognized platform `{platform}` | |
|
||||||
|
|
||||||
### flake8-pytest-style (PT)
|
### flake8-pytest-style (PT)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,11 @@
|
||||||
|
import sys
|
||||||
|
|
||||||
|
if sys.platform == "platform_name_1": ... # OK
|
||||||
|
|
||||||
|
if sys.platform != "platform_name_2": ... # OK
|
||||||
|
|
||||||
|
if sys.platform in ["linux"]: ... # OK
|
||||||
|
|
||||||
|
if sys.platform > 3: ... # OK
|
||||||
|
|
||||||
|
if sys.platform == 10.12: ... # OK
|
||||||
|
|
@ -0,0 +1,11 @@
|
||||||
|
import sys
|
||||||
|
|
||||||
|
if sys.platform == "platform_name_1": ... # OK
|
||||||
|
|
||||||
|
if sys.platform != "platform_name_2": ... # OK
|
||||||
|
|
||||||
|
if sys.platform in ["linux"]: ... # Error: PYI007 Unrecognized sys.platform check
|
||||||
|
|
||||||
|
if sys.platform > 3: ... # Error: PYI007 Unrecognized sys.platform check
|
||||||
|
|
||||||
|
if sys.platform == 10.12: ... # Error: PYI007 Unrecognized sys.platform check
|
||||||
|
|
@ -0,0 +1,11 @@
|
||||||
|
import sys
|
||||||
|
|
||||||
|
if sys.platform == "linus": ... # OK
|
||||||
|
|
||||||
|
if sys.platform != "linux": ... # OK
|
||||||
|
|
||||||
|
if sys.platform == "win32": ... # OK
|
||||||
|
|
||||||
|
if sys.platform != "darwin": ... # OK
|
||||||
|
|
||||||
|
if sys.platform == "cygwin": ... # OK
|
||||||
|
|
@ -0,0 +1,11 @@
|
||||||
|
import sys
|
||||||
|
|
||||||
|
if sys.platform == "linus": ... # Error: PYI008 Unrecognized platform `linus`
|
||||||
|
|
||||||
|
if sys.platform != "linux": ... # OK
|
||||||
|
|
||||||
|
if sys.platform == "win32": ... # OK
|
||||||
|
|
||||||
|
if sys.platform != "darwin": ... # OK
|
||||||
|
|
||||||
|
if sys.platform == "cygwin": ... # OK
|
||||||
|
|
@ -3155,6 +3155,23 @@ where
|
||||||
if self.settings.rules.enabled(&Rule::YodaConditions) {
|
if self.settings.rules.enabled(&Rule::YodaConditions) {
|
||||||
flake8_simplify::rules::yoda_conditions(self, expr, left, ops, comparators);
|
flake8_simplify::rules::yoda_conditions(self, expr, left, ops, comparators);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if self
|
||||||
|
.settings
|
||||||
|
.rules
|
||||||
|
.enabled(&Rule::UnrecognizedPlatformCheck)
|
||||||
|
|| self.settings.rules.enabled(&Rule::UnrecognizedPlatformName)
|
||||||
|
{
|
||||||
|
if self.path.extension().map_or(false, |ext| ext == "pyi") {
|
||||||
|
flake8_pyi::rules::unrecognized_platform(
|
||||||
|
self,
|
||||||
|
expr,
|
||||||
|
left,
|
||||||
|
ops,
|
||||||
|
comparators,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
ExprKind::Constant {
|
ExprKind::Constant {
|
||||||
value: Constant::Str(value),
|
value: Constant::Str(value),
|
||||||
|
|
|
||||||
|
|
@ -446,6 +446,8 @@ ruff_macros::define_rule_mapping!(
|
||||||
EM103 => rules::flake8_errmsg::rules::DotFormatInException,
|
EM103 => rules::flake8_errmsg::rules::DotFormatInException,
|
||||||
// flake8-pyi
|
// flake8-pyi
|
||||||
PYI001 => rules::flake8_pyi::rules::PrefixTypeParams,
|
PYI001 => rules::flake8_pyi::rules::PrefixTypeParams,
|
||||||
|
PYI007 => rules::flake8_pyi::rules::UnrecognizedPlatformCheck,
|
||||||
|
PYI008 => rules::flake8_pyi::rules::UnrecognizedPlatformName,
|
||||||
// flake8-pytest-style
|
// flake8-pytest-style
|
||||||
PT001 => rules::flake8_pytest_style::rules::IncorrectFixtureParenthesesStyle,
|
PT001 => rules::flake8_pytest_style::rules::IncorrectFixtureParenthesesStyle,
|
||||||
PT002 => rules::flake8_pytest_style::rules::FixturePositionalArgs,
|
PT002 => rules::flake8_pytest_style::rules::FixturePositionalArgs,
|
||||||
|
|
|
||||||
|
|
@ -14,6 +14,10 @@ mod tests {
|
||||||
|
|
||||||
#[test_case(Rule::PrefixTypeParams, Path::new("PYI001.pyi"))]
|
#[test_case(Rule::PrefixTypeParams, Path::new("PYI001.pyi"))]
|
||||||
#[test_case(Rule::PrefixTypeParams, Path::new("PYI001.py"))]
|
#[test_case(Rule::PrefixTypeParams, Path::new("PYI001.py"))]
|
||||||
|
#[test_case(Rule::UnrecognizedPlatformCheck, Path::new("PYI007.pyi"))]
|
||||||
|
#[test_case(Rule::UnrecognizedPlatformCheck, Path::new("PYI007.py"))]
|
||||||
|
#[test_case(Rule::UnrecognizedPlatformName, Path::new("PYI008.pyi"))]
|
||||||
|
#[test_case(Rule::UnrecognizedPlatformName, Path::new("PYI008.py"))]
|
||||||
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy());
|
let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy());
|
||||||
let diagnostics = test_path(
|
let diagnostics = test_path(
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,7 @@
|
||||||
|
pub use prefix_type_params::{prefix_type_params, PrefixTypeParams};
|
||||||
|
pub use unrecognized_platform::{
|
||||||
|
unrecognized_platform, UnrecognizedPlatformCheck, UnrecognizedPlatformName,
|
||||||
|
};
|
||||||
|
|
||||||
|
mod prefix_type_params;
|
||||||
|
mod unrecognized_platform;
|
||||||
|
|
@ -0,0 +1,154 @@
|
||||||
|
use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind};
|
||||||
|
|
||||||
|
use ruff_macros::{define_violation, derive_message_formats};
|
||||||
|
|
||||||
|
use crate::ast::types::Range;
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
use crate::registry::{Diagnostic, Rule};
|
||||||
|
use crate::violation::Violation;
|
||||||
|
|
||||||
|
define_violation!(
|
||||||
|
/// ## What it does
|
||||||
|
/// Check for unrecognized `sys.platform` checks. Platform checks should be
|
||||||
|
/// simple string comparisons.
|
||||||
|
///
|
||||||
|
/// **Note**: this rule is only enabled in `.pyi` stub files.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// Some `sys.platform` checks are too complex for type checkers to
|
||||||
|
/// understand, and thus result in false positives. `sys.platform` checks
|
||||||
|
/// should be simple string comparisons, like `sys.platform == "linux"`.
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// if sys.platform.startswith("linux"):
|
||||||
|
/// # Linux specific definitions
|
||||||
|
/// else:
|
||||||
|
/// # Posix specific definitions
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Instead, use a simple string comparison, such as `==` or `!=`:
|
||||||
|
/// ```python
|
||||||
|
/// if sys.platform == "linux":
|
||||||
|
/// # Linux specific definitions
|
||||||
|
/// else:
|
||||||
|
/// # Posix specific definitions
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// ## References
|
||||||
|
/// - [PEP 484](https://peps.python.org/pep-0484/#version-and-platform-checking)
|
||||||
|
pub struct UnrecognizedPlatformCheck;
|
||||||
|
);
|
||||||
|
impl Violation for UnrecognizedPlatformCheck {
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
format!("Unrecognized sys.platform check")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
define_violation!(
|
||||||
|
/// ## What it does
|
||||||
|
/// Check for unrecognized platform names in `sys.platform` checks.
|
||||||
|
///
|
||||||
|
/// **Note**: this rule is only enabled in `.pyi` stub files.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// If a `sys.platform` check compares to a platform name outside of a
|
||||||
|
/// small set of known platforms (e.g. "linux", "win32", etc.), it's likely
|
||||||
|
/// a typo or a platform name that is not recognized by type checkers.
|
||||||
|
///
|
||||||
|
/// The list of known platforms is: "linux", "win32", "cygwin", "darwin".
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// if sys.platform == "linus":
|
||||||
|
/// ...
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```python
|
||||||
|
/// if sys.platform == "linux":
|
||||||
|
/// ...
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// ## References
|
||||||
|
/// - [PEP 484](https://peps.python.org/pep-0484/#version-and-platform-checking)
|
||||||
|
pub struct UnrecognizedPlatformName {
|
||||||
|
pub platform: String,
|
||||||
|
}
|
||||||
|
);
|
||||||
|
impl Violation for UnrecognizedPlatformName {
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
let UnrecognizedPlatformName { platform } = self;
|
||||||
|
format!("Unrecognized platform `{platform}`")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// PYI007, PYI008
|
||||||
|
pub fn unrecognized_platform(
|
||||||
|
checker: &mut Checker,
|
||||||
|
expr: &Expr,
|
||||||
|
left: &Expr,
|
||||||
|
ops: &[Cmpop],
|
||||||
|
comparators: &[Expr],
|
||||||
|
) {
|
||||||
|
let ([op], [right]) = (ops, comparators) else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
|
||||||
|
let diagnostic_unrecognized_platform_check =
|
||||||
|
Diagnostic::new(UnrecognizedPlatformCheck, Range::from_located(expr));
|
||||||
|
if !checker.resolve_call_path(left).map_or(false, |call_path| {
|
||||||
|
call_path.as_slice() == ["sys", "platform"]
|
||||||
|
}) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// "in" might also make sense but we don't currently have one.
|
||||||
|
if !matches!(op, Cmpop::Eq | Cmpop::NotEq)
|
||||||
|
&& checker
|
||||||
|
.settings
|
||||||
|
.rules
|
||||||
|
.enabled(&Rule::UnrecognizedPlatformCheck)
|
||||||
|
{
|
||||||
|
checker
|
||||||
|
.diagnostics
|
||||||
|
.push(diagnostic_unrecognized_platform_check);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
match &right.node {
|
||||||
|
ExprKind::Constant {
|
||||||
|
value: Constant::Str(value),
|
||||||
|
..
|
||||||
|
} => {
|
||||||
|
// Other values are possible but we don't need them right now.
|
||||||
|
// This protects against typos.
|
||||||
|
if !["linux", "win32", "cygwin", "darwin"].contains(&value.as_str())
|
||||||
|
&& checker
|
||||||
|
.settings
|
||||||
|
.rules
|
||||||
|
.enabled(&Rule::UnrecognizedPlatformName)
|
||||||
|
{
|
||||||
|
checker.diagnostics.push(Diagnostic::new(
|
||||||
|
UnrecognizedPlatformName {
|
||||||
|
platform: value.clone(),
|
||||||
|
},
|
||||||
|
Range::from_located(right),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => {
|
||||||
|
if checker
|
||||||
|
.settings
|
||||||
|
.rules
|
||||||
|
.enabled(&Rule::UnrecognizedPlatformCheck)
|
||||||
|
{
|
||||||
|
checker
|
||||||
|
.diagnostics
|
||||||
|
.push(diagnostic_unrecognized_platform_check);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/flake8_pyi/mod.rs
|
||||||
|
expression: diagnostics
|
||||||
|
---
|
||||||
|
[]
|
||||||
|
|
||||||
|
|
@ -0,0 +1,35 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/flake8_pyi/mod.rs
|
||||||
|
expression: diagnostics
|
||||||
|
---
|
||||||
|
- kind:
|
||||||
|
UnrecognizedPlatformCheck: ~
|
||||||
|
location:
|
||||||
|
row: 7
|
||||||
|
column: 3
|
||||||
|
end_location:
|
||||||
|
row: 7
|
||||||
|
column: 28
|
||||||
|
fix: ~
|
||||||
|
parent: ~
|
||||||
|
- kind:
|
||||||
|
UnrecognizedPlatformCheck: ~
|
||||||
|
location:
|
||||||
|
row: 9
|
||||||
|
column: 3
|
||||||
|
end_location:
|
||||||
|
row: 9
|
||||||
|
column: 19
|
||||||
|
fix: ~
|
||||||
|
parent: ~
|
||||||
|
- kind:
|
||||||
|
UnrecognizedPlatformCheck: ~
|
||||||
|
location:
|
||||||
|
row: 11
|
||||||
|
column: 3
|
||||||
|
end_location:
|
||||||
|
row: 11
|
||||||
|
column: 24
|
||||||
|
fix: ~
|
||||||
|
parent: ~
|
||||||
|
|
||||||
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/flake8_pyi/mod.rs
|
||||||
|
expression: diagnostics
|
||||||
|
---
|
||||||
|
[]
|
||||||
|
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff/src/rules/flake8_pyi/mod.rs
|
||||||
|
expression: diagnostics
|
||||||
|
---
|
||||||
|
- kind:
|
||||||
|
UnrecognizedPlatformName:
|
||||||
|
platform: linus
|
||||||
|
location:
|
||||||
|
row: 3
|
||||||
|
column: 19
|
||||||
|
end_location:
|
||||||
|
row: 3
|
||||||
|
column: 26
|
||||||
|
fix: ~
|
||||||
|
parent: ~
|
||||||
|
|
||||||
|
|
@ -0,0 +1,33 @@
|
||||||
|
# unrecognized-platform-check (PYI007)
|
||||||
|
|
||||||
|
Derived from the **flake8-pyi** linter.
|
||||||
|
|
||||||
|
## What it does
|
||||||
|
Check for unrecognized `sys.platform` checks. Platform checks should be
|
||||||
|
simple string comparisons.
|
||||||
|
|
||||||
|
**Note**: this rule is only enabled in `.pyi` stub files.
|
||||||
|
|
||||||
|
## Why is this bad?
|
||||||
|
Some `sys.platform` checks are too complex for type checkers to
|
||||||
|
understand, and thus result in false positives. `sys.platform` checks
|
||||||
|
should be simple string comparisons, like `sys.platform == "linux"`.
|
||||||
|
|
||||||
|
## Example
|
||||||
|
```python
|
||||||
|
if sys.platform.startswith("linux"):
|
||||||
|
# Linux specific definitions
|
||||||
|
else:
|
||||||
|
# Posix specific definitions
|
||||||
|
```
|
||||||
|
|
||||||
|
Instead, use a simple string comparison, such as `==` or `!=`:
|
||||||
|
```python
|
||||||
|
if sys.platform == "linux":
|
||||||
|
# Linux specific definitions
|
||||||
|
else:
|
||||||
|
# Posix specific definitions
|
||||||
|
```
|
||||||
|
|
||||||
|
## References
|
||||||
|
- [PEP 484](https://peps.python.org/pep-0484/#version-and-platform-checking)
|
||||||
|
|
@ -0,0 +1,30 @@
|
||||||
|
# unrecognized-platform-name (PYI008)
|
||||||
|
|
||||||
|
Derived from the **flake8-pyi** linter.
|
||||||
|
|
||||||
|
## What it does
|
||||||
|
Check for unrecognized platform names in `sys.platform` checks.
|
||||||
|
|
||||||
|
**Note**: this rule is only enabled in `.pyi` stub files.
|
||||||
|
|
||||||
|
## Why is this bad?
|
||||||
|
If a `sys.platform` check compares to a platform name outside of a
|
||||||
|
small set of known platforms (e.g. "linux", "win32", etc.), it's likely
|
||||||
|
a typo or a platform name that is not recognized by type checkers.
|
||||||
|
|
||||||
|
The list of known platforms is: "linux", "win32", "cygwin", "darwin".
|
||||||
|
|
||||||
|
## Example
|
||||||
|
```python
|
||||||
|
if sys.platform == "linus":
|
||||||
|
...
|
||||||
|
```
|
||||||
|
|
||||||
|
Use instead:
|
||||||
|
```python
|
||||||
|
if sys.platform == "linux":
|
||||||
|
...
|
||||||
|
```
|
||||||
|
|
||||||
|
## References
|
||||||
|
- [PEP 484](https://peps.python.org/pep-0484/#version-and-platform-checking)
|
||||||
|
|
@ -1823,6 +1823,8 @@
|
||||||
"PYI0",
|
"PYI0",
|
||||||
"PYI00",
|
"PYI00",
|
||||||
"PYI001",
|
"PYI001",
|
||||||
|
"PYI007",
|
||||||
|
"PYI008",
|
||||||
"Q",
|
"Q",
|
||||||
"Q0",
|
"Q0",
|
||||||
"Q00",
|
"Q00",
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue