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 PYI053 #4770

Merged
merged 9 commits into from Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
38 changes: 38 additions & 0 deletions 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"
30 changes: 30 additions & 0 deletions 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 stringggggggggggggggggggggggggggggg\U0001f600", # OK
) -> None: ...
def f4(
x: str = "51 character stringgggggggggggggggggggggggggggggg\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"50 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
11 changes: 11 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Expand Up @@ -3393,6 +3393,14 @@ where
}
}
}
Expr::Constant(ast::ExprConstant {
value: Constant::Bytes(_),
..
}) => {
if self.is_stub && self.enabled(Rule::LongStringOrBytesInStub) {
flake8_pyi::rules::long_string_or_bytes_in_stub(self, expr);
}
}
Expr::Constant(ast::ExprConstant {
value: Constant::Str(value),
kind,
Expand Down Expand Up @@ -3427,6 +3435,9 @@ where
if self.enabled(Rule::UnicodeKindPrefix) {
pyupgrade::rules::unicode_kind_prefix(self, expr, kind.as_deref());
}
if self.is_stub && self.enabled(Rule::LongStringOrBytesInStub) {
flake8_pyi::rules::long_string_or_bytes_in_stub(self, expr);
}
}
Expr::Lambda(
lambda @ ast::ExprLambda {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Expand Up @@ -599,6 +599,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::LongStringOrBytesInStub),

// flake8-pytest-style
(Flake8PytestStyle, "001") => (RuleGroup::Unspecified, Rule::PytestFixtureIncorrectParenthesesStyle),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Expand Up @@ -519,6 +519,7 @@ ruff_macros::register_rules!(
rules::flake8_pyi::rules::DuplicateUnionMember,
rules::flake8_pyi::rules::EllipsisInNonEmptyClassBody,
rules::flake8_pyi::rules::CollectionsNamedTuple,
rules::flake8_pyi::rules::LongStringOrBytesInStub,
rules::flake8_pyi::rules::NonEmptyStubBody,
rules::flake8_pyi::rules::PassInClassBody,
rules::flake8_pyi::rules::PassStatementStubBody,
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/mod.rs
Expand Up @@ -50,6 +50,8 @@ mod tests {
#[test_case(Rule::TypedArgumentDefaultInStub, Path::new("PYI011.pyi"))]
#[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.py"))]
#[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.pyi"))]
#[test_case(Rule::LongStringOrBytesInStub, Path::new("PYI053.py"))]
#[test_case(Rule::LongStringOrBytesInStub, 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"))]
Expand Down
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_in_stub feels redundant in the name given that it's in flake8_pyi. lmk if you prefer a different name.

@@ -0,0 +1,43 @@
use ruff_diagnostics::{Diagnostic, Violation};
use rustpython_parser::ast::{self, Constant, Expr, Ranged};

use ruff_macros::{derive_message_formats, violation};

use crate::checkers::ast::Checker;

#[violation]
pub struct LongStringOrBytesInStub;

/// ## What it does
/// Checks for `str` or `bytes` literals longer than 50 characters in stubs.
///
/// ## Why is this bad?
/// Extremely long strings used as argument defaults or otherwise are unlikely to be useful for
/// users. Consider replacing them with ellipses.
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved
impl Violation for LongStringOrBytesInStub {
#[derive_message_formats]
fn message(&self) -> String {
format!("`str` and `bytes` literals longer than 50 characters should not be used in stubs.")
}
}

/// PYI053
pub(crate) fn long_string_or_bytes_in_stub(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 {
checker
.diagnostics
.push(Diagnostic::new(LongStringOrBytesInStub, expr.range()));
}
}
4 changes: 4 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Expand Up @@ -11,6 +11,9 @@ pub(crate) use ellipsis_in_non_empty_class_body::{
pub(crate) use iter_method_return_iterable::{
iter_method_return_iterable, IterMethodReturnIterable,
};
pub(crate) use long_string_or_bytes_in_stub::{
long_string_or_bytes_in_stub, LongStringOrBytesInStub,
};
pub(crate) use non_empty_stub_body::{non_empty_stub_body, NonEmptyStubBody};
pub(crate) use pass_in_class_body::{pass_in_class_body, PassInClassBody};
pub(crate) use pass_statement_stub_body::{pass_statement_stub_body, PassStatementStubBody};
Expand Down Expand Up @@ -39,6 +42,7 @@ mod docstring_in_stubs;
mod duplicate_union_member;
mod ellipsis_in_non_empty_class_body;
mod iter_method_return_iterable;
mod long_string_or_bytes_in_stub;
mod non_empty_stub_body;
mod pass_in_class_body;
mod pass_statement_stub_body;
Expand Down
@@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---

@@ -0,0 +1,41 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---
PYI053.pyi:3:14: PYI053 `str` and `bytes` literals longer than 50 characters should not be used in stubs.
|
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:21:16: PYI053 `str` and `bytes` literals longer than 50 characters should not be used in stubs.
|
21 | ) -> None: ...
22 | def f8(
23 | x: bytes = b"50 character byte stringgggggggggggggggggggggggggg\xff", # Error: PYI053
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053
24 | ) -> None: ...
|

PYI053.pyi:26:12: PYI053 `str` and `bytes` literals longer than 50 characters should not be used in stubs.
|
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 `str` and `bytes` literals longer than 50 characters should not be used in stubs.
|
30 | baz: bytes = b"50 character byte stringgggggggggggggggggggggggggg" # OK
31 |
32 | qux: bytes = b"51 character byte stringggggggggggggggggggggggggggg\xff" # Error: PYI053
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.