Skip to content

Commit

Permalink
[flake8-pyi] Implement PYI054 (#4775)
Browse files Browse the repository at this point in the history
  • Loading branch information
density authored and konstin committed Jun 13, 2023
1 parent af3edd9 commit 06e20c5
Show file tree
Hide file tree
Showing 14 changed files with 364 additions and 65 deletions.
20 changes: 20 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI054.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
field01: int = 0xFFFFFFFF
field02: int = 0xFFFFFFFFF
field03: int = -0xFFFFFFFF
field04: int = -0xFFFFFFFFF

field05: int = 1234567890
field06: int = 12_456_890
field07: int = 12345678901
field08: int = -1234567801
field09: int = -234_567_890

field10: float = 123.456789
field11: float = 123.4567890
field12: float = -123.456789
field13: float = -123.567_890

field14: complex = 1e1234567j
field15: complex = 1e12345678j
field16: complex = -1e1234567j
field17: complex = 1e123456789j
20 changes: 20 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI054.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
field01: int = 0xFFFFFFFF
field02: int = 0xFFFFFFFFF # Error: PYI054
field03: int = -0xFFFFFFFF
field04: int = -0xFFFFFFFFF # Error: PYI054

field05: int = 1234567890
field06: int = 12_456_890
field07: int = 12345678901 # Error: PYI054
field08: int = -1234567801
field09: int = -234_567_890 # Error: PYI054

field10: float = 123.456789
field11: float = 123.4567890 # Error: PYI054
field12: float = -123.456789
field13: float = -123.567_890 # Error: PYI054

field14: complex = 1e1234567j
field15: complex = 1e12345678j # Error: PYI054
field16: complex = -1e1234567j
field17: complex = 1e123456789j # Error: PYI054
12 changes: 11 additions & 1 deletion crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3410,9 +3410,19 @@ where
}
}
}
Expr::Constant(ast::ExprConstant {
value: Constant::Int(_) | Constant::Float(_) | Constant::Complex { .. },
kind: _,
range: _,
}) => {
if self.is_stub && self.enabled(Rule::NumericLiteralTooLong) {
flake8_pyi::rules::numeric_literal_too_long(self, expr);
}
}
Expr::Constant(ast::ExprConstant {
value: Constant::Bytes(_),
..
kind: _,
range: _,
}) => {
if self.is_stub && self.enabled(Rule::StringOrBytesTooLong) {
flake8_pyi::rules::string_or_bytes_too_long(self, expr);
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Pyi, "045") => (RuleGroup::Unspecified, Rule::IterMethodReturnIterable),
(Flake8Pyi, "048") => (RuleGroup::Unspecified, Rule::StubBodyMultipleStatements),
(Flake8Pyi, "052") => (RuleGroup::Unspecified, Rule::UnannotatedAssignmentInStub),
(Flake8Pyi, "054") => (RuleGroup::Unspecified, Rule::NumericLiteralTooLong),
(Flake8Pyi, "053") => (RuleGroup::Unspecified, Rule::StringOrBytesTooLong),

// flake8-pytest-style
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@ ruff_macros::register_rules!(
rules::flake8_pyi::rules::NonEmptyStubBody,
rules::flake8_pyi::rules::PassInClassBody,
rules::flake8_pyi::rules::PassStatementStubBody,
rules::flake8_pyi::rules::NumericLiteralTooLong,
rules::flake8_pyi::rules::QuotedAnnotationInStub,
rules::flake8_pyi::rules::SnakeCaseTypeAlias,
rules::flake8_pyi::rules::StubBodyMultipleStatements,
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ mod tests {
#[test_case(Rule::CollectionsNamedTuple, Path::new("PYI024.pyi"))]
#[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.py"))]
#[test_case(Rule::IterMethodReturnIterable, Path::new("PYI045.pyi"))]
#[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.py"))]
#[test_case(Rule::NumericLiteralTooLong, Path::new("PYI054.pyi"))]
#[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.py"))]
#[test_case(Rule::NonEmptyStubBody, Path::new("PYI010.pyi"))]
#[test_case(Rule::PassInClassBody, Path::new("PYI012.py"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub(crate) use iter_method_return_iterable::{
iter_method_return_iterable, IterMethodReturnIterable,
};
pub(crate) use non_empty_stub_body::{non_empty_stub_body, NonEmptyStubBody};
pub(crate) use numeric_literal_too_long::{numeric_literal_too_long, NumericLiteralTooLong};
pub(crate) use pass_in_class_body::{pass_in_class_body, PassInClassBody};
pub(crate) use pass_statement_stub_body::{pass_statement_stub_body, PassStatementStubBody};
pub(crate) use prefix_type_params::{prefix_type_params, UnprefixedTypeParam};
Expand Down Expand Up @@ -44,6 +45,7 @@ mod duplicate_union_member;
mod ellipsis_in_non_empty_class_body;
mod iter_method_return_iterable;
mod non_empty_stub_body;
mod numeric_literal_too_long;
mod pass_in_class_body;
mod pass_statement_stub_body;
mod prefix_type_params;
Expand Down
58 changes: 58 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/numeric_literal_too_long.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use ruff_text_size::TextSize;
use rustpython_parser::ast::{Expr, Ranged};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;
use crate::registry::AsRule;

#[violation]
pub struct NumericLiteralTooLong;

/// ## What it does
/// Checks for numeric literals with a string representation longer than ten
/// characters.
///
/// ## Why is this bad?
/// If a function has a default value where the literal representation is
/// greater than 50 characters, it is likely to be an implementation detail or
/// a constant that varies depending on the system you're running on.
///
/// Consider replacing such constants with ellipses (`...`).
///
/// ## Example
/// ```python
/// def foo(arg: int = 12345678901) -> None: ...
/// ```
///
/// Use instead:
/// ```python
/// def foo(arg: int = ...) -> None: ...
/// ```
impl AlwaysAutofixableViolation for NumericLiteralTooLong {
#[derive_message_formats]
fn message(&self) -> String {
format!("Numeric literals with a string representation longer than ten characters are not permitted")
}

fn autofix_title(&self) -> String {
"Replace with `...`".to_string()
}
}

/// PYI054
pub(crate) fn numeric_literal_too_long(checker: &mut Checker, expr: &Expr) {
if expr.range().len() <= TextSize::new(10) {
return;
}

let mut diagnostic = Diagnostic::new(NumericLiteralTooLong, expr.range());
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
"...".to_string(),
expr.range(),
)));
}
checker.diagnostics.push(diagnostic);
}
73 changes: 20 additions & 53 deletions crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ fn is_valid_default_value_with_annotation(
model: &SemanticModel,
) -> bool {
match default {
Expr::Constant(_) => {
return true;
}
Expr::List(ast::ExprList { elts, .. })
| Expr::Tuple(ast::ExprTuple { elts, .. })
| Expr::Set(ast::ExprSet { elts, range: _ }) => {
Expand All @@ -118,65 +121,29 @@ fn is_valid_default_value_with_annotation(
}) && is_valid_default_value_with_annotation(v, false, locator, model)
});
}
Expr::Constant(ast::ExprConstant {
value: Constant::Ellipsis | Constant::None,
..
}) => {
return true;
}
Expr::Constant(ast::ExprConstant {
value: Constant::Str(..) | Constant::Bytes(..),
..
}) => return true,
// Ex) `123`, `True`, `False`, `3.14`
Expr::Constant(ast::ExprConstant {
value: Constant::Int(..) | Constant::Bool(..) | Constant::Float(..),
..
}) => {
return locator.slice(default.range()).len() <= 10;
}
// Ex) `2j`
Expr::Constant(ast::ExprConstant {
value: Constant::Complex { real, .. },
..
}) => {
if *real == 0.0 {
return locator.slice(default.range()).len() <= 10;
}
}
Expr::UnaryOp(ast::ExprUnaryOp {
op: Unaryop::USub,
operand,
range: _,
}) => {
// Ex) `-1`, `-3.14`
if let Expr::Constant(ast::ExprConstant {
value: Constant::Int(..) | Constant::Float(..),
..
}) = operand.as_ref()
{
return locator.slice(operand.range()).len() <= 10;
}
// Ex) `-2j`
if let Expr::Constant(ast::ExprConstant {
value: Constant::Complex { real, .. },
..
}) = operand.as_ref()
{
if *real == 0.0 {
return locator.slice(operand.range()).len() <= 10;
}
}
// Ex) `-math.inf`, `-math.pi`, etc.
if let Expr::Attribute(_) = operand.as_ref() {
if model.resolve_call_path(operand).map_or(false, |call_path| {
ALLOWED_MATH_ATTRIBUTES_IN_DEFAULTS.iter().any(|target| {
// reject `-math.nan`
call_path.as_slice() == *target && *target != ["math", "nan"]
})
}) {
return true;
match operand.as_ref() {
// Ex) `-1`, `-3.14`, `2j`
Expr::Constant(ast::ExprConstant {
value: Constant::Int(..) | Constant::Float(..) | Constant::Complex { .. },
..
}) => return true,
// Ex) `-math.inf`, `-math.pi`, etc.
Expr::Attribute(_) => {
if model.resolve_call_path(operand).map_or(false, |call_path| {
ALLOWED_MATH_ATTRIBUTES_IN_DEFAULTS.iter().any(|target| {
// reject `-math.nan`
call_path.as_slice() == *target && *target != ["math", "nan"]
})
}) {
return true;
}
}
_ => {}
}
}
Expr::BinOp(ast::ExprBinOp {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustpython_parser::ast::{self, Constant, Expr, Ranged};

use ruff_diagnostics::{Diagnostic, Edit, Fix, Violation};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -28,11 +28,15 @@ pub struct StringOrBytesTooLong;
/// ```python
/// def foo(arg: str = ...) -> None: ...
/// ```
impl Violation for StringOrBytesTooLong {
impl AlwaysAutofixableViolation for StringOrBytesTooLong {
#[derive_message_formats]
fn message(&self) -> String {
format!("String and bytes literals longer than 50 characters are not permitted")
}

fn autofix_title(&self) -> String {
"Replace with `...`".to_string()
}
}

/// PYI053
Expand All @@ -48,7 +52,6 @@ pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, expr: &Expr) {
}) => bytes.len(),
_ => return,
};

if length <= 50 {
return;
}
Expand All @@ -60,7 +63,5 @@ pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, expr: &Expr) {
expr.range(),
)));
}
checker
.diagnostics
.push(Diagnostic::new(StringOrBytesTooLong, expr.range()));
checker.diagnostics.push(diagnostic);
}
Loading

0 comments on commit 06e20c5

Please sign in to comment.