From 1b1b19a902e38a61cd083c85e48953b4cf8dc5f2 Mon Sep 17 00:00:00 2001
From: JP Simard <jp@jpsim.com>
Date: Fri, 12 May 2023 10:35:59 -0400
Subject: [PATCH] Fix some `unhandled_throwing_task` false positives (#5001)

Fixes some of the cases reported in
https://github.com/realm/SwiftLint/issues/4987
---
 CHANGELOG.md                                  |  7 +-
 .../Lint/UnhandledThrowingTaskRule.swift      | 74 ++++++++++++++++++-
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dbb45059b..c5a254b2f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,7 +19,12 @@
 
 * Make `unhandled_throwing_task` opt-in instead of enabled by default. The rule
   is still prone to false positives at this point, so this makes enabling the
-  rule a conscious decision by end-users instead of enabled by default.  
+  rule a conscious decision by end-users.  
+  [JP Simard](https://github.com/jpsim)
+  [#4987](https://github.com/realm/SwiftLint/issues/4987)
+
+* Fix `unhandled_throwing_task` false positives when the `Task` is returned or
+  where the throwing code is handled in a `Result` initializer.  
   [JP Simard](https://github.com/jpsim)
   [#4987](https://github.com/realm/SwiftLint/issues/4987)
 
diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/UnhandledThrowingTaskRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/UnhandledThrowingTaskRule.swift
index a04761ae7..f61a83997 100644
--- a/Source/SwiftLintBuiltInRules/Rules/Lint/UnhandledThrowingTaskRule.swift
+++ b/Source/SwiftLintBuiltInRules/Rules/Lint/UnhandledThrowingTaskRule.swift
@@ -82,6 +82,28 @@ struct UnhandledThrowingTaskRule: ConfigurationProviderRule, SwiftSyntaxRule, Op
             let result = await Task {
               throw CancellationError()
             }.result
+            """),
+            Example("""
+            func makeTask() -> Task<String, Error> {
+              return Task {
+                try await someThrowingFunction()
+              }
+            }
+            """),
+            Example("""
+            func makeTask() -> Task<String, Error> {
+              // Implicit return
+              Task {
+                try await someThrowingFunction()
+              }
+            }
+            """),
+            Example("""
+            Task {
+              return Result {
+                  try someThrowingFunc()
+              }
+            }
             """)
         ],
         triggeringExamples: [
@@ -151,6 +173,13 @@ struct UnhandledThrowingTaskRule: ConfigurationProviderRule, SwiftSyntaxRule, Op
                 throw BarError()
               }
             }
+            """),
+            Example("""
+            func doTask() {
+              ↓Task {
+                try await someThrowingFunction()
+              }
+            }
             """)
         ]
     )
@@ -174,7 +203,7 @@ private extension FunctionCallExprSyntax {
     var hasViolation: Bool {
         isTaskWithImplicitErrorType &&
             doesThrow &&
-            !(isAssigned || isValueOrResultAccessed)
+            !(isAssigned || isValueOrResultAccessed || isReturnValue)
     }
 
     var isTaskWithImplicitErrorType: Bool {
@@ -260,6 +289,22 @@ private final class ThrowsVisitor: SyntaxVisitor {
         return .skipChildren
     }
 
+    override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
+        // No need to continue traversing if we already throw.
+        if doesThrow {
+            return .skipChildren
+        }
+
+        // Result initializers with trailing closures handle thrown errors.
+        if let typeIdentifier = node.calledExpression.as(IdentifierExprSyntax.self),
+           typeIdentifier.identifier.text == "Result",
+           node.trailingClosure != nil {
+            return .skipChildren
+        }
+
+        return .visitChildren
+    }
+
     override func visitPost(_ node: TryExprSyntax) {
         if node.questionOrExclamationMark == nil {
             doesThrow = true
@@ -270,3 +315,30 @@ private final class ThrowsVisitor: SyntaxVisitor {
         doesThrow = true
     }
 }
+
+private extension SyntaxProtocol {
+    var isExplicitReturnValue: Bool {
+        parent?.is(ReturnStmtSyntax.self) == true
+    }
+
+    var isImplicitReturnValue: Bool {
+        // 4th parent: FunctionDecl
+        // 3rd parent: | CodeBlock
+        // 2nd parent:   | CodeBlockItemList
+        // 1st parent:     | CodeBlockItem
+        // Current node:     | FunctionDeclSyntax
+        guard
+            let parentFunctionDecl = parent?.parent?.parent?.parent?.as(FunctionDeclSyntax.self),
+            parentFunctionDecl.body?.statements.count == 1,
+            parentFunctionDecl.signature.output != nil
+        else {
+            return false
+        }
+
+        return true
+    }
+
+    var isReturnValue: Bool {
+        isExplicitReturnValue || isImplicitReturnValue
+    }
+}