-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add SwiftLint rule to prevent multiple variable declarations on one line #5990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 11 commits
ee27243
2a712e0
d542734
3d185cd
8999cf3
ec21694
92b6a81
f866597
51b29cb
70f41fa
47ff674
fe48a67
1c7565d
7087a64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,10 @@ | |
|
||
### Enhancements | ||
|
||
* Add `multiple_statements_declaration` opt-in rule | ||
Check failure on line 15 in CHANGELOG.md
|
||
that triggers when the there's multiple statements at the same line. | ||
AhmedZaghloul19 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
[Ahmad Zaghloul](https://github.com/AhmedZaghloul19) | ||
|
||
* Exclude types with a `@Suite` attribute and functions annotated with `@Test` from `no_magic_numbers` rule. | ||
Also treat a type as a `@Suite` if it contains `@Test` functions. | ||
[SimplyDanny](https://github.com/SimplyDanny) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import SwiftSyntax | ||
|
||
@SwiftSyntaxRule(explicitRewriter: true, optIn: true) | ||
struct MultipleStatementsDeclarationRule: Rule { | ||
var configuration = SeverityConfiguration<Self>(.warning) | ||
|
||
static let description = RuleDescription( | ||
identifier: "multiple_statements_declaration", | ||
name: "Multiple Statements Declaration", | ||
description: "Statements should not be on the same line", | ||
AhmedZaghloul19 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
kind: .idiomatic, | ||
nonTriggeringExamples: [ | ||
Example("let a = 1;\nlet b = 2;"), | ||
AhmedZaghloul19 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
Example("var x = 10\nvar y = 20;"), | ||
Example("let a = 1;\nreturn a"), | ||
Example("if b { return }; \nlet a = 1"), | ||
], | ||
triggeringExamples: [ | ||
Example("let a = 1; return a"), | ||
Example("if b { return }; let a = 1"), | ||
Example("if b { return }; if c { return }"), | ||
Example("let a = 1; let b = 2; let c = 0"), | ||
Example("var x = 10; var y = 20"), | ||
Example("let x = 10; var y = 20"), | ||
], | ||
corrections: [ | ||
Example("let a = 0↓; let b = 0"): Example("let a = 0\nlet b = 0"), | ||
Example("let a = 0↓; let b = 0↓; let c = 0"): | ||
Example("let a = 0\nlet b = 0\nlet c = 0"), | ||
Example("let a = 0↓; print(\"Hello\")"): Example("let a = 0\nprint(\"Hello\")"), | ||
] | ||
) | ||
} | ||
|
||
private extension MultipleStatementsDeclarationRule { | ||
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> { | ||
override func visitPost(_ node: TokenSyntax) { | ||
if node.isThereStatementAfterSemicolon { | ||
violations.append(node.positionAfterSkippingLeadingTrivia) | ||
} | ||
} | ||
} | ||
|
||
final class Rewriter: ViolationsSyntaxRewriter<ConfigurationType> { | ||
override func visit(_ node: TokenSyntax) -> TokenSyntax { | ||
guard node.isThereStatementAfterSemicolon else { | ||
return super.visit(node) | ||
} | ||
|
||
correctionPositions.append(node.positionAfterSkippingLeadingTrivia) | ||
let newNode = TokenSyntax( | ||
.unknown(""), | ||
leadingTrivia: node.leadingTrivia, | ||
trailingTrivia: .newlines(1), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't indent the following statement. Consider the example {
let a = 0; let b = 0
} which should be corrected to {
let a = 0;
let b = 0
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The autocorrect version is
as we're removing the semicolon, as it's swift code styling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. My bad. Except this example wouldn't be formatted like that with the current implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SimplyDanny So, do i have to do anything else for this rule except fixing the conflict? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The examples isn't formatted like you suggest. That needs to be fixed. |
||
presence: .present | ||
) | ||
return super.visit(newNode) | ||
} | ||
} | ||
} | ||
|
||
private extension TokenSyntax { | ||
var isThereStatementAfterSemicolon: Bool { | ||
guard tokenKind == .semicolon, | ||
!trailingTrivia.isEmpty else { return false } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't comprehend this condition. Why is no trivia after the semicolon allowed? I think an example like |
||
|
||
if let nextToken = nextToken(viewMode: .sourceAccurate), | ||
isFollowedOnlyByWhitespaceOrNewline { | ||
return nextToken.leadingTrivia.containsNewlines() == false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think what you actually want to know is whether the next token is on the same line as the semicolon. You can use |
||
} | ||
return true | ||
} | ||
|
||
var isFollowedOnlyByWhitespaceOrNewline: Bool { | ||
guard let nextToken = nextToken(viewMode: .sourceAccurate), | ||
!nextToken.trailingTrivia.isEmpty else { | ||
return true | ||
} | ||
return nextToken.leadingTrivia.allSatisfy { $0.isWhitespaceOrNewline } | ||
} | ||
} | ||
|
||
private extension TriviaPiece { | ||
var isWhitespaceOrNewline: Bool { | ||
switch self { | ||
case .spaces, .tabs, .newlines, .carriageReturns, .carriageReturnLineFeeds: | ||
return true | ||
default: | ||
return false | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,7 +86,10 @@ internal extension Configuration.FileGraph.FilePath { | |
// Block main thread until timeout is reached / task is done | ||
while true { | ||
if taskDone { break } | ||
if Date().timeIntervalSince(startDate) > timeout { task.cancel(); break } | ||
if Date().timeIntervalSince(startDate) > timeout { | ||
task.cancel() | ||
break | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't look like it got auto-fixed by the rule. The only cases that can be fixed automatically are the ones where the statements are on their own line without leading tokens. We should probably restrict the rewriter to these cases. |
||
usleep(50_000) // Sleep for 50 ms | ||
} | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.