From 8b1ecf733de627bec3372158a063c7ee7833de88 Mon Sep 17 00:00:00 2001 From: alisher-zinullayev Date: Thu, 6 Aug 2026 16:32:11 +0500 Subject: [PATCH] Add defer_before_unstructured_task rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flags a `defer` block that assigns to shared state right before a sibling unstructured `Task` (or `Task.detached`) reads that same state. The deferred assignment runs the instant the enclosing synchronous scope returns, before the Task's body has a chance to run, inverting the intended order — most commonly seen as a loading flag being reset before the async work it guards has actually finished. Closes #6619 --- CHANGELOG.md | 8 + .../Models/BuiltInRules.swift | 1 + .../DeferBeforeUnstructuredTaskRule.swift | 258 ++++++++++++++++++ Tests/GeneratedTests/GeneratedTests_02.swift | 16 +- Tests/GeneratedTests/GeneratedTests_03.swift | 16 +- Tests/GeneratedTests/GeneratedTests_04.swift | 16 +- Tests/GeneratedTests/GeneratedTests_05.swift | 16 +- Tests/GeneratedTests/GeneratedTests_06.swift | 16 +- Tests/GeneratedTests/GeneratedTests_07.swift | 16 +- Tests/GeneratedTests/GeneratedTests_08.swift | 16 +- Tests/GeneratedTests/GeneratedTests_09.swift | 16 +- Tests/GeneratedTests/GeneratedTests_10.swift | 16 +- Tests/GeneratedTests/GeneratedTests_11.swift | 8 + .../Resources/default_rule_configurations.yml | 5 + 14 files changed, 352 insertions(+), 72 deletions(-) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Lint/DeferBeforeUnstructuredTaskRule.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 32d12bc071..7879842eba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,14 @@ [arimu1](https://github.com/arimu1) [#6839](https://github.com/realm/SwiftLint/issues/6839) +* Add `defer_before_unstructured_task` rule (opt-in) that flags a `defer` + block assigning to shared state right before a sibling unstructured `Task` + reads that same state. The `defer`red assignment runs the instant the + enclosing synchronous function returns, before the `Task`'s body has a + chance to execute, which is a common source of inverted loading-flag bugs. + [alisher-zinullayev](https://github.com/alisher-zinullayev) + [#6619](https://github.com/realm/SwiftLint/issues/6619) + ### Bug Fixes * Add an opt-in `allow_explicit_unsafe_unowned` option to let the diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index 51b2a4eee4..ab80066887 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -35,6 +35,7 @@ public let builtInRules: [any Rule.Type] = [ ControlStatementRule.self, ConvenienceTypeRule.self, CyclomaticComplexityRule.self, + DeferBeforeUnstructuredTaskRule.self, DeploymentTargetRule.self, DirectReturnRule.self, DiscardedNotificationCenterObserverRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/DeferBeforeUnstructuredTaskRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/DeferBeforeUnstructuredTaskRule.swift new file mode 100644 index 0000000000..1dc0b1f394 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/DeferBeforeUnstructuredTaskRule.swift @@ -0,0 +1,258 @@ +import SwiftLintCore +import SwiftSyntax + +@SwiftSyntaxRule(foldExpressions: true, optIn: true) +struct DeferBeforeUnstructuredTaskRule: Rule { + var configuration = SeverityConfiguration(.warning) + + static let description = RuleDescription( + identifier: "defer_before_unstructured_task", + name: "Defer Before Unstructured Task", + description: """ + A `defer` block runs the moment its enclosing synchronous scope returns, before a sibling \ + unstructured `Task` has a chance to run its body. Assigning to shared state in such a `defer` \ + while a `Task` in the same scope reads that state is usually a bug: the state is reset before \ + the asynchronous work it's guarding has actually finished. Move the assignment inside the \ + `Task`, or make the enclosing function `async` and `await` the work directly + """, + kind: .lint, + nonTriggeringExamples: #examples([ + """ + func f() async { + isLoading = true + defer { isLoading = false } + await work() + } + """, + """ + func f() { + defer { print("done") } + Task { await work() } + } + """, + """ + func f() { + let t = Task { await work() } + defer { print("leaving") } + _ = t + } + """, + """ + func f() { + var localFlag = true + defer { localFlag = false } + Task { await work() } + _ = localFlag + } + """, + """ + func f() { + isLoading = true + defer { isLoading = false } + let t = Task { + await work() + _ = isLoading + } + _ = t + } + """, + ]), + triggeringExamples: #examples([ + """ + func login() { + isLoading = true + ↓defer { isLoading = false } + Task { + await doSomethingAsync() + _ = isLoading + } + } + """, + """ + func login() { + isLoading = true + ↓defer { isLoading = false } + Task.detached { + await doSomethingAsync() + _ = await self.isLoading + } + } + """, + """ + func login() { + isLoading = true + errorMessage = nil + ↓defer { + isLoading = false + errorMessage = nil + } + Task { + await doSomethingAsync() + _ = isLoading + } + } + """, + """ + func login() { + isLoading = true + ↓defer { isLoading = false } + Task { + await doSomethingAsync() + _ = isLoading + } + } + """, + ]) + ) +} + +private extension DeferBeforeUnstructuredTaskRule { + final class Visitor: ViolationsSyntaxVisitor { + override func visitPost(_ node: DeferStmtSyntax) { + guard + !node.isInAsyncScope, + let identifiers = node.assignedIdentifiers, + node.hasSiblingDiscardedTask(referencing: identifiers) + else { + return + } + + violations.append(node.deferKeyword.positionAfterSkippingLeadingTrivia) + } + } +} + +private extension DeferStmtSyntax { + /// Whether the nearest enclosing function-like scope is `async`. + var isInAsyncScope: Bool { + var current = parent + while let node = current { + if let function = node.as(FunctionDeclSyntax.self) { + return function.signature.effectSpecifiers?.asyncSpecifier != nil + } + if let closure = node.as(ClosureExprSyntax.self) { + return closure.signature?.effectSpecifiers?.asyncSpecifier != nil + } + if let accessor = node.as(AccessorDeclSyntax.self) { + return accessor.effectSpecifiers?.asyncSpecifier != nil + } + if let initializer = node.as(InitializerDeclSyntax.self) { + return initializer.signature.effectSpecifiers?.asyncSpecifier != nil + } + current = node.parent + } + return false + } + + /// The identifiers simply assigned to in this `defer`'s body, if the body consists of one or two + /// straightforward assignments only. Returns `nil` for anything else (logging calls, lock releases, + /// control flow, etc.), which excludes those defers from consideration. + var assignedIdentifiers: Set? { + let statements = body.statements + guard (1...2).contains(statements.count) else { + return nil + } + + var identifiers = Set() + for statement in statements { + guard + let infix = statement.item.as(InfixOperatorExprSyntax.self), + infix.operator.is(AssignmentExprSyntax.self), + let identifier = infix.leftOperand.assignmentTargetIdentifier + else { + return nil + } + identifiers.insert(identifier) + } + + return identifiers + } + + /// Whether a sibling statement at the same scope as this `defer` is a discarded `Task { ... }` or + /// `Task.detached { ... }` call whose trailing closure references at least one of `identifiers`. + func hasSiblingDiscardedTask(referencing identifiers: Set) -> Bool { + guard let siblings = parent?.parent?.as(CodeBlockItemListSyntax.self) else { + return false + } + + return siblings.contains { sibling in + guard + let call = sibling.item.as(FunctionCallExprSyntax.self), + call.isUnstructuredTaskInitializer, + let closure = call.trailingClosure + else { + return false + } + + return closure.references(any: identifiers) + } + } +} + +private extension ExprSyntax { + /// The identifier this expression assigns to, considering plain identifiers (`foo`) and + /// `self`-qualified member access (`self.foo`). `nil` for anything more complex. + var assignmentTargetIdentifier: String? { + if let reference = `as`(DeclReferenceExprSyntax.self) { + return reference.baseName.text + } + + if let member = `as`(MemberAccessExprSyntax.self), + member.base?.as(DeclReferenceExprSyntax.self)?.baseName.text == "self" { + return member.declName.baseName.text + } + + return nil + } +} + +private extension FunctionCallExprSyntax { + var isUnstructuredTaskInitializer: Bool { + if let reference = calledExpression.as(DeclReferenceExprSyntax.self) { + return reference.baseName.text == "Task" + } + + if let specialized = calledExpression.as(GenericSpecializationExprSyntax.self), + let reference = specialized.expression.as(DeclReferenceExprSyntax.self) { + return reference.baseName.text == "Task" + } + + if let member = calledExpression.as(MemberAccessExprSyntax.self), + member.base?.as(DeclReferenceExprSyntax.self)?.baseName.text == "Task", + member.declName.baseName.text == "detached" { + return true + } + + return false + } +} + +private extension ClosureExprSyntax { + func references(any identifiers: Set) -> Bool { + IdentifierReferenceVisitor(identifiers: identifiers, viewMode: .sourceAccurate) + .walk(tree: self, handler: \.found) + } +} + +private final class IdentifierReferenceVisitor: SyntaxVisitor { + private let identifiers: Set + var found = false + + init(identifiers: Set, viewMode: SyntaxTreeViewMode) { + self.identifiers = identifiers + super.init(viewMode: viewMode) + } + + override func visitPost(_ node: DeclReferenceExprSyntax) { + if identifiers.contains(node.baseName.text) { + found = true + } + } + + override func visitPost(_ node: MemberAccessExprSyntax) { + if node.base?.as(DeclReferenceExprSyntax.self)?.baseName.text == "self", + identifiers.contains(node.declName.baseName.text) { + found = true + } + } +} diff --git a/Tests/GeneratedTests/GeneratedTests_02.swift b/Tests/GeneratedTests/GeneratedTests_02.swift index 4f940ea714..73b04aeaaf 100644 --- a/Tests/GeneratedTests/GeneratedTests_02.swift +++ b/Tests/GeneratedTests/GeneratedTests_02.swift @@ -73,6 +73,14 @@ struct CyclomaticComplexityRuleGeneratedTests { } } +@Suite(.rulesRegistered) +struct DeferBeforeUnstructuredTaskRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(DeferBeforeUnstructuredTaskRule.description) + } +} + @Suite(.rulesRegistered) struct DeploymentTargetRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct EmptyCollectionLiteralRuleGeneratedTests { verifyRule(EmptyCollectionLiteralRule.description) } } - -@Suite(.rulesRegistered) -struct EmptyCountRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(EmptyCountRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_03.swift b/Tests/GeneratedTests/GeneratedTests_03.swift index 6e40beed59..a59a9ed61f 100644 --- a/Tests/GeneratedTests/GeneratedTests_03.swift +++ b/Tests/GeneratedTests/GeneratedTests_03.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct EmptyCountRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(EmptyCountRule.description) + } +} + @Suite(.rulesRegistered) struct EmptyEnumArgumentsRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct FlatMapOverMapReduceRuleGeneratedTests { verifyRule(FlatMapOverMapReduceRule.description) } } - -@Suite(.rulesRegistered) -struct ForWhereRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(ForWhereRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_04.swift b/Tests/GeneratedTests/GeneratedTests_04.swift index 52c56efca3..59bc011dc9 100644 --- a/Tests/GeneratedTests/GeneratedTests_04.swift +++ b/Tests/GeneratedTests/GeneratedTests_04.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct ForWhereRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(ForWhereRule.description) + } +} + @Suite(.rulesRegistered) struct ForceCastRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct LastWhereRuleGeneratedTests { verifyRule(LastWhereRule.description) } } - -@Suite(.rulesRegistered) -struct LeadingWhitespaceRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(LeadingWhitespaceRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_05.swift b/Tests/GeneratedTests/GeneratedTests_05.swift index 6e42efeddb..1843544e48 100644 --- a/Tests/GeneratedTests/GeneratedTests_05.swift +++ b/Tests/GeneratedTests/GeneratedTests_05.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct LeadingWhitespaceRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(LeadingWhitespaceRule.description) + } +} + @Suite(.rulesRegistered) struct LegacyCGGeometryFunctionsRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct MultilineParametersBracketsRuleGeneratedTests { verifyRule(MultilineParametersBracketsRule.description) } } - -@Suite(.rulesRegistered) -struct MultilineParametersRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(MultilineParametersRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_06.swift b/Tests/GeneratedTests/GeneratedTests_06.swift index 25430d5f6d..9a4afb1e90 100644 --- a/Tests/GeneratedTests/GeneratedTests_06.swift +++ b/Tests/GeneratedTests/GeneratedTests_06.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct MultilineParametersRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(MultilineParametersRule.description) + } +} + @Suite(.rulesRegistered) struct MultipleClosuresWithTrailingClosureRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct OrphanedDocCommentRuleGeneratedTests { verifyRule(OrphanedDocCommentRule.description) } } - -@Suite(.rulesRegistered) -struct OverriddenSuperCallRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(OverriddenSuperCallRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_07.swift b/Tests/GeneratedTests/GeneratedTests_07.swift index 37a1913ba4..f1a50dd1f2 100644 --- a/Tests/GeneratedTests/GeneratedTests_07.swift +++ b/Tests/GeneratedTests/GeneratedTests_07.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct OverriddenSuperCallRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(OverriddenSuperCallRule.description) + } +} + @Suite(.rulesRegistered) struct OverrideInExtensionRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct QuickDiscouragedPendingTestRuleGeneratedTests { verifyRule(QuickDiscouragedPendingTestRule.description) } } - -@Suite(.rulesRegistered) -struct RawValueForCamelCasedCodableEnumRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(RawValueForCamelCasedCodableEnumRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_08.swift b/Tests/GeneratedTests/GeneratedTests_08.swift index 9a9e613da4..06f02dc630 100644 --- a/Tests/GeneratedTests/GeneratedTests_08.swift +++ b/Tests/GeneratedTests/GeneratedTests_08.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct RawValueForCamelCasedCodableEnumRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(RawValueForCamelCasedCodableEnumRule.description) + } +} + @Suite(.rulesRegistered) struct ReduceBooleanRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct SortedFirstLastRuleGeneratedTests { verifyRule(SortedFirstLastRule.description) } } - -@Suite(.rulesRegistered) -struct SortedImportsRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(SortedImportsRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_09.swift b/Tests/GeneratedTests/GeneratedTests_09.swift index 63724b0191..f891cb0f10 100644 --- a/Tests/GeneratedTests/GeneratedTests_09.swift +++ b/Tests/GeneratedTests/GeneratedTests_09.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct SortedImportsRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(SortedImportsRule.description) + } +} + @Suite(.rulesRegistered) struct StatementPositionRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct UnhandledThrowingTaskRuleGeneratedTests { verifyRule(UnhandledThrowingTaskRule.description) } } - -@Suite(.rulesRegistered) -struct UnneededBreakInSwitchRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(UnneededBreakInSwitchRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_10.swift b/Tests/GeneratedTests/GeneratedTests_10.swift index 6e8193c399..6b51b6d07c 100644 --- a/Tests/GeneratedTests/GeneratedTests_10.swift +++ b/Tests/GeneratedTests/GeneratedTests_10.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct UnneededBreakInSwitchRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(UnneededBreakInSwitchRule.description) + } +} + @Suite(.rulesRegistered) struct UnneededEscapingRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct VoidFunctionInTernaryConditionRuleGeneratedTests { verifyRule(VoidFunctionInTernaryConditionRule.description) } } - -@Suite(.rulesRegistered) -struct VoidReturnRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(VoidReturnRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_11.swift b/Tests/GeneratedTests/GeneratedTests_11.swift index 61a9b2b37e..8d40b52c40 100644 --- a/Tests/GeneratedTests/GeneratedTests_11.swift +++ b/Tests/GeneratedTests/GeneratedTests_11.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct VoidReturnRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(VoidReturnRule.description) + } +} + @Suite(.rulesRegistered) struct WeakDelegateRuleGeneratedTests { @Test diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index adb51a3d16..5cbcd5756e 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -177,6 +177,11 @@ cyclomatic_complexity: meta: opt-in: false correctable: false +defer_before_unstructured_task: + severity: warning + meta: + opt-in: true + correctable: false deployment_target: severity: warning iOSApplicationExtension_deployment_target: 7.0