Draft: [New rule] Space with guard #1801
Conversation
Sources/Rules.swift
Outdated
| } | ||
| } | ||
|
|
||
| public let spacesWithGuard = FormatRule(help: "Remove space between guard and add spaces after last guard.", |
There was a problem hiding this comment.
A name like guardStatementSpacing or guardSpacing could be nice -- we have an existing rule named consistentSwitchCaseSpacing, which is somewhat similar. What do you think?
There was a problem hiding this comment.
Renamed to guardSpacing
| @@ -0,0 +1,156 @@ | |||
| // Created by @NikeKov on 27.07.2024 | |||
There was a problem hiding this comment.
As of #1789 this file should be Tests/Rules/SpaceWithGuardTests.swift (or whatever we end up naming the rule)
There was a problem hiding this comment.
Added
// GuardSpacingTests.swift
// GuardSpacingTests
//
Tests/RulesTests+GuardSpacing.swift
Outdated
| import XCTest | ||
| @testable import SwiftFormat | ||
|
|
||
| class GuardSpacingTests: RulesTests { |
There was a problem hiding this comment.
The name of the test class should match the rule name
| class GuardSpacingTests: RulesTests { | |
| class SpaceWithGuardTests: RulesTests { |
Sources/Rules.swift
Outdated
| public let spacesWithGuard = FormatRule(help: "Remove space between guard and add spaces after last guard.", | ||
| disabledByDefault: true) | ||
| { formatter in | ||
| func leaveOrSetLinebreaksInIndexes(_ indexes: Set<Int>, linebreaksCount: Int) { |
There was a problem hiding this comment.
When you move this rule to its own file (e.g. Sources/Rules/SpacesWithGuard.swift, it would probably be nicer to move this func leaveOrSetLinebreaksInIndexes to be in a separate extension Formatter at the bottom of the file, rather than being nested within the rule implementation closure.
Sources/Rules.swift
Outdated
| let guardKeyword = Token.keyword("guard") | ||
| formatter.forEach(guardKeyword) { guardIndex, _ in |
There was a problem hiding this comment.
I think it's more common to just do:
| let guardKeyword = Token.keyword("guard") | |
| formatter.forEach(guardKeyword) { guardIndex, _ in | |
| formatter.forEach(.keyword("guard")) { guardIndex, _ in |
Sources/Rules.swift
Outdated
|
|
||
| let guardKeyword = Token.keyword("guard") | ||
| formatter.forEach(guardKeyword) { guardIndex, _ in | ||
| print("✳️ start \(guardIndex)") |
There was a problem hiding this comment.
Remember to clean up these print statements before merging!
Sources/Rules.swift
Outdated
| switch nextNonSpaceAndNonLinebreakToken { | ||
| case let .endOfScope(string): | ||
| if string == "}" { | ||
| // Do not add space for end bracket | ||
| return | ||
| } | ||
| default: | ||
| break | ||
| } |
There was a problem hiding this comment.
A simpler way to write this would be:
| switch nextNonSpaceAndNonLinebreakToken { | |
| case let .endOfScope(string): | |
| if string == "}" { | |
| // Do not add space for end bracket | |
| return | |
| } | |
| default: | |
| break | |
| } | |
| if nextNonSpaceAndNonLinebreakToken == .endOfScope("}") { | |
| // Do not add space for end bracket | |
| return | |
| } |
Sources/Rules.swift
Outdated
| if alreadyHasLinebreaksCount != linebreaksCount, | ||
| let firstIndex = indexes.first | ||
| { | ||
| formatter.insert(.linebreak("\n", 0), at: firstIndex) |
There was a problem hiding this comment.
This should be:
| formatter.insert(.linebreak("\n", 0), at: firstIndex) | |
| formatter.insertLinebreak(at: firstIndex) |
calda
left a comment
calda
left a comment
There was a problem hiding this comment.
Looks good! I'll take another look after the merge conflicts are resolved.
Rules.md
Outdated
| * [wrapEnumCases](#wrapEnumCases) | ||
| * [wrapMultilineConditionalAssignment](#wrapMultilineConditionalAssignment) | ||
| * [wrapSwitchCases](#wrapSwitchCases) | ||
| * [spaceWithGuard](#spaceWithGuard) |
There was a problem hiding this comment.
Did you modify the Rules.md file by hand?
This file is generated by MetadataTests.testGenerateRulesDocumentation(), which is currently failing on this branch. You can run the test case and commit the result, then the test should pass.
There was a problem hiding this comment.
Yes, I did it by myself. How is needed to be?
There was a problem hiding this comment.
Oh, see in next thread. I'll change it.
Rules.md
Outdated
| <details> | ||
| <summary>Examples</summary> | ||
|
|
||
| ```diff |
There was a problem hiding this comment.
You'll need to put this example in Examples.swift, which is used when generating the Rules.md file
3 participants
Blocked by #1800