[DiagnosticsQoI] SR-3359: Add a fix-it to remove @discardableResult on functions that return Void or Never

This commit adds a fix-it to remove @discardableResult on functions that return Void or Never. The fix-it is at the warning level. A test was added to verify that the fix-it removes the @discardableResult. This issue was reported in SR-3359:
https://bugs.swift.org/browse/SR-3359

Changes:
TypeCheckAttr.cpp: implemented AttributeChecker::visitDiscardableResultAttr to add a fix-it to remove @discardableResult on functions returning Void or Never.

DiagnosticsSema.def: Added a warning with a diagnostic message.

LoggingWrappers.swift.gyb, HashedCollections.swift.gyb: Removed @discardableResult on functions returning Void.

fixits-apply-all.swift, fixits-apply-all.swift.result: Added tests to verify that @discardableResult is removed from functions returning Void or Never.

Author: matthewcarroll

include/swift/AST/DiagnosticsSema.def

@@ -3295,6 +3295,14 @@ NOTE(availability_protocol_requirement_here, none,
 NOTE(availability_conformance_introduced_here, none,
      "conformance introduced here", ())
 
+//------------------------------------------------------------------------------
+// @discardableResult
+//------------------------------------------------------------------------------
+
+WARNING(discardable_result_on_void_never_function, none,
+        "@discardableResult declared on a function returning %select{Never|Void}0 is unnecessary",
+        (bool))
+
 //------------------------------------------------------------------------------
 // Resilience diagnostics
 //------------------------------------------------------------------------------

lib/Sema/TypeCheckAttr.cpp

@@ -726,7 +726,6 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
     IGNORED_ATTR(Testable)
     IGNORED_ATTR(WarnUnqualifiedAccess)
     IGNORED_ATTR(ShowInInterface)
-    IGNORED_ATTR(DiscardableResult)
 #undef IGNORED_ATTR
 
   void visitAvailableAttr(AvailableAttr *attr);
@@ -763,6 +762,8 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
   void visitSpecializeAttr(SpecializeAttr *attr);
 
   void visitVersionedAttr(VersionedAttr *attr);
+  
+  void visitDiscardableResultAttr(DiscardableResultAttr *attr);
 };
 } // end anonymous namespace
 
@@ -1534,6 +1535,20 @@ void AttributeChecker::visitVersionedAttr(VersionedAttr *attr) {
   }
 }
 
+void AttributeChecker::visitDiscardableResultAttr(DiscardableResultAttr *attr) {
+  if (auto *FD = dyn_cast<FuncDecl>(D)) {
+    if (auto result = FD->getResultInterfaceType()) {
+      auto resultIsVoid = result->isVoid();
+      if (resultIsVoid || result->isUninhabited()) {
+        auto warn = diag::discardable_result_on_void_never_function;
+        auto diagnostic = TC.diagnose(D->getStartLoc(), warn, resultIsVoid);
+        diagnostic.fixItRemove(attr->getRangeWithAt());
+        attr->setInvalid();
+      }
+    }
+  }
+}
+
 void TypeChecker::checkDeclAttributes(Decl *D) {
   AttributeChecker Checker(*this, D);
 

stdlib/private/StdlibCollectionUnittest/LoggingWrappers.swift.gyb

@@ -541,7 +541,6 @@ public struct ${Self}<
     return base.removeFirst()
   }
 
-  @discardableResult
   public mutating func removeFirst(_ n: Int) {
     Log.removeFirstN[selfType] += 1
     base.removeFirst(n)

stdlib/public/core/HashedCollections.swift.gyb

@@ -243,7 +243,6 @@ internal protocol _HashBuffer {
   @discardableResult
   mutating func removeValue(forKey key: Key) -> Value?
 
-  @discardableResult
   mutating func removeAll(keepingCapacity keepCapacity: Bool)
 
   var count: Int { get }

test/FixCode/fixits-apply-all.swift

@@ -20,3 +20,7 @@ func goo(_ e: Error) {
 
 @warn_unused_result(message="test message")
 func warn_unused_result_removal() -> Int { return 5 }
+
+@discardableResult func discardableResultOnVoidFunc() {}
+
+@discardableResult func discardableResultOnNeverFunc() -> Never { fatalError() }

test/FixCode/fixits-apply-all.swift.result

@@ -20,3 +20,7 @@ func goo(_ e: Error) {
 
 
 func warn_unused_result_removal() -> Int { return 5 }
+
+func discardableResultOnVoidFunc() {}
+
+func discardableResultOnNeverFunc() -> Never { fatalError() }