Skip to content

Commit

Permalink
Merge pull request #2597 from realm/mf-weak_computed_property
Browse files Browse the repository at this point in the history
Add `weak_computed_property` rule
  • Loading branch information
marcelofabri authored Jan 27, 2019
2 parents 7c38722 + 81ffa9b commit ea9c638
Show file tree
Hide file tree
Showing 7 changed files with 315 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
[Marcelo Fabri](https://github.com/marcelofabri)
[#2589](https://github.com/realm/SwiftLint/issues/2589)

* Add `weak_computed_property` rule to warn against using `weak` in a computed
property as it has no effect.
[Marcelo Fabri](https://github.com/marcelofabri)
[#2596](https://github.com/realm/SwiftLint/issues/2596)

#### Bug Fixes

* Fix `explicit_type_interface` when used in statements.
Expand Down
71 changes: 71 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@
* [Vertical Whitespace before Closing Braces](#vertical-whitespace-before-closing-braces)
* [Vertical Whitespace after Opening Braces](#vertical-whitespace-after-opening-braces)
* [Void Return](#void-return)
* [Weak Computed Property](#weak-computed-property)
* [Weak Delegate](#weak-delegate)
* [XCTest Specific Matcher](#xctest-specific-matcher)
* [XCTFail Message](#xctfail-message)
Expand Down Expand Up @@ -23539,6 +23540,76 @@ let foo: (ConfigurationTests) -> () throws -> ↓())



## Weak Computed Property

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
--- | --- | --- | --- | --- | ---
`weak_computed_property` | Enabled | Yes | lint | No | 4.1.0

Adding weak to a computed property has no effect.

### Examples

<details>
<summary>Non Triggering Examples</summary>

```swift
class Foo {
weak var delegate: SomeProtocol?
}
```

```swift
class Foo {
var delegate: SomeProtocol?
}
```

```swift
class Foo {
weak var delegate: SomeProtocol? {
didSet {
update(with: delegate)
}
}
}
```

```swift
class Foo {
weak var delegate: SomeProtocol? {
willSet {
update(with: delegate)
}
}
}
```

</details>
<details>
<summary>Triggering Examples</summary>

```swift
class Foo {
weak var delegate: SomeProtocol? { return bar() }
}
```

```swift
class Foo {
private weak var _delegate: SomeProtocol?

↓weak var delegate: SomeProtocol? {
get { return _delegate }
set { _delegate = newValue }
}
}
```

</details>



## Weak Delegate

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/MasterRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ public let masterRuleList = RuleList(rules: [
VerticalWhitespaceOpeningBracesRule.self,
VerticalWhitespaceRule.self,
VoidReturnRule.self,
WeakComputedProperyRule.self,
WeakDelegateRule.self,
XCTFailMessageRule.self,
XCTSpecificMatcherRule.self,
Expand Down
221 changes: 221 additions & 0 deletions Source/SwiftLintFramework/Rules/Lint/WeakComputedProperyRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
import Foundation
import SourceKittenFramework

public struct WeakComputedProperyRule: ASTRule, CorrectableRule, ConfigurationProviderRule, AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static let description = RuleDescription(
identifier: "weak_computed_property",
name: "Weak Computed Property",
description: "Adding weak to a computed property has no effect.",
kind: .lint,
minSwiftVersion: .fourDotOne,
nonTriggeringExamples: [
" weak var delegate: SomeProtocol?",
" var delegate: SomeProtocol?",
"""
weak var delegate: SomeProtocol? {
didSet {
update(with: delegate)
}
}
""",
"""
weak var delegate: SomeProtocol? {
willSet {
update(with: delegate)
}
}
"""
].map(wrapExample),
triggeringExamples: [
" weak var delegate: SomeProtocol? { return bar() }",
"""
private weak var _delegate: SomeProtocol?
↓weak var delegate: SomeProtocol? {
get { return _delegate }
set { _delegate = newValue }
}
"""
].map(wrapExample),
corrections: [
wrapExample(" ↓weak var delegate: SomeProtocol? { return bar() }"):
wrapExample(" var delegate: SomeProtocol? { return bar() }"),
wrapExample("""
private weak var _delegate: SomeProtocol?
↓weak var delegate: SomeProtocol? {
get { return _delegate }
set { _delegate = newValue }
}
"""):
wrapExample("""
private weak var _delegate: SomeProtocol?
var delegate: SomeProtocol? {
get { return _delegate }
set { _delegate = newValue }
}
""")
]
)

// MARK: - ASTRule

public func validate(file: File,
kind: SwiftDeclarationKind,
dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
return violationRanges(in: file, kind: kind, dictionary: dictionary).map {
StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, characterOffset: $0.location))
}
}

// MARK: - CorrectableRule

public func correct(file: File) -> [Correction] {
let violatingRanges = file.ruleEnabled(violatingRanges: violationRanges(in: file), for: self)
guard !violatingRanges.isEmpty else { return [] }

let description = type(of: self).description
var corrections = [Correction]()
var contents = file.contents
for range in violatingRanges {
var rangeToRemove = range
let contentsNSString = contents.bridge()
if let byteRange = contentsNSString.NSRangeToByteRange(start: range.location, length: range.length),
let nextToken = file.syntaxMap.tokens.first(where: { $0.offset > byteRange.location }),
let nextTokenLocation = contentsNSString.byteRangeToNSRange(start: nextToken.offset, length: 0) {
rangeToRemove.length = nextTokenLocation.location - range.location
}

contents = contentsNSString.replacingCharacters(in: rangeToRemove, with: "")
let location = Location(file: file, characterOffset: range.location)
corrections.append(Correction(ruleDescription: description, location: location))
}

file.write(contents)
return corrections
}

// MARK: - Private

private let allowedKinds = SwiftDeclarationKind.variableKinds.subtracting([.varParameter])

private func violationRanges(in file: File) -> [NSRange] {
return violationRanges(in: file, dictionary: file.structure.dictionary).sorted {
$0.location > $1.location
}
}

private func violationRanges(in file: File,
dictionary: [String: SourceKitRepresentable]) -> [NSRange] {
let ranges = dictionary.substructure.flatMap { subDict -> [NSRange] in
var ranges = violationRanges(in: file, dictionary: subDict)

if let kind = subDict.kind.flatMap(SwiftDeclarationKind.init(rawValue:)) {
ranges += violationRanges(in: file, kind: kind, dictionary: subDict)
}

return ranges
}

return ranges.unique
}

private func violationRanges(in file: File,
kind: SwiftDeclarationKind,
dictionary: [String: SourceKitRepresentable]) -> [NSRange] {
guard allowedKinds.contains(kind),
let bodyOffset = dictionary.bodyOffset,
let bodyLength = dictionary.bodyLength, bodyLength > 0,
let weakAttribute = dictionary.swiftAttributes.first(where: { $0.isWeakAttribute }),
let attributeOffset = weakAttribute.offset,
let attributeLength = weakAttribute.length, attributeLength > 0,
case let contents = file.contents.bridge(),
let attributeRange = contents.byteRangeToNSRange(start: attributeOffset, length: attributeLength),
let bodyRange = contents.byteRangeToNSRange(start: bodyOffset, length: bodyLength),
!containsObserverToken(in: bodyRange, file: file, propertyStructure: dictionary) else {
return []
}

return [attributeRange]
}

private func containsObserverToken(in range: NSRange, file: File,
propertyStructure: [String: SourceKitRepresentable]) -> Bool {
let tokens = file.rangesAndTokens(matching: "\\b(?:didSet|willSet)\\b", range: range).keywordTokens()
return tokens.contains(where: { token -> Bool in
// the last element is the deepest structure
guard let dict = declarations(forByteOffset: token.offset, structure: file.structure).last,
propertyStructure.isEqualTo(dict) else {
return false
}

return true
})
}

private func declarations(forByteOffset byteOffset: Int,
structure: Structure) -> [[String: SourceKitRepresentable]] {
var results = [[String: SourceKitRepresentable]]()

func parse(dictionary: [String: SourceKitRepresentable], parentKind: SwiftDeclarationKind?) {
// Only accepts declarations which contains a body and contains the
// searched byteOffset
guard let kindString = dictionary.kind,
let kind = SwiftDeclarationKind(rawValue: kindString),
let bodyOffset = dictionary.bodyOffset,
let bodyLength = dictionary.bodyLength,
case let byteRange = NSRange(location: bodyOffset, length: bodyLength),
NSLocationInRange(byteOffset, byteRange) else {
return
}

if parentKind != .protocol && allowedKinds.contains(kind) {
results.append(dictionary)
}

for dictionary in dictionary.substructure {
parse(dictionary: dictionary, parentKind: kind)
}
}

for dictionary in structure.dictionary.substructure {
parse(dictionary: dictionary, parentKind: nil)
}

return results
}
}

private extension Dictionary where Key == String, Value == SourceKitRepresentable {
var isWeakAttribute: Bool {
return attribute.flatMap(SwiftDeclarationAttributeKind.init) == .weak
}
}

private extension Array where Element == (NSRange, [SyntaxToken]) {
func keywordTokens() -> [SyntaxToken] {
return compactMap { _, tokens in
guard let token = tokens.last,
SyntaxKind(rawValue: token.type) == .keyword else {
return nil
}

return token
}
}
}

private func wrapExample(_ text: String) -> String {
return """
class Foo {
\(text)
}
"""
}
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@
D41985EB21FAB63E003BE2B7 /* DeploymentTargetConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41985EA21FAB63E003BE2B7 /* DeploymentTargetConfiguration.swift */; };
D41985ED21FAD033003BE2B7 /* DeploymentTargetConfigurationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41985EC21FAD033003BE2B7 /* DeploymentTargetConfigurationTests.swift */; };
D41985EF21FAD5E8003BE2B7 /* DeploymentTargetRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41985EE21FAD5E8003BE2B7 /* DeploymentTargetRuleTests.swift */; };
D41985F121FC5AF7003BE2B7 /* WeakComputedProperyRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41985F021FC5AF7003BE2B7 /* WeakComputedProperyRule.swift */; };
D41B57781ED8CEE0007B0470 /* ExtensionAccessModifierRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41B57771ED8CEE0007B0470 /* ExtensionAccessModifierRule.swift */; };
D41E7E0B1DF9DABB0065259A /* RedundantStringEnumValueRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */; };
D4246D6D1F30D8620097E658 /* PrivateOverFilePrivateRuleConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4246D6C1F30D8620097E658 /* PrivateOverFilePrivateRuleConfiguration.swift */; };
Expand Down Expand Up @@ -698,6 +699,7 @@
D41985EA21FAB63E003BE2B7 /* DeploymentTargetConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeploymentTargetConfiguration.swift; sourceTree = "<group>"; };
D41985EC21FAD033003BE2B7 /* DeploymentTargetConfigurationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeploymentTargetConfigurationTests.swift; sourceTree = "<group>"; };
D41985EE21FAD5E8003BE2B7 /* DeploymentTargetRuleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeploymentTargetRuleTests.swift; sourceTree = "<group>"; };
D41985F021FC5AF7003BE2B7 /* WeakComputedProperyRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WeakComputedProperyRule.swift; sourceTree = "<group>"; };
D41B57771ED8CEE0007B0470 /* ExtensionAccessModifierRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExtensionAccessModifierRule.swift; sourceTree = "<group>"; };
D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantStringEnumValueRule.swift; sourceTree = "<group>"; };
D4246D6C1F30D8620097E658 /* PrivateOverFilePrivateRuleConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateOverFilePrivateRuleConfiguration.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1069,6 +1071,7 @@
8F6B3153213CDCD100858E44 /* UnusedPrivateDeclarationRule.swift */,
D4B3409C21F16B110038C79A /* UnusedSetterValueRule.swift */,
D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */,
D41985F021FC5AF7003BE2B7 /* WeakComputedProperyRule.swift */,
094384FF1D5D2382009168CF /* WeakDelegateRule.swift */,
1872906F1FC37A9B0016BEA2 /* YodaConditionRule.swift */,
);
Expand Down Expand Up @@ -2031,6 +2034,7 @@
D4130D971E16183F00242361 /* IdentifierNameRuleExamples.swift in Sources */,
7250948A1D0859260039B353 /* StatementModeConfiguration.swift in Sources */,
E81619531BFC162C00946723 /* QueuedPrint.swift in Sources */,
D41985F121FC5AF7003BE2B7 /* WeakComputedProperyRule.swift in Sources */,
E87E4A051BFB927C00FCFE46 /* TrailingSemicolonRule.swift in Sources */,
D4B472411F66486300BD6EF1 /* FallthroughRule.swift in Sources */,
B25DCD0B1F7E9F9E0028A199 /* MultilineArgumentsRuleExamples.swift in Sources */,
Expand Down
7 changes: 7 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1390,6 +1390,12 @@ extension VoidReturnRuleTests {
]
}

extension WeakComputedProperyRuleTests {
static var allTests: [(String, (WeakComputedProperyRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
]
}

extension WeakDelegateRuleTests {
static var allTests: [(String, (WeakDelegateRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
Expand Down Expand Up @@ -1628,6 +1634,7 @@ XCTMain([
testCase(VerticalWhitespaceOpeningBracesRuleTests.allTests),
testCase(VerticalWhitespaceRuleTests.allTests),
testCase(VoidReturnRuleTests.allTests),
testCase(WeakComputedProperyRuleTests.allTests),
testCase(WeakDelegateRuleTests.allTests),
testCase(XCTFailMessageRuleTests.allTests),
testCase(XCTSpecificMatcherRuleTests.allTests),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,12 @@ class VoidReturnRuleTests: XCTestCase {
}
}

class WeakComputedProperyRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(WeakComputedProperyRule.description)
}
}

class WeakDelegateRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(WeakDelegateRule.description)
Expand Down

0 comments on commit ea9c638

Please sign in to comment.