[Diagnostics] Warn when the result of a Void-returning function is ignored (by assigning into '_') (#29576)

Author: theblixguy

include/swift/AST/DiagnosticsSema.def

@@ -3193,6 +3193,9 @@ ERROR(object_literal_broken_proto,none,
 ERROR(discard_expr_outside_of_assignment,none,
       "'_' can only appear in a pattern or on the left side of an assignment",
       ())
+WARNING(discard_expr_void_result_redundant, none,
+        "using '_' to ignore the result of a Void-returning function "
+        "is redundant", ())
 ERROR(collection_literal_heterogeneous,none,
       "heterogeneous collection literal could only be inferred to %0; add"
       " explicit type annotation if this is intentional", (Type))

lib/Sema/MiscDiagnostics.cpp

@@ -221,8 +221,24 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
       }
 
       // If we have an assignment expression, scout ahead for acceptable _'s.
-      if (auto *AE = dyn_cast<AssignExpr>(E))
-        markAcceptableDiscardExprs(AE->getDest());
+      if (auto *AE = dyn_cast<AssignExpr>(E)) {
+        auto destExpr = AE->getDest();
+        markAcceptableDiscardExprs(destExpr);
+        // If the user is assigning the result of a function that returns
+        // Void to _ then warn, because that is redundant.
+        if (auto DAE = dyn_cast<DiscardAssignmentExpr>(destExpr)) {
+          if (auto CE = dyn_cast<CallExpr>(AE->getSrc())) {
+            if (CE->getCalledValue() && isa<FuncDecl>(CE->getCalledValue()) &&
+                CE->getType()->isVoid()) {
+              Ctx.Diags
+                  .diagnose(DAE->getLoc(),
+                            diag::discard_expr_void_result_redundant)
+                  .fixItRemoveChars(DAE->getStartLoc(),
+                                    AE->getSrc()->getStartLoc());
+            }
+          }
+        }
+      }
 
       /// Diagnose a '_' that isn't on the immediate LHS of an assignment.
       if (auto *DAE = dyn_cast<DiscardAssignmentExpr>(E)) {
@@ -409,7 +425,7 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC,
     /// in simple pattern-like expressions, so we reject anything complex here.
     void markAcceptableDiscardExprs(Expr *E) {
       if (!E) return;
-      
+
       if (auto *PE = dyn_cast<ParenExpr>(E))
         return markAcceptableDiscardExprs(PE->getSubExpr());
       if (auto *TE = dyn_cast<TupleExpr>(E)) {

test/ClangImporter/MixedSource/submodule.swift

@@ -4,6 +4,6 @@
 @_exported import Mixed.Submodule
 
 func test() {
-  _ = topLevel()
-  _ = fromSubmodule()
+  topLevel()
+  fromSubmodule()
 }

test/ClangImporter/private_frameworks.swift

@@ -39,8 +39,8 @@
 import SomeKit
 
 func testWidget(widget: SKWidget) {
-  _ = widget.someObjCMethod()
-  _ = widget.someObjCExtensionMethod()
+  widget.someObjCMethod()
+  widget.someObjCExtensionMethod()
 
   let ext = widget.extensionMethod()
   ext.foo()

test/Compatibility/special_func_name.swift

@@ -34,16 +34,16 @@ struct S3 {
 }
 
 _ = S11(0) // expected-error {{argument passed to call that takes no arguments}}
-_ = S11.init(0)
-_ = S11.`init`(0)
+S11.init(0)
+S11.`init`(0)
 
 _ = S12(0)
 _ = S12.init(0)
 _ = S12.`init`(0) // expected-error {{type 'S12' has no member 'init'}}
 
 _ = S21(0) // expected-error {{argument passed to call that takes no arguments}}
-_ = S21.init(0)
-_ = S21.`init`(0)
+S21.init(0)
+S21.`init`(0)
 
 _ = S22(0)
 _ = S22.init(0)

test/Constraints/assignment.swift

@@ -72,3 +72,8 @@ func f23798944() {
 }
 
 .sr_3506 = 0 // expected-error {{type 'Int' has no member 'sr_3506'}}
+
+// SR-1553
+
+func returnsVoid() {}
+_ = returnsVoid() // expected-warning {{using '_' to ignore the result of a Void-returning function is redundant}}{{1-5=}}

test/Constraints/dynamic_lookup.swift

@@ -224,7 +224,7 @@ let anyValue: Any = X()
 _ = anyValue.bar() // expected-error {{value of type 'Any' has no member 'bar'}}
 // expected-note@-1 {{cast 'Any' to 'AnyObject' or use 'as!' to force downcast to a more specific type to access members}}{{5-5=(}}{{13-13= as AnyObject)}}
 _ = (anyValue as AnyObject).bar()
-_ = (anyValue as! X).bar()
+(anyValue as! X).bar()
 
 var anyDict: [String : Any] = Dictionary<String, Any>()
 anyDict["test"] = anyValue

test/Constraints/optional.swift

@@ -391,8 +391,8 @@ func sr8411() {
 
   _ = S(&foo)      // Ok
   _ = S.init(&foo) // Ok
-  _ = S.foo(&foo)  // Ok
-  _ = S.bar(&foo, 42) // Ok
+  S.foo(&foo)  // Ok
+  S.bar(&foo, 42) // Ok
 }
 
 // SR-11104 - Slightly misleading diagnostics for contextual failures with multiple fixes

test/Constraints/override.swift

@@ -9,8 +9,8 @@ class Sub: Base {
 }
 
 func removeOverrides<SomeSub: Sub>(concrete: Sub, generic: SomeSub) {
-  _ = concrete.foo()
-  _ = generic.foo()
+  concrete.foo()
+  generic.foo()
 }
 
 class Base1 {

test/Constraints/rdar39931475.swift

@@ -5,5 +5,5 @@ protocol P {
 }
 
 func foo<T: P>(_ bar: T) {
-  _ = bar.b { a in Double((a, a += 1).0) }
+  bar.b { a in Double((a, a += 1).0) }
 }

test/Constraints/tuple_arguments.swift

@@ -1696,8 +1696,8 @@ class Mappable<T> {
 
 let x = Mappable(())
 // expected-note@-1 2{{'x' declared here}}
-_ = x.map { (_: Void) in return () }
-_ = x.map { (_: ()) in () }
+x.map { (_: Void) in return () }
+x.map { (_: ()) in () }
 
 // https://bugs.swift.org/browse/SR-9470
 do {

test/NameBinding/name_lookup.swift

@@ -609,7 +609,7 @@ class ShadowingGenericParameter<T> {
   func foo (t : T) {}
 }
 
-_ = ShadowingGenericParameter<String>().foo(t: "hi")
+ShadowingGenericParameter<String>().foo(t: "hi")
 
 // rdar://problem/51266778
 struct PatternBindingWithTwoVars1 { var x = 3, y = x }

test/Sema/call_as_function_simple.swift

@@ -106,8 +106,8 @@ func testMutating(_ x: Mutating, _ y: inout Mutating) {
   _ = x()
   // expected-error @+1 {{cannot use mutating member on immutable value: 'x' is a 'let' constant}}
   _ = x.callAsFunction()
-  _ = y()
-  _ = y.callAsFunction()
+  y()
+  y.callAsFunction()
 }
 
 struct Inout {
@@ -185,8 +185,8 @@ func testIUO(a: SimpleCallable!, b: MultipleArgsCallable!, c: Extended!,
   _ = d()?.callAsFunction()?()
   _ = e()
   _ = e(1, 2, 3)
-  _ = f()
-  _ = g(&inoutInt)
+  f()
+  g(&inoutInt)
   _ = try? h()
   _ = try? h { throw DummyError() }
 }

test/Sema/implementation-only-import-inlinable-conformances.swift

@@ -135,7 +135,7 @@ extension NormalProtoAssocHolder {
 }
 
 @inlinable func testMultipleConformances() {
-  _ = NormalProtoAssocHolder<NormalStruct>.testAnotherConformance(NormalClass.self)
+  NormalProtoAssocHolder<NormalStruct>.testAnotherConformance(NormalClass.self)
  // expected-error@-1 2 {{cannot use conformance of 'NormalStruct' to 'NormalProto' here; 'BADLibrary' has been imported as implementation-only}}
   // expected-error@-2 {{cannot use conformance of 'NormalClass' to 'NormalProto' here; 'BADLibrary' has been imported as implementation-only}}
 }

test/Serialization/Recovery/typedefs-in-protocols.swift

@@ -44,7 +44,7 @@ public func testGenericDispatch<T: Proto>(user: T) {
   // CHECK-IR: [[METHOD:%.+]] = bitcast i8* [[RAW_METHOD]] to void (%swift.opaque*, %swift.type*, i8**)*
   // CHECK-IR-NOT: ret
   // CHECK-IR: call swiftcc void [[METHOD]](
-  _ = user.lastMethod()
+  user.lastMethod()
 } // CHECK-IR: ret void
 
 #if VERIFY

test/Serialization/Recovery/typedefs.swift

@@ -39,7 +39,7 @@ public func testVTableBuilding(user: User) {
   // changes, please check that offset is still correct.
   // CHECK-IR-NOT: ret
   // CHECK-IR: getelementptr inbounds void (%T3Lib4UserC*)*, void (%T3Lib4UserC*)** %{{[0-9]+}}, {{i64 28|i32 31}}
-  _ = user.lastMethod()
+  user.lastMethod()
 } // CHECK-IR: ret void
 
 #if VERIFY

test/attr/attr_dynamic_callable.swift

@@ -451,7 +451,7 @@ struct A {
 }
 
 func test9239() {
-  _ = A()() // ok
+  A()() // ok
 }
 
 // SR-10313

test/decl/func/operator.swift

@@ -391,10 +391,10 @@ func testPrefixOperatorOnTuple() {
   _ = (∫)foo
   // expected-error@-1 {{consecutive statements on a line must be separated by ';'}}
   // expected-warning@-2 {{expression of type '(Int, Int)' is unused}}
-  _ = (∫)(foo)
+  (∫)(foo)
   _ = ∫(1, 2)
   _ = (∫)(1, 2) // expected-error {{operator function '∫' expects a single parameter of type '(Int, Int)'}}
-  _ = (∫)((1, 2))
+  (∫)((1, 2))
 }
 
 postfix operator §
@@ -412,9 +412,9 @@ func testPostfixOperatorOnTuple<A, B>(a: A, b: B) {
   // expected-error@-2 {{generic parameter 'T' could not be inferred}}
   // expected-error@-3 {{generic parameter 'U' could not be inferred}}
   // expected-warning@-4 {{expression of type '(A, (B, B), A)' is unused}}
-  _ = (§)(foo)
+  (§)(foo)
   _ = (a, (b, b), a)§
   _ = (§)(a, (b, b), a) // expected-error {{operator function '§' expects a single parameter of type '(T, (U, U), T)'}}
-  _ = (§)((a, (b, b), a))
+  (§)((a, (b, b), a))
   _ = (a, ((), (b, (a, a), b)§), a)§
 }

test/decl/func/special_func_name.swift

@@ -35,15 +35,15 @@ struct S3 {
 
 _ = S11(0) // expected-error {{argument passed to call that takes no arguments}}
 _ = S11.init(0) // expected-error {{argument passed to call that takes no arguments}}
-_ = S11.`init`(0)
+S11.`init`(0)
 
 _ = S12(0)
 _ = S12.init(0)
 _ = S12.`init`(0) // expected-error {{type 'S12' has no member 'init'}}
 
 _ = S21(0) // expected-error {{argument passed to call that takes no arguments}}
 _ = S21.init(0) // expected-error {{argument passed to call that takes no arguments}}
-_ = S21.`init`(0)
+S21.`init`(0)
 
 _ = S22(0)
 _ = S22.init(0)

test/decl/protocol/req/optional_visibility.swift

@@ -11,5 +11,5 @@ class Conforms : Opt {
 }
 
 func g(x: Conforms) {
-  _ = x.f(callback: {})
+  x.f(callback: {})
 }

test/expr/unary/keypath/keypath.swift

@@ -814,7 +814,7 @@ func test_keypath_with_method_refs() {
 // SR-10467 - Argument type 'KeyPath<String, Int>' does not conform to expected type 'Any'
 func test_keypath_in_any_context() {
   func foo(_: Any) {}
-  _ = foo(\String.count) // Ok
+  foo(\String.count) // Ok
 }
 
 protocol PWithTypeAlias {

test/stdlib/UnsafePointerDiagnostics.swift

@@ -131,7 +131,7 @@ struct SR9800 {
   func foo(_: UnsafePointer<UInt8>) {}
 
   func ambiguityTest(buf: UnsafeMutablePointer<CChar>) {
-    _ = foo(UnsafePointer(buf)) // this call should be unambiguoius
+    foo(UnsafePointer(buf)) // this call should be unambiguoius
   }
 }