Skip to content

Commit

Permalink
Fix false positives and invalid corrections with unused import rule (r…
Browse files Browse the repository at this point in the history
…ealm#2492)

* Fix wrong correction when removing testable imports with UnusedImportRule

* Fix false positive with UnusedImportRule when importing Foundation when there are attributes in that file requiring Foundation.

* Update rules documentation

* Add changelog entries
  • Loading branch information
jpsim authored and sjavora committed Mar 9, 2019
1 parent 0fcf7a7 commit 9b167d3
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 4 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@
`rules` command when there are one or more configured custom rules.
[jhildensperger](https://github.com/jhildensperger)

* Fix wrong correction when removing testable imports with the `unused_import`
rule.
[JP Simard](https://github.com/jpsim)

* Fix false positive with the `unused_import` rule when importing Foundation
when there are attributes in that file requiring Foundation.
[JP Simard](https://github.com/jpsim)

## 0.29.0: A Laundry List of Changes

#### Breaking
Expand Down
17 changes: 17 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -21447,6 +21447,17 @@ import Dispatch
dispatchMain()
```

```swift
@testable import Dispatch
dispatchMain()
```

```swift
import Foundation
@objc
class A {}
```

</details>
<details>
<summary>Triggering Examples</summary>
Expand All @@ -21464,6 +21475,12 @@ A.dispatchMain()
dispatchMain()
```

```swift
import Foundation
// @objc
class A {}
```

</details>


Expand Down
72 changes: 68 additions & 4 deletions Source/SwiftLintFramework/Rules/Lint/UnusedImportRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ public struct UnusedImportRule: CorrectableRule, ConfigurationProviderRule, Anal
"""
import Dispatch
dispatchMain()
""",
"""
@testable import Dispatch
dispatchMain()
""",
"""
import Foundation
@objc
class A {}
"""
],
triggeringExamples: [
Expand All @@ -26,6 +35,11 @@ public struct UnusedImportRule: CorrectableRule, ConfigurationProviderRule, Anal
"""
↓import Foundation
dispatchMain()
""",
"""
↓import Foundation
// @objc
class A {}
"""
],
corrections: [
Expand All @@ -44,6 +58,24 @@ public struct UnusedImportRule: CorrectableRule, ConfigurationProviderRule, Anal
""":
"""
dispatchMain()
""",
"""
↓@testable import Foundation
import Dispatch
dispatchMain()
""":
"""
import Dispatch
dispatchMain()
""",
"""
↓import Foundation
// @objc
class A {}
""":
"""
// @objc
class A {}
"""
],
requiresFileOnDisk: true
Expand Down Expand Up @@ -89,6 +121,7 @@ public struct UnusedImportRule: CorrectableRule, ConfigurationProviderRule, Anal

private extension File {
func unusedImports(compilerArguments: [String]) -> [(String, NSRange)] {
let contentsNSString = contents.bridge()
var imports = [String]()
var usrFragments = [String]()
var nextIsModuleImport = false
Expand All @@ -97,8 +130,7 @@ private extension File {
continue
}
if tokenKind == .keyword,
let substring = contents.bridge()
.substringWithByteRange(start: token.offset, length: token.length),
let substring = contentsNSString.substringWithByteRange(start: token.offset, length: token.length),
substring == "import" {
nextIsModuleImport = true
continue
Expand Down Expand Up @@ -129,11 +161,36 @@ private extension File {
}
// Always disallow 'import Swift' because it's available without importing.
usrFragments = usrFragments.filter { $0 != "Swift" }
let unusedImports = imports.filter { !usrFragments.contains($0) }
var unusedImports = imports.filter { !usrFragments.contains($0) }
// Certain Swift attributes requires importing Foundation.
if unusedImports.contains("Foundation") && containsAttributesRequiringFoundation() {
unusedImports.removeAll(where: { $0 == "Foundation" })
}
return unusedImports.map { module in
return (module, contents.bridge().range(of: "import \(module)\n"))
let testableImportRange = contentsNSString.range(of: "@testable import \(module)\n")
if testableImportRange.location != NSNotFound {
return (module, testableImportRange)
}

return (module, contentsNSString.range(of: "import \(module)\n"))
}
}

private func containsAttributesRequiringFoundation() -> Bool {
guard contents.contains("@objc") else {
return false
}

func containsAttributesRequiringFoundation(dict: [String: SourceKitRepresentable]) -> Bool {
if !attributesRequiringFoundation.isDisjoint(with: dict.enclosedSwiftAttributes) {
return true
} else {
return dict.substructure.contains(where: containsAttributesRequiringFoundation)
}
}

return containsAttributesRequiringFoundation(dict: self.structure.dictionary)
}
}

private let syntaxKindsToSkip: Set<SyntaxKind> = [
Expand All @@ -150,3 +207,10 @@ private let syntaxKindsToSkip: Set<SyntaxKind> = [
.comment,
.docCommentField
]

private let attributesRequiringFoundation: Set<SwiftDeclarationAttributeKind> = [
.objc,
.objcName,
.objcMembers,
.objcNonLazyRealization
]

0 comments on commit 9b167d3

Please sign in to comment.