From fdbd1f1836ac3e417bd61b948d7a31c144acf631 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 9 Mar 2024 16:40:34 +0530 Subject: [PATCH 1/4] Use `ExprFString` for `StringLike::FString` variant --- crates/ruff_linter/src/checkers/ast/mod.rs | 13 +----- .../rules/hardcoded_bind_all_interfaces.rs | 42 ++++++++++++----- .../rules/hardcoded_tmp_directory.rs | 31 ++++++++++--- .../rules/string_or_bytes_too_long.rs | 31 +++++++++++-- .../ruff/rules/ambiguous_unicode_character.rs | 46 +++++++++++++------ crates/ruff_python_ast/src/expression.rs | 6 +-- 6 files changed, 120 insertions(+), 49 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index cda951ecba299..43aaf86ccf3ea 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -45,7 +45,7 @@ use ruff_python_ast::helpers::{ use ruff_python_ast::identifier::Identifier; use ruff_python_ast::name::QualifiedName; use ruff_python_ast::str::Quote; -use ruff_python_ast::visitor::{walk_except_handler, walk_f_string_element, walk_pattern, Visitor}; +use ruff_python_ast::visitor::{walk_except_handler, walk_pattern, Visitor}; use ruff_python_ast::{helpers, str, visitor, PySourceType}; use ruff_python_codegen::{Generator, Stylist}; use ruff_python_index::Indexer; @@ -1407,6 +1407,7 @@ impl<'a> Visitor<'a> for Checker<'a> { analyze::string_like(string_literal.into(), self); } Expr::BytesLiteral(bytes_literal) => analyze::string_like(bytes_literal.into(), self), + Expr::FString(f_string) => analyze::string_like(f_string.into(), self), _ => {} } @@ -1573,16 +1574,6 @@ impl<'a> Visitor<'a> for Checker<'a> { .push((bound, self.semantic.snapshot())); } } - - fn visit_f_string_element(&mut self, f_string_element: &'a ast::FStringElement) { - // Step 2: Traversal - walk_f_string_element(self, f_string_element); - - // Step 4: Analysis - if let Some(literal) = f_string_element.as_literal() { - analyze::string_like(literal.into(), self); - } - } } impl<'a> Checker<'a> { diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs index 0e4301ee44c07..7e65372ab72e7 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs @@ -38,17 +38,37 @@ impl Violation for HardcodedBindAllInterfaces { /// S104 pub(crate) fn hardcoded_bind_all_interfaces(checker: &mut Checker, string: StringLike) { - let is_bind_all_interface = match string { - StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => value == "0.0.0.0", - StringLike::FStringLiteral(ast::FStringLiteralElement { value, .. }) => { - &**value == "0.0.0.0" + match string { + StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => { + if value == "0.0.0.0" { + checker + .diagnostics + .push(Diagnostic::new(HardcodedBindAllInterfaces, string.range())); + } } - StringLike::BytesLiteral(_) => return, + StringLike::FStringLiteral(ast::ExprFString { value, .. }) => { + for part in value { + match part { + ast::FStringPart::Literal(literal) => { + if &**literal == "0.0.0.0" { + checker + .diagnostics + .push(Diagnostic::new(HardcodedBindAllInterfaces, literal.range())); + } + } + ast::FStringPart::FString(f_string) => { + for literal in f_string.literals() { + if &**literal == "0.0.0.0" { + checker.diagnostics.push(Diagnostic::new( + HardcodedBindAllInterfaces, + literal.range(), + )); + } + } + } + } + } + } + StringLike::BytesLiteral(_) => (), }; - - if is_bind_all_interface { - checker - .diagnostics - .push(Diagnostic::new(HardcodedBindAllInterfaces, string.range())); - } } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs index fc74174fe32be..69b19fdb3578f 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs @@ -1,5 +1,5 @@ use ruff_python_ast::{self as ast, Expr, StringLike}; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; @@ -53,12 +53,29 @@ impl Violation for HardcodedTempFile { /// S108 pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, string: StringLike) { - let value = match string { - StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.to_str(), - StringLike::FStringLiteral(ast::FStringLiteralElement { value, .. }) => value, - StringLike::BytesLiteral(_) => return, - }; + match string { + StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => { + check(checker, value.to_str(), string.range()); + } + StringLike::FStringLiteral(ast::ExprFString { value, .. }) => { + for part in value { + match part { + ast::FStringPart::Literal(literal) => { + check(checker, literal, literal.range()); + } + ast::FStringPart::FString(f_string) => { + for literal in f_string.literals() { + check(checker, literal, literal.range()); + } + } + } + } + } + StringLike::BytesLiteral(_) => (), + } +} +fn check(checker: &mut Checker, value: &str, range: TextRange) { if !checker .settings .flake8_bandit @@ -85,6 +102,6 @@ pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, string: StringLike) HardcodedTempFile { string: value.to_string(), }, - string.range(), + range, )); } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs index 7be2510933e32..b4f4be3402b6d 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs @@ -59,9 +59,7 @@ pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, string: StringLike let length = match string { StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.chars().count(), StringLike::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.len(), - StringLike::FStringLiteral(ast::FStringLiteralElement { value, .. }) => { - value.chars().count() - } + StringLike::FStringLiteral(node) => count_f_string_chars(node), }; if length <= 50 { return; @@ -75,6 +73,33 @@ pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, string: StringLike checker.diagnostics.push(diagnostic); } +/// Count the number of characters in an f-string. This accounts for implicitly concatenated +/// f-strings as well. For example, the following f-string has 12 characters as highlighted +/// by the caret symbols: +/// +/// ```python +/// x = "one" f"one{expr}one" f"one" f"{expr}" +/// # ^^^ ^^^ ^^^ ^^^ +/// ```` +fn count_f_string_chars(f_string: &ast::ExprFString) -> usize { + f_string + .value + .iter() + .map(|part| match part { + ast::FStringPart::Literal(string) => string.chars().count(), + ast::FStringPart::FString(f_string) => f_string + .elements + .iter() + .map(|element| { + element + .as_literal() + .map_or(0, |literal| literal.chars().count()) + }) + .sum(), + }) + .sum() +} + fn is_warnings_dot_deprecated(expr: Option<&ast::Expr>, semantic: &SemanticModel) -> bool { // Does `expr` represent a call to `warnings.deprecated` or `typing_extensions.deprecated`? let Some(expr) = expr else { diff --git a/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs b/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs index 2a228f1a4c5b5..88112233a28bc 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs @@ -4,7 +4,7 @@ use bitflags::bitflags; use ruff_diagnostics::{Diagnostic, DiagnosticKind, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::StringLike; +use ruff_python_ast::{self as ast, StringLike}; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -193,27 +193,45 @@ pub(crate) fn ambiguous_unicode_character_string(checker: &mut Checker, string_l }; match string_like { - StringLike::StringLiteral(string_literal) => { - for string in &string_literal.value { - let text = checker.locator().slice(string); + StringLike::StringLiteral(node) => { + for literal in &node.value { + let text = checker.locator().slice(literal); ambiguous_unicode_character( &mut checker.diagnostics, text, - string.range(), + literal.range(), context, checker.settings, ); } } - StringLike::FStringLiteral(f_string_literal) => { - let text = checker.locator().slice(f_string_literal); - ambiguous_unicode_character( - &mut checker.diagnostics, - text, - f_string_literal.range(), - context, - checker.settings, - ); + StringLike::FStringLiteral(node) => { + for part in &node.value { + match part { + ast::FStringPart::Literal(literal) => { + let text = checker.locator().slice(literal); + ambiguous_unicode_character( + &mut checker.diagnostics, + text, + literal.range(), + context, + checker.settings, + ); + } + ast::FStringPart::FString(f_string) => { + for literal in f_string.literals() { + let text = checker.locator().slice(literal); + ambiguous_unicode_character( + &mut checker.diagnostics, + text, + literal.range(), + context, + checker.settings, + ); + } + } + } + } } StringLike::BytesLiteral(_) => (), } diff --git a/crates/ruff_python_ast/src/expression.rs b/crates/ruff_python_ast/src/expression.rs index 707737c403309..9dc40b53a30aa 100644 --- a/crates/ruff_python_ast/src/expression.rs +++ b/crates/ruff_python_ast/src/expression.rs @@ -401,7 +401,7 @@ impl LiteralExpressionRef<'_> { pub enum StringLike<'a> { StringLiteral(&'a ast::ExprStringLiteral), BytesLiteral(&'a ast::ExprBytesLiteral), - FStringLiteral(&'a ast::FStringLiteralElement), + FStringLiteral(&'a ast::ExprFString), } impl<'a> From<&'a ast::ExprStringLiteral> for StringLike<'a> { @@ -416,8 +416,8 @@ impl<'a> From<&'a ast::ExprBytesLiteral> for StringLike<'a> { } } -impl<'a> From<&'a ast::FStringLiteralElement> for StringLike<'a> { - fn from(value: &'a ast::FStringLiteralElement) -> Self { +impl<'a> From<&'a ast::ExprFString> for StringLike<'a> { + fn from(value: &'a ast::ExprFString) -> Self { StringLike::FStringLiteral(value) } } From 534cbb56d45ed2de44071b3ff8e8dacb9dd79387 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 9 Mar 2024 16:40:54 +0530 Subject: [PATCH 2/4] Add test case, update snapshots --- .../test/fixtures/flake8_bandit/S104.py | 4 ++++ .../test/fixtures/flake8_bandit/S108.py | 7 ++++++ .../test/fixtures/flake8_pyi/PYI053.pyi | 2 ++ .../test/fixtures/ruff/confusables.py | 3 +++ ...s__flake8_bandit__tests__S104_S104.py.snap | 19 +++++++++++++++ ...s__flake8_bandit__tests__S108_S108.py.snap | 24 +++++++++++++++++++ ...es__flake8_bandit__tests__S108_extend.snap | 24 +++++++++++++++++++ ..._flake8_pyi__tests__PYI053_PYI053.pyi.snap | 21 +++++++++++++--- ...nter__rules__ruff__tests__confusables.snap | 12 ++++++++++ ...les__ruff__tests__preview_confusables.snap | 14 +++++++++++ 10 files changed, 127 insertions(+), 3 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S104.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S104.py index 7e50db007619c..d7f9716ef1a6c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S104.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S104.py @@ -18,3 +18,7 @@ def func(address): def my_func(): x = "0.0.0.0" print(x) + + +# Implicit string concatenation +"0.0.0.0" f"0.0.0.0{expr}0.0.0.0" diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S108.py b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S108.py index c7cc7dd4809f5..610a6700cdba7 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S108.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bandit/S108.py @@ -18,6 +18,13 @@ with open("/foo/bar", "w") as f: f.write("def") +# Implicit string concatenation +with open("/tmp/" "abc", "w") as f: + f.write("def") + +with open("/tmp/abc" f"/tmp/abc", "w") as f: + f.write("def") + # Using `tempfile` module should be ok import tempfile from tempfile import TemporaryDirectory diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI053.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI053.pyi index 668005de39f98..7bb29dc1c468e 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI053.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI053.pyi @@ -64,3 +64,5 @@ def not_warnings_dot_deprecated( "Not warnings.deprecated, so this one *should* lead to PYI053 in a stub!" # Error: PYI053 ) def not_a_deprecated_function() -> None: ... + +fbaz: str = f"51 character {foo} stringggggggggggggggggggggggggggggggg" # Error: PYI053 diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/confusables.py b/crates/ruff_linter/resources/test/fixtures/ruff/confusables.py index 7791fb5dc4d76..b838a284947c3 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/confusables.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/confusables.py @@ -53,3 +53,6 @@ class Labware: assert getattr(Labware(), "ยตL") == 1.5 + +# Implicit string concatenation +x = "๐ad" f"๐ad string" diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S104_S104.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S104_S104.py.snap index b3b9ad07d38d4..3f87d24d80ef0 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S104_S104.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S104_S104.py.snap @@ -42,4 +42,23 @@ S104.py:19:9: S104 Possible binding to all interfaces 20 | print(x) | +S104.py:24:1: S104 Possible binding to all interfaces + | +23 | # Implicit string concatenation +24 | "0.0.0.0" f"0.0.0.0{expr}0.0.0.0" + | ^^^^^^^^^ S104 + | + +S104.py:24:13: S104 Possible binding to all interfaces + | +23 | # Implicit string concatenation +24 | "0.0.0.0" f"0.0.0.0{expr}0.0.0.0" + | ^^^^^^^ S104 + | +S104.py:24:26: S104 Possible binding to all interfaces + | +23 | # Implicit string concatenation +24 | "0.0.0.0" f"0.0.0.0{expr}0.0.0.0" + | ^^^^^^^ S104 + | diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S108_S108.py.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S108_S108.py.snap index 7336a5015aa28..d7cf9f3ec0a0e 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S108_S108.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S108_S108.py.snap @@ -37,4 +37,28 @@ S108.py:14:11: S108 Probable insecure usage of temporary file or directory: "/de 15 | f.write("def") | +S108.py:22:11: S108 Probable insecure usage of temporary file or directory: "/tmp/abc" + | +21 | # Implicit string concatenation +22 | with open("/tmp/" "abc", "w") as f: + | ^^^^^^^^^^^^^ S108 +23 | f.write("def") + | +S108.py:25:11: S108 Probable insecure usage of temporary file or directory: "/tmp/abc" + | +23 | f.write("def") +24 | +25 | with open("/tmp/abc" f"/tmp/abc", "w") as f: + | ^^^^^^^^^^ S108 +26 | f.write("def") + | + +S108.py:25:24: S108 Probable insecure usage of temporary file or directory: "/tmp/abc" + | +23 | f.write("def") +24 | +25 | with open("/tmp/abc" f"/tmp/abc", "w") as f: + | ^^^^^^^^ S108 +26 | f.write("def") + | diff --git a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S108_extend.snap b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S108_extend.snap index b562794a05d7c..9cebf409e715d 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S108_extend.snap +++ b/crates/ruff_linter/src/rules/flake8_bandit/snapshots/ruff_linter__rules__flake8_bandit__tests__S108_extend.snap @@ -45,4 +45,28 @@ S108.py:18:11: S108 Probable insecure usage of temporary file or directory: "/fo 19 | f.write("def") | +S108.py:22:11: S108 Probable insecure usage of temporary file or directory: "/tmp/abc" + | +21 | # Implicit string concatenation +22 | with open("/tmp/" "abc", "w") as f: + | ^^^^^^^^^^^^^ S108 +23 | f.write("def") + | + +S108.py:25:11: S108 Probable insecure usage of temporary file or directory: "/tmp/abc" + | +23 | f.write("def") +24 | +25 | with open("/tmp/abc" f"/tmp/abc", "w") as f: + | ^^^^^^^^^^ S108 +26 | f.write("def") + | +S108.py:25:24: S108 Probable insecure usage of temporary file or directory: "/tmp/abc" + | +23 | f.write("def") +24 | +25 | with open("/tmp/abc" f"/tmp/abc", "w") as f: + | ^^^^^^^^ S108 +26 | f.write("def") + | diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap index 096a8ce038263..a6829a787fe0b 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap @@ -105,12 +105,12 @@ PYI053.pyi:34:14: PYI053 [*] String and bytes literals longer than 50 characters 36 36 | ffoo: str = f"50 character stringggggggggggggggggggggggggggggggg" # OK 37 37 | -PYI053.pyi:38:15: PYI053 [*] String and bytes literals longer than 50 characters are not permitted +PYI053.pyi:38:13: PYI053 [*] String and bytes literals longer than 50 characters are not permitted | 36 | ffoo: str = f"50 character stringggggggggggggggggggggggggggggggg" # OK 37 | 38 | fbar: str = f"51 character stringgggggggggggggggggggggggggggggggg" # Error: PYI053 - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 39 | 40 | class Demo: | @@ -121,7 +121,7 @@ PYI053.pyi:38:15: PYI053 [*] String and bytes literals longer than 50 characters 36 36 | ffoo: str = f"50 character stringggggggggggggggggggggggggggggggg" # OK 37 37 | 38 |-fbar: str = f"51 character stringgggggggggggggggggggggggggggggggg" # Error: PYI053 - 38 |+fbar: str = f"..." # Error: PYI053 + 38 |+fbar: str = ... # Error: PYI053 39 39 | 40 40 | class Demo: 41 41 | """Docstrings are excluded from this rule. Some padding.""" # OK @@ -144,5 +144,20 @@ PYI053.pyi:64:5: PYI053 [*] String and bytes literals longer than 50 characters 64 |+ ... # Error: PYI053 65 65 | ) 66 66 | def not_a_deprecated_function() -> None: ... +67 67 | +PYI053.pyi:68:13: PYI053 [*] String and bytes literals longer than 50 characters are not permitted + | +66 | def not_a_deprecated_function() -> None: ... +67 | +68 | fbaz: str = f"51 character {foo} stringggggggggggggggggggggggggggggggg" # Error: PYI053 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 + | + = help: Replace with `...` +โ„น Safe fix +65 65 | ) +66 66 | def not_a_deprecated_function() -> None: ... +67 67 | +68 |-fbaz: str = f"51 character {foo} stringggggggggggggggggggggggggggggggg" # Error: PYI053 + 68 |+fbaz: str = ... # Error: PYI053 diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__confusables.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__confusables.snap index 541fc82af67a1..7bbfa68012c1c 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__confusables.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__confusables.snap @@ -155,4 +155,16 @@ confusables.py:46:62: RUF003 Comment contains ambiguous `แœต` (PHILIPPINE SINGLE 47 | }" | +confusables.py:58:6: RUF001 String contains ambiguous `๐` (MATHEMATICAL BOLD CAPITAL B). Did you mean `B` (LATIN CAPITAL LETTER B)? + | +57 | # Implicit string concatenation +58 | x = "๐ad" f"๐ad string" + | ^ RUF001 + | +confusables.py:58:13: RUF001 String contains ambiguous `๐` (MATHEMATICAL BOLD CAPITAL B). Did you mean `B` (LATIN CAPITAL LETTER B)? + | +57 | # Implicit string concatenation +58 | x = "๐ad" f"๐ad string" + | ^ RUF001 + | diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview_confusables.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview_confusables.snap index 1a7b2d480542d..46ac0fafeb4a9 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview_confusables.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview_confusables.snap @@ -159,6 +159,20 @@ confusables.py:55:28: RUF001 String contains ambiguous `ยต` (MICRO SIGN). Did yo | 55 | assert getattr(Labware(), "ยตL") == 1.5 | ^ RUF001 +56 | +57 | # Implicit string concatenation | +confusables.py:58:6: RUF001 String contains ambiguous `๐` (MATHEMATICAL BOLD CAPITAL B). Did you mean `B` (LATIN CAPITAL LETTER B)? + | +57 | # Implicit string concatenation +58 | x = "๐ad" f"๐ad string" + | ^ RUF001 + | +confusables.py:58:13: RUF001 String contains ambiguous `๐` (MATHEMATICAL BOLD CAPITAL B). Did you mean `B` (LATIN CAPITAL LETTER B)? + | +57 | # Implicit string concatenation +58 | x = "๐ad" f"๐ad string" + | ^ RUF001 + | From d607009b7e30fa9438bcbc0a207aa0e428304d3c Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 9 Mar 2024 17:29:13 +0530 Subject: [PATCH 3/4] Rename variants to remove `Literal` prefix --- .../rules/hardcoded_bind_all_interfaces.rs | 6 +++--- .../rules/hardcoded_tmp_directory.rs | 6 +++--- .../rules/string_or_bytes_too_long.rs | 6 +++--- .../ruff/rules/ambiguous_unicode_character.rs | 6 +++--- crates/ruff_python_ast/src/expression.rs | 18 +++++++++--------- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs index 7e65372ab72e7..aea1850771de0 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_bind_all_interfaces.rs @@ -39,14 +39,14 @@ impl Violation for HardcodedBindAllInterfaces { /// S104 pub(crate) fn hardcoded_bind_all_interfaces(checker: &mut Checker, string: StringLike) { match string { - StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => { + StringLike::String(ast::ExprStringLiteral { value, .. }) => { if value == "0.0.0.0" { checker .diagnostics .push(Diagnostic::new(HardcodedBindAllInterfaces, string.range())); } } - StringLike::FStringLiteral(ast::ExprFString { value, .. }) => { + StringLike::FString(ast::ExprFString { value, .. }) => { for part in value { match part { ast::FStringPart::Literal(literal) => { @@ -69,6 +69,6 @@ pub(crate) fn hardcoded_bind_all_interfaces(checker: &mut Checker, string: Strin } } } - StringLike::BytesLiteral(_) => (), + StringLike::Bytes(_) => (), }; } diff --git a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs index 69b19fdb3578f..4304e1482907c 100644 --- a/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs +++ b/crates/ruff_linter/src/rules/flake8_bandit/rules/hardcoded_tmp_directory.rs @@ -54,10 +54,10 @@ impl Violation for HardcodedTempFile { /// S108 pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, string: StringLike) { match string { - StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => { + StringLike::String(ast::ExprStringLiteral { value, .. }) => { check(checker, value.to_str(), string.range()); } - StringLike::FStringLiteral(ast::ExprFString { value, .. }) => { + StringLike::FString(ast::ExprFString { value, .. }) => { for part in value { match part { ast::FStringPart::Literal(literal) => { @@ -71,7 +71,7 @@ pub(crate) fn hardcoded_tmp_directory(checker: &mut Checker, string: StringLike) } } } - StringLike::BytesLiteral(_) => (), + StringLike::Bytes(_) => (), } } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs index b4f4be3402b6d..a51886046dbc3 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs @@ -57,9 +57,9 @@ pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, string: StringLike } let length = match string { - StringLike::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.chars().count(), - StringLike::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.len(), - StringLike::FStringLiteral(node) => count_f_string_chars(node), + StringLike::String(ast::ExprStringLiteral { value, .. }) => value.chars().count(), + StringLike::Bytes(ast::ExprBytesLiteral { value, .. }) => value.len(), + StringLike::FString(node) => count_f_string_chars(node), }; if length <= 50 { return; diff --git a/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs b/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs index 88112233a28bc..85f73f3db9610 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/ambiguous_unicode_character.rs @@ -193,7 +193,7 @@ pub(crate) fn ambiguous_unicode_character_string(checker: &mut Checker, string_l }; match string_like { - StringLike::StringLiteral(node) => { + StringLike::String(node) => { for literal in &node.value { let text = checker.locator().slice(literal); ambiguous_unicode_character( @@ -205,7 +205,7 @@ pub(crate) fn ambiguous_unicode_character_string(checker: &mut Checker, string_l ); } } - StringLike::FStringLiteral(node) => { + StringLike::FString(node) => { for part in &node.value { match part { ast::FStringPart::Literal(literal) => { @@ -233,7 +233,7 @@ pub(crate) fn ambiguous_unicode_character_string(checker: &mut Checker, string_l } } } - StringLike::BytesLiteral(_) => (), + StringLike::Bytes(_) => (), } } diff --git a/crates/ruff_python_ast/src/expression.rs b/crates/ruff_python_ast/src/expression.rs index 9dc40b53a30aa..1ef7247f5a373 100644 --- a/crates/ruff_python_ast/src/expression.rs +++ b/crates/ruff_python_ast/src/expression.rs @@ -399,35 +399,35 @@ impl LiteralExpressionRef<'_> { /// f-strings. #[derive(Copy, Clone, Debug, PartialEq)] pub enum StringLike<'a> { - StringLiteral(&'a ast::ExprStringLiteral), - BytesLiteral(&'a ast::ExprBytesLiteral), - FStringLiteral(&'a ast::ExprFString), + String(&'a ast::ExprStringLiteral), + Bytes(&'a ast::ExprBytesLiteral), + FString(&'a ast::ExprFString), } impl<'a> From<&'a ast::ExprStringLiteral> for StringLike<'a> { fn from(value: &'a ast::ExprStringLiteral) -> Self { - StringLike::StringLiteral(value) + StringLike::String(value) } } impl<'a> From<&'a ast::ExprBytesLiteral> for StringLike<'a> { fn from(value: &'a ast::ExprBytesLiteral) -> Self { - StringLike::BytesLiteral(value) + StringLike::Bytes(value) } } impl<'a> From<&'a ast::ExprFString> for StringLike<'a> { fn from(value: &'a ast::ExprFString) -> Self { - StringLike::FStringLiteral(value) + StringLike::FString(value) } } impl Ranged for StringLike<'_> { fn range(&self) -> TextRange { match self { - StringLike::StringLiteral(literal) => literal.range(), - StringLike::BytesLiteral(literal) => literal.range(), - StringLike::FStringLiteral(literal) => literal.range(), + StringLike::String(literal) => literal.range(), + StringLike::Bytes(literal) => literal.range(), + StringLike::FString(literal) => literal.range(), } } } From 1ef5e952f61d0a809755a8fb1ee8f849ddc9efd8 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 14 Mar 2024 13:12:46 +0530 Subject: [PATCH 4/4] Count expression length as well --- .../test/fixtures/flake8_pyi/PYI053.pyi | 2 +- .../rules/string_or_bytes_too_long.rs | 17 +++++------------ ...s__flake8_pyi__tests__PYI053_PYI053.pyi.snap | 6 +++--- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI053.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI053.pyi index 7bb29dc1c468e..a711b7e9156d9 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI053.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI053.pyi @@ -65,4 +65,4 @@ def not_warnings_dot_deprecated( ) def not_a_deprecated_function() -> None: ... -fbaz: str = f"51 character {foo} stringggggggggggggggggggggggggggggggg" # Error: PYI053 +fbaz: str = f"51 character {foo} stringgggggggggggggggggggggggggg" # Error: PYI053 diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs index a51886046dbc3..145d38f6da6b4 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs @@ -73,14 +73,8 @@ pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, string: StringLike checker.diagnostics.push(diagnostic); } -/// Count the number of characters in an f-string. This accounts for implicitly concatenated -/// f-strings as well. For example, the following f-string has 12 characters as highlighted -/// by the caret symbols: -/// -/// ```python -/// x = "one" f"one{expr}one" f"one" f"{expr}" -/// # ^^^ ^^^ ^^^ ^^^ -/// ```` +/// Count the number of visible characters in an f-string. This accounts for +/// implicitly concatenated f-strings as well. fn count_f_string_chars(f_string: &ast::ExprFString) -> usize { f_string .value @@ -90,10 +84,9 @@ fn count_f_string_chars(f_string: &ast::ExprFString) -> usize { ast::FStringPart::FString(f_string) => f_string .elements .iter() - .map(|element| { - element - .as_literal() - .map_or(0, |literal| literal.chars().count()) + .map(|element| match element { + ast::FStringElement::Literal(string) => string.chars().count(), + ast::FStringElement::Expression(expr) => expr.range().len().to_usize(), }) .sum(), }) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap index a6829a787fe0b..19ca04f611b51 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI053_PYI053.pyi.snap @@ -150,8 +150,8 @@ PYI053.pyi:68:13: PYI053 [*] String and bytes literals longer than 50 characters | 66 | def not_a_deprecated_function() -> None: ... 67 | -68 | fbaz: str = f"51 character {foo} stringggggggggggggggggggggggggggggggg" # Error: PYI053 - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 +68 | fbaz: str = f"51 character {foo} stringgggggggggggggggggggggggggg" # Error: PYI053 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI053 | = help: Replace with `...` @@ -159,5 +159,5 @@ PYI053.pyi:68:13: PYI053 [*] String and bytes literals longer than 50 characters 65 65 | ) 66 66 | def not_a_deprecated_function() -> None: ... 67 67 | -68 |-fbaz: str = f"51 character {foo} stringggggggggggggggggggggggggggggggg" # Error: PYI053 +68 |-fbaz: str = f"51 character {foo} stringgggggggggggggggggggggggggg" # Error: PYI053 68 |+fbaz: str = ... # Error: PYI053