diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/bad_str_strip_call.py b/crates/ruff_linter/resources/test/fixtures/pylint/bad_str_strip_call.py index feb9fd8531..9beed03ead 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/bad_str_strip_call.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/bad_str_strip_call.py @@ -80,6 +80,16 @@ b''.rstrip(b'http://') ''.strip(r'\b\x09') ''.strip('\\\x5C') +# Errors: Type inference +b = b'' +b.strip(b'//') + +# Errors: Type inference (preview) +foo: str = ""; bar: bytes = b"" +foo.rstrip("//") +bar.lstrip(b"//") + + # OK: Different types b"".strip("//") "".strip(b"//") @@ -93,13 +103,8 @@ b"".lstrip(b"//", foo = "bar") "".rstrip() # OK: Not literals -foo: str = ""; bar: bytes = b"" "".strip(foo) b"".strip(bar) -# False negative -foo.rstrip("//") -bar.lstrip(b"//") - # OK: Not `.[lr]?strip` "".mobius_strip("") diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index fc0ed99f6c..79bb1eafb7 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -441,6 +441,7 @@ mod tests { Path::new("repeated_equality_comparison.py") )] #[test_case(Rule::InvalidEnvvarDefault, Path::new("invalid_envvar_default.py"))] + #[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( "preview__{}_{}", diff --git a/crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs b/crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs index 5f50cb5a62..9ed65dc8eb 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/bad_str_strip_call.rs @@ -5,6 +5,8 @@ use rustc_hash::FxHashSet; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; +use ruff_python_semantic::analyze::typing; +use ruff_python_semantic::SemanticModel; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -72,10 +74,22 @@ pub(crate) enum ValueKind { } impl ValueKind { - fn from(expr: &Expr) -> Option { + fn from(expr: &Expr, semantic: &SemanticModel) -> Option { match expr { Expr::StringLiteral(_) => Some(Self::String), Expr::BytesLiteral(_) => Some(Self::Bytes), + Expr::Name(name) => { + let binding_id = semantic.only_binding(name)?; + let binding = semantic.binding(binding_id); + + if typing::is_string(binding, semantic) { + Some(Self::String) + } else if typing::is_bytes(binding, semantic) { + Some(Self::Bytes) + } else { + None + } + } _ => None, } } @@ -171,7 +185,15 @@ pub(crate) fn bad_str_strip_call(checker: &Checker, call: &ast::ExprCall) { return; }; - let Some(value_kind) = ValueKind::from(value.as_ref()) else { + let value = &**value; + + if checker.settings.preview.is_disabled() + && !matches!(value, Expr::StringLiteral(_) | Expr::BytesLiteral(_)) + { + return; + } + + let Some(value_kind) = ValueKind::from(value, checker.semantic()) else { return; }; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1310_bad_str_strip_call.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1310_bad_str_strip_call.py.snap index c6d17717c5..d017b7b062 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1310_bad_str_strip_call.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE1310_bad_str_strip_call.py.snap @@ -179,5 +179,5 @@ bad_str_strip_call.py:81:10: PLE1310 String `strip` call contains duplicate char 81 | ''.strip('\\\x5C') | ^^^^^^^^ PLE1310 82 | -83 | # OK: Different types +83 | # Errors: Type inference | diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE1310_bad_str_strip_call.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE1310_bad_str_strip_call.py.snap new file mode 100644 index 0000000000..a3d6f646c0 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE1310_bad_str_strip_call.py.snap @@ -0,0 +1,210 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +bad_str_strip_call.py:2:21: PLE1310 String `strip` call contains duplicate characters + | +1 | # PLE1310 +2 | "Hello World".strip("Hello") + | ^^^^^^^ PLE1310 +3 | +4 | # PLE1310 + | + +bad_str_strip_call.py:5:21: PLE1310 String `strip` call contains duplicate characters + | +4 | # PLE1310 +5 | "Hello World".strip("Hello") + | ^^^^^^^ PLE1310 +6 | +7 | # PLE1310 + | + +bad_str_strip_call.py:8:21: PLE1310 String `strip` call contains duplicate characters + | + 7 | # PLE1310 + 8 | "Hello World".strip(u"Hello") + | ^^^^^^^^ PLE1310 + 9 | +10 | # PLE1310 + | + +bad_str_strip_call.py:11:21: PLE1310 String `strip` call contains duplicate characters + | +10 | # PLE1310 +11 | "Hello World".strip(r"Hello") + | ^^^^^^^^ PLE1310 +12 | +13 | # PLE1310 + | + +bad_str_strip_call.py:14:21: PLE1310 String `strip` call contains duplicate characters + | +13 | # PLE1310 +14 | "Hello World".strip("Hello\t") + | ^^^^^^^^^ PLE1310 +15 | +16 | # PLE1310 + | + +bad_str_strip_call.py:17:21: PLE1310 String `strip` call contains duplicate characters + | +16 | # PLE1310 +17 | "Hello World".strip(r"Hello\t") + | ^^^^^^^^^^ PLE1310 +18 | +19 | # PLE1310 + | + +bad_str_strip_call.py:20:21: PLE1310 String `strip` call contains duplicate characters + | +19 | # PLE1310 +20 | "Hello World".strip("Hello\\") + | ^^^^^^^^^ PLE1310 +21 | +22 | # PLE1310 + | + +bad_str_strip_call.py:23:21: PLE1310 String `strip` call contains duplicate characters + | +22 | # PLE1310 +23 | "Hello World".strip(r"Hello\\") + | ^^^^^^^^^^ PLE1310 +24 | +25 | # PLE1310 + | + +bad_str_strip_call.py:26:21: PLE1310 String `strip` call contains duplicate characters + | +25 | # PLE1310 +26 | "Hello World".strip("🤣🤣🤣🤣🙃👀😀") + | ^^^^^^^^^^^^^^^^ PLE1310 +27 | +28 | # PLE1310 + | + +bad_str_strip_call.py:30:5: PLE1310 String `strip` call contains duplicate characters + | +28 | # PLE1310 +29 | "Hello World".strip( +30 | / """ +31 | | there are a lot of characters to strip +32 | | """ + | |___^ PLE1310 +33 | ) + | + +bad_str_strip_call.py:36:21: PLE1310 String `strip` call contains duplicate characters + | +35 | # PLE1310 +36 | "Hello World".strip("can we get a long " \ + | _____________________^ +37 | | "string of characters to strip " \ +38 | | "please?") + | |_____________________________^ PLE1310 +39 | +40 | # PLE1310 + | + +bad_str_strip_call.py:42:5: PLE1310 String `strip` call contains duplicate characters + | +40 | # PLE1310 +41 | "Hello World".strip( +42 | / "can we get a long " +43 | | "string of characters to strip " +44 | | "please?" + | |_____________^ PLE1310 +45 | ) + | + +bad_str_strip_call.py:49:5: PLE1310 String `strip` call contains duplicate characters + | +47 | # PLE1310 +48 | "Hello World".strip( +49 | / "can \t we get a long" +50 | | "string \t of characters to strip" +51 | | "please?" + | |_____________^ PLE1310 +52 | ) + | + +bad_str_strip_call.py:61:11: PLE1310 String `strip` call contains duplicate characters + | +60 | # PLE1310 +61 | u''.strip('http://') + | ^^^^^^^^^ PLE1310 +62 | +63 | # PLE1310 + | + +bad_str_strip_call.py:64:12: PLE1310 String `lstrip` call contains duplicate characters (did you mean `removeprefix`?) + | +63 | # PLE1310 +64 | u''.lstrip('http://') + | ^^^^^^^^^ PLE1310 +65 | +66 | # PLE1310 + | + +bad_str_strip_call.py:67:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?) + | +66 | # PLE1310 +67 | b''.rstrip(b'http://') + | ^^^^^^^^^^ PLE1310 +68 | +69 | # OK + | + +bad_str_strip_call.py:79:10: PLE1310 String `strip` call contains duplicate characters + | +78 | # Errors: Multiple backslashes +79 | ''.strip('\\b\\x09') + | ^^^^^^^^^^ PLE1310 +80 | ''.strip(r'\b\x09') +81 | ''.strip('\\\x5C') + | + +bad_str_strip_call.py:80:10: PLE1310 String `strip` call contains duplicate characters + | +78 | # Errors: Multiple backslashes +79 | ''.strip('\\b\\x09') +80 | ''.strip(r'\b\x09') + | ^^^^^^^^^ PLE1310 +81 | ''.strip('\\\x5C') + | + +bad_str_strip_call.py:81:10: PLE1310 String `strip` call contains duplicate characters + | +79 | ''.strip('\\b\\x09') +80 | ''.strip(r'\b\x09') +81 | ''.strip('\\\x5C') + | ^^^^^^^^ PLE1310 +82 | +83 | # Errors: Type inference + | + +bad_str_strip_call.py:85:9: PLE1310 String `strip` call contains duplicate characters + | +83 | # Errors: Type inference +84 | b = b'' +85 | b.strip(b'//') + | ^^^^^ PLE1310 +86 | +87 | # Errors: Type inference (preview) + | + +bad_str_strip_call.py:89:12: PLE1310 String `rstrip` call contains duplicate characters (did you mean `removesuffix`?) + | +87 | # Errors: Type inference (preview) +88 | foo: str = ""; bar: bytes = b"" +89 | foo.rstrip("//") + | ^^^^ PLE1310 +90 | bar.lstrip(b"//") + | + +bad_str_strip_call.py:90:12: PLE1310 String `lstrip` call contains duplicate characters (did you mean `removeprefix`?) + | +88 | foo: str = ""; bar: bytes = b"" +89 | foo.rstrip("//") +90 | bar.lstrip(b"//") + | ^^^^^ PLE1310 + | diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 2b4d644f36..c9d1858ecc 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -741,6 +741,22 @@ impl BuiltinTypeChecker for SetChecker { const EXPR_TYPE: PythonType = PythonType::Set; } +struct StringChecker; + +impl BuiltinTypeChecker for StringChecker { + const BUILTIN_TYPE_NAME: &'static str = "str"; + const TYPING_NAME: Option<&'static str> = None; + const EXPR_TYPE: PythonType = PythonType::String; +} + +struct BytesChecker; + +impl BuiltinTypeChecker for BytesChecker { + const BUILTIN_TYPE_NAME: &'static str = "bytes"; + const TYPING_NAME: Option<&'static str> = None; + const EXPR_TYPE: PythonType = PythonType::Bytes; +} + struct TupleChecker; impl BuiltinTypeChecker for TupleChecker { @@ -984,6 +1000,16 @@ pub fn is_float(binding: &Binding, semantic: &SemanticModel) -> bool { check_type::(binding, semantic) } +/// Test whether the given binding can be considered an instance of `str`. +pub fn is_string(binding: &Binding, semantic: &SemanticModel) -> bool { + check_type::(binding, semantic) +} + +/// Test whether the given binding can be considered an instance of `bytes`. +pub fn is_bytes(binding: &Binding, semantic: &SemanticModel) -> bool { + check_type::(binding, semantic) +} + /// Test whether the given binding can be considered a set. /// /// For this, we check what value might be associated with it through it's initialization and