From ded37c3d5b80fbe2e6ce02d2c5fd1091373101d2 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 17 Jan 2019 23:42:29 +0000 Subject: [PATCH] Add `DuplicateImportsRule` (#2004) --- CHANGELOG.md | 4 +- Rules.md | 202 ++++++++++++++++++ .../Models/MasterRuleList.swift | 1 + .../Idiomatic/DuplicateImportsRule.swift | 107 ++++++++++ .../DuplicateImportsRuleExamples.swift | 56 +++++ SwiftLint.xcodeproj/project.pbxproj | 8 + Tests/LinuxMain.swift | 7 + .../AutomaticRuleTests.generated.swift | 6 + 8 files changed, 390 insertions(+), 1 deletion(-) create mode 100644 Source/SwiftLintFramework/Rules/Idiomatic/DuplicateImportsRule.swift create mode 100644 Source/SwiftLintFramework/Rules/Idiomatic/DuplicateImportsRuleExamples.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 76f5b830462..fadb68d82c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,9 @@ #### Enhancements -* None. +* Add `duplicate_imports` rule to prevent importing the same module twice. + [Samuel Susla](https://github.com/sammy-sc) + [#1881](https://github.com/realm/SwiftLint/issues/1881) #### Bug Fixes diff --git a/Rules.md b/Rules.md index 3f2ad5f5765..72d88f3b0e9 100644 --- a/Rules.md +++ b/Rules.md @@ -26,6 +26,7 @@ * [Discouraged Object Literal](#discouraged-object-literal) * [Discouraged Optional Boolean](#discouraged-optional-boolean) * [Discouraged Optional Collection](#discouraged-optional-collection) +* [Duplicate Imports](#duplicate-imports) * [Dynamic Inline](#dynamic-inline) * [Empty Count](#empty-count) * [Empty Enum Arguments](#empty-enum-arguments) @@ -4623,6 +4624,207 @@ enum Foo { +## Duplicate Imports + +Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version +--- | --- | --- | --- | --- | --- +`duplicate_imports` | Enabled | No | idiomatic | No | 3.0.0 + +Imports should be unique. + +### Examples + +
+Non Triggering Examples + +```swift +import A +import B +import C +``` + +```swift +import A.B +import A.C +``` + +```swift +#if DEBUG + @testable import KsApi +#else + import KsApi +#endif +``` + +```swift +import A // module +import B // module +``` + +
+
+Triggering Examples + +```swift +import Foundation +import Dispatch +↓import Foundation +``` + +```swift +import Foundation +↓import Foundation.NSString +``` + +```swift +↓import Foundation.NSString +import Foundation +``` + +```swift +↓import A.B.C +import A.B +``` + +```swift +import A.B +↓import A.B.C +``` + +```swift +import A +#if DEBUG + @testable import KsApi +#else + import KsApi +#endif +↓import A +``` + +```swift +import A +↓import typealias A.Foo +``` + +```swift +import A +↓import struct A.Foo +``` + +```swift +import A +↓import class A.Foo +``` + +```swift +import A +↓import enum A.Foo +``` + +```swift +import A +↓import protocol A.Foo +``` + +```swift +import A +↓import let A.Foo +``` + +```swift +import A +↓import var A.Foo +``` + +```swift +import A +↓import func A.Foo +``` + +```swift +import A +↓import typealias A.B.Foo +``` + +```swift +import A +↓import struct A.B.Foo +``` + +```swift +import A +↓import class A.B.Foo +``` + +```swift +import A +↓import enum A.B.Foo +``` + +```swift +import A +↓import protocol A.B.Foo +``` + +```swift +import A +↓import let A.B.Foo +``` + +```swift +import A +↓import var A.B.Foo +``` + +```swift +import A +↓import func A.B.Foo +``` + +```swift +import A.B +↓import typealias A.B.Foo +``` + +```swift +import A.B +↓import struct A.B.Foo +``` + +```swift +import A.B +↓import class A.B.Foo +``` + +```swift +import A.B +↓import enum A.B.Foo +``` + +```swift +import A.B +↓import protocol A.B.Foo +``` + +```swift +import A.B +↓import let A.B.Foo +``` + +```swift +import A.B +↓import var A.B.Foo +``` + +```swift +import A.B +↓import func A.B.Foo +``` + +
+ + + ## Dynamic Inline Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index d45edea5cf1..dbf32130ef0 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -27,6 +27,7 @@ public let masterRuleList = RuleList(rules: [ DiscouragedObjectLiteralRule.self, DiscouragedOptionalBooleanRule.self, DiscouragedOptionalCollectionRule.self, + DuplicateImportsRule.self, DynamicInlineRule.self, EmptyCountRule.self, EmptyEnumArgumentsRule.self, diff --git a/Source/SwiftLintFramework/Rules/Idiomatic/DuplicateImportsRule.swift b/Source/SwiftLintFramework/Rules/Idiomatic/DuplicateImportsRule.swift new file mode 100644 index 00000000000..81547b635e7 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/Idiomatic/DuplicateImportsRule.swift @@ -0,0 +1,107 @@ +import Foundation +import SourceKittenFramework + +public struct DuplicateImportsRule: ConfigurationProviderRule, AutomaticTestableRule { + public var configuration = SeverityConfiguration(.warning) + + // List of all possible import kinds + static let importKinds = [ + "typealias", "struct", "class", + "enum", "protocol", "let", + "var", "func" + ] + + public init() {} + + public static let description = RuleDescription( + identifier: "duplicate_imports", + name: "Duplicate Imports", + description: "Imports should be unique.", + kind: .idiomatic, + nonTriggeringExamples: DuplicateImportsRuleExamples.nonTriggeringExamples, + triggeringExamples: DuplicateImportsRuleExamples.triggeringExamples + ) + + private func rangesInConditionalCompilation(file: File) -> [NSRange] { + let contents = file.contents.bridge() + + let ranges = file.syntaxMap.tokens + .filter { SyntaxKind(rawValue: $0.type) == .buildconfigKeyword } + .map { NSRange(location: $0.offset, length: $0.length) } + .filter { range in + let keyword = contents.substringWithByteRange(start: range.location, length: range.length) + return ["#if", "#endif"].contains(keyword) + } + + return stride(from: 0, to: ranges.count, by: 2).reduce(into: []) { result, rangeIndex in + result.append(NSUnionRange(ranges[rangeIndex], ranges[rangeIndex + 1])) + } + } + + public func validate(file: File) -> [StyleViolation] { + let contents = file.contents.bridge() + + let ignoredRanges = self.rangesInConditionalCompilation(file: file) + + let importKinds = DuplicateImportsRule.importKinds.joined(separator: "|") + + // Grammar of import declaration + // attributes(optional) import import-kind(optional) import-path + let regex = "^(\\w\\s)?import(\\s(\(importKinds)))?\\s+[a-zA-Z0-9._]+$" + let importRanges = file.match(pattern: regex) + .filter { $0.1.allSatisfy { [.keyword, .identifier].contains($0) } } + .compactMap { contents.NSRangeToByteRange(start: $0.0.location, length: $0.0.length) } + .filter { importRange -> Bool in + return !importRange.intersects(ignoredRanges) + } + + let lines = contents.lines() + + let importLines: [Line] = importRanges.compactMap { range in + guard let line = contents.lineAndCharacter(forByteOffset: range.location)?.line + else { return nil } + return lines[line - 1] + } + + var violations = [StyleViolation]() + + for indexI in 0.. Bool { + guard let firstImportIdentifiers = self.importIdentifier?.split(separator: "."), + let secondImportIdentifiers = otherLine.importIdentifier?.split(separator: ".") + else { return false } + + return zip(firstImportIdentifiers, secondImportIdentifiers).allSatisfy { $0 == $1 } + } +} diff --git a/Source/SwiftLintFramework/Rules/Idiomatic/DuplicateImportsRuleExamples.swift b/Source/SwiftLintFramework/Rules/Idiomatic/DuplicateImportsRuleExamples.swift new file mode 100644 index 00000000000..244db52fb05 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/Idiomatic/DuplicateImportsRuleExamples.swift @@ -0,0 +1,56 @@ +internal struct DuplicateImportsRuleExamples { + static let nonTriggeringExamples: [String] = [ + "import A\nimport B\nimport C", + "import A.B\nimport A.C", + """ + #if DEBUG + @testable import KsApi + #else + import KsApi + #endif + """, + "import A // module\nimport B // module" + ] + + static let triggeringExamples: [String] = { + var list: [String] = [ + "import Foundation\nimport Dispatch\n↓import Foundation", + "import Foundation\n↓import Foundation.NSString", + "↓import Foundation.NSString\nimport Foundation", + "↓import A.B.C\nimport A.B", + "import A.B\n↓import A.B.C", + """ + import A + #if DEBUG + @testable import KsApi + #else + import KsApi + #endif + ↓import A + """ + ] + + list += DuplicateImportsRule.importKinds.map { importKind in + return """ + import A + ↓import \(importKind) A.Foo + """ + } + + list += DuplicateImportsRule.importKinds.map { importKind in + return """ + import A + ↓import \(importKind) A.B.Foo + """ + } + + list += DuplicateImportsRule.importKinds.map { importKind in + return """ + import A.B + ↓import \(importKind) A.B.Foo + """ + } + + return list + }() +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 46e8ed6d78d..e488104425b 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -35,6 +35,8 @@ 24B4DF0D1D6DFDE90097803B /* RedundantNilCoalescingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24B4DF0B1D6DFA370097803B /* RedundantNilCoalescingRule.swift */; }; 24E17F721B14BB3F008195BE /* File+Cache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24E17F701B1481FF008195BE /* File+Cache.swift */; }; 29AD4C661F6EA1D5009B66E1 /* ContainsOverFirstNotNilRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29AD4C641F6EA16C009B66E1 /* ContainsOverFirstNotNilRule.swift */; }; + 29FC197921382C07006D208C /* DuplicateImportsRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29FC197721382C06006D208C /* DuplicateImportsRule.swift */; }; + 29FC197A21382C07006D208C /* DuplicateImportsRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29FC197821382C07006D208C /* DuplicateImportsRuleExamples.swift */; }; 29FFC37A1F15764D007E4825 /* FileLengthRuleConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29FFC3781F1574FD007E4825 /* FileLengthRuleConfiguration.swift */; }; 29FFC37D1F157BDE007E4825 /* FileLengthRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29FFC37B1F157BA8007E4825 /* FileLengthRuleTests.swift */; }; 2E02005F1C54BF680024D09D /* CyclomaticComplexityRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E02005E1C54BF680024D09D /* CyclomaticComplexityRule.swift */; }; @@ -472,6 +474,8 @@ 24B4DF0B1D6DFA370097803B /* RedundantNilCoalescingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantNilCoalescingRule.swift; sourceTree = ""; }; 24E17F701B1481FF008195BE /* File+Cache.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "File+Cache.swift"; sourceTree = ""; }; 29AD4C641F6EA16C009B66E1 /* ContainsOverFirstNotNilRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContainsOverFirstNotNilRule.swift; sourceTree = ""; }; + 29FC197721382C06006D208C /* DuplicateImportsRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DuplicateImportsRule.swift; sourceTree = ""; }; + 29FC197821382C07006D208C /* DuplicateImportsRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DuplicateImportsRuleExamples.swift; sourceTree = ""; }; 29FFC3781F1574FD007E4825 /* FileLengthRuleConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileLengthRuleConfiguration.swift; sourceTree = ""; }; 29FFC37B1F157BA8007E4825 /* FileLengthRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileLengthRuleTests.swift; sourceTree = ""; }; 2E02005E1C54BF680024D09D /* CyclomaticComplexityRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CyclomaticComplexityRule.swift; sourceTree = ""; }; @@ -1132,6 +1136,8 @@ 6264015320155533005B9C4A /* DiscouragedOptionalBooleanRuleExamples.swift */, 62FE5D30200CAB6E00F68793 /* DiscouragedOptionalCollectionExamples.swift */, 629ADD052006302D0009E362 /* DiscouragedOptionalCollectionRule.swift */, + 29FC197721382C06006D208C /* DuplicateImportsRule.swift */, + 29FC197821382C07006D208C /* DuplicateImportsRuleExamples.swift */, 72EA17B51FD31F10009D5CE6 /* ExplicitACLRule.swift */, 827169B21F488181003FB9AF /* ExplicitEnumRawValueRule.swift */, 7C0C2E791D2866CB0076435A /* ExplicitInitRule.swift */, @@ -1840,6 +1846,7 @@ 6258783B1FFC458100AC34F2 /* DiscouragedObjectLiteralRule.swift in Sources */, 62A3E95D209E084000547A86 /* EmptyXCTestMethodRule.swift in Sources */, D4C4A34C1DEA4FF000E0E04C /* AttributesConfiguration.swift in Sources */, + 29FC197A21382C07006D208C /* DuplicateImportsRuleExamples.swift in Sources */, 83D71E281B131ECE000395DE /* RuleDescription.swift in Sources */, D4470D571EB69225008A1B2E /* ImplicitReturnRule.swift in Sources */, 3B12C9C51C322032000B423F /* MasterRuleList.swift in Sources */, @@ -1861,6 +1868,7 @@ D4246D6D1F30D8620097E658 /* PrivateOverFilePrivateRuleConfiguration.swift in Sources */, 3A915E5B20A1543700519F3A /* ClosureEndIndentationRuleExamples.swift in Sources */, 629ADD062006302D0009E362 /* DiscouragedOptionalCollectionRule.swift in Sources */, + 29FC197921382C07006D208C /* DuplicateImportsRule.swift in Sources */, E816194E1BFBFEAB00946723 /* ForceTryRule.swift in Sources */, 8F2CC1CB20A6A070006ED34F /* FileNameConfiguration.swift in Sources */, D4CFC5D2209EC95A00668488 /* FunctionDefaultParameterAtEndRule.swift in Sources */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index e5daa0ce5b4..8f07d52af10 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -287,6 +287,12 @@ extension DocumentationTests { ] } +extension DuplicateImportsRuleTests { + static var allTests: [(String, (DuplicateImportsRuleTests) -> () throws -> Void)] = [ + ("testWithDefaultConfiguration", testWithDefaultConfiguration) + ] +} + extension DynamicInlineRuleTests { static var allTests: [(String, (DynamicInlineRuleTests) -> () throws -> Void)] = [ ("testWithDefaultConfiguration", testWithDefaultConfiguration) @@ -1434,6 +1440,7 @@ XCTMain([ testCase(DiscouragedOptionalBooleanRuleTests.allTests), testCase(DiscouragedOptionalCollectionRuleTests.allTests), testCase(DocumentationTests.allTests), + testCase(DuplicateImportsRuleTests.allTests), testCase(DynamicInlineRuleTests.allTests), testCase(EmptyCountRuleTests.allTests), testCase(EmptyEnumArgumentsRuleTests.allTests), diff --git a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift index f04c63e6f03..d59e411e0c8 100644 --- a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift +++ b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift @@ -102,6 +102,12 @@ class DiscouragedOptionalCollectionRuleTests: XCTestCase { } } +class DuplicateImportsRuleTests: XCTestCase { + func testWithDefaultConfiguration() { + verifyRule(DuplicateImportsRule.description) + } +} + class DynamicInlineRuleTests: XCTestCase { func testWithDefaultConfiguration() { verifyRule(DynamicInlineRule.description)