lint: allow return try for optional error payloads

Z017 flags return try expressions as redundant, but Zig can require try when the enclosing function returns an optional error-union payload. In cases like !?Thing, try unwraps !Thing to Thing before return coerces the payload to ?Thing.

Skip the diagnostic for optional error-union return payloads and document the exception. Add regression coverage so the normal redundant case still warns while !?Thing returns do not.

Author: rockorager

README.md

@@ -38,7 +38,7 @@ Directories are scanned recursively for `.zig` files.
 | Z014 | Error set names should be PascalCase |
 | Z015 | Public function exposes private error set |
 | Z016 | Split compound assert: `assert(a and b)` → `assert(a); assert(b);` |
-| Z017 | Redundant `try` in return: `return try expr` → `return expr` |
+| Z017 | Redundant `try` in return when no payload coercion is needed |
 | Z018 | Redundant `@as` when type is already known from context |
 | Z019 | `@This()` in named struct; use the type name instead |
 | Z020 | Inline `@This()`; assign to a constant first |

docs/rules/Z017.md

@@ -6,7 +6,7 @@ enabled: true
 
 # Z017: Redundant try in return
 
-Detects `return try expr` where `try` is redundant. When returning an error union, `return expr` handles error propagation automatically.
+Detects `return try expr` where `try` is redundant. When returning an error union with the same payload type, `return expr` handles error propagation automatically.
 
 ## Examples
 
@@ -38,6 +38,20 @@ fn process() !void {
 }
 ```
 
+The `try` can also be needed when the function return type performs an additional payload coercion, such as returning `Thing` as `?Thing` in an `!?Thing` function:
+
+```zig
+const Thing = struct {};
+
+fn allocThing() !Thing {
+    return .{};
+}
+
+fn maybeThing() !?Thing {
+    return try allocThing();
+}
+```
+
 ## Rationale
 
-In Zig, `return expr` already propagates errors when the function returns an error union. Adding `try` is redundant and adds visual noise. The only purpose of `try` is to convert an error union to its payload type for intermediate operations.
+In Zig, `return expr` already propagates errors when the function returns a matching error union. Adding `try` is redundant and adds visual noise. Keep `try` when it converts an error union to its payload for another coercion or intermediate operation.

src/Linter.zig

@@ -1884,6 +1884,10 @@ fn checkReturnTry(self: *Linter, return_node: Ast.Node.Index, return_expr: Ast.N
     // Check if the return expression is a try
     if (self.tree.nodeTag(return_expr) != .@"try") return;
 
+    // In `!?T` functions, `try` can be required to coerce `!T` into `!?T`:
+    // the `try` unwraps the error union, then `return` coerces `T` to `?T`.
+    if (self.currentReturnTypeMayNeedTryForOptionalPayload()) return;
+
     // Get the inner expression being tried
     const try_expr = self.tree.nodeData(return_expr).node;
     const expr_source = self.getNodeSource(try_expr);
@@ -1893,6 +1897,23 @@ fn checkReturnTry(self: *Linter, return_node: Ast.Node.Index, return_expr: Ast.N
     self.report(loc, .Z017, truncated);
 }
 
+fn currentReturnTypeMayNeedTryForOptionalPayload(self: *Linter) bool {
+    const fn_return_type = self.current_fn_return_type.unwrap() orelse return false;
+    const tag = self.tree.nodeTag(fn_return_type);
+
+    // Bare `!?T` function returns store only the payload node in FnProto.return_type;
+    // the `!` is the token immediately before the optional payload.
+    if (tag == .optional_type) {
+        const first_token = self.tree.firstToken(fn_return_type);
+        return first_token > 0 and self.tree.tokenTag(first_token - 1) == .bang;
+    }
+
+    if (tag != .error_union) return false;
+
+    const data = self.tree.nodeData(fn_return_type).node_and_node;
+    return self.tree.nodeTag(data[1]) == .optional_type;
+}
+
 fn checkCatchReturnAll(self: *Linter) void {
     for (0..self.tree.nodes.len) |i| {
         const node: Ast.Node.Index = @enumFromInt(i);
@@ -4377,6 +4398,35 @@ test "Z016: simple assert is ok" {
     }
 }
 
+test "Z017: detect redundant try in return" {
+    var linter: Linter = .init(std.testing.allocator,
+        \\fn bar() !u32 {
+        \\    return 1;
+        \\}
+        \\fn foo() !u32 {
+        \\    return try bar();
+        \\}
+    , "test.zig", null);
+    defer linter.deinit();
+    linter.lint();
+    try std.testing.expectEqual(1, linter.diagnosticCount(.Z017));
+}
+
+test "Z017: allow try when error union payload coerces to optional return" {
+    var linter: Linter = .init(std.testing.allocator,
+        \\const Thing = struct {};
+        \\fn allocThing() !Thing {
+        \\    return .{};
+        \\}
+        \\fn foo() !?Thing {
+        \\    return try allocThing();
+        \\}
+    , "test.zig", null);
+    defer linter.deinit();
+    linter.lint();
+    try std.testing.expectEqual(0, linter.diagnosticCount(.Z017));
+}
+
 test "Z019: @This() in named struct should warn" {
     var linter: Linter = .init(std.testing.allocator,
         \\const Foo = struct {