[clang-tools-extra] 6c21409 - [clang-tidy] Suppress google-objc-avoid-throwing-exception in system macros 🫢

[Stephane Moore via cfe-commits]

Author: Stephane Moore
Date: 2022-11-30T16:44:45-08:00
New Revision: 6c2140943cbe257c85f7121349c5bca950a26e0d

URL: https://github.com/llvm/llvm-project/commit/6c2140943cbe257c85f7121349c5bca950a26e0d
DIFF: https://github.com/llvm/llvm-project/commit/6c2140943cbe257c85f7121349c5bca950a26e0d.diff

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

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.

Reviewed By: gribozavr2

Differential Revision: https://reviews.llvm.org/D137738

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

Modified: 
    clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
index 2263806fb6a7f..9205125e5ac1f 100644
--- a/clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp
@@ -36,6 +36,22 @@ void AvoidThrowingObjCExceptionCheck::check(
       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");

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 44c3743549ef0..52346e5b75589 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@ Changes in existing checks
   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.
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h
new file mode 100644
index 0000000000000..22d1bd45387b5
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/system-header-throw.h
@@ -0,0 +1,6 @@
+#pragma clang system_header
+
+#define SYS_THROW(e) @throw e
+
+#define SYS_RAISE [NSException raise:@"example" format:@"fmt"]
+

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m b/clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
index 7fa32e7a5aa59..c28bf6414bce6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/objc-avoid-throwing-exception.m
@@ -1,4 +1,5 @@
-// 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
@@ -21,12 +22,29 @@ - (void)f {
     // 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