Skip to content

Commit

Permalink
Merge pull request realm#2663 from matthew-healy/matthew-healy/equata…
Browse files Browse the repository at this point in the history
…ble-nsobject

Add NSObjectPreferIsEqualRule
  • Loading branch information
marcelofabri authored Mar 4, 2019
2 parents a8c108e + eded960 commit 6244d98
Show file tree
Hide file tree
Showing 8 changed files with 333 additions and 3 deletions.
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

#### Enhancements

* None.
* Add `nsobject_prefer_isequal` rule to warn against implementing `==` on an
`NSObject` subclass as calling `isEqual` (i.e. when using the class from
Objective-C) will will not use the defined `==` method.
[Matthew Healy](https://github.com/matthew-healy)
[#2663](https://github.com/realm/SwiftLint/pull/2663)

#### Bug Fixes

Expand All @@ -27,7 +31,6 @@
* None.

#### Enhancements

* Add `deployment_target` rule to validate that `@availability` attributes and
`#available` conditions are not using a version that is satisfied by the
deployment target. Since SwiftLint can't read an Xcode project, you need to
Expand Down
126 changes: 126 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
* [No Grouping Extension](#no-grouping-extension)
* [Notification Center Detachment](#notification-center-detachment)
* [NSLocalizedString Key](#nslocalizedstring-key)
* [NSObject Prefer isEqual](#nsobject-prefer-isequal)
* [Number Separator](#number-separator)
* [Object Literal](#object-literal)
* [Opening Brace Spacing](#opening-brace-spacing)
Expand Down Expand Up @@ -13274,6 +13275,131 @@ NSLocalizedString(↓"key_\(param)", comment: nil)



## NSObject Prefer isEqual

Identifier | Enabled by default | Supports autocorrection | Kind | Analyzer | Minimum Swift Compiler Version
--- | --- | --- | --- | --- | ---
`nsobject_prefer_isequal` | Enabled | No | lint | No | 3.0.0

NSObject subclasses should implement isEqual instead of ==.

### Examples

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

```swift
class AClass: NSObject {
}
```

```swift
@objc class AClass: SomeNSObjectSubclass {
}
```

```swift
class AClass: Equatable {
static func ==(lhs: AClass, rhs: AClass) -> Bool {
return true
}
```

```swift
class AClass: NSObject {
override func isEqual(_ object: Any?) -> Bool {
return true
}
}
```

```swift
@objc class AClass: SomeNSObjectSubclass {
override func isEqual(_ object: Any?) -> Bool {
return false
}
}
```

```swift
class AClass: NSObject {
func ==(lhs: AClass, rhs: AClass) -> Bool {
return true
}
}
```

```swift
class AClass: NSObject {
static func ==(lhs: AClass, rhs: BClass) -> Bool {
return true
}
}
```

```swift
struct AStruct: Equatable {
static func ==(lhs: AStruct, rhs: AStruct) -> Bool {
return false
}
}
```

```swift
enum AnEnum: Equatable {
static func ==(lhs: AnEnum, rhs: AnEnum) -> Bool {
return true
}
}
```

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

```swift
class AClass: NSObject {
↓static func ==(lhs: AClass, rhs: AClass) -> Bool {
return false
}
}
```

```swift
@objc class AClass: SomeOtherNSObjectSubclass {
↓static func ==(lhs: AClass, rhs: AClass) -> Bool {
return true
}
}
```

```swift
class AClass: NSObject, Equatable {
↓static func ==(lhs: AClass, rhs: AClass) -> Bool {
return false
}
}
```

```swift
class AClass: NSObject {
override func isEqual(_ object: Any?) -> Bool {
guard let other = object as? AClass else {
return false
}
return true
}

↓static func ==(lhs: AClass, rhs: AClass) -> Bool {
return false
}
}
```

</details>



## Number Separator

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 @@ -89,6 +89,7 @@ public let masterRuleList = RuleList(rules: [
MultilineParametersRule.self,
MultipleClosuresWithTrailingClosureRule.self,
NSLocalizedStringKeyRule.self,
NSObjectPreferIsEqualRule.self,
NestingRule.self,
NimbleOperatorRule.self,
NoExtensionAccessModifierRule.self,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import SourceKittenFramework

public struct NSObjectPreferIsEqualRule: Rule, ConfigurationProviderRule, AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static let description = RuleDescription(
identifier: "nsobject_prefer_isequal",
name: "NSObject Prefer isEqual",
description: "NSObject subclasses should implement isEqual instead of ==.",
kind: .lint,
nonTriggeringExamples: NSObjectPreferIsEqualRuleExamples.nonTriggeringExamples,
triggeringExamples: NSObjectPreferIsEqualRuleExamples.triggeringExamples
)

public func validate(file: File) -> [StyleViolation] {
return objcVisibleClasses(in: file).flatMap { violations(in: file, for: $0) }
}

// MARK: - Private

private func objcVisibleClasses(in file: File) -> [[String: SourceKitRepresentable]] {
return file.structure.dictionary.substructure.filter { dictionary in
guard
let kind = dictionary.kind,
SwiftDeclarationKind(rawValue: kind) == .class
else { return false }
let isDirectNSObjectSubclass = dictionary.inheritedTypes.contains("NSObject")
let isMarkedObjc = dictionary.enclosedSwiftAttributes.contains(.objc)
return isDirectNSObjectSubclass || isMarkedObjc
}
}

private func violations(in file: File,
for dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
guard let typeName = dictionary.name else { return [] }
return dictionary.substructure.compactMap { subDictionary -> StyleViolation? in
guard
isDoubleEqualsMethod(subDictionary, onType: typeName),
let offset = subDictionary.offset
else { return nil }
return StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, byteOffset: offset))
}
}

private func isDoubleEqualsMethod(_ method: [String: SourceKitRepresentable],
onType typeName: String) -> Bool {
guard
let kind = method.kind.flatMap(SwiftDeclarationKind.init),
let name = method.name,
kind == .functionMethodStatic,
name == "==(_:_:)",
areAllArguments(toMethod: method, ofType: typeName)
else { return false }
return true
}

private func areAllArguments(toMethod method: [String: SourceKitRepresentable],
ofType typeName: String) -> Bool {
return method.enclosedVarParameters.reduce(true) { soFar, param -> Bool in
soFar && (param.typeName == typeName)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
internal struct NSObjectPreferIsEqualRuleExamples {
static let nonTriggeringExamples: [String] = [
// NSObject subclass without ==
"""
class AClass: NSObject {
}
""",
// @objc class without ==
"""
@objc class AClass: SomeNSObjectSubclass {
}
""",
// Class with == which does not subclass NSObject
"""
class AClass: Equatable {
static func ==(lhs: AClass, rhs: AClass) -> Bool {
return true
}
""",
// NSObject subclass implementing isEqual
"""
class AClass: NSObject {
override func isEqual(_ object: Any?) -> Bool {
return true
}
}
""",
// @objc class implementing isEqual
"""
@objc class AClass: SomeNSObjectSubclass {
override func isEqual(_ object: Any?) -> Bool {
return false
}
}
""",
// NSObject subclass with non-static ==
"""
class AClass: NSObject {
func ==(lhs: AClass, rhs: AClass) -> Bool {
return true
}
}
""",
// NSObject subclass implementing == with different signature
"""
class AClass: NSObject {
static func ==(lhs: AClass, rhs: BClass) -> Bool {
return true
}
}
""",
// Equatable struct
"""
struct AStruct: Equatable {
static func ==(lhs: AStruct, rhs: AStruct) -> Bool {
return false
}
}
""",
// Equatable enum
"""
enum AnEnum: Equatable {
static func ==(lhs: AnEnum, rhs: AnEnum) -> Bool {
return true
}
}
"""
]

static let triggeringExamples: [String] = [
// NSObject subclass implementing ==
"""
class AClass: NSObject {
↓static func ==(lhs: AClass, rhs: AClass) -> Bool {
return false
}
}
""",
// @objc class implementing ==
"""
@objc class AClass: SomeOtherNSObjectSubclass {
↓static func ==(lhs: AClass, rhs: AClass) -> Bool {
return true
}
}
""",
// Equatable NSObject subclass implementing ==
"""
class AClass: NSObject, Equatable {
↓static func ==(lhs: AClass, rhs: AClass) -> Bool {
return false
}
}
""",
// NSObject subclass overriding isEqual and implementing ==
"""
class AClass: NSObject {
override func isEqual(_ object: Any?) -> Bool {
guard let other = object as? AClass else {
return false
}
return true
}
↓static func ==(lhs: AClass, rhs: AClass) -> Bool {
return false
}
}
"""
]
}
8 changes: 8 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
1F11B3CF1C252F23002E8FA8 /* ClosingBraceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */; };
24B4DF0D1D6DFDE90097803B /* RedundantNilCoalescingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24B4DF0B1D6DFA370097803B /* RedundantNilCoalescingRule.swift */; };
24E17F721B14BB3F008195BE /* File+Cache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24E17F701B1481FF008195BE /* File+Cache.swift */; };
2882895F222975D00037CF5F /* NSObjectPreferIsEqualRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2882895B22287C9C0037CF5F /* NSObjectPreferIsEqualRule.swift */; };
288289602229776C0037CF5F /* NSObjectPreferIsEqualRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2882895D22287E2C0037CF5F /* NSObjectPreferIsEqualRuleExamples.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 */; };
Expand Down Expand Up @@ -486,6 +488,8 @@
1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosingBraceRule.swift; sourceTree = "<group>"; };
24B4DF0B1D6DFA370097803B /* RedundantNilCoalescingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantNilCoalescingRule.swift; sourceTree = "<group>"; };
24E17F701B1481FF008195BE /* File+Cache.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "File+Cache.swift"; sourceTree = "<group>"; };
2882895B22287C9C0037CF5F /* NSObjectPreferIsEqualRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSObjectPreferIsEqualRule.swift; sourceTree = "<group>"; };
2882895D22287E2C0037CF5F /* NSObjectPreferIsEqualRuleExamples.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSObjectPreferIsEqualRuleExamples.swift; sourceTree = "<group>"; };
29AD4C641F6EA16C009B66E1 /* ContainsOverFirstNotNilRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContainsOverFirstNotNilRule.swift; sourceTree = "<group>"; };
29FC197721382C06006D208C /* DuplicateImportsRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DuplicateImportsRule.swift; sourceTree = "<group>"; };
29FC197821382C07006D208C /* DuplicateImportsRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DuplicateImportsRuleExamples.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1052,6 +1056,8 @@
D4DABFD61E2C23B1009617B6 /* NotificationCenterDetachmentRule.swift */,
D4DABFD81E2C59BC009617B6 /* NotificationCenterDetachmentRuleExamples.swift */,
D41985E621F85014003BE2B7 /* NSLocalizedStringKeyRule.swift */,
2882895B22287C9C0037CF5F /* NSObjectPreferIsEqualRule.swift */,
2882895D22287E2C0037CF5F /* NSObjectPreferIsEqualRuleExamples.swift */,
78F032441D7C877800BE709A /* OverriddenSuperCallRule.swift */,
D40FE89C1F867BFF006433E2 /* OverrideInExtensionRule.swift */,
62DEA1651FB21A9E00BCCCC6 /* PrivateActionRule.swift */,
Expand Down Expand Up @@ -1877,6 +1883,7 @@
181D9E172038343D001F6887 /* UntypedErrorInCatchRule.swift in Sources */,
47FF3BE11E7C75B600187E6D /* ImplicitlyUnwrappedOptionalRule.swift in Sources */,
623E36F01F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift in Sources */,
288289602229776C0037CF5F /* NSObjectPreferIsEqualRuleExamples.swift in Sources */,
BFF028AE1CBCF8A500B38A9D /* TrailingWhitespaceConfiguration.swift in Sources */,
3B034B6E1E0BE549005D49A9 /* LineLengthConfiguration.swift in Sources */,
B25DCD0C1F7E9FA20028A199 /* MultilineArgumentsRule.swift in Sources */,
Expand Down Expand Up @@ -2026,6 +2033,7 @@
D93DA3D11E699E6300809827 /* NestingConfiguration.swift in Sources */,
CC26ED07204DEB510013BBBC /* RuleIdentifier.swift in Sources */,
C328A2F71E6759AE00A9E4D7 /* ExplicitTypeInterfaceRule.swift in Sources */,
2882895F222975D00037CF5F /* NSObjectPreferIsEqualRule.swift in Sources */,
18B90B6B21ADD99800B60749 /* RedundantObjcAttributeRule.swift in Sources */,
93E0C3CE1D67BD7F007FA25D /* ConditionalReturnsOnNewlineRule.swift in Sources */,
D43DB1081DC573DA00281215 /* ImplicitGetterRule.swift in Sources */,
Expand Down
Loading

0 comments on commit 6244d98

Please sign in to comment.