Skip to content

Preserve breaking conditional statement#1222

Open
matthewnessworthy wants to merge 5 commits intocarthage-software:mainfrom
matthewnessworthy:preserve-breaking-conditional-statement
Open

Preserve breaking conditional statement#1222
matthewnessworthy wants to merge 5 commits intocarthage-software:mainfrom
matthewnessworthy:preserve-breaking-conditional-statement

Conversation

@matthewnessworthy
Copy link

@matthewnessworthy matthewnessworthy commented Feb 26, 2026

📌 What Does This PR Do?

Adds a new preserve_breaking_condition_statement formatter setting that preserves multi-line
condition layouts in control structures (if, elseif, while, do-while, switch, match)
following PER Coding Style 3.0 rules.

🔍 Context & Motivation

The formatter already has preserve_breaking_conditional_expression for ternary expressions, but
there's no equivalent for control structure conditions. When a developer intentionally breaks a
complex condition across multiple lines for readability, the formatter should respect that layout
when this setting is enabled — placing each boolean operator on its own line and the closing
parenthesis on its own line.

php-fig.org/per/coding-style#51-if-elseif-else

Expressions in parentheses MAY be split across multiple lines, where each subsequent line is indented at least once. When doing so, the first condition MUST be on the next line. The closing parenthesis and opening brace MUST be placed together on their own line with one space between them. Boolean operators between conditions MUST always be at the beginning.

🛠️ Summary of Changes

  • Feature: Added preserve_breaking_condition_statement bool field to FormatSettings and
    RawFormatSettings via the generate_formatter_settings! macro, defaulting to false.
  • Feature: Set preset values — Pint, Tempest, Drupal = true; Default, PSR-12, Hack = false.
  • Feature: Implemented must_break detection in print_condition() using
    has_new_line_in_range between the opening parenthesis and the condition.
  • Feature: Added must_break_condition flag to FormatterState, threaded through to
    binaryish.rs to force hard line breaks before binary operators when inside a must-break condition.

📂 Affected Areas

  • Linter
  • Formatter
  • CLI
  • Dependencies
  • Documentation
  • Other (please specify):

🔗 Related Issues or PRs

Related to #869

📝 Notes for Reviewers

When enabled, a multi-line condition like:

if (
    $foo && $bar
) {
becomes:
if (
    $foo
    && $bar
) {

Single-line conditions are never forced to break. When disabled (the default), existing behavior is
unchanged — all 366 existing tests pass with zero regressions.

Copy link
Member

@azjezz azjezz left a comment

Choose a reason for hiding this comment

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

LGTM. But before we can merge, please remove Claude from git history.

I don't mind using Claude, but it should never be in commit history for git blame :)

@azjezz azjezz added c-formatter PHP code reformatting and style rules t-enhancement New feature or request labels Feb 26, 2026
@matthewnessworthy
Copy link
Author

@azjezz sure thing, I'll make the changes soon.
Thanks for the review and being accepting of the change

@matthewnessworthy matthewnessworthy force-pushed the preserve-breaking-conditional-statement branch from 2c3b0c2 to de924db Compare February 26, 2026 13:28
@azjezz
Copy link
Member

azjezz commented Feb 26, 2026

@matthewnessworthy you should use just fix, it will fix formatting and clippy issue

@matthewnessworthy matthewnessworthy force-pushed the preserve-breaking-conditional-statement branch 2 times, most recently from 1422fe6 to 0c5198b Compare February 27, 2026 13:30
@matthewnessworthy
Copy link
Author

@azjezz i ran just fix, but it changed a couple of extra files, please let me know if that's fine, or if i should revert that commit in favour of running cargo fix

Add a new bool field `preserve_breaking_condition_statement` to the
`generate_formatter_settings!` macro in settings.rs, with `default_false`.
This generates the field in both `FormatSettings` and `RawFormatSettings`.

Set correct values in all 6 presets:
- Default=false, PSR-12=false, Hack=false (no preservation)
- Pint=true, Tempest=true, Drupal=true (preserve multi-line conditions)

No behavioral change — the field is declared but not yet consumed.
When `preserve_breaking_condition_statement` is enabled and a condition
was originally broken across multiple lines, force PER Coding Style 3.0
output: first condition on indented next line, closing paren on own line,
and each boolean operator on its own line.

Changes to print_condition() in misc.rs:
- Compute `must_break` from setting + `has_new_line_in_range` detection
- Guard expandable expression branch with `&& !must_break`
- Replace soft line breaks with conditional hard/soft based on must_break
- Add `.with_break(must_break)` to force group breaking
- Thread `must_break_condition` flag through FormatterState

Changes to binaryish.rs:
- When `must_break_condition` is set, force hard line breaks before
  binary operators so each condition appears on its own line

Automatically covers all 7 call sites (if, elseif, while, do-while,
switch, match) without touching any call-site code.
Add enabled/disabled test fixture pairs:

Enabled test covers all 6 control structures (if, elseif, while,
do-while, switch, match) with multi-line conditions producing PER-style
output where each boolean operator appears on its own line. Single-line
conditions remain untouched.

Disabled test confirms the formatter collapses multi-line conditions
when the setting is off.

Idempotency is verified automatically by the test_case! macro.
@matthewnessworthy matthewnessworthy force-pushed the preserve-breaking-conditional-statement branch from 0c5198b to e7c1a8f Compare March 2, 2026 15:57
@matthewnessworthy
Copy link
Author

@azjezz rebased, formatted, and conflicts fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c-formatter PHP code reformatting and style rules t-enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants