fix(lint): noUselessStringConcat now correctly detects leading concatenation operators from operatorLinebreak=before
#7874
Conversation
…catenation operators from `operatorLinebreak=before` I initially added a simple or check for the `+` operator having leading whitespace. I realized that check could and will fire on regular addition, so it needed to check that the thing actually was a string. `
🦋 Changeset detectedLatest commit: 5ef9571 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Linter
Bertie690
changed the title
noUselessStringConcat now correctly detects leading concatenation operators from operatorLinebreak=beforenoUselessStringConcat now correctly detects leading concatenation operators from operatorLinebreak=before
we love it when you move 18-month old code into a new function and it trips all the linters again... (not salty just funny)
ffs man since when
WalkthroughReworks the noUselessStringConcat lint rule to correctly handle multi-line concatenations where the Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
crates/biome_js_analyze/tests/specs/complexity/noUselessStringConcat/valid.js (1)
16-20: Nice additions; consider two extra edge cases.Add leading-operator cases with a parenthesised left and with a chained group to lock in the fix:
- ('foo' + 'bar')
- 'baz'
- (
foo+bar)
bazKeeps parity with trailing-operator tests and guards regressions.
Please run the suite for this spec to confirm these pass alongside the new logic.
.changeset/quick-apples-count.md (1)
5-5: Typos and header style.
- “detectes” → “detects”.
- Prefer a colon after the issue per guidelines.
Apply:
-Fixed [#7230](https://github.com/biomejs/biome/issues/7230) - [`noUselessStringConcat`](https://biomejs.dev/linter/rules/no-useless-string-concat/) now correctly detectes multiline strings with leading `+` operators (such as those from `operatorLinebreak="before"`). +Fixed [#7230](https://github.com/biomejs/biome/issues/7230): [`noUselessStringConcat`](https://biomejs.dev/linter/rules/no-useless-string-concat/) now correctly detects multiline strings with leading `+` operators (such as those from `operatorLinebreak="before"`).crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (7)
63-71: Docs: call out leading-operator multiline explicitly.You added a leading-operator example (great). Consider adding a short line stating that both trailing- and leading-operator multiline concatenations are exempt, so readers don’t infer it’s an accident.
95-100: Avoid?inside boolean guard for clarity.The
parent_binary_expression?within theifcondition is unconventional and risks accidental early-returns if refactored. Prefer a straightif let:- if parent_binary_expression.is_some() - && get_concatenation_range(&parent_binary_expression?).is_some() - { + if let Some(parent) = parent_binary_expression { + if get_concatenation_range(&parent).is_some() { + return None; + } + } - return None; - }Please ensure no duplicate diagnostics appear for chains like
"a" + "b" + "c"after this tweak.
183-212: Behaviour vs docs: numeric+string now considered concatenation.
is_string_concatenationtreats"a" + 1and1 + "a"as concatenations (andactioncan fold them when both sides resolve to constants). The top-of-file docs currently say “2 literals … strings or template literals”. Either:
- update docs to note numeric literals also fold into strings, or
- constrain detection to string/template on both sides.
Your call, but please align docs/tests with behaviour.
240-248: Unary numeric expressions are not recognised as pure numeric calculations.
is_numeric_calculationonly matches binary expressions whose children are numeric literals; cases like1 + +2or1 + -2won’t be considered “numeric calculation” here. If you intend to suppress diagnostics/fixes when the numeric side is a constant-but-unary expression, extend detection to simple unary+/-over number literals.Would you like a small follow-up to handle
JsUnaryExpressionwith+/-overJsNumberLiteralExpression?
275-304: Range start for parenthesised left-hand side.When the left is a parenthesised concatenation,
range_startuses the(token rather than the inner first string. That’s fine for highlighting, but it makes the underline a tad broader than needed. If you prefer tighter spans, consider extracting from within the parentheses similarly toextract_concat_range.
306-312: Stylistic check looks solid; consider CRLF/comments adjacency.
has_leading_newline()should handle CRLF, but cases like+ /* c */\n "bar"may place the newline on the comment rather than the operator/right token. If you want to exempt those too, you could treat “operator followed by only trivia up to a newline” as stylistic as well. Optional polish.If you have bandwidth, add a test with an inline comment between the operator and the right literal on the next line.
Also applies to: 332-332
118-180: Fix output: quote style preservation.
js_string_literal(...)may choose a default quote style. If we want stable diffs with the formatter, consider preserving the left-most string’s quote style where possible.Please confirm current output matches house style (single vs double quotes) after formatting.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/complexity/noUselessStringConcat/valid.js.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/quick-apples-count.md(1 hunks)crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs(6 hunks)crates/biome_js_analyze/tests/specs/complexity/noUselessStringConcat/valid.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period
Files:
.changeset/quick-apples-count.md
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rscrates/biome_js_analyze/tests/specs/complexity/noUselessStringConcat/valid.js
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rscrates/biome_js_analyze/tests/specs/complexity/noUselessStringConcat/valid.js
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_js_analyze/tests/specs/complexity/noUselessStringConcat/valid.js
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (1)
crates/biome_js_parser/src/syntax/stmt.rs (1)
parenthesized_expression(933-943)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: autofix
CodSpeed Performance ReportMerging #7874 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (3)
crates/biome_js_parser/src/syntax/stmt.rs (1)
parenthesized_expression(933-943)packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
TextRange(9327-9327)crates/biome_suppression/src/lib.rs (1)
range_start(512-527)
🪛 GitHub Actions: autofix.ci
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
[error] 277-303: mismatched types: expected Option<TextRange>, found (); the function likely returns Option<TextRange> but ends with an implicit () due to a stray semicolon. Remove the semicolon to return the value.
🪛 GitHub Actions: Benchmarks JS
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
[error] 277-303: mismatched types: get_concatenation_range() is declared to return Option but returns (). The log suggests removing a stray semicolon to return the value (e.g., remove the semicolon after the value).
🪛 GitHub Actions: Lint rule docs
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
[error] 277-277: mismatched types: expected Option<TextRange>, found () in get_concatenation_range. Implicitly returns (); likely a return/semicolon issue. See note at line 303 suggesting removing the semicolon to return the value.
🪛 GitHub Actions: Pull request
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
[error] 277-277: mismatched types: get_concatenation_range is expected to return Option but currently returns () (likely due to a stray semicolon). This caused the build in biome_js_analyze to fail during 'cargo documentation'.
🪛 GitHub Actions: Pull request Node.js
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
[error] 277-277: mismatched types: expected Option<TextRange>, found () in get_concatenation_range. The function implicitly returns () due to an empty body; consider returning Some(TextRange) or remove the trailing semicolon to return the value. (E0308)
[error] 303-303: help: remove this semicolon to return this value
[error] 277-277: Note: this is part of a compilation failure for biome_js_analyze (library). Fixing the above type mismatch should allow the build to proceed.
🔇 Additional comments (4)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (4)
21-71: LGTM! Clear documentation of multiline concatenation styles.The new examples effectively demonstrate both trailing and leading operator styles, aligning well with the fix for issue #7230.
183-212: LGTM! Comprehensive string concatenation detection.The refactored predicate correctly handles string literals, template literals, nested binary expressions, parenthesised forms, and numeric coercion cases. The inline examples in the match arms are helpful.
265-273: LGTM! Clean helper for parenthesised concatenations.The logic correctly unwraps parenthesised expressions and delegates to the existing predicate.
306-333: LGTM! Correctly detects leading-operator multiline concatenations.The addition of
has_newline_in_operatorproperly supports theoperatorLinebreak=beforestyle, addressing the core issue from #7230.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (2)
272-280: Consider usingis_string_concatenationfor more robust validation.
is_binary_expression_with_literal_stringchecks a narrow pattern (non-identifier left + string right). For parenthesised concatenations, you should verify the expression is a valid string concatenation usingis_string_concatenation, which handles all the cases including numeric coercion and nested concatenations.Apply this diff to improve the validation:
fn is_parenthesized_concatenation(expression: &Option<AnyJsExpression>) -> bool { if let Some(AnyJsExpression::JsParenthesizedExpression(parenthesized_expression)) = expression { - return is_binary_expression_with_literal_string( - &parenthesized_expression.expression().ok(), - ); + if let Ok(AnyJsExpression::JsBinaryExpression(binary_expr)) = parenthesized_expression.expression() { + return is_string_concatenation(&binary_expr); + } } false }
316-342: LGTM! The TODO comment is worth addressing in future work.The function correctly detects multiline concatenations with leading operators (
operatorLinebreak=before), which resolves issue #7230. The TODO at Line 317 raises a valid point about numeric first operands—consider tracking this as a follow-up if the current behaviour isn't ideal.Would you like me to open an issue to track the TODO about reviewing the behaviour when the first operand is numeric?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/quick-apples-count.md(1 hunks)crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs(7 hunks)crates/biome_js_analyze/tests/specs/complexity/noUselessStringConcat/valid.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_js_analyze/tests/specs/complexity/noUselessStringConcat/valid.js
- .changeset/quick-apples-count.md
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (2)
crates/biome_js_parser/src/syntax/stmt.rs (1)
parenthesized_expression(933-943)packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
TextRange(9327-9327)
🪛 GitHub Actions: Lint rule docs
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
[error] 102-102: E0308: mismatched types. expected &JsBinaryExpression, found &Option. This occurs in the condition using get_concatenation_range(&parent_binary_expression).is_some().
🪛 GitHub Actions: Pull request Node.js
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
[error] 102-102: cargo build failed during step 'cargo build -p biome_cli --release': mismatched types: expected &JsBinaryExpression, found &Option in get_concatenation_range (line 102). This occurs in crate biome_js_analyze during type checking (E0308).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: autofix
- GitHub Check: Check Dependencies
🔇 Additional comments (3)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (3)
191-219: LGTM!The predicate correctly identifies string concatenations, including cases where one operand is numeric (e.g.
1 + "a"or"a" + 1), which aligns with the enhanced rule behaviour described in the PR objectives.
284-311: LGTM!The stray semicolon issue from the previous review has been resolved. The function now correctly returns
Option<TextRange>without a type mismatch, and the logic properly verifies string concatenation before extracting the diagnostic range.
20-22: LGTM!The documentation updates clearly describe the enhanced behaviour, including numeric concatenation and the multiline leading-operator style. The examples align perfectly with the PR objectives and the code changes.
Also applies to: 34-36, 67-78
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (2)
23-23: Consider clarifying the multi-line exemption wording."Allowed" might imply the rule permits or encourages multi-line concatenations. Consider "ignored" or "not flagged" for clarity.
-/// Concatenation of multiple strings is allowed for multi-line strings (such as ones used to prevent exceeding the maximum line width). +/// Multi-line string concatenations are not flagged by this rule (such as ones used to prevent exceeding the maximum line width).
415-427: Simplify the control flow.The
else if let Some(c)on line 421 is redundant—the outerwhilealready ensurescurrent_node.is_some().fn get_parent_binary_expression(node: &JsBinaryExpression) -> Option<JsBinaryExpression> { let mut current_node = node.parent(); - while current_node.is_some() { - if let Some(AnyJsExpression::JsBinaryExpression(binary_expression)) = current_node { + while let Some(node) = current_node { + if let AnyJsExpression::JsBinaryExpression(binary_expression) = node { return Some(binary_expression); - } else if let Some(c) = current_node { - current_node = c.parent() } + current_node = node.parent(); } None }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs(7 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
🔇 Additional comments (8)
crates/biome_js_analyze/src/lint/complexity/no_useless_string_concat.rs (8)
96-108: LGTM!The suppression logic correctly prevents duplicate reports by checking the parent expression, ensuring only the outermost concatenation is flagged.
190-219: LGTM!The logic correctly identifies string concatenations including mixed string+number cases, whilst avoiding false positives on pure numeric operations.
272-280: LGTM!The new predicate correctly identifies parenthesised concatenations, matching the expanded scope of the rule.
282-311: LGTM!The function correctly checks for string concatenation before testing stylistic exemptions, and the range calculation properly handles nested expressions. This successfully addresses issue #7230 by detecting leading-operator multi-line concatenations.
313-342: LGTM!The operator newline check (line 318-320) correctly implements the fix for issue #7230. The TODO on line 317 is a reasonable flag for future review, though the current behaviour (treating numeric+string multi-line concatenations as stylistic) seems sensible.
344-400: LGTM!The expanded
extract_string_valuecorrectly handles number literals (lines 353-357), parenthesised expressions, and templates, enabling the fix action for mixed string+number concatenations.
402-413: LGTM!The recursive range extraction correctly identifies the leftmost operand in nested concatenations.
429-469: LGTM!The recursive concatenation builder correctly handles nested expressions, though its complexity is justified by the requirement to flatten multi-level concatenations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/quick-apples-count.md (1)
15-15: Clarify rule behaviour in code comment.The comment "the rule correctly flags this as a stylistic concatenation and ignores it" is slightly contradictory—"flags" typically implies reporting an error, whilst "ignores" implies not reporting. For clarity, consider rewording to "the rule now correctly recognises this as a stylistic concatenation and does not flag it" or similar.
Applied to line 15:
-// Now, the rule correctly flags this as a stylistic concatenation and ignores it. +// Now, the rule correctly recognises this as a stylistic concatenation and does not flag it.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/quick-apples-count.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period
Files:
.changeset/quick-apples-count.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Test Node.js API
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: autofix
…atenation operators from `operatorLinebreak=before` (#7874) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2 participants
Summary
Fixes #7230
For why this ended up as more than a simple 3-loc change:
I initially added a very simple boolean for the
+operator having leading whitespace insideis_stylistic_concatenation.I then realized that the function was being checked before the (woefully named)
is_concatenationactually checked that it was in fact a valid concatenation.I also moved around a bit of code so that
is_concatenation(now rechristenedget_concatenation_range) now callsis_stylistic_concatenationafter verifying that the operators involved are in fact ones that would otherwise trigger the rule.Note
Whether this was actually necessary for the rule to work is up for debate, but the prior code flow and naming weirdness was a code smell that I didn't really appreciate.
Also this would make it easier to resolve future issues/configurations with this stuff
(P.S: I could've denested more but chose not to due to time constraints/laziness).
Test Plan
I added a few tests to
valid.js(seen as all we're doing is adding more valid constructs, there's no need to editinvalid.js.Docs
Added a few extra
rustdocs for completeness' sake, but other than that nothing else much