diff --git a/CHANGELOG.md b/CHANGELOG.md index 968431bc00..73c18130f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ [Martin Redington](https://github.com/mildm8nnered) [#5552](https://github.com/realm/SwiftLint/issues/5552) -* Add `no_empty_block` default rule to validate that code blocks are not empty. +* Add `no_empty_block` opt-in rule to validate that code blocks are not empty. They should at least contain a comment. [Ueeek](https://github.com/Ueeek) [#5615](https://github.com/realm/SwiftLint/issues/5615) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoEmptyBlockRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoEmptyBlockRule.swift index 2ed5b456f1..51efc92461 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoEmptyBlockRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/NoEmptyBlockRule.swift @@ -1,8 +1,9 @@ +import SwiftLintCore import SwiftSyntax @SwiftSyntaxRule -struct NoEmptyBlockRule: Rule { - var configuration = SeverityConfiguration(.warning) +struct NoEmptyBlockRule: OptInRule { + var configuration = NoEmptyBlockConfiguration() static let description = RuleDescription( identifier: "no_empty_block", @@ -11,85 +12,122 @@ struct NoEmptyBlockRule: Rule { kind: .idiomatic, nonTriggeringExamples: [ Example(""" + func f() { + /* do something */ + } + var flag = true { willSet { /* do something */ } } """), + Example(""" - do { - /* do something */ - } catch { - /* do something */ + class Apple { + init() { /* do something */ } + + deinit { /* do something */ } } """), + Example(""" - defer { + for _ in 0..<10 { /* do something */ } + + do { + /* do something */ + } catch { /* do something */ } - """), - Example(""" - deinit { /* do something */ } - """), - Example(""" - for _ in 0..<10 { /* do something */ } - """), - Example(""" + func f() { - /* do something */ + defer { + /* do something */ + } + print("other code") } - """), - Example(""" + if flag { /* do something */ } else { /* do something */ } + + repeat { /* do something */ } while (flag) + + while i < 10 { /* do something */ } """), + Example(""" - init() { /* do something */ } - """), + func f() {} + + var flag = true { + willSet {} + } + """, configuration: ["disabled_block_types": ["function_bodies"]]), + Example(""" - repeat { /* do something */ } while (flag) - """), + class Apple { + init() {} + + deinit {} + } + """, configuration: ["disabled_block_types": ["initializer_bodies"]]), + Example(""" - while i < 10 { /* do something */ } - """), + for _ in 0..<10 {} + + do { + } catch { + } + + func f() { + defer {} + print("other code") + } + + if flag { + } else { + } + + repeat {} while (flag) + + while i < 10 {} + """, configuration: ["disabled_block_types": ["statement_blocks"]]), ], triggeringExamples: [ Example(""" + func f() ↓{} + var flag = true { willSet ↓{} } """), + Example(""" - do ↓{ - } catch ↓{ + class Apple { + init() ↓{} + + deinit ↓{} } """), - Example(""" - defer ↓{} - """), - Example(""" - deinit ↓{} - """), + Example(""" for _ in 0..<10 ↓{} - """), - Example(""" - func f() ↓{} - """), - Example(""" + + do ↓{ + } catch ↓{ + } + + func f() { + defer ↓{} + print("other code") + } + if flag ↓{ } else ↓{ } - """), - Example(""" - init() ↓{} - """), - Example(""" + repeat ↓{} while (flag) - """), - Example(""" + while i < 10 ↓{} """), ] @@ -99,16 +137,36 @@ struct NoEmptyBlockRule: Rule { private extension NoEmptyBlockRule { final class Visitor: ViolationsSyntaxVisitor { override func visitPost(_ node: CodeBlockSyntax) { - // No need to show a warning since Empty Block of `guard` is compile error. - guard node.parent?.kind != .guardStmt else { return } + if let codeBlockType = node.codeBlockType, configuration.enabledBlockTypes.contains(codeBlockType) { + validate(node: node) + } + } + func validate(node: CodeBlockSyntax) { guard node.statements.isEmpty, !node.leftBrace.trailingTrivia.containsComments, !node.rightBrace.leadingTrivia.containsComments else { return } - violations.append(node.leftBrace.positionAfterSkippingLeadingTrivia) } } } + +private extension CodeBlockSyntax { + var codeBlockType: NoEmptyBlockConfiguration.CodeBlockType? { + switch parent?.kind { + case .functionDecl, .accessorDecl: + .functionBodies + case .initializerDecl, .deinitializerDecl: + .initializerBodies + case .forStmt, .doStmt, .whileStmt, .repeatStmt, .ifExpr, .catchClause, .deferStmt: + .statementBlocks + case .guardStmt: + // No need to handle this case since Empty Block of `guard` is compile error. + nil + default: + nil + } + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/NoEmptyBlockConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/NoEmptyBlockConfiguration.swift new file mode 100644 index 0000000000..1e8625c4bc --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/NoEmptyBlockConfiguration.swift @@ -0,0 +1,25 @@ +import SwiftLintCore + +@AutoApply +struct NoEmptyBlockConfiguration: SeverityBasedRuleConfiguration { + typealias Parent = NoEmptyBlockRule + + @MakeAcceptableByConfigurationElement + enum CodeBlockType: String, CaseIterable { + case functionBodies = "function_bodies" + case initializerBodies = "initializer_bodies" + case statementBlocks = "statement_blocks" + + static let all = Set(allCases) + } + + @ConfigurationElement(key: "severity") + private(set) var severityConfiguration = SeverityConfiguration(.warning) + + @ConfigurationElement(key: "disabled_block_types") + private(set) var disabledBlockTypes: [CodeBlockType] = [] + + var enabledBlockTypes: Set { + CodeBlockType.all.subtracting(disabledBlockTypes) + } +} diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index 653b753cf5..990c7b2e64 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -336,6 +336,7 @@ nimble_operator: severity: warning no_empty_block: severity: warning + disabled_block_types: [] no_extension_access_modifier: severity: error no_fallthrough_only: diff --git a/Tests/SwiftLintFrameworkTests/NoEmptyBlockConfigurationTests.swift b/Tests/SwiftLintFrameworkTests/NoEmptyBlockConfigurationTests.swift new file mode 100644 index 0000000000..39168dfd30 --- /dev/null +++ b/Tests/SwiftLintFrameworkTests/NoEmptyBlockConfigurationTests.swift @@ -0,0 +1,54 @@ +@testable import SwiftLintBuiltInRules +@testable import SwiftLintCore +import XCTest + +final class NoEmptyBlockConfigurationTests: SwiftLintTestCase { + func testDefaultConfiguration() { + let config = NoEmptyBlockConfiguration() + XCTAssertEqual(config.severityConfiguration.severity, .warning) + XCTAssertEqual(config.enabledBlockTypes, NoEmptyBlockConfiguration.CodeBlockType.all) + } + + func testApplyingCustomConfiguration() throws { + var config = NoEmptyBlockConfiguration() + try config.apply( + configuration: [ + "severity": "error", + "disabled_block_types": ["function_bodies"], + ] as [String: any Sendable] + ) + XCTAssertEqual(config.severityConfiguration.severity, .error) + XCTAssertEqual(config.enabledBlockTypes, Set([.initializerBodies, .statementBlocks])) + } + + func testInvalidKeyInCustomConfiguration() { + var config = NoEmptyBlockConfiguration() + XCTAssertEqual( + try Issue.captureConsole { try config.apply(configuration: ["invalidKey": "error"]) }, + "warning: Configuration for 'no_empty_block' rule contains the invalid key(s) 'invalidKey'." + ) + } + + func testInvalidTypeOfCustomConfiguration() { + var config = NoEmptyBlockConfiguration() + checkError(Issue.invalidConfiguration(ruleID: NoEmptyBlockRule.description.identifier)) { + try config.apply(configuration: "invalidKey") + } + } + + func testInvalidTypeOfValueInCustomConfiguration() { + var config = NoEmptyBlockConfiguration() + checkError(Issue.invalidConfiguration(ruleID: NoEmptyBlockRule.description.identifier)) { + try config.apply(configuration: ["severity": "foo"]) + } + } + + func testConsoleDescription() throws { + var config = NoEmptyBlockConfiguration() + try config.apply(configuration: ["disabled_block_types": ["initializer_bodies", "statement_blocks"]]) + XCTAssertEqual( + RuleConfigurationDescription.from(configuration: config).oneLiner(), + "severity: warning; disabled_block_types: [initializer_bodies, statement_blocks]" + ) + } +}