diff --git a/CHANGELOG.md b/CHANGELOG.md index b6b56c21bab..6e57381f56c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ * Improve performance of `line_length` rule. [Marcelo Fabri](https://github.com/marcelofabri) +* Add `closure_body_length` rule to enforce the maximum number + of lines a closure should have. Requires Swift 4.2. + [Ornithologist Coder](https://github.com/ornithocoder) + [#52](https://github.com/realm/SwiftLint/issues/52) + #### Bug Fixes * None. diff --git a/Rules.md b/Rules.md index 4bb9f5794b4..c4fea8ba2eb 100644 --- a/Rules.md +++ b/Rules.md @@ -7,6 +7,7 @@ * [Block Based KVO](#block-based-kvo) * [Class Delegate Protocol](#class-delegate-protocol) * [Closing Brace Spacing](#closing-brace-spacing) +* [Closure Body Length](#closure-body-length) * [Closure End Indentation](#closure-end-indentation) * [Closure Parameter Position](#closure-parameter-position) * [Closure Spacing](#closure-spacing) @@ -828,6 +829,559 @@ Closing brace with closing parenthesis should not have any whitespaces in the mi +## Closure Body Length + +Identifier | Enabled by default | Supports autocorrection | Kind | Minimum Swift Compiler Version +--- | --- | --- | --- | --- +`closure_body_length` | Disabled | No | metrics | 4.2.0 + +Closure bodies should not span too many lines. + +### Examples + +
+Non Triggering Examples + +```swift +foo.bar { $0 } +``` + +```swift +foo.bar { toto in +} +``` + +```swift +foo.bar { toto in + let a = 0 + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + + + + + + + + + + +} +``` + +```swift +foo.bar { toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +} +``` + +```swift +foo.bar { toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + + + + + + + + + + +} +``` + +```swift +foo.bar({ toto in +}) +``` + +```swift +foo.bar({ toto in + let a = 0 +}) +``` + +```swift +foo.bar({ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}) +``` + +```swift +foo.bar(label: { toto in +}) +``` + +```swift +foo.bar(label: { toto in + let a = 0 +}) +``` + +```swift +foo.bar(label: { toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}) +``` + +```swift +foo.bar(label: { toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}, anotherLabel: { toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}) +``` + +```swift +foo.bar(label: { toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}) { toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +} +``` + +```swift +let foo: Bar = { toto in + let bar = Bar() + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + return bar +}() +``` + +
+
+Triggering Examples + +```swift +foo.bar ↓{ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +} +``` + +```swift +foo.bar ↓{ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + // toto + + + + + + + + + + +} +``` + +```swift +foo.bar(↓{ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}) +``` + +```swift +foo.bar(label: ↓{ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}) +``` + +```swift +foo.bar(label: ↓{ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}, anotherLabel: ↓{ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}) +``` + +```swift +foo.bar(label: ↓{ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +}) ↓{ toto in + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 +} +``` + +```swift +let foo: Bar = ↓{ toto in + let bar = Bar() + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + let a = 0 + return bar +}() +``` + +
+ + + ## Closure End Indentation Identifier | Enabled by default | Supports autocorrection | Kind | Minimum Swift Compiler Version diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index bf5f343447a..b49d85ef073 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -8,6 +8,7 @@ public let masterRuleList = RuleList(rules: [ BlockBasedKVORule.self, ClassDelegateProtocolRule.self, ClosingBraceRule.self, + ClosureBodyLengthRule.self, ClosureEndIndentationRule.self, ClosureParameterPositionRule.self, ClosureSpacingRule.self, diff --git a/Source/SwiftLintFramework/Rules/Metrics/ClosureBodyLengthRule.swift b/Source/SwiftLintFramework/Rules/Metrics/ClosureBodyLengthRule.swift new file mode 100644 index 00000000000..930d6800cf3 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/Metrics/ClosureBodyLengthRule.swift @@ -0,0 +1,50 @@ +import SourceKittenFramework + +public struct ClosureBodyLengthRule: OptInRule, ASTRule, ConfigurationProviderRule, AutomaticTestableRule { + public var configuration = SeverityLevelsConfiguration(warning: 20, error: 100) + + public init() {} + + public static let description = RuleDescription( + identifier: "closure_body_length", + name: "Closure Body Length", + description: "Closure bodies should not span too many lines.", + kind: .metrics, + minSwiftVersion: .fourDotTwo, + nonTriggeringExamples: ClosureBodyLengthRuleExamples.nonTriggeringExamples, + triggeringExamples: ClosureBodyLengthRuleExamples.triggeringExamples + ) + + // MARK: - ASTRule + + public func validate(file: File, + kind: SwiftExpressionKind, + dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { + guard + kind == .closure, + let offset = dictionary.offset, + let bodyOffset = dictionary.bodyOffset, + let bodyLength = dictionary.bodyLength, + let startLine = file.contents.bridge().lineAndCharacter(forByteOffset: bodyOffset)?.line, + let endLine = file.contents.bridge().lineAndCharacter(forByteOffset: bodyOffset + bodyLength)?.line + else { + return [] + } + + return configuration.params.compactMap { parameter in + let (exceeds, lineCount) = file.exceedsLineCountExcludingCommentsAndWhitespace(startLine, + endLine, + parameter.value) + + guard exceeds else { return nil } + + let reason = "Closure body should span \(configuration.warning) lines or less " + + "excluding comments and whitespace: currently spans \(lineCount) lines" + + return StyleViolation(ruleDescription: type(of: self).description, + severity: parameter.severity, + location: Location(file: file, byteOffset: offset), + reason: reason) + } + } +} diff --git a/Source/SwiftLintFramework/Rules/Metrics/ClosureBodyLengthRuleExamples.swift b/Source/SwiftLintFramework/Rules/Metrics/ClosureBodyLengthRuleExamples.swift new file mode 100644 index 00000000000..ce1f62f47ce --- /dev/null +++ b/Source/SwiftLintFramework/Rules/Metrics/ClosureBodyLengthRuleExamples.swift @@ -0,0 +1,83 @@ +import Foundation + +internal struct ClosureBodyLengthRuleExamples { + static let nonTriggeringExamples: [String] = [ + singleLineClosure(), + trailingClosure(codeLinesCount: 0, commentLinesCount: 0, emptyLinesCount: 0), + trailingClosure(codeLinesCount: 1, commentLinesCount: 10, emptyLinesCount: 10), + trailingClosure(codeLinesCount: 19, commentLinesCount: 0, emptyLinesCount: 0), + trailingClosure(codeLinesCount: 19, commentLinesCount: 10, emptyLinesCount: 10), + argumentClosure(codeLinesCount: 0), + argumentClosure(codeLinesCount: 1), + argumentClosure(codeLinesCount: 19), + labeledArgumentClosure(codeLinesCount: 0), + labeledArgumentClosure(codeLinesCount: 1), + labeledArgumentClosure(codeLinesCount: 19), + multiLabeledArgumentClosures(codeLinesCount: 19), + labeledAndTrailingClosures(codeLinesCount: 19), + lazyInitialization(codeLinesCount: 18) + ] + + static let triggeringExamples: [String] = [ + trailingClosure("↓", codeLinesCount: 21, commentLinesCount: 0, emptyLinesCount: 0), + trailingClosure("↓", codeLinesCount: 21, commentLinesCount: 10, emptyLinesCount: 10), + argumentClosure("↓", codeLinesCount: 21), + labeledArgumentClosure("↓", codeLinesCount: 21), + multiLabeledArgumentClosures("↓", codeLinesCount: 21), + labeledAndTrailingClosures("↓", codeLinesCount: 21), + lazyInitialization("↓", codeLinesCount: 19) + ] +} + +// MARK: - Private + +private func singleLineClosure() -> String { + return "foo.bar { $0 }" +} + +private func trailingClosure(_ violationSymbol: String = "", + codeLinesCount: Int, + commentLinesCount: Int, + emptyLinesCount: Int) -> String { + return "foo.bar \(violationSymbol){ toto in\n" + + repeatElement("\tlet a = 0\n", count: codeLinesCount).joined() + + repeatElement("\t// toto\n", count: commentLinesCount).joined() + + repeatElement("\t\n", count: emptyLinesCount).joined() + + "}" +} + +private func argumentClosure(_ violationSymbol: String = "", codeLinesCount: Int) -> String { + return "foo.bar(\(violationSymbol){ toto in\n" + + repeatElement("\tlet a = 0\n", count: codeLinesCount).joined() + + "})" +} + +private func labeledArgumentClosure(_ violationSymbol: String = "", codeLinesCount: Int) -> String { + return "foo.bar(label: \(violationSymbol){ toto in\n" + + repeatElement("\tlet a = 0\n", count: codeLinesCount).joined() + + "})" +} + +private func multiLabeledArgumentClosures(_ violationSymbol: String = "", codeLinesCount: Int) -> String { + return "foo.bar(label: \(violationSymbol){ toto in\n" + + repeatElement("\tlet a = 0\n", count: codeLinesCount).joined() + + "}, anotherLabel: \(violationSymbol){ toto in\n" + + repeatElement("\tlet a = 0\n", count: codeLinesCount).joined() + + "})" +} + +private func labeledAndTrailingClosures(_ violationSymbol: String = "", codeLinesCount: Int) -> String { + return "foo.bar(label: \(violationSymbol){ toto in\n" + + repeatElement("\tlet a = 0\n", count: codeLinesCount).joined() + + "}) \(violationSymbol){ toto in\n" + + repeatElement("\tlet a = 0\n", count: codeLinesCount).joined() + + "}" +} + +private func lazyInitialization(_ violationSymbol: String = "", codeLinesCount: Int) -> String { + return "let foo: Bar = \(violationSymbol){ toto in\n" + + "\tlet bar = Bar()\n" + + repeatElement("\tlet a = 0\n", count: codeLinesCount).joined() + + "\treturn bar\n" + + "}()" +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 5d3f6fd5de8..9ac64047c6b 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -85,6 +85,8 @@ 623675B21F962FC4009BE6F3 /* QuickDiscouragedPendingTestRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = 623675B11F962FC4009BE6F3 /* QuickDiscouragedPendingTestRuleExamples.swift */; }; 623E36F01F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 623E36EF1F3DB1B1002E5B71 /* QuickDiscouragedCallRule.swift */; }; 623E36F21F3DB988002E5B71 /* QuickDiscouragedCallRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = 623E36F11F3DB988002E5B71 /* QuickDiscouragedCallRuleExamples.swift */; }; + 62426A032118BA6E007E6340 /* ClosureBodyLengthRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62426A022118BA6E007E6340 /* ClosureBodyLengthRule.swift */; }; + 62426A062118F995007E6340 /* ClosureBodyLengthRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62426A042118F991007E6340 /* ClosureBodyLengthRuleExamples.swift */; }; 6250D32A1ED4DFEB00735129 /* MultilineParametersRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6238AE411ED4D734006C3601 /* MultilineParametersRule.swift */; }; 6258783B1FFC458100AC34F2 /* DiscouragedObjectLiteralRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6258783A1FFC458100AC34F2 /* DiscouragedObjectLiteralRule.swift */; }; 62622F6B1F2F2E3500D5D099 /* DiscouragedDirectInitRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62622F6A1F2F2E3500D5D099 /* DiscouragedDirectInitRule.swift */; }; @@ -485,6 +487,8 @@ 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 /* QuickDiscouragedCallRuleExamples.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QuickDiscouragedCallRuleExamples.swift; sourceTree = ""; }; + 62426A022118BA6E007E6340 /* ClosureBodyLengthRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClosureBodyLengthRule.swift; sourceTree = ""; }; + 62426A042118F991007E6340 /* ClosureBodyLengthRuleExamples.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClosureBodyLengthRuleExamples.swift; sourceTree = ""; }; 6258783A1FFC458100AC34F2 /* DiscouragedObjectLiteralRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiscouragedObjectLiteralRule.swift; sourceTree = ""; }; 62622F6A1F2F2E3500D5D099 /* DiscouragedDirectInitRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiscouragedDirectInitRule.swift; sourceTree = ""; }; 62640150201552E0005B9C4A /* DiscouragedOptionalBooleanRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiscouragedOptionalBooleanRule.swift; sourceTree = ""; }; @@ -892,6 +896,8 @@ 8F2892B421176E1200691D58 /* Metrics */ = { isa = PBXGroup; children = ( + 62426A022118BA6E007E6340 /* ClosureBodyLengthRule.swift */, + 62426A042118F991007E6340 /* ClosureBodyLengthRuleExamples.swift */, 2E02005E1C54BF680024D09D /* CyclomaticComplexityRule.swift */, E88DEA891B0992B300A66CB0 /* FileLengthRule.swift */, E88DEA8F1B099A3100A66CB0 /* FunctionBodyLengthRule.swift */, @@ -1678,6 +1684,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 62426A062118F995007E6340 /* ClosureBodyLengthRuleExamples.swift in Sources */, 8B01E4FD20A41C8700C9233E /* FunctionParameterCountConfiguration.swift in Sources */, 740DF1B1203F62BB0081F694 /* EmptyStringRule.swift in Sources */, 4DB7815E1CAD72BA00BC4723 /* LegacyCGGeometryFunctionsRule.swift in Sources */, @@ -1878,6 +1885,7 @@ 3BD9CD3D1C37175B009A5D25 /* YamlParser.swift in Sources */, F22314B01D4FA4D7009AD165 /* LegacyNSGeometryFunctionsRule.swift in Sources */, E88DEA8C1B0999A000A66CB0 /* ASTRule.swift in Sources */, + 62426A032118BA6E007E6340 /* ClosureBodyLengthRule.swift in Sources */, 1E82D5591D7775C7009553D7 /* ClosureSpacingRule.swift in Sources */, E80746F61ECB722F00548D31 /* CacheDescriptionProvider.swift in Sources */, 094385041D5D4F7C009168CF /* PrivateOutletRule.swift in Sources */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 04df4a8f22a..f581909d918 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -45,6 +45,12 @@ extension ClosingBraceRuleTests { ] } +extension ClosureBodyLengthRuleTests { + static var allTests: [(String, (ClosureBodyLengthRuleTests) -> () throws -> Void)] = [ + ("testWithDefaultConfiguration", testWithDefaultConfiguration) + ] +} + extension ClosureEndIndentationRuleTests { static var allTests: [(String, (ClosureEndIndentationRuleTests) -> () throws -> Void)] = [ ("testWithDefaultConfiguration", testWithDefaultConfiguration) @@ -1233,6 +1239,7 @@ XCTMain([ testCase(BlockBasedKVORuleTests.allTests), testCase(ClassDelegateProtocolRuleTests.allTests), testCase(ClosingBraceRuleTests.allTests), + testCase(ClosureBodyLengthRuleTests.allTests), testCase(ClosureEndIndentationRuleTests.allTests), testCase(ClosureParameterPositionRuleTests.allTests), testCase(ClosureSpacingRuleTests.allTests), diff --git a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift index 210edec91a2..d267596207c 100644 --- a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift +++ b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift @@ -36,6 +36,12 @@ class ClosingBraceRuleTests: XCTestCase { } } +class ClosureBodyLengthRuleTests: XCTestCase { + func testWithDefaultConfiguration() { + verifyRule(ClosureBodyLengthRule.description) + } +} + class ClosureEndIndentationRuleTests: XCTestCase { func testWithDefaultConfiguration() { verifyRule(ClosureEndIndentationRule.description)