-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Vertical parameter alignment rule #1035
Vertical parameter alignment rule #1035
Conversation
Current coverage is 82.79% (diff: 97.29%)@@ master #1035 diff @@
==========================================
Files 141 142 +1
Lines 6891 6927 +36
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 5700 5735 +35
- Misses 1191 1192 +1
Partials 0 0
|
// VerticalParameterAlignmentRule.swift | ||
// SwiftLint | ||
// | ||
// Created by Marcelo Fabri on 22/12/16. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
πΊπΈ π π₯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
obviously π
Some minor ideas you may or may not want to incorporate: diff --git a/Source/SwiftLintFramework/Rules/VerticalParameterAlignmentRule.swift b/Source/SwiftLintFramework/Rules/VerticalParameterAlignmentRule.swift
index f9f6dc3..6484c90 100644
--- a/Source/SwiftLintFramework/Rules/VerticalParameterAlignmentRule.swift
+++ b/Source/SwiftLintFramework/Rules/VerticalParameterAlignmentRule.swift
@@ -59,17 +59,15 @@ public struct VerticalParameterAlignmentRule: ASTRule, ConfigurationProviderRule
let paramStart = regex(pattern).firstMatch(in: file.contents,
options: [], range: nameRange)?.rangeAt(1).location,
let (startLine, startCharacter) = contents.lineAndCharacter(forCharacterOffset: paramStart),
- let (endLine, _) = contents.lineAndCharacter(forByteOffset: nameOffset + nameLength - 1),
+ let endLine = contents.lineAndCharacter(forByteOffset: nameOffset + nameLength - 1)?.0,
endLine > startLine else {
return []
}
- let linesRange = (startLine + 1)...endLine
- let violationLocations = linesRange.flatMap { lineIndex -> Int? in
- let line = file.lines[lineIndex - 1]
+ let violationLocations = (startLine..<endLine).flatMap { lineIndex -> Int? in
guard let paramLocation = regex("\\S").firstMatch(in: file.contents, options: [],
- range: line.range)?.range.location,
- let (_, paramCharacter) = contents.lineAndCharacter(forCharacterOffset: paramLocation),
+ range: file.lines[lineIndex].range)?.range.location,
+ let paramCharacter = contents.lineAndCharacter(forCharacterOffset: paramLocation)?.1,
paramCharacter != startCharacter else {
return nil
} |
I actually prefer the current style if it's ok. Even if it's smarter because it avoids the |
It's absolutely ok! My sample basically abuses the fact that line numbers are 1-indexed. |
If you end up closing #1033 after merging this, make sure you open a new issue to track improving this rule by also checking function calls. |
Fixes #1033
This isn't correctable yet because I wasn't sure if it'd be ok to use spaces to correct the indentation. I heard some people use tabs π€·ββοΈ
Also #1033 mentions checking function calls, but I think it's better to do it later as it's much more complex IMO.