Skip to content
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-pyi] Implement PYI054 #4775

Merged
merged 11 commits into from Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI054.py
@@ -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
@@ -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
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
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
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
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
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
@@ -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
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;
}
Comment on lines +129 to +144
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the checks on numeric literal length since it's now covered by PYI054.

}
_ => {}
}
}
Expr::BinOp(ast::ExprBinOp {
Expand Down
@@ -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);
}