Skip to content

Avoid dropping inline comments in control_statement fixes - #6545

Open
theamodhshetty wants to merge 1 commit into
realm:mainfrom
theamodhshetty:codex/fix-control-statement-inline-comments
Open

theamodhshetty wants to merge 1 commit into
realm:mainfrom
theamodhshetty:codex/fix-control-statement-inline-comments

Conversation

@theamodhshetty

@theamodhshetty theamodhshetty commented Mar 18, 2026 •

Copy link
Copy Markdown
Contributor

control_statement autocorrection could delete comments while removing parentheses, or drop a line-comment newline so that a following brace/else became part of the comment.

This preserves comments before and after conditions, around statement keywords and parentheses, and at the end of a condition list. It also visits nested statements and else if branches during correction. Parentheses are still removed rather than skipping commented cases.

Correction coverage now includes if, guard, while, switch, and catch; block and line comments; multiple conditions; and nested statements. The branch is rebased onto current main, uses current example macros, and places the changelog entry under Main with the requested Markdown line break.

Validation: swift test --filter ControlStatementRuleGeneratedTests passes. Both affected rule suites also pass in a temporary combined checkout (swift test --filter 'ControlStatementRuleGeneratedTests|VerticalParameterAlignmentOnCallRuleGeneratedTests'). Strict lint passes, and a manual swiftlint lint --fix --only-rule control_statement reproduction preserves each comment and the newline before else. Whitespace checks pass with Markdown hard-break trailing spaces allowed. Full platform validation remains with CI.

Closes #6207.

@SwiftLintBot

SwiftLintBot commented Mar 18, 2026 •

Copy link
Copy Markdown
1 Warning
⚠️ This PR may need tests.
20 Messages
📖 Building this branch resulted in a binary size of 28772.41 KiB vs 28755.54 KiB when built on main (0% larger).
📖 Linting Aerial with this PR took 0.1 s vs 0.1 s on main (0% slower).
📖 Linting Alamofire with this PR took 0.11 s vs 0.11 s on main (0% slower).
📖 Linting Brave with this PR took 0.37 s vs 0.35 s on main (5% slower).
📖 Linting Brigade with this PR took 18.51 s vs 18.66 s on main (0% faster).
📖 Linting DuckDuckGo with this PR took 2.0 s vs 1.99 s on main (0% slower).
📖 Linting Firefox with this PR took 0.72 s vs 0.72 s on main (0% slower).
📖 Linting Kickstarter with this PR took 0.45 s vs 0.44 s on main (2% slower).
📖 Linting Moya with this PR took 0.09 s vs 0.09 s on main (0% slower).
📖 Linting NetNewsWire with this PR took 0.2 s vs 0.18 s on main (11% slower).
📖 Linting Nimble with this PR took 0.11 s vs 0.09 s on main (22% slower).
📖 Linting PocketCasts with this PR took 0.5 s vs 0.49 s on main (2% slower).
📖 Linting Quick with this PR took 0.09 s vs 0.09 s on main (0% slower).
📖 Linting Realm with this PR took 0.19 s vs 0.2 s on main (5% faster).
📖 Linting Sourcery with this PR took 0.14 s vs 0.14 s on main (0% slower).
📖 Linting Swift with this PR took 0.22 s vs 0.21 s on main (4% slower).
📖 Linting SwiftLintPerformanceTests with this PR took 3.81 s vs 3.79 s on main (0% slower).
📖 Linting VLC with this PR took 0.18 s vs 0.18 s on main (0% slower).
📖 Linting Wire with this PR took 0.75 s vs 0.72 s on main (4% slower).
📖 Linting WordPress with this PR took 0.57 s vs 0.55 s on main (3% slower).

Generated by 🚫 Danger

@SimplyDanny

Copy link
Copy Markdown
Collaborator

Why not remove the parentheses while keeping the comment as is?

@theamodhshetty

Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense. I pushed ba5b107 so the correction removes the outer parentheses and keeps inline comments in place instead of skipping those cases.

I also checked the same trivia path for guard, while, and switch locally so the fix does not only work for the added if example.

Validation rerun:

  • swift test --filter ControlStatementRuleGeneratedTests
  • ./.build/debug/swiftlint lint Source/SwiftLintBuiltInRules/Rules/Style/ControlStatementRule.swift CHANGELOG.md --strict

@SimplyDanny SimplyDanny left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's have a few more test cases. Quite a few cases are currently not covered.

Comment thread CHANGELOG.md Resolved
@theamodhshetty
theamodhshetty force-pushed the codex/fix-control-statement-inline-comments branch from ba5b107 to 7a59172 Compare October 1, 2026 22:03
@theamodhshetty

Copy link
Copy Markdown
Contributor Author

@SimplyDanny Could you re-review this when you have time? Head 7a591722 addresses the requested correction coverage across if, guard, while, switch, and catch, including leading/trailing comments, line-comment newlines, multiple conditions, and nested statements. The requested changelog line break is included, the recorded review threads are resolved, and the current head has 20 successful checks with Deploy skipped. Please let me know if anything else needs adjusting. Thanks!

This branch has not been deployed

No deployments
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.

control_statement kills inline comments

3 participants