diff --git a/.gitignore b/.gitignore index e6d5048be..41cde72e5 100644 --- a/.gitignore +++ b/.gitignore @@ -70,3 +70,7 @@ bin/ # Danger osscheck/ oss-check-summary.md + + +# VS Code +.vscode \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 3daa4a24f..f9a0188e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -98,6 +98,11 @@ [SimplyDanny](https://github.com/SimplyDanny) [#5849](https://github.com/realm/SwiftLint/issues/5849) +* Add `implicit_optional_initialization` rule to enforce implicit or explicit + initialization of optional variables, configurable via `style: always | never`. + [@leolem](https://github.com/leo-lem) + [#4943](https://github.com/realm/SwiftLint/issues/4943) + ### Bug Fixes * Fix `no_extension_access_modifier` rule incorrectly triggering for `nonisolated extension`. diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index bea8255b9..62b1ad065 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -87,6 +87,7 @@ public let builtInRules: [any Rule.Type] = [ IdenticalOperandsRule.self, IdentifierNameRule.self, ImplicitGetterRule.self, + ImplicitOptionalInitializationRule.self, ImplicitReturnRule.self, ImplicitlyUnwrappedOptionalRule.self, InclusiveLanguageRule.self, @@ -174,7 +175,6 @@ public let builtInRules: [any Rule.Type] = [ RedundantDiscardableLetRule.self, RedundantNilCoalescingRule.self, RedundantObjcAttributeRule.self, - RedundantOptionalInitializationRule.self, RedundantSelfInClosureRule.self, RedundantSendableRule.self, RedundantSetAccessControlRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/RedundantOptionalInitializationRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/RedundantOptionalInitializationRule.swift deleted file mode 100644 index 2c689fd39..000000000 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/RedundantOptionalInitializationRule.swift +++ /dev/null @@ -1,193 +0,0 @@ -import SwiftSyntax - -@SwiftSyntaxRule(explicitRewriter: true) -struct RedundantOptionalInitializationRule: Rule { - var configuration = SeverityConfiguration(.warning) - - static let description = RuleDescription( - identifier: "redundant_optional_initialization", - name: "Redundant Optional Initialization", - description: "Initializing an optional variable with nil is redundant", - kind: .idiomatic, - nonTriggeringExamples: [ - Example("var myVar: Int?"), - Example("let myVar: Int? = nil"), - Example("var myVar: Int? = 0"), - Example("func foo(bar: Int? = 0) { }"), - Example("var myVar: Optional"), - Example("let myVar: Optional = nil"), - Example("var myVar: Optional = 0"), - // properties with body should be ignored - Example(""" - var foo: Int? { - if bar != nil { } - return 0 - } - """), - // properties with a closure call - Example(""" - var foo: Int? = { - if bar != nil { } - return 0 - }() - """), - // lazy variables need to be initialized - Example("lazy var test: Int? = nil"), - // local variables - Example(""" - func funcName() { - var myVar: String? - } - """), - Example(""" - func funcName() { - let myVar: String? = nil - } - """), - ], - triggeringExamples: triggeringExamples, - corrections: corrections - ) - - private static let triggeringExamples: [Example] = [ - Example("var myVar: Int?↓ = nil"), - Example("var myVar: Optional↓ = nil"), - Example("var myVar: Int?↓=nil"), - Example("var myVar: Optional↓=nil\n)"), - Example(""" - var myVar: String?↓ = nil { - didSet { print("didSet") } - } - """), - Example(""" - func funcName() { - var myVar: String?↓ = nil - } - """), - ] - - private static let corrections: [Example: Example] = [ - Example("var myVar: Int?↓ = nil"): Example("var myVar: Int?"), - Example("var myVar: Optional↓ = nil"): Example("var myVar: Optional"), - Example("var myVar: Int?↓=nil"): Example("var myVar: Int?"), - Example("var myVar: Optional↓=nil"): Example("var myVar: Optional"), - Example("class C {\n#if true\nvar myVar: Int?↓ = nil\n#endif\n}"): - Example("class C {\n#if true\nvar myVar: Int?\n#endif\n}"), - Example(""" - var myVar: Int?↓ = nil { - didSet { } - } - """): - Example(""" - var myVar: Int? { - didSet { } - } - """), - Example(""" - var myVar: Int?↓=nil{ - didSet { } - } - """): - Example(""" - var myVar: Int?{ - didSet { } - } - """), - Example(""" - func foo() { - var myVar: String?↓ = nil, b: Int - } - """): - Example(""" - func foo() { - var myVar: String?, b: Int - } - """), - ] -} - -private extension RedundantOptionalInitializationRule { - final class Visitor: ViolationsSyntaxVisitor { - override func visitPost(_ node: VariableDeclSyntax) { - guard node.bindingSpecifier.tokenKind == .keyword(.var), - !node.modifiers.contains(keyword: .lazy) else { - return - } - - violations.append(contentsOf: node.bindings.compactMap(\.violationPosition)) - } - } - - final class Rewriter: ViolationsSyntaxRewriter { - override func visitAny(_: Syntax) -> Syntax? { nil } - - override func visit(_ node: VariableDeclSyntax) -> DeclSyntax { - guard node.bindingSpecifier.tokenKind == .keyword(.var), - !node.modifiers.contains(keyword: .lazy) else { - return super.visit(node) - } - - let violations = node.bindings - .compactMap { binding in - binding.violationPosition.map { ($0, binding) } - } - .filter { position, _ in - !position.isContainedIn(regions: disabledRegions, locationConverter: locationConverter) - } - - guard violations.isNotEmpty else { - return super.visit(node) - } - numberOfCorrections += violations.count - let violatingBindings = violations.map(\.1) - let newBindings = PatternBindingListSyntax(node.bindings.map { binding in - guard violatingBindings.contains(binding) else { - return binding - } - let newBinding = binding.with(\.initializer, nil) - if newBinding.accessorBlock != nil { - return newBinding - } - if binding.trailingComma != nil { - return newBinding.with(\.typeAnnotation, binding.typeAnnotation?.with(\.trailingTrivia, Trivia())) - } - return newBinding.with(\.trailingTrivia, binding.initializer?.trailingTrivia ?? Trivia()) - }) - - return super.visit(node.with(\.bindings, newBindings)) - } - } -} - -private extension PatternBindingSyntax { - var violationPosition: AbsolutePosition? { - guard let initializer, - let type = typeAnnotation, - initializer.isInitializingToNil, - type.isOptionalType else { - return nil - } - - return type.endPositionBeforeTrailingTrivia - } -} - -private extension InitializerClauseSyntax { - var isInitializingToNil: Bool { - value.is(NilLiteralExprSyntax.self) - } -} - -private extension TypeAnnotationSyntax { - var isOptionalType: Bool { - if type.is(OptionalTypeSyntax.self) { - return true - } - - if let type = type.as(IdentifierTypeSyntax.self), let genericClause = type.genericArgumentClause { - return genericClause.arguments.count == 1 && type.name.text == "Optional" - } - - return false - } -} diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/ImplicitOptionalInitializationConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/ImplicitOptionalInitializationConfiguration.swift new file mode 100644 index 000000000..34a8ab346 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/ImplicitOptionalInitializationConfiguration.swift @@ -0,0 +1,17 @@ +import SwiftLintCore + +@AutoConfigParser +struct ImplicitOptionalInitializationConfiguration: SeverityBasedRuleConfiguration { // swiftlint:disable:this type_name + typealias Parent = ImplicitOptionalInitializationRule + + @AcceptableByConfigurationElement + enum Style: String { + case always + case never + } + + @ConfigurationElement(key: "severity") + private(set) var severityConfiguration = SeverityConfiguration(.warning) + @ConfigurationElement(key: "style") + private(set) var style: Style = .always +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/ImplicitOptionalInitializationRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/ImplicitOptionalInitializationRule.swift new file mode 100644 index 000000000..402c21dd6 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Style/ImplicitOptionalInitializationRule.swift @@ -0,0 +1,125 @@ +import SwiftSyntax + +@SwiftSyntaxRule(explicitRewriter: true) +struct ImplicitOptionalInitializationRule: Rule { + var configuration = ImplicitOptionalInitializationConfiguration() + + static let description = RuleDescription( + identifier: "implicit_optional_initialization", + name: "Implicit Optional Initialization", + description: "Optionals should be consistently initialized, either with `= nil` or without.", + kind: .style, + nonTriggeringExamples: ImplicitOptionalInitializationRuleExamples.nonTriggeringExamples, + triggeringExamples: ImplicitOptionalInitializationRuleExamples.triggeringExamples, + corrections: ImplicitOptionalInitializationRuleExamples.corrections, + deprecatedAliases: ["redundant_optional_initialization"] + ) +} + +private extension ImplicitOptionalInitializationRule { + final class Visitor: ViolationsSyntaxVisitor { + var reason: String { + switch configuration.style { + case .always: "Optional should be implicitly initialized without nil" + case .never: "Optional should be explicitly initialized to nil" + } + } + + override func visitPost(_ node: PatternBindingSyntax) { + guard let violationPosition = node.violationPosition(for: configuration.style) else { return } + + violations.append(ReasonedRuleViolation(position: violationPosition, reason: reason)) + } + } +} + +private extension ImplicitOptionalInitializationRule { + final class Rewriter: ViolationsSyntaxRewriter { + override func visit(_ node: PatternBindingSyntax) -> PatternBindingSyntax { + guard node.violationPosition(for: configuration.style) != nil else { + return super.visit(node) + } + + self.numberOfCorrections += 1 + + return switch configuration.style { + case .never: + node + .with( + \.initializer, + InitializerClauseSyntax( + equal: .equalToken( + leadingTrivia: .space, + trailingTrivia: .space + ), + value: ExprSyntax(NilLiteralExprSyntax(nilKeyword: .keyword(.nil))), + trailingTrivia: node.typeAnnotation?.trailingTrivia ?? Trivia() + ) + ) + .with(\.typeAnnotation, node.typeAnnotation?.with(\.trailingTrivia, Trivia())) + case .always: + node + .with(\.initializer, nil) + .with( + \.trailingTrivia, + node.accessorBlock == nil + ? node.initializer?.trailingTrivia ?? Trivia() + : node.trailingTrivia + ) + } + } + } +} + +private extension PatternBindingSyntax { + func violationPosition( + for style: ImplicitOptionalInitializationConfiguration.Style + ) -> AbsolutePosition? { + guard + let parent = parent?.parent?.as(VariableDeclSyntax.self), + parent.bindingSpecifier.tokenKind == .keyword(.var), + !parent.modifiers.contains(keyword: .lazy), + let typeAnnotation, + typeAnnotation.isOptionalType + else { return nil } + + // ignore properties with accessors unless they have only willSet or didSet + if let accessorBlock { + if let accessors = accessorBlock.accessors.as(AccessorDeclListSyntax.self) { + if accessors.contains(where: { + $0.accessorSpecifier.tokenKind != .keyword(.willSet) + && $0.accessorSpecifier.tokenKind != .keyword(.didSet) + }) { // we have more than willSet or didSet + return nil + } + } else { // code block, i.e. getter + return nil + } + } + + if (style == .never && !initializer.isNil) || (style == .always && initializer.isNil) { + return positionAfterSkippingLeadingTrivia + } + + return nil + } +} + +private extension InitializerClauseSyntax? { + var isNil: Bool { + self?.value.is(NilLiteralExprSyntax.self) ?? false + } +} + +private extension TypeAnnotationSyntax { + var isOptionalType: Bool { + if type.is(OptionalTypeSyntax.self) { return true } + + if let type = type.as(IdentifierTypeSyntax.self), + let genericClause = type.genericArgumentClause { + return genericClause.arguments.count == 1 && type.name.text == "Optional" + } + + return false + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/ImplicitOptionalInitializationRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Style/ImplicitOptionalInitializationRuleExamples.swift new file mode 100644 index 000000000..a985f9787 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Style/ImplicitOptionalInitializationRuleExamples.swift @@ -0,0 +1,160 @@ +enum ImplicitOptionalInitializationRuleExamples { // swiftlint:disable:this type_name + static let nonTriggeringExamples = [ + Example( // properties with body should be ignored + """ + var foo: Int? { + if bar != nil { } + return 0 + } + """), + Example( // properties with a closure call + """ + var foo: Int? = { + if bar != nil { } + return 0 + }() + """ + ), + Example("lazy var test: Int? = nil"), // lazy variables need to be initialized + Example("let myVar: String? = nil"), // let variables need to be initialized + Example("var myVar: Int? { nil }"), // computed properties should be ignored + Example("var x: Int? = 1"), // initialized with a value + + // never style + Example("private var myVar: Int? = nil", configuration: ["style": "never"]), + Example("var myVar: Optional = nil", configuration: ["style": "never"]), + Example( + "var myVar: Int? { nil }, myOtherVar: Int? = nil", configuration: ["style": "never"] + ), + Example( + """ + var myVar: String? = nil { + didSet { print("didSet") } + } + """, configuration: ["style": "never"]), + Example( + """ + func funcName() { + var myVar: String? = nil + } + """, configuration: ["style": "never"]), + Example("var x: Int? = nil // comment", configuration: ["style": "never"]), // with comment after + + // always style + Example("public var myVar: Int?", configuration: ["style": "always"]), + Example("var myVar: Optional", configuration: ["style": "always"]), + Example( + "var myVar: Int? { nil }, myOtherVar: Int?", configuration: ["style": "always"]), + Example( + """ + var myVar: String? { + didSet { print("didSet") } + } + """, configuration: ["style": "always"]), + Example( + """ + func funcName() { + var myVar: String? + } + """, configuration: ["style": "always"]), + Example("var x: Int? // comment", configuration: ["style": "always"]), // with comment after + ] + + static let triggeringExamples = [ + // never style + Example("var ↓myVar: Int? ", configuration: ["style": "never"]), + Example("var ↓myVar: Optional ", configuration: ["style": "never"]), + Example("var myVar: Int? = nil, ↓myOtherVar: Int? ", configuration: ["style": "never"]), + Example( + """ + var ↓myVar: String? { + didSet { print("didSet") } + } + """, configuration: ["style": "never"]), + Example( + """ + func funcName() { + var ↓myVar: String? + } + """, configuration: ["style": "never"] + ), + + // always style + Example("var ↓myVar: Int? = nil", configuration: ["style": "always"]), + Example("var ↓myVar: Optional = nil", configuration: ["style": "always"]), + Example("var myVar: Int?, ↓myOtherVar: Int? = nil", configuration: ["style": "always"]), + Example( + """ + var ↓myVar: String? = nil { + didSet { print("didSet") } + } + """, configuration: ["style": "always"]), + Example( + """ + func funcName() { + var ↓myVar: String? = nil + } + """, configuration: ["style": "always"]), + ] + + static let corrections = [ + // never style + Example("var ↓myVar: Int? // comment", configuration: ["style": "never"]): + Example("var myVar: Int? = nil // comment"), + Example("var ↓myVar: Optional // comment", configuration: ["style": "never"]): + Example("var myVar: Optional = nil // comment"), + Example( + """ + var ↓myVar: String? { + didSet { print("didSet") } + } + """, configuration: ["style": "never"]): + Example( + """ + var myVar: String? = nil { + didSet { print("didSet") } + } + """), + Example( + """ + func funcName() { + var ↓myVar: String? + } + """, configuration: ["style": "never"] + ): Example( + """ + func funcName() { + var myVar: String? = nil + } + """), + + Example("var ↓myVar: Int? = nil // comment", configuration: ["style": "always"]): + Example("var myVar: Int? // comment"), + Example("var ↓myVar: Optional = nil // comment", configuration: ["style": "always"]): + Example("var myVar: Optional // comment"), + Example( + """ + var ↓myVar: String? = nil { + didSet { print("didSet") } + } + """, configuration: ["style": "always"]): + Example( + """ + var myVar: String? { + didSet { print("didSet") } + } + """), + Example( + """ + func funcName() { + var ↓myVar: String? = nil + } + """, configuration: ["style": "always"]): + Example( + """ + func funcName() { + var myVar: String? + } + """), + ] +} diff --git a/Tests/GeneratedTests/GeneratedTests_04.swift b/Tests/GeneratedTests/GeneratedTests_04.swift index 4cdd72ea8..1cca943d6 100644 --- a/Tests/GeneratedTests/GeneratedTests_04.swift +++ b/Tests/GeneratedTests/GeneratedTests_04.swift @@ -67,6 +67,12 @@ final class ImplicitGetterRuleGeneratedTests: SwiftLintTestCase { } } +final class ImplicitOptionalInitializationRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(ImplicitOptionalInitializationRule.description) + } +} + final class ImplicitReturnRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(ImplicitReturnRule.description) @@ -150,9 +156,3 @@ final class LegacyHashingRuleGeneratedTests: SwiftLintTestCase { verifyRule(LegacyHashingRule.description) } } - -final class LegacyMultipleRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(LegacyMultipleRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_05.swift b/Tests/GeneratedTests/GeneratedTests_05.swift index 751aa6087..00a334ad6 100644 --- a/Tests/GeneratedTests/GeneratedTests_05.swift +++ b/Tests/GeneratedTests/GeneratedTests_05.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class LegacyMultipleRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(LegacyMultipleRule.description) + } +} + final class LegacyNSGeometryFunctionsRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(LegacyNSGeometryFunctionsRule.description) @@ -150,9 +156,3 @@ final class NimbleOperatorRuleGeneratedTests: SwiftLintTestCase { verifyRule(NimbleOperatorRule.description) } } - -final class NoEmptyBlockRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(NoEmptyBlockRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_06.swift b/Tests/GeneratedTests/GeneratedTests_06.swift index 014ababfd..78a240aab 100644 --- a/Tests/GeneratedTests/GeneratedTests_06.swift +++ b/Tests/GeneratedTests/GeneratedTests_06.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class NoEmptyBlockRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(NoEmptyBlockRule.description) + } +} + final class NoExtensionAccessModifierRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(NoExtensionAccessModifierRule.description) @@ -150,9 +156,3 @@ final class PreferNimbleRuleGeneratedTests: SwiftLintTestCase { verifyRule(PreferNimbleRule.description) } } - -final class PreferSelfInStaticReferencesRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(PreferSelfInStaticReferencesRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_07.swift b/Tests/GeneratedTests/GeneratedTests_07.swift index e503444da..a27cb25e9 100644 --- a/Tests/GeneratedTests/GeneratedTests_07.swift +++ b/Tests/GeneratedTests/GeneratedTests_07.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class PreferSelfInStaticReferencesRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(PreferSelfInStaticReferencesRule.description) + } +} + final class PreferSelfTypeOverTypeOfSelfRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(PreferSelfTypeOverTypeOfSelfRule.description) @@ -139,12 +145,6 @@ final class RedundantObjcAttributeRuleGeneratedTests: SwiftLintTestCase { } } -final class RedundantOptionalInitializationRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(RedundantOptionalInitializationRule.description) - } -} - final class RedundantSelfInClosureRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(RedundantSelfInClosureRule.description) diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index 094ad89c0..4394bb530 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -497,6 +497,12 @@ implicit_getter: meta: opt-in: false correctable: false +implicit_optional_initialization: + severity: warning + style: always + meta: + opt-in: false + correctable: true implicit_return: severity: warning included: [closure, function, getter, initializer, subscript] @@ -988,11 +994,6 @@ redundant_objc_attribute: meta: opt-in: false correctable: true -redundant_optional_initialization: - severity: warning - meta: - opt-in: false - correctable: true redundant_self_in_closure: severity: warning meta: