diff --git a/crates/ruff/resources/test/fixtures/flake8_implicit_str_concat/ISC.py b/crates/ruff/resources/test/fixtures/flake8_implicit_str_concat/ISC.py index ef4517b5b1..5dc543d853 100644 --- a/crates/ruff/resources/test/fixtures/flake8_implicit_str_concat/ISC.py +++ b/crates/ruff/resources/test/fixtures/flake8_implicit_str_concat/ISC.py @@ -59,3 +59,13 @@ _ = "abc" + "def" + foo _ = foo + bar + "abc" _ = "abc" + foo + bar _ = foo + "abc" + bar + +_ = ( + a + f"abc" + + "def" +) + +_ = ( + f"abc" + + "def" + a +) diff --git a/crates/ruff/src/checkers/ast/analyze/expression.rs b/crates/ruff/src/checkers/ast/analyze/expression.rs index b485527b5b..a23a01c11d 100644 --- a/crates/ruff/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff/src/checkers/ast/analyze/expression.rs @@ -12,10 +12,10 @@ use crate::registry::Rule; use crate::rules::{ flake8_2020, flake8_async, flake8_bandit, flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, flake8_debugger, flake8_django, - flake8_future_annotations, flake8_gettext, flake8_implicit_str_concat, flake8_logging_format, - flake8_pie, flake8_print, flake8_pyi, flake8_pytest_style, flake8_self, flake8_simplify, - flake8_tidy_imports, flake8_use_pathlib, flynt, numpy, pandas_vet, pep8_naming, pycodestyle, - pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, + flake8_future_annotations, flake8_gettext, flake8_logging_format, flake8_pie, flake8_print, + flake8_pyi, flake8_pytest_style, flake8_self, flake8_simplify, flake8_tidy_imports, + flake8_use_pathlib, flynt, numpy, pandas_vet, pep8_naming, pycodestyle, pyflakes, pygrep_hooks, + pylint, pyupgrade, ruff, }; use crate::settings::types::PythonVersion; @@ -1055,13 +1055,6 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { Expr::BinOp(ast::ExprBinOp { op: Operator::Add, .. }) => { - if checker.enabled(Rule::ExplicitStringConcatenation) { - if let Some(diagnostic) = - flake8_implicit_str_concat::rules::explicit(expr, checker.locator) - { - checker.diagnostics.push(diagnostic); - } - } if checker.enabled(Rule::CollectionLiteralConcatenation) { ruff::rules::collection_literal_concatenation(checker, expr); } diff --git a/crates/ruff/src/checkers/tokens.rs b/crates/ruff/src/checkers/tokens.rs index 06cfafd4a7..73802cdf88 100644 --- a/crates/ruff/src/checkers/tokens.rs +++ b/crates/ruff/src/checkers/tokens.rs @@ -119,8 +119,8 @@ pub(crate) fn check_tokens( } if settings.rules.any_enabled(&[ - Rule::SingleLineImplicitStringConcatenation, Rule::MultiLineImplicitStringConcatenation, + Rule::SingleLineImplicitStringConcatenation, ]) { flake8_implicit_str_concat::rules::implicit( &mut diagnostics, @@ -130,6 +130,10 @@ pub(crate) fn check_tokens( ); } + if settings.rules.enabled(Rule::ExplicitStringConcatenation) { + flake8_implicit_str_concat::rules::explicit(&mut diagnostics, tokens); + } + if settings.rules.any_enabled(&[ Rule::MissingTrailingComma, Rule::TrailingCommaOnBareTuple, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index f4d5bd2b60..e0713dfe5f 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -258,6 +258,7 @@ impl Rule { | Rule::BlanketNOQA | Rule::BlanketTypeIgnore | Rule::CommentedOutCode + | Rule::ExplicitStringConcatenation | Rule::ExtraneousParentheses | Rule::InvalidCharacterBackspace | Rule::InvalidCharacterEsc diff --git a/crates/ruff/src/rules/flake8_implicit_str_concat/rules/explicit.rs b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/explicit.rs index 0d93bae425..160a5d6f40 100644 --- a/crates/ruff/src/rules/flake8_implicit_str_concat/rules/explicit.rs +++ b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/explicit.rs @@ -1,8 +1,11 @@ -use rustpython_parser::ast::{self, Constant, Expr, Operator, Ranged}; +use itertools::Itertools; +use ruff_text_size::TextRange; +use rustpython_parser::ast::Ranged; +use rustpython_parser::lexer::LexResult; +use rustpython_parser::Tok; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::source_code::Locator; /// ## What it does /// Checks for string literals that are explicitly concatenated (using the @@ -39,34 +42,31 @@ impl Violation for ExplicitStringConcatenation { } /// ISC003 -pub(crate) fn explicit(expr: &Expr, locator: &Locator) -> Option { - if let Expr::BinOp(ast::ExprBinOp { - left, - op, - right, - range, - }) = expr +pub(crate) fn explicit(diagnostics: &mut Vec, tokens: &[LexResult]) { + for ((a_tok, a_range), (b_tok, _), (c_tok, _), (d_tok, d_range)) in tokens + .iter() + .flatten() + .filter(|(tok, _)| !tok.is_comment()) + .tuple_windows() { - if matches!(op, Operator::Add) { - if matches!( - left.as_ref(), - Expr::JoinedStr(_) - | Expr::Constant(ast::ExprConstant { - value: Constant::Str(..) | Constant::Bytes(..), - .. - }) - ) && matches!( - right.as_ref(), - Expr::JoinedStr(_) - | Expr::Constant(ast::ExprConstant { - value: Constant::Str(..) | Constant::Bytes(..), - .. - }) - ) && locator.contains_line_break(*range) - { - return Some(Diagnostic::new(ExplicitStringConcatenation, expr.range())); - } + if matches!( + (a_tok, b_tok, c_tok, d_tok), + ( + Tok::String { .. }, + Tok::NonLogicalNewline, + Tok::Plus, + Tok::String { .. } + ) | ( + Tok::String { .. }, + Tok::Plus, + Tok::NonLogicalNewline, + Tok::String { .. } + ) + ) { + diagnostics.push(Diagnostic::new( + ExplicitStringConcatenation, + TextRange::new(a_range.start(), d_range.end()), + )); } } - None } diff --git a/crates/ruff/src/rules/flake8_implicit_str_concat/snapshots/ruff__rules__flake8_implicit_str_concat__tests__ISC003_ISC.py.snap b/crates/ruff/src/rules/flake8_implicit_str_concat/snapshots/ruff__rules__flake8_implicit_str_concat__tests__ISC003_ISC.py.snap index f27c2bfd1b..0418c2c0e1 100644 --- a/crates/ruff/src/rules/flake8_implicit_str_concat/snapshots/ruff__rules__flake8_implicit_str_concat__tests__ISC003_ISC.py.snap +++ b/crates/ruff/src/rules/flake8_implicit_str_concat/snapshots/ruff__rules__flake8_implicit_str_concat__tests__ISC003_ISC.py.snap @@ -31,4 +31,24 @@ ISC.py:19:3: ISC003 Explicitly concatenated string should be implicitly concaten 21 | ) | +ISC.py:64:7: ISC003 Explicitly concatenated string should be implicitly concatenated + | +63 | _ = ( +64 | a + f"abc" + + | _______^ +65 | | "def" + | |_______^ ISC003 +66 | ) + | + +ISC.py:69:3: ISC003 Explicitly concatenated string should be implicitly concatenated + | +68 | _ = ( +69 | f"abc" + + | ___^ +70 | | "def" + a + | |_______^ ISC003 +71 | ) + | + diff --git a/crates/ruff/src/rules/flake8_implicit_str_concat/snapshots/ruff__rules__flake8_implicit_str_concat__tests__multiline_ISC003_ISC.py.snap b/crates/ruff/src/rules/flake8_implicit_str_concat/snapshots/ruff__rules__flake8_implicit_str_concat__tests__multiline_ISC003_ISC.py.snap index f27c2bfd1b..0418c2c0e1 100644 --- a/crates/ruff/src/rules/flake8_implicit_str_concat/snapshots/ruff__rules__flake8_implicit_str_concat__tests__multiline_ISC003_ISC.py.snap +++ b/crates/ruff/src/rules/flake8_implicit_str_concat/snapshots/ruff__rules__flake8_implicit_str_concat__tests__multiline_ISC003_ISC.py.snap @@ -31,4 +31,24 @@ ISC.py:19:3: ISC003 Explicitly concatenated string should be implicitly concaten 21 | ) | +ISC.py:64:7: ISC003 Explicitly concatenated string should be implicitly concatenated + | +63 | _ = ( +64 | a + f"abc" + + | _______^ +65 | | "def" + | |_______^ ISC003 +66 | ) + | + +ISC.py:69:3: ISC003 Explicitly concatenated string should be implicitly concatenated + | +68 | _ = ( +69 | f"abc" + + | ___^ +70 | | "def" + a + | |_______^ ISC003 +71 | ) + | +