From d2f9768ec21a863d597f96f35a0c850bb71d2be2 Mon Sep 17 00:00:00 2001 From: Matthew Carroll Date: Wed, 11 Jan 2017 15:39:02 -0500 Subject: [PATCH] [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. --- include/swift/AST/DiagnosticsSema.def | 8 ++++++++ lib/Sema/TypeCheckAttr.cpp | 17 ++++++++++++++++- .../LoggingWrappers.swift.gyb | 1 - stdlib/public/core/HashedCollections.swift.gyb | 1 - test/FixCode/fixits-apply-all.swift | 4 ++++ test/FixCode/fixits-apply-all.swift.result | 4 ++++ 6 files changed, 32 insertions(+), 3 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 70889445b979c..b33ae8d55d737 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/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 //------------------------------------------------------------------------------ diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index b3923c11d1942..2060c545325c2 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -726,7 +726,6 @@ class AttributeChecker : public AttributeVisitor { 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 { 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(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); diff --git a/stdlib/private/StdlibCollectionUnittest/LoggingWrappers.swift.gyb b/stdlib/private/StdlibCollectionUnittest/LoggingWrappers.swift.gyb index 35cfc9837d423..d440cf494c08c 100644 --- a/stdlib/private/StdlibCollectionUnittest/LoggingWrappers.swift.gyb +++ b/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) diff --git a/stdlib/public/core/HashedCollections.swift.gyb b/stdlib/public/core/HashedCollections.swift.gyb index 446ce5f294152..d22a3d5643a62 100644 --- a/stdlib/public/core/HashedCollections.swift.gyb +++ b/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 } diff --git a/test/FixCode/fixits-apply-all.swift b/test/FixCode/fixits-apply-all.swift index f9610e58ac911..2b7ec0310f90a 100644 --- a/test/FixCode/fixits-apply-all.swift +++ b/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() } diff --git a/test/FixCode/fixits-apply-all.swift.result b/test/FixCode/fixits-apply-all.swift.result index 776c9491f8cdf..af4204def1c16 100644 --- a/test/FixCode/fixits-apply-all.swift.result +++ b/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() }