[clang-tidy] Suppress google-objc-avoid-throwing-exception in system macros 🫢

[stephanemoore]

Summary

The google-objc-avoid-throwing-exception check enforces the Google
Objective-C Style Guide's prohibition on throwing exceptions in user
code but the check incorrectly triggers findings for code emitted from
system headers. This commit suppresses any findings that do not have
valid locations or are emitted from macros in system headers.

Avoid Throwing Exceptions, Google Objective-C Style Guide:
https://github.com/google/styleguide/blob/gh-pages/objcguide.md#avoid-throwing-exceptions

Test Notes:
Ran clang-tidy lit tests.

[stephanemoore]

[clang-tidy] Fix namespace comments in AvoidThrowingObjCExceptionCheck.cpp 🧹

This commit fixes namespace comments as suggested by the linter.

[stephanemoore]

Restore original diff that was replaced by accident.

[stephanemoore]

Included namespace comment linter fixes from D137740 as advised by @Eugene.Zelenko,

[stephanemoore]

The false positives from this check are causing some disruption for XCTest framework users after some changes that we believe happened in Xcode 14. It would be nice if we could land some form of fix soon. I am happy to discuss options and iterate as needed.

clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp

 }
 
 void AvoidThrowingObjCExceptionCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *MatchedStmt =
       Result.Nodes.getNodeAs<ObjCAtThrowStmt>("throwStmt");
   const auto *MatchedExpr =
       Result.Nodes.getNodeAs<ObjCMessageExpr>("raiseException");
   auto SourceLoc = MatchedStmt == nullptr ? MatchedExpr->getSelectorStartLoc()
                                           : MatchedStmt->getThrowLoc();
+
+  // Early return on invalid locations.
+  if (SourceLoc.isInvalid())
+    return;
+
+  // If the match location was in a macro, check if the macro was in a system
+  // header.
+  if (SourceLoc.isMacroID()) {
+    SourceManager &SM = *Result.SourceManager;
+    auto MacroLoc = SM.getImmediateMacroCallerLoc(SourceLoc);
+
+    // Matches in system header macros should be ignored.
+    if (SM.isInSystemHeader(MacroLoc))
+      return;
+  }
+
   diag(SourceLoc,
        "pass in NSError ** instead of throwing exception to indicate "
        "Objective-C errors");
 }
 
 } // namespace objc
 } // namespace google
 } // namespace tidy
 } // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 - Improved :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone/signal-handler>` check. Partial
   support for C++14 signal handler rules was added. Bug report generation was
   improved.
 
 - Fixed a false positive in :doc:`cppcoreguidelines-pro-type-member-init
   <clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` when warnings
   would be emitted for uninitialized members of an anonymous union despite
   there being an initializer for one of the other members.
 
+- Fixed false positives in :doc:`google-objc-avoid-throwing-exception
+  <clang-tidy/checks/google/objc-avoid-throwing-exception>` check for exceptions
+  thrown by code emitted from macros in system headers.
+
 - Improved :doc:`modernize-use-emplace <clang-tidy/checks/modernize/use-emplace>`
   check.
 
   The check now supports detecting inefficient invocations of ``push`` and
   ``push_front`` on STL-style containers and replacing them with ``emplace``
   or ``emplace_front``.
 
   The check now supports detecting alias cases of ``push_back`` ``push`` and
   ``push_front`` on STL-style containers and replacing them with ``emplace_back``,
   ``emplace`` or ``emplace_front``.

clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h

+#pragma clang system_header
+
+#define SYS_THROW(e) @throw e
+
+#define SYS_RAISE [NSException raise:@"example" format:@"fmt"]
+

clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m

-// RUN: %check_clang_tidy %s google-objc-avoid-throwing-exception %t
+// RUN: %check_clang_tidy %s google-objc-avoid-throwing-exception %t -- -- -I %S/Inputs/
+
 @class NSString;
 
 @interface NSException
 
 + (void)raise:(NSString *)name format:(NSString *)format;
 + (void)raise:(NSString *)name format:(NSString *)format arguments:(NSString *)args; // using NSString type since va_list cannot be recognized here
 
 @end
 
 @interface NotException

 
 @end
 
 @implementation Foo
 - (void)f {
     NSString *foo = @"foo";
     @throw foo;
     // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
 }
 
+#include "system-header-throw.h"
+
+#define THROW(e) @throw e
+
+#define RAISE [NSException raise:@"example" format:@"fmt"]
+
 - (void)f2 {
     [NSException raise:@"TestException" format:@"Test"];
     // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
     [NSException raise:@"TestException" format:@"Test %@" arguments:@"bar"];
     // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
     [NotException raise:@"NotException" format:@"Test"];
+
+    NSException *e;
+    SYS_THROW(e);
+
+    SYS_RAISE;
+
+    THROW(e);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
+
+    RAISE;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: pass in NSError ** instead of throwing exception to indicate Objective-C errors [google-objc-avoid-throwing-exception]
 }
 @end