Skip to content

Commit

Permalink
Always allow explicit multi-line concatenations when implicit are ban…
Browse files Browse the repository at this point in the history
…ned (#12532)

## Summary

Closes #11582.
  • Loading branch information
charliermarsh authored Jul 26, 2024
1 parent 1fe4a5f commit 49f5158
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 62 deletions.
8 changes: 5 additions & 3 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1197,9 +1197,11 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
op: Operator::Add, ..
}) => {
if checker.enabled(Rule::ExplicitStringConcatenation) {
if let Some(diagnostic) =
flake8_implicit_str_concat::rules::explicit(expr, checker.locator)
{
if let Some(diagnostic) = flake8_implicit_str_concat::rules::explicit(
expr,
checker.locator,
checker.settings,
) {
checker.diagnostics.push(diagnostic);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ pub(crate) fn check_tokens(
flake8_implicit_str_concat::rules::implicit(
&mut diagnostics,
tokens,
settings,
locator,
indexer,
settings,
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use ruff_python_ast::{self as ast, Expr, Operator};

use crate::settings::LinterSettings;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_source_file::Locator;
Expand Down Expand Up @@ -40,7 +41,19 @@ impl Violation for ExplicitStringConcatenation {
}

/// ISC003
pub(crate) fn explicit(expr: &Expr, locator: &Locator) -> Option<Diagnostic> {
pub(crate) fn explicit(
expr: &Expr,
locator: &Locator,
settings: &LinterSettings,
) -> Option<Diagnostic> {
// If the user sets `allow-multiline` to `false`, then we should allow explicitly concatenated
// strings that span multiple lines even if this rule is enabled. Otherwise, there's no way
// for the user to write multiline strings, and that setting is "more explicit" than this rule
// being enabled.
if !settings.flake8_implicit_str_concat.allow_multiline {
return None;
}

if let Expr::BinOp(ast::ExprBinOp {
left,
op,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ impl Violation for MultiLineImplicitStringConcatenation {
pub(crate) fn implicit(
diagnostics: &mut Vec<Diagnostic>,
tokens: &Tokens,
settings: &LinterSettings,
locator: &Locator,
indexer: &Indexer,
settings: &LinterSettings,
) {
for (a_token, b_token) in tokens
.iter()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,55 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_implicit_str_concat/mod.rs
---
ISC.py:9:3: ISC003 Explicitly concatenated string should be implicitly concatenated
|
8 | _ = (
9 | "abc" +
| ___^
10 | | "def"
| |_______^ ISC003
11 | )
|

ISC.py:14:3: ISC003 Explicitly concatenated string should be implicitly concatenated
|
13 | _ = (
14 | f"abc" +
| ___^
15 | | "def"
| |_______^ ISC003
16 | )
|

ISC.py:19:3: ISC003 Explicitly concatenated string should be implicitly concatenated
|
18 | _ = (
19 | b"abc" +
| ___^
20 | | b"def"
| |________^ ISC003
21 | )
|

ISC.py:78:10: ISC003 Explicitly concatenated string should be implicitly concatenated
|
77 | # Explicitly concatenated nested f-strings
78 | _ = f"a {f"first"
| __________^
79 | | + f"second"} d"
| |_______________^ ISC003
80 | _ = f"a {f"first {f"middle"}"
81 | + f"second"} d"
|

ISC.py:80:10: ISC003 Explicitly concatenated string should be implicitly concatenated
|
78 | _ = f"a {f"first"
79 | + f"second"} d"
80 | _ = f"a {f"first {f"middle"}"
| __________^
81 | | + f"second"} d"
| |_______________^ ISC003
|


9 changes: 5 additions & 4 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1260,10 +1260,11 @@ pub struct Flake8ImplicitStrConcatOptions {
/// allowed (but continuation lines, delimited with a backslash, are
/// prohibited).
///
/// Note that setting `allow-multiline = false` should typically be coupled
/// with disabling `explicit-string-concatenation` (`ISC003`). Otherwise,
/// both explicit and implicit multiline string concatenations will be seen
/// as violations.
/// Setting `allow-multiline = false` will automatically disable the
/// `explicit-string-concatenation` (`ISC003`) rule. Otherwise, both
/// implicit and explicit multiline string concatenations would be seen
/// as violations, making it impossible to write a linter-compliant multiline
/// string.
#[option(
default = r#"true"#,
value_type = "bool",
Expand Down
2 changes: 1 addition & 1 deletion ruff.schema.json

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

0 comments on commit 49f5158

Please sign in to comment.