Add `unhandled_throwing_task` rule (#4958)

This rule will check Task's that are not explicitly annotated with success and
failure types for unhandled try expressions. These trys will silently fail if an
error is thrown.

See this forum thread for more details:
https://forums.swift.org/t/task-initializer-with-throwing-closure-swallows-error/56066

CHANGELOG.md

@@ -80,6 +80,12 @@
   [SimplyDanny](https://github.com/SimplyDanny)
   [#4843](https://github.com/realm/SwiftLint/issues/4843)
 
+* Add new `unhandled_throwing_task` rule that triggers when a Task with an
+  implicit error type has unhandled trys or errors thrown inside its body.
+  This results in errors being silently discarded, which may be unexpected.
+  [kylebshr](https://github.com/kylebshr)
+  [#4958](https://github.com/realm/SwiftLint/pull/4958)
+
 #### Bug Fixes
 
 * Fix `lower_acl_than_parent` rule rewriter by preserving leading whitespace.  

Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift

@@ -204,6 +204,7 @@ public let builtInRules: [Rule.Type] = [
     TypesafeArrayInitRule.self,
     UnavailableConditionRule.self,
     UnavailableFunctionRule.self,
+    UnhandledThrowingTaskRule.self,
     UnneededBreakInSwitchRule.self,
     UnneededParenthesesInClosureArgumentRule.self,
     UnownedVariableCaptureRule.self,

Source/SwiftLintBuiltInRules/Rules/Lint/UnhandledThrowingTaskRule.swift

@@ -0,0 +1,267 @@
+import SwiftSyntax
+
+struct UnhandledThrowingTaskRule: ConfigurationProviderRule, SwiftSyntaxRule {
+    var configuration = SeverityConfiguration(.error)
+
+    static let description = RuleDescription(
+        identifier: "unhandled_throwing_task",
+        name: "Unhandled Throwing Task",
+        description: """
+            Errors thrown inside this task are not handled, which may be unexpected. \
+            Handle errors inside the task, or use `try await` to access the Tasks value and handle errors. \
+            See this forum thread for more details: \
+            https://forums.swift.org/t/task-initializer-with-throwing-closure-swallows-error/56066)
+            """,
+        kind: .lint,
+        nonTriggeringExamples: [
+            Example("""
+            Task<Void, Never> {
+              try await myThrowingFunction()
+            }
+            """),
+            Example("""
+            Task {
+              try? await myThrowingFunction()
+            }
+            """),
+            Example("""
+            Task {
+              try! await myThrowingFunction()
+            }
+            """),
+            Example("""
+            Task<Void, String> {
+              let text = try myThrowingFunction()
+              return text
+            }
+            """),
+            Example("""
+            Task {
+              do {
+                try myThrowingFunction()
+              } catch let e {
+                print(e)
+              }
+            }
+            """),
+            Example("""
+            func someFunction() throws {
+              Task {
+                anotherFunction()
+                do {
+                  try myThrowingFunction()
+                } catch {
+                  print(error)
+                }
+              }
+
+              try something()
+            }
+            """),
+            Example("""
+            let task = Task {
+              try await myThrowingFunction()
+            }
+            """),
+            Example("""
+            var task = Task {
+              try await myThrowingFunction()
+            }
+            """),
+            Example("""
+            try await Task {
+              try await myThrowingFunction()
+            }.value
+            """),
+            Example("""
+            executor.task = Task {
+              try await isolatedOpen(.init(executor.asUnownedSerialExecutor()))
+            }
+            """)
+        ],
+        triggeringExamples: [
+            Example("""
+            ↓Task {
+              try await myThrowingFunction()
+            }
+            """),
+            Example("""
+            ↓Task {
+              let text = try myThrowingFunction()
+              return text
+            }
+            """),
+            Example("""
+            ↓Task {
+              do {
+                try myThrowingFunction()
+              }
+            }
+            """),
+            Example("""
+            ↓Task {
+              do {
+                try myThrowingFunction()
+              } catch let e as FooError {
+                print(e)
+              }
+            }
+            """),
+            Example("""
+            ↓Task {
+              do {
+                throw FooError.bar
+              }
+            }
+            """),
+            Example("""
+            ↓Task {
+              throw FooError.bar
+            }
+            """),
+            Example("""
+            ↓Task<_, _> {
+              throw FooError.bar
+            }
+            """),
+            Example("""
+            ↓Task<Void,_> {
+              throw FooError.bar
+            }
+            """),
+            Example("""
+            ↓Task {
+              do {
+                try foo()
+              } catch {
+                try bar()
+              }
+            }
+            """),
+            Example("""
+            ↓Task {
+              do {
+                try foo()
+              } catch {
+                throw BarError()
+              }
+            }
+            """)
+        ]
+    )
+
+    func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor {
+        Visitor(viewMode: .sourceAccurate)
+    }
+}
+
+private extension UnhandledThrowingTaskRule {
+    final class Visitor: ViolationsSyntaxVisitor {
+        override func visitPost(_ node: FunctionCallExprSyntax) {
+            if node.hasViolation {
+                violations.append(node.calledExpression.positionAfterSkippingLeadingTrivia)
+            }
+        }
+    }
+}
+
+private extension FunctionCallExprSyntax {
+    var hasViolation: Bool {
+        isTaskWithImplicitErrorType &&
+            doesThrow &&
+            !(isAssigned || isValueAccessed)
+    }
+
+    var isTaskWithImplicitErrorType: Bool {
+        if let typeIdentifier = calledExpression.as(IdentifierExprSyntax.self),
+           typeIdentifier.identifier.text == "Task" {
+            return true
+        }
+
+        if let specializedExpression = calledExpression.as(SpecializeExprSyntax.self),
+           let typeIdentifier = specializedExpression.expression.as(IdentifierExprSyntax.self),
+           typeIdentifier.identifier.text == "Task",
+           let lastGeneric = specializedExpression.genericArgumentClause
+            .arguments.last?.argumentType.as(SimpleTypeIdentifierSyntax.self),
+           lastGeneric.typeName == "_" {
+            return true
+        }
+
+        return false
+    }
+
+    var isAssigned: Bool {
+        guard let parent else {
+            return false
+        }
+
+        if parent.is(InitializerClauseSyntax.self) {
+            return true
+        }
+
+        if let list = parent.as(ExprListSyntax.self),
+           list.contains(where: { $0.is(AssignmentExprSyntax.self) }) {
+            return true
+        }
+
+        return false
+    }
+
+    var isValueAccessed: Bool {
+        guard let parent = parent?.as(MemberAccessExprSyntax.self) else {
+            return false
+        }
+
+        return parent.name.text == "value"
+    }
+
+    var doesThrow: Bool {
+        ThrowsVisitor(viewMode: .sourceAccurate)
+            .walk(tree: self, handler: \.doesThrow)
+    }
+}
+
+/// If the `doesThrow` property is true after visiting, then this node throws an error that is "unhandled."
+/// Try statements inside a `do` with a `catch` that handles all errors will not be marked as throwing.
+private final class ThrowsVisitor: SyntaxVisitor {
+    var doesThrow = false
+
+    override func visit(_ node: DoStmtSyntax) -> SyntaxVisitorContinueKind {
+        // No need to continue traversing if we already throw.
+        if doesThrow {
+            return .skipChildren
+        }
+
+        // If there are no catch clauses, visit children to see if there are any try expressions.
+        guard let lastCatchClause = node.catchClauses?.last else {
+            return .visitChildren
+        }
+
+        let catchItems = lastCatchClause.catchItems ?? []
+
+        // If we have a value binding pattern, only an IdentifierPatternSyntax will catch
+        // any error; if it's not an IdentifierPatternSyntax, we need to visit children.
+        if let pattern = catchItems.last?.pattern?.as(ValueBindingPatternSyntax.self),
+           !pattern.valuePattern.is(IdentifierPatternSyntax.self) {
+            return .visitChildren
+        }
+
+        // Check the catch clause tree for unhandled throws.
+        if ThrowsVisitor(viewMode: .sourceAccurate).walk(tree: lastCatchClause, handler: \.doesThrow) {
+            doesThrow = true
+        }
+
+        // We don't need to visit children of the `do` node, since all errors are handled by the catch.
+        return .skipChildren
+    }
+
+    override func visitPost(_ node: TryExprSyntax) {
+        if node.questionOrExclamationMark == nil {
+            doesThrow = true
+        }
+    }
+
+    override func visitPost(_ node: ThrowStmtSyntax) {
+        doesThrow = true
+    }
+}

Tests/GeneratedTests/GeneratedTests.swift

@@ -1220,6 +1220,12 @@ class UnavailableFunctionRuleGeneratedTests: SwiftLintTestCase {
     }
 }
 
+class UnhandledThrowingTaskRuleGeneratedTests: SwiftLintTestCase {
+    func testWithDefaultConfiguration() {
+        verifyRule(UnhandledThrowingTaskRule.description)
+    }
+}
+
 class UnneededBreakInSwitchRuleGeneratedTests: SwiftLintTestCase {
     func testWithDefaultConfiguration() {
         verifyRule(UnneededBreakInSwitchRule.description)