From 583411a29f0e64d27255ac4407a863f2b5dd37b6 Mon Sep 17 00:00:00 2001 From: Justin Prieto Date: Thu, 1 Jun 2023 19:00:15 -0400 Subject: [PATCH] [`flake8-pyi`] Implement PYI053 (#4770) --- .../test/fixtures/flake8_pyi/PYI053.py | 38 +++++++++++ .../test/fixtures/flake8_pyi/PYI053.pyi | 30 +++++++++ crates/ruff/src/checkers/ast/mod.rs | 11 ++++ crates/ruff/src/codes.rs | 1 + crates/ruff/src/registry.rs | 1 + crates/ruff/src/rules/flake8_pyi/mod.rs | 2 + crates/ruff/src/rules/flake8_pyi/rules/mod.rs | 2 + .../rules/flake8_pyi/rules/simple_defaults.rs | 8 +-- .../rules/string_or_bytes_too_long.rs | 66 +++++++++++++++++++ ...__flake8_pyi__tests__PYI053_PYI053.py.snap | 4 ++ ..._flake8_pyi__tests__PYI053_PYI053.pyi.snap | 51 ++++++++++++++ ruff.schema.json | 1 + 12 files changed, 209 insertions(+), 6 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI053.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI053.pyi create mode 100644 crates/ruff/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI053_PYI053.py.snap create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI053.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI053.py new file mode 100644 index 0000000000000..15631d15f2acd --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI053.py @@ -0,0 +1,38 @@ +def f1(x: str = "50 character stringggggggggggggggggggggggggggggggg") -> None: + ... + + +def f2(x: str = "51 character stringgggggggggggggggggggggggggggggggg") -> None: + ... + + +def f3(x: str = "50 character stringggggggggggggggggggggggggggggg\U0001f600") -> None: + ... + + +def f4(x: str = "51 character stringgggggggggggggggggggggggggggggg\U0001f600") -> None: + ... + + +def f5(x: bytes = b"50 character byte stringgggggggggggggggggggggggggg") -> None: + ... + + +def f6(x: bytes = b"51 character byte stringgggggggggggggggggggggggggg") -> None: + ... + + +def f7(x: bytes = b"50 character byte stringggggggggggggggggggggggggg\xff") -> None: + ... + + +def f8(x: bytes = b"50 character byte stringgggggggggggggggggggggggggg\xff") -> None: + ... + + +foo: str = "50 character stringggggggggggggggggggggggggggggggg" +bar: str = "51 character stringgggggggggggggggggggggggggggggggg" + +baz: bytes = b"50 character byte stringgggggggggggggggggggggggggg" + +qux: bytes = b"51 character byte stringggggggggggggggggggggggggggg\xff" diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI053.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI053.pyi new file mode 100644 index 0000000000000..d2f55531a2660 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI053.pyi @@ -0,0 +1,30 @@ +def f1(x: str = "50 character stringggggggggggggggggggggggggggggggg") -> None: ... # OK +def f2( + x: str = "51 character stringgggggggggggggggggggggggggggggggg", # Error: PYI053 +) -> None: ... +def f3( + x: str = "50 character stringgggggggggggggggggggggggggggggg\U0001f600", # OK +) -> None: ... +def f4( + x: str = "51 character stringggggggggggggggggggggggggggggggg\U0001f600", # Error: PYI053 +) -> None: ... +def f5( + x: bytes = b"50 character byte stringgggggggggggggggggggggggggg", # OK +) -> None: ... +def f6( + x: bytes = b"51 character byte stringgggggggggggggggggggggggggg", # Error: PYI053 +) -> None: ... +def f7( + x: bytes = b"50 character byte stringggggggggggggggggggggggggg\xff", # OK +) -> None: ... +def f8( + x: bytes = b"51 character byte stringgggggggggggggggggggggggggg\xff", # Error: PYI053 +) -> None: ... + +foo: str = "50 character stringggggggggggggggggggggggggggggggg" # OK + +bar: str = "51 character stringgggggggggggggggggggggggggggggggg" # Error: PYI053 + +baz: bytes = b"50 character byte stringgggggggggggggggggggggggggg" # OK + +qux: bytes = b"51 character byte stringggggggggggggggggggggggggggg\xff" # Error: PYI053 diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 97c328059db64..514844ee48f9a 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -3410,6 +3410,14 @@ where } } } + Expr::Constant(ast::ExprConstant { + value: Constant::Bytes(_), + .. + }) => { + if self.is_stub && self.enabled(Rule::StringOrBytesTooLong) { + flake8_pyi::rules::string_or_bytes_too_long(self, expr); + } + } Expr::Constant(ast::ExprConstant { value: Constant::Str(value), kind, @@ -3444,6 +3452,9 @@ where if self.enabled(Rule::UnicodeKindPrefix) { pyupgrade::rules::unicode_kind_prefix(self, expr, kind.as_deref()); } + if self.is_stub && self.enabled(Rule::StringOrBytesTooLong) { + flake8_pyi::rules::string_or_bytes_too_long(self, expr); + } } Expr::Lambda( lambda @ ast::ExprLambda { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 75c56b5cfefca..e7d12a17ae0f1 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -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, "053") => (RuleGroup::Unspecified, Rule::StringOrBytesTooLong), // flake8-pytest-style (Flake8PytestStyle, "001") => (RuleGroup::Unspecified, Rule::PytestFixtureIncorrectParenthesesStyle), diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 92a21b475daac..67af1e4572ae6 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -520,6 +520,7 @@ ruff_macros::register_rules!( rules::flake8_pyi::rules::DuplicateUnionMember, rules::flake8_pyi::rules::EllipsisInNonEmptyClassBody, rules::flake8_pyi::rules::CollectionsNamedTuple, + rules::flake8_pyi::rules::StringOrBytesTooLong, rules::flake8_pyi::rules::NonEmptyStubBody, rules::flake8_pyi::rules::PassInClassBody, rules::flake8_pyi::rules::PassStatementStubBody, diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index c724ff4df1294..9a2d112be7955 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -52,6 +52,8 @@ mod tests { #[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025.pyi"))] #[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.py"))] #[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.pyi"))] + #[test_case(Rule::StringOrBytesTooLong, Path::new("PYI053.py"))] + #[test_case(Rule::StringOrBytesTooLong, Path::new("PYI053.pyi"))] #[test_case(Rule::UnprefixedTypeParam, Path::new("PYI001.py"))] #[test_case(Rule::UnprefixedTypeParam, Path::new("PYI001.pyi"))] #[test_case(Rule::UnrecognizedPlatformCheck, Path::new("PYI007.py"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 7978118cd8c2c..0be2c5c3bfd67 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -21,6 +21,7 @@ pub(crate) use simple_defaults::{ typed_argument_simple_defaults, unannotated_assignment_in_stub, ArgumentDefaultInStub, AssignmentDefaultInStub, TypedArgumentDefaultInStub, UnannotatedAssignmentInStub, }; +pub(crate) use string_or_bytes_too_long::{string_or_bytes_too_long, StringOrBytesTooLong}; pub(crate) use stub_body_multiple_statements::{ stub_body_multiple_statements, StubBodyMultipleStatements, }; @@ -48,6 +49,7 @@ mod pass_statement_stub_body; mod prefix_type_params; mod quoted_annotation_in_stub; mod simple_defaults; +mod string_or_bytes_too_long; mod stub_body_multiple_statements; mod type_alias_naming; mod type_comment_in_stub; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs b/crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs index 57c4c18bd450e..c9224d9c5759b 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/simple_defaults.rs @@ -125,13 +125,9 @@ fn is_valid_default_value_with_annotation( return true; } Expr::Constant(ast::ExprConstant { - value: Constant::Str(..), + value: Constant::Str(..) | Constant::Bytes(..), .. - }) => return locator.slice(default.range()).len() <= 50, - Expr::Constant(ast::ExprConstant { - value: Constant::Bytes(..), - .. - }) => return locator.slice(default.range()).len() <= 50, + }) => return true, // Ex) `123`, `True`, `False`, `3.14` Expr::Constant(ast::ExprConstant { value: Constant::Int(..) | Constant::Bool(..) | Constant::Float(..), diff --git a/crates/ruff/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs b/crates/ruff/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs new file mode 100644 index 0000000000000..459f17f489170 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs @@ -0,0 +1,66 @@ +use rustpython_parser::ast::{self, Constant, Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Edit, Fix, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; +use crate::registry::AsRule; + +#[violation] +pub struct StringOrBytesTooLong; + +/// ## What it does +/// Checks for the use of string and bytes literals longer than 50 characters. +/// +/// ## Why is this bad? +/// If a function has a default value where the string or bytes 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: str = "51 character stringgggggggggggggggggggggggggggggggg") -> None: ... +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(arg: str = ...) -> None: ... +/// ``` +impl Violation for StringOrBytesTooLong { + #[derive_message_formats] + fn message(&self) -> String { + format!("String and bytes literals longer than 50 characters are not permitted") + } +} + +/// PYI053 +pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, expr: &Expr) { + let length = match expr { + Expr::Constant(ast::ExprConstant { + value: Constant::Str(s), + .. + }) => s.chars().count(), + Expr::Constant(ast::ExprConstant { + value: Constant::Bytes(bytes), + .. + }) => bytes.len(), + _ => return, + }; + + if length <= 50 { + return; + } + + let mut diagnostic = Diagnostic::new(StringOrBytesTooLong, expr.range()); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.set_fix(Fix::suggested(Edit::range_replacement( + "...".to_string(), + expr.range(), + ))); + } + checker + .diagnostics + .push(Diagnostic::new(StringOrBytesTooLong, expr.range())); +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI053_PYI053.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI053_PYI053.py.snap new file mode 100644 index 0000000000000..d1aa2e9116558 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI053_PYI053.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap new file mode 100644 index 0000000000000..04eb3a9799ed5 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap @@ -0,0 +1,51 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI053.pyi:3:14: PYI053 String and bytes literals longer than 50 characters are not permitted + | +3 | def f1(x: str = "50 character stringggggggggggggggggggggggggggggggg") -> None: ... # OK +4 | def f2( +5 | x: str = "51 character stringgggggggggggggggggggggggggggggggg", # Error: PYI053 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 +6 | ) -> None: ... +7 | def f3( + | + +PYI053.pyi:9:14: PYI053 String and bytes literals longer than 50 characters are not permitted + | + 9 | ) -> None: ... +10 | def f4( +11 | x: str = "51 character stringggggggggggggggggggggggggggggggg\U0001f600", # Error: PYI053 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 +12 | ) -> None: ... +13 | def f5( + | + +PYI053.pyi:21:16: PYI053 String and bytes literals longer than 50 characters are not permitted + | +21 | ) -> None: ... +22 | def f8( +23 | x: bytes = b"51 character byte stringgggggggggggggggggggggggggg\xff", # Error: PYI053 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 +24 | ) -> None: ... + | + +PYI053.pyi:26:12: PYI053 String and bytes literals longer than 50 characters are not permitted + | +26 | foo: str = "50 character stringggggggggggggggggggggggggggggggg" # OK +27 | +28 | bar: str = "51 character stringgggggggggggggggggggggggggggggggg" # Error: PYI053 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 +29 | +30 | baz: bytes = b"50 character byte stringgggggggggggggggggggggggggg" # OK + | + +PYI053.pyi:30:14: PYI053 String and bytes literals longer than 50 characters are not permitted + | +30 | baz: bytes = b"50 character byte stringgggggggggggggggggggggggggg" # OK +31 | +32 | qux: bytes = b"51 character byte stringggggggggggggggggggggggggggg\xff" # Error: PYI053 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 + | + + diff --git a/ruff.schema.json b/ruff.schema.json index e07a609d392c3..57be720b3937d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2251,6 +2251,7 @@ "PYI048", "PYI05", "PYI052", + "PYI053", "Q", "Q0", "Q00",