SwiftLint
SwiftLint copied to clipboard
realm
Add new `attribute_name_spacing` rule
Resolves https://github.com/realm/SwiftLint/issues/5667
Seeing as private (set) would cause issues with the Swift 6 compiler in Xcode 16+, this PR implements the changes discussed in the issue linked above.
The rule is now a default rule and prevents the following code from triggering errors:
Triggers:
private (set) var type: ClipType = .undetermined
Doesn't Trigger:
private(set) var type: ClipType = .undetermined
First time contributing to SwiftLint, so apologies for any contribution mistakes on my end. You managed to convince me to try and contribute @SimplyDanny (re. SwiftCraft)
| 17 Messages | |
|---|---|
| :book: | Linting Aerial with this PR took 0.91s vs 0.92s on main (1% faster) |
| :book: | Linting Alamofire with this PR took 1.27s vs 1.26s on main (0% slower) |
| :book: | Linting Brave with this PR took 7.43s vs 7.39s on main (0% slower) |
| :book: | Linting DuckDuckGo with this PR took 4.86s vs 4.87s on main (0% faster) |
| :book: | Linting Firefox with this PR took 10.95s vs 10.93s on main (0% slower) |
| :book: | Linting Kickstarter with this PR took 9.99s vs 9.96s on main (0% slower) |
| :book: | Linting Moya with this PR took 0.53s vs 0.53s on main (0% slower) |
| :book: | Linting NetNewsWire with this PR took 2.54s vs 2.54s on main (0% slower) |
| :book: | Linting Nimble with this PR took 0.77s vs 0.76s on main (1% slower) |
| :book: | Linting PocketCasts with this PR took 8.57s vs 8.53s on main (0% slower) |
| :book: | Linting Quick with this PR took 0.45s vs 0.43s on main (4% slower) |
| :book: | Linting Realm with this PR took 4.65s vs 4.64s on main (0% slower) |
| :book: | Linting Sourcery with this PR took 2.33s vs 2.35s on main (0% faster) |
| :book: | Linting Swift with this PR took 4.55s vs 4.55s on main (0% slower) |
| :book: | Linting VLC with this PR took 1.24s vs 1.25s on main (0% faster) |
| :book: | Linting Wire with this PR took 18.15s vs 18.1s on main (0% slower) |
| :book: | Linting WordPress with this PR took 11.73s vs 11.76s on main (0% faster) |
Generated by :no_entry_sign: Danger
Hey @aryamansharda, nice to hear from you and even better via a contribution!
The implementation looks good to me. Yet, checking the commits that introduced this warning/error in the Swift compiler and SwiftSyntax, I just found out that <access-control-modifier> (set) is only one special case. The warning/error is actually more general and applies to all modifiers and attributes likewise, e.g. @available (*, deprecated) or nonisolated (unsafe).
For the attribute case, we could also check that there is no trivia between the @ sign and the attribute name as this is another related compiler diagnostic. It also seems that while
let closure1 = { @MainActor (a, b) in
}
leads to the warning/error,
let closure2 = { @MainActor
(a, b) in
}
does not. In other words, a line break serves as a disambiguator.
I hope this isn't too much to ask for, but it seems that only supporting all these cases makes this rule a real help in preparation for Swift 6. I'm happy to help in case of any issues. 😉
Sounds good @SimplyDanny! I'll explore that today - just wanted to double-check my understanding before I continue.
First, I should should update my existing implementation to check that there is no trivia between any DeclModifier and DeclModifierDetail which would handle cases like (set) and nonisolated(unsafe)
Next, based off the examples in the commit you referenced:
struct PropertyWrapperTest {
@MyPropertyWrapper (param: 2) // expected-warning {{extraneous whitespace between attribute name and '('; this is an error in Swift 6}}
var x: Int
}
let closure1 = { @MainActor (a, b) in // expected-warning {{extraneous whitespace between attribute name and '('; this is an error in Swift 6}}
}
let closure2 = { @MainActor
(a: Int, b: Int) in
}
When I plug them into the Swift AST Explorer, I see that AttributeList and Attribute are the type we're after.
So, in addition to the existing check above, we want to add a new check for all AttributeList->Attribute->IdentifierType to ensure that there is no leading (ex. @ MainActor / @ escaping) or trailing trivia (ex. @MyPropertyWrapper (param: 2) / @MainActor (a, b))?
Yes, you mentioned all the cases we can find in the related Swift compiler diagnostics.
Not sure if this will work or not, but you might be able to avoid the Rewriter implementation by doing something like ImplicitReturnRule does with SwiftSyntaxCorrectableRule.
@mildm8nnered Ah, very cool. Just updated the implementation to use that 👍
Hey @aryamansharda, anything that's blocking this PR? I'd like to include it into the next major release. Swift 6 is not too far off. To drive this rule useable, we shouldn't wait for too long.
Hey, sorry for the delay! I'll make the updates in the next day or two!
@SimplyDanny Sorry for the delay, but I've made the requested changes. If there's any other changes or documentation changes let me know - I promise the following turn around time will be much faster.
Thanks @aryamansharda! I only made the wording more serious to better match the error severity and the fact that these violation will be compiler errors in the future.
Once the build passes, this is ready to be merged.