From 04202ba455e4a9ad8e9bc4ff670499bb85af2518 Mon Sep 17 00:00:00 2001 From: Ornithologist Coder Date: Fri, 11 Aug 2017 16:08:02 +0200 Subject: [PATCH] Add quick_discouraged_call opt-in rule Implements #1781 (Method calls and object initialization inside Quick 'describe' and 'context' blocks can be harmful) --- CHANGELOG.md | 5 + Rules.md | 208 ++++++++++++++++++ .../Models/MasterRuleList.swift | 1 + .../Rules/QuickDiscouragedCallRule.swift | 107 +++++++++ .../QuickInitializationRuleExamples.swift | 151 +++++++++++++ SwiftLint.xcodeproj/project.pbxproj | 8 + Tests/LinuxMain.swift | 1 + .../SwiftLintFrameworkTests/RulesTests.swift | 4 + 8 files changed, 485 insertions(+) create mode 100644 Source/SwiftLintFramework/Rules/QuickDiscouragedCallRule.swift create mode 100644 Source/SwiftLintFramework/Rules/QuickInitializationRuleExamples.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e0b0b89d0f..1361998961e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,11 @@ [Ryan Booker](https://github.com/ryanbooker) [#1761](https://github.com/realm/SwiftLint/issues/1761) +* Add `quick_discouraged_call` opt-in rule to discourage calls and object + initialization inside 'describe' and 'context' block in Quick tests. + [Ornithologist Coder](https://github.com/ornithocoder) + [#1781](https://github.com/realm/SwiftLint/issues/1781) + ##### Bug Fixes * Fix false positive on `force_unwrapping` rule when declaring diff --git a/Rules.md b/Rules.md index 9c894713602..2f2411f1a37 100644 --- a/Rules.md +++ b/Rules.md @@ -67,6 +67,7 @@ * [Private Unit Test](#private-unit-test) * [Prohibited calls to super](#prohibited-calls-to-super) * [Protocol Property Accessors Order](#protocol-property-accessors-order) +* [Quick Discouraged Call](#quick-discouraged-call) * [Redundant Discardable Let](#redundant-discardable-let) * [Redundant Nil Coalescing](#redundant-nil-coalescing) * [Redundant Optional Initialization](#redundant-optional-initialization) @@ -7906,6 +7907,213 @@ protocol Foo { +## Quick Discouraged Call + +Identifier | Enabled by default | Supports autocorrection | Kind +--- | --- | --- | --- +`quick_discouraged_call` | Disabled | No | style + +Discouraged call inside 'describe' and/or 'context' block. + +### Examples + +
+Non Triggering Examples + +```swift +class TotoTests: QuickSpec { + override func spec() { + describe("foo") { + beforeEach { + let foo = Foo() + foo.toto() + } + } + } +} + +``` + +```swift +class TotoTests: QuickSpec { + override func spec() { + describe("foo") { + beforeEach { + let foo = Foo() + foo.toto() + } + afterEach { + let foo = Foo() + foo.toto() + } + describe("bar") { + } + context("bar") { + } + it("bar") { + let foo = Foo() + foo.toto() + } + } + } +} + +``` + +```swift +class TotoTests: QuickSpec { + override func spec() { + describe("foo") { + itBehavesLike("bar") + } + } +} + +``` + +```swift +class TotoTests: QuickSpec { + override func spec() { + describe("foo") { + it("does something") { + let foo = Foo() + foo.toto() + } + } + } +} + +``` + +```swift +class TotoTests: QuickSpec { + override func spec() { + context("foo") { + afterEach { toto.append(foo) } + } + } +} + +``` + +
+
+Triggering Examples + +```swift +class TotoTests: QuickSpec { + override func spec() { + describe("foo") { + let foo = ↓Foo() + } + } +} + +``` + +```swift +class TotoTests: QuickSpec { + override func spec() { + describe("foo") { + context("foo") { + let foo = ↓Foo() + } + context("bar") { + let foo = ↓Foo() + ↓foo.bar() + it("does something") { + let foo = Foo() + foo.toto() + } + } + } + } +} + +``` + +```swift +class TotoTests: QuickSpec { + override func spec() { + describe("foo") { + context("foo") { + context("foo") { + beforeEach { + let foo = Foo() + foo.toto() + } + it("bar") { + } + context("foo") { + let foo = ↓Foo() + } + } + } + } + } +} + +``` + +```swift +class TotoTests: QuickSpec + override func spec() + context("foo") { + let foo = ↓Foo() + } + } +} + +``` + +```swift +class TotoTests: QuickSpec + override func spec() + sharedExamples("foo") { + let foo = ↓Foo() + } + } +} + +``` + +```swift +class TotoTests: QuickSpec + override func spec() + describe("foo") { + ↓foo() + } + } +} + +``` + +```swift +class TotoTests: QuickSpec + override func spec() + context("foo") { + ↓foo() + } + } +} + +``` + +```swift +class TotoTests: QuickSpec + override func spec() + sharedExamples("foo") { + ↓foo() + } + } +} + +``` + +
+ + + ## Redundant Discardable Let Identifier | Enabled by default | Supports autocorrection | Kind diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index b1ce8c90ce6..86086c60b8d 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -75,6 +75,7 @@ public let masterRuleList = RuleList(rules: [ PrivateUnitTestRule.self, ProhibitedSuperRule.self, ProtocolPropertyAccessorsOrderRule.self, + QuickDiscouragedCallRule.self, RedundantDiscardableLetRule.self, RedundantNilCoalescingRule.self, RedundantOptionalInitializationRule.self, diff --git a/Source/SwiftLintFramework/Rules/QuickDiscouragedCallRule.swift b/Source/SwiftLintFramework/Rules/QuickDiscouragedCallRule.swift new file mode 100644 index 00000000000..3ac6b68c750 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/QuickDiscouragedCallRule.swift @@ -0,0 +1,107 @@ +// +// QuickInitializationRule.swift +// SwiftLint +// +// Created by Ornithologist Coder on 8/11/17. +// Copyright © 2017 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +// swiftlint:disable identifier_name + +public struct QuickDiscouragedCallRule: ASTRule, OptInRule, ConfigurationProviderRule { + public var configuration = SeverityConfiguration(.warning) + + public init() {} + + public static let description = RuleDescription( + identifier: "quick_discouraged_call", + name: "Quick Discouraged Call", + description: "Discouraged call inside 'describe' and/or 'context' block.", + kind: .style, + nonTriggeringExamples: QuickDiscouragedCallRuleExamples.nonTriggeringExamples, + triggeringExamples: QuickDiscouragedCallRuleExamples.triggeringExamples + ) + + public func validate(file: File, + kind: SwiftExpressionKind, + dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { + guard + fileContainsQuickSpec(file: file), + kind == .call, + let name = dictionary.name, + let kindName = QuickCallKind(rawValue: name), + QuickCallKind.restrictiveKinds().contains(kindName) + else { return [] } + + return violationOffsets(in: dictionary.enclosedArguments) + .map { + StyleViolation(ruleDescription: type(of: self).description, + severity: configuration.severity, + location: Location(file: file, byteOffset: $0)) + } + } + + // MARK: - Private + + private func fileContainsQuickSpec(file: File) -> Bool { + return !file.structure.dictionary.substructure.filter { $0.inheritedTypes.contains("QuickSpec") }.isEmpty + } + + typealias ViolationOffset = Int + + private func violationOffsets(in substructure: [[String: SourceKitRepresentable]]) -> [ViolationOffset] { + return substructure.flatMap { dictionary -> [ViolationOffset] in + return dictionary.substructure.flatMap(toViolationOffsets) + } + } + + private func toViolationOffsets(dictionary: [String: SourceKitRepresentable]) -> [ViolationOffset] { + guard + let kind = dictionary.kind, + let offset = dictionary.offset + else { return [] } + + if SwiftExpressionKind(rawValue: kind) == .call, + let name = dictionary.name, QuickCallKind(rawValue: name) == nil { + return [offset] + } + + guard SwiftExpressionKind(rawValue: kind) != .call else { return [] } + + return dictionary.substructure.flatMap(toViolationOffset) + } + + private func toViolationOffset(dictionary: [String: SourceKitRepresentable]) -> ViolationOffset? { + guard + let name = dictionary.name, + let offset = dictionary.offset, + let kind = dictionary.kind, + SwiftExpressionKind(rawValue: kind) == .call, + QuickCallKind(rawValue: name) == nil + else { return nil } + + return offset + } +} + +// MARK: - Private + +private enum QuickCallKind: String { + case describe + case context + case sharedExamples + case itBehavesLike + case beforeEach + case beforeSuite + case afterEach + case afterSuite + case it + case pending + + static func restrictiveKinds() -> [QuickCallKind] { + return [.describe, .context, .sharedExamples] + } +} diff --git a/Source/SwiftLintFramework/Rules/QuickInitializationRuleExamples.swift b/Source/SwiftLintFramework/Rules/QuickInitializationRuleExamples.swift new file mode 100644 index 00000000000..f7096372441 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/QuickInitializationRuleExamples.swift @@ -0,0 +1,151 @@ +// +// QuickInitializationRuleExamples.swift +// SwiftLint +// +// Created by Ornithologist Coder on 8/11/17. +// Copyright © 2017 Realm. All rights reserved. +// + +import Foundation + +internal struct QuickDiscouragedCallRuleExamples { + static let nonTriggeringExamples: [String] = [ + "class TotoTests: QuickSpec {\n" + + " override func spec() {\n" + + " describe(\"foo\") {\n" + + " beforeEach {\n" + + " let foo = Foo()\n" + + " foo.toto()\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec {\n" + + " override func spec() {\n" + + " describe(\"foo\") {\n" + + " beforeEach {\n" + + " let foo = Foo()\n" + + " foo.toto()\n" + + " }\n" + + " afterEach {\n" + + " let foo = Foo()\n" + + " foo.toto()\n" + + " }\n" + + " describe(\"bar\") {\n" + + " }\n" + + " context(\"bar\") {\n" + + " }\n" + + " it(\"bar\") {\n" + + " let foo = Foo()\n" + + " foo.toto()\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec {\n" + + " override func spec() {\n" + + " describe(\"foo\") {\n" + + " itBehavesLike(\"bar\")\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec {\n" + + " override func spec() {\n" + + " describe(\"foo\") {\n" + + " it(\"does something\") {\n" + + " let foo = Foo()\n" + + " foo.toto()\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec {\n" + + " override func spec() {\n" + + " context(\"foo\") {\n" + + " afterEach { toto.append(foo) }\n" + + " }\n" + + " }\n" + + "}\n" + ] + + static let triggeringExamples: [String] = [ + "class TotoTests: QuickSpec {\n" + + " override func spec() {\n" + + " describe(\"foo\") {\n" + + " let foo = ↓Foo()\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec {\n" + + " override func spec() {\n" + + " describe(\"foo\") {\n" + + " context(\"foo\") {\n" + + " let foo = ↓Foo()\n" + + " }\n" + + " context(\"bar\") {\n" + + " let foo = ↓Foo()\n" + + " ↓foo.bar()\n" + + " it(\"does something\") {\n" + + " let foo = Foo()\n" + + " foo.toto()\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec {\n" + + " override func spec() {\n" + + " describe(\"foo\") {\n" + + " context(\"foo\") {\n" + + " context(\"foo\") {\n" + + " beforeEach {\n" + + " let foo = Foo()\n" + + " foo.toto()\n" + + " }\n" + + " it(\"bar\") {\n" + + " }\n" + + " context(\"foo\") {\n" + + " let foo = ↓Foo()\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec\n" + + " override func spec()\n" + + " context(\"foo\") {\n" + + " let foo = ↓Foo()\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec\n" + + " override func spec()\n" + + " sharedExamples(\"foo\") {\n" + + " let foo = ↓Foo()\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec\n" + + " override func spec()\n" + + " describe(\"foo\") {\n" + + " ↓foo()\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec\n" + + " override func spec()\n" + + " context(\"foo\") {\n" + + " ↓foo()\n" + + " }\n" + + " }\n" + + "}\n", + "class TotoTests: QuickSpec\n" + + " override func spec()\n" + + " sharedExamples(\"foo\") {\n" + + " ↓foo()\n" + + " }\n" + + " }\n" + + "}\n" + ] +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index b9e25b3a1e4..a4d58d11890 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -66,6 +66,8 @@ 57ED827B1CF656E3002B3513 /* JUnitReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57ED82791CF65183002B3513 /* JUnitReporter.swift */; }; 621061BF1ED57E640082D51E /* MultilineParametersRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = 621061BE1ED57E640082D51E /* MultilineParametersRuleExamples.swift */; }; 62329C2B1F30B2310035737E /* DiscouragedDirectInitRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62AF35D71F30B183009B11EE /* DiscouragedDirectInitRuleTests.swift */; }; + 623E36F01F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 623E36EF1F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift */; }; + 623E36F21F3DB988002E5B71 /* QuickInitializationRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = 623E36F11F3DB988002E5B71 /* QuickInitializationRuleExamples.swift */; }; 6250D32A1ED4DFEB00735129 /* MultilineParametersRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6238AE411ED4D734006C3601 /* MultilineParametersRule.swift */; }; 62622F6B1F2F2E3500D5D099 /* DiscouragedDirectInitRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62622F6A1F2F2E3500D5D099 /* DiscouragedDirectInitRule.swift */; }; 626D02971F31CBCC0054788D /* XCTFailMessageRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 626D02961F31CBCC0054788D /* XCTFailMessageRule.swift */; }; @@ -375,6 +377,8 @@ 57ED82791CF65183002B3513 /* JUnitReporter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JUnitReporter.swift; sourceTree = ""; }; 621061BE1ED57E640082D51E /* MultilineParametersRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultilineParametersRuleExamples.swift; sourceTree = ""; }; 6238AE411ED4D734006C3601 /* MultilineParametersRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MultilineParametersRule.swift; sourceTree = ""; }; + 623E36EF1F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QuickDiscouragedCallRule.swift; sourceTree = ""; }; + 623E36F11F3DB988002E5B71 /* QuickInitializationRuleExamples.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QuickInitializationRuleExamples.swift; sourceTree = ""; }; 62622F6A1F2F2E3500D5D099 /* DiscouragedDirectInitRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiscouragedDirectInitRule.swift; sourceTree = ""; }; 626D02961F31CBCC0054788D /* XCTFailMessageRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XCTFailMessageRule.swift; sourceTree = ""; }; 62A498551F306A7700D766E4 /* DiscouragedDirectInitConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiscouragedDirectInitConfiguration.swift; sourceTree = ""; }; @@ -996,6 +1000,8 @@ B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */, 009E09271DFEE4C200B588A7 /* ProhibitedSuperRule.swift */, D47F31141EC918B600E3E1CA /* ProtocolPropertyAccessorsOrderRule.swift */, + 623E36EF1F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift */, + 623E36F11F3DB988002E5B71 /* QuickInitializationRuleExamples.swift */, D4C889701E385B7B00BAE88D /* RedundantDiscardableLetRule.swift */, 24B4DF0B1D6DFA370097803B /* RedundantNilCoalescingRule.swift */, D4B022951E0EF80C007E5297 /* RedundantOptionalInitializationRule.swift */, @@ -1344,6 +1350,7 @@ 6250D32A1ED4DFEB00735129 /* MultilineParametersRule.swift in Sources */, 009E092A1DFEE4DD00B588A7 /* ProhibitedSuperConfiguration.swift in Sources */, 47FF3BE11E7C75B600187E6D /* ImplicitlyUnwrappedOptionalRule.swift in Sources */, + 623E36F01F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift in Sources */, BFF028AE1CBCF8A500B38A9D /* TrailingWhitespaceConfiguration.swift in Sources */, 3B034B6E1E0BE549005D49A9 /* LineLengthConfiguration.swift in Sources */, D4C4A34C1DEA4FF000E0E04C /* AttributesConfiguration.swift in Sources */, @@ -1377,6 +1384,7 @@ E88198561BEA94D800333A11 /* FileLengthRule.swift in Sources */, D47079A91DFDBED000027086 /* ClosureParameterPositionRule.swift in Sources */, E8B67C3E1C095E6300FDED8E /* Correction.swift in Sources */, + 623E36F21F3DB988002E5B71 /* QuickInitializationRuleExamples.swift in Sources */, E88198531BEA944400333A11 /* LineLengthRule.swift in Sources */, D47F31151EC918B600E3E1CA /* ProtocolPropertyAccessorsOrderRule.swift in Sources */, 92CCB2D71E1EEFA300C8E5A3 /* UnusedOptionalBindingRule.swift in Sources */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 40c9e004fec..f0525b4df51 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -388,6 +388,7 @@ extension RulesTests { ("testPrivateUnitTest", testPrivateUnitTest), ("testProhibitedSuper", testProhibitedSuper), ("testProtocolPropertyAccessorsOrder", testProtocolPropertyAccessorsOrder), + ("testQuickDiscouragedCall", testQuickDiscouragedCall), ("testRedundantDiscardableLet", testRedundantDiscardableLet), ("testRedundantNilCoalescing", testRedundantNilCoalescing), ("testRedundantOptionalInitialization", testRedundantOptionalInitialization), diff --git a/Tests/SwiftLintFrameworkTests/RulesTests.swift b/Tests/SwiftLintFrameworkTests/RulesTests.swift index 3c0713b3d46..ff9560830c4 100644 --- a/Tests/SwiftLintFrameworkTests/RulesTests.swift +++ b/Tests/SwiftLintFrameworkTests/RulesTests.swift @@ -237,6 +237,10 @@ class RulesTests: XCTestCase { verifyRule(ProtocolPropertyAccessorsOrderRule.description) } + func testQuickDiscouragedCall() { + verifyRule(QuickDiscouragedCallRule.description) + } + func testRedundantDiscardableLet() { verifyRule(RedundantDiscardableLetRule.description) }