From de3a24008f354c90c564f5d6ad0557ae847c31cb Mon Sep 17 00:00:00 2001 From: Ben Staveley-Taylor Date: Sat, 5 Jan 2019 14:04:50 +0000 Subject: [PATCH 1/3] Fix false positives on vertical_whitespace_between_cases Fixes #2538 --- .../VerticalWhitespaceBetweenCasesRule.swift | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/Style/VerticalWhitespaceBetweenCasesRule.swift b/Source/SwiftLintFramework/Rules/Style/VerticalWhitespaceBetweenCasesRule.swift index 45052c287..7aeb842f9 100644 --- a/Source/SwiftLintFramework/Rules/Style/VerticalWhitespaceBetweenCasesRule.swift +++ b/Source/SwiftLintFramework/Rules/Style/VerticalWhitespaceBetweenCasesRule.swift @@ -104,6 +104,28 @@ public struct VerticalWhitespaceBetweenCasesRule: ConfigurationProviderRule { ] private let pattern = "([^\\n{][ \\t]*\\n)([ \\t]*(?:case[^\\n]+|default):[ \\t]*\\n)" + + private func violationRanges(in file: File) -> [NSRange] { + return file.violatingRanges(for: pattern).filter { + !isFalsePositive(in: file, range: $0) + } + } + + private func isFalsePositive(in file: File, range: NSRange) -> Bool { + // Regex incorrectly flags blank lines that contain trailing whitespace (#2538) + let patternRegex = regex(pattern) + let substring = file.contents.substring(from: range.location, length: range.length) + let substringRange = NSRange(location: 0, length: substring.count) + guard let matchResult = patternRegex.firstMatch(in: substring, options: [], range: substringRange), + matchResult.numberOfRanges > 1 else { + return false + } + + let matchFirstRange = matchResult.range(at: 1) + let matchFirstString = substring.substring(from: matchFirstRange.location, length: matchFirstRange.length) + let isAllWhitespace = matchFirstString.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + return isAllWhitespace + } } extension VerticalWhitespaceBetweenCasesRule: OptInRule, AutomaticTestableRule { @@ -119,7 +141,7 @@ extension VerticalWhitespaceBetweenCasesRule: OptInRule, AutomaticTestableRule { public func validate(file: File) -> [StyleViolation] { let patternRegex = regex(pattern) - return file.violatingRanges(for: pattern).compactMap { violationRange in + return violationRanges(in: file).compactMap { violationRange in let substring = file.contents.substring(from: violationRange.location, length: violationRange.length) let substringRange = NSRange(location: 0, length: substring.count) guard let matchResult = patternRegex.firstMatch(in: substring, options: [], range: substringRange) else { @@ -140,7 +162,7 @@ extension VerticalWhitespaceBetweenCasesRule: OptInRule, AutomaticTestableRule { extension VerticalWhitespaceBetweenCasesRule: CorrectableRule { public func correct(file: File) -> [Correction] { - let violatingRanges = file.ruleEnabled(violatingRanges: file.violatingRanges(for: pattern), for: self) + let violatingRanges = file.ruleEnabled(violatingRanges: violationRanges(in: file), for: self) guard !violatingRanges.isEmpty else { return [] } let patternRegex = regex(pattern) From c4e01a3ec40b74f49a5b7c2c55d46d02eb22e93e Mon Sep 17 00:00:00 2001 From: Ben Staveley-Taylor Date: Sat, 5 Jan 2019 16:17:52 +0000 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 820df3e6a..afe91f62b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,11 @@ types. [Marcelo Fabri](https://github.com/marcelofabri) [#2539](https://github.com/realm/SwiftLint/issues/2539) + +* Fix false positives on `vertical_whitespace_between_cases` rule when a blank + line is present but it contains trailing whitespace. + [Ben Staveley-Taylor](https://github.com/BenStaveleyTaylor) + [#2538](https://github.com/realm/SwiftLint/issues/2538) ## 0.29.2: Washateria From 09c266613d8c179196af6ff4259082d960704527 Mon Sep 17 00:00:00 2001 From: Ben Staveley-Taylor Date: Sun, 6 Jan 2019 15:51:39 +0000 Subject: [PATCH 3/3] Implement review changes Add new nonTriggeringExample for testing, with trailing spaces --- Rules.md | 10 ++++++++++ .../VerticalWhitespaceBetweenCasesRule.swift | 16 ++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/Rules.md b/Rules.md index 12238eb92..95a18a672 100644 --- a/Rules.md +++ b/Rules.md @@ -22321,6 +22321,16 @@ default: print("x is invalid") } ``` +```swift +switch x { +case 1: + print("one") + +default: + print("not one") +} +``` +
Triggering Examples diff --git a/Source/SwiftLintFramework/Rules/Style/VerticalWhitespaceBetweenCasesRule.swift b/Source/SwiftLintFramework/Rules/Style/VerticalWhitespaceBetweenCasesRule.swift index 7aeb842f9..9c46b0a2a 100644 --- a/Source/SwiftLintFramework/Rules/Style/VerticalWhitespaceBetweenCasesRule.swift +++ b/Source/SwiftLintFramework/Rules/Style/VerticalWhitespaceBetweenCasesRule.swift @@ -46,6 +46,15 @@ public struct VerticalWhitespaceBetweenCasesRule: ConfigurationProviderRule { default: print("x is invalid") } """ + , + // Testing handling of trailing spaces: do not convert to """ style + "switch x { \n" + + "case 1: \n" + + " print(\"one\") \n" + + " \n" + + "default: \n" + + " print(\"not one\") \n" + + "} " ] private static let violatingToValidExamples: [String: String] = [ @@ -115,8 +124,7 @@ public struct VerticalWhitespaceBetweenCasesRule: ConfigurationProviderRule { // Regex incorrectly flags blank lines that contain trailing whitespace (#2538) let patternRegex = regex(pattern) let substring = file.contents.substring(from: range.location, length: range.length) - let substringRange = NSRange(location: 0, length: substring.count) - guard let matchResult = patternRegex.firstMatch(in: substring, options: [], range: substringRange), + guard let matchResult = patternRegex.firstMatch(in: substring, options: [], range: substring.fullNSRange), matchResult.numberOfRanges > 1 else { return false } @@ -143,8 +151,8 @@ extension VerticalWhitespaceBetweenCasesRule: OptInRule, AutomaticTestableRule { let patternRegex = regex(pattern) return violationRanges(in: file).compactMap { violationRange in let substring = file.contents.substring(from: violationRange.location, length: violationRange.length) - let substringRange = NSRange(location: 0, length: substring.count) - guard let matchResult = patternRegex.firstMatch(in: substring, options: [], range: substringRange) else { + guard let matchResult = patternRegex.firstMatch(in: substring, options: [], + range: substring.fullNSRange) else { return nil }