Skip to content

SA1143: Allow multiline conditions - #4374

Merged
AArnott merged 3 commits into
masterfrom
aarnott-issue-4367-sa1143-conditional-expressions-must-be-f-5ea37a
Oct 21, 2026
Merged

AArnott merged 3 commits into
masterfrom
aarnott-issue-4367-sa1143-conditional-expressions-must-be-f-5ea37a

Conversation

@AArnott

@AArnott AArnott commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Fixes: #4367

SA1143 currently rejects readable expanded conditional expressions when the condition spans multiple lines. Allow multiline conditions in the expanded layout, requiring the ? and : lines to align and be indented beyond the condition's final line. The expanded code fix continues to produce a valid layout by flattening the condition onto one line.

Added regression coverage for the reported layout, the expanded fix, and the existing analyzer tests' invalid indentation case. The regression test fails on master with SA1143 at the ? token. Updated the English and localized SA1143 documentation.

The analyzer's registration is unchanged; the existing conditional-expression callback now uses the condition's last token to determine its indentation. No additional callbacks or work for files without conditional expressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@AArnott
AArnott requested review from bjornhellander and a balanced review from Copilot October 21, 2026 18:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Conditions ending in multiline literals use the opening line’s indentation instead of the final line’s indentation.

1 open issue
What changed in this PR

Updates SA1143 to allow multiline conditions in expanded conditional expressions.

Changes:

  • Checks operator indentation against the condition’s last token.
  • Adds C# 7 analyzer and expanded-fix regression tests.
  • Updates English and localized documentation.
File Description
test/​StyleCop.Analyzers.Test.CSharp7/​ReadabilityRules/​SA1143CSharp7UnitTests.cs Adds multiline-condition regression tests.
src/​StyleCop.Analyzers/​ReadabilityRules/​SA1143ConditionalExpressionsMustBeFormattedConsistently.cs Relaxes expanded-layout validation.
documentation/​zh-cn/​SA1143.md Updates Chinese guidance and example.
documentation/​SA1143.md Documents multiline conditions and indentation.
documentation/​ru-ru/​SA1143.md Updates Russian guidance and example.
documentation/​cs-cz/​SA1143.md Synchronizes the Czech documentation page.

Review effort: Balanced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 18:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Move the C# 6-compatible regression theory into the base test project to cover the oldest supported compiler.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread test/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1143CSharp7UnitTests.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 18:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Nested false-branch conditions can bypass the enclosing branch’s continuation-indentation requirement.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Nested false branches bypass continuation indentation checks

src/​StyleCop.Analyzers/​ReadabilityRules/​SA1143ConditionalExpressionsMustBeFormattedConsistently.cs:100

Allowing multiline conditions also lets nested false branches bypass the enclosing branch's continuation-indentation check. For example, this layout is now accepted:

var x = firstCondition
    ? first
    : left &&
    right
        ? second
        : third;

right is aligned with the outer :, violating the continuation rule documented in documentation/SA1143.md:41–42. IsExpanded advances to next without checking its condition against the outer operator column, and the nested expression's callback is skipped. Validate the nested condition's continuation indentation before advancing, and require its operators to remain deeper than the enclosing operators. Add a regression test for this layout and a correctly indented counterpart.

Review effort: Balanced

@codecov

codecov Bot commented Oct 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.63%. Comparing base (6a9040d) to head (af7d037).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4374   +/-   ##
=======================================
  Coverage   97.63%   97.63%           
=======================================
  Files        1344     1345    +1     
  Lines      151640   151674   +34     
  Branches     5215     5216    +1     
=======================================
+ Hits       148048   148082   +34     
  Misses       2268     2268           
  Partials     1324     1324           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bjornhellander

Copy link
Copy Markdown
Contributor

Allowing multi-line conditions seems fine. It allows for some very unreadable code if misused, but forbidding it seem overly strict. Thumbs up on that one.

The other part about comparing indentation to the condition's last line is something that hasn't clicked for me yet. I would need to dig into that more, and I don't think I will manage that this week.

@AArnott
AArnott merged commit 1313b16 into master Oct 21, 2026
30 checks passed
@AArnott
AArnott deleted the aarnott-issue-4367-sa1143-conditional-expressions-must-be-f-5ea37a branch October 21, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SA1143 (Conditional expressions must be formatted consistently) false positive?

3 participants