[clang-tools-extra] [clang-tidy] Skip user headers named like C headers in `modernize-deprecated-headers` (PR #195507)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Zeyi Xu (zeyi2)

<details>
<summary>Changes</summary>

Previously `modernize-deprecated-headers` would match on any header with the same name as standard library headers. This commit fixes the problem by checking whether the include resolves to a system header.

Closes #<!-- -->45991 

---
Full diff: https://github.com/llvm/llvm-project/pull/195507.diff


4 Files Affected:

- (modified) clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp (+4) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) 
- (added) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/deprecated-headers/user/assert.h (+1) 
- (added) clang-tools-extra/test/clang-tidy/checkers/modernize/deprecated-headers-user.cpp (+9) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
index eff7c2f36d49d..88c411fcc6633 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -175,6 +175,10 @@ void IncludeModernizePPCallbacks::InclusionDirective(
   if (SM.isInSystemHeader(HashLoc))
     return;
 
+  // Skip headers that happen to use the same name as a standard library header.
+  if (!File || !SrcMgr::isSystem(FileType))
+    return;
+
   // FIXME: Take care of library symbols from the global namespace.
   //
   // Reasonable options for the check:
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c9749df481bcd..ede3942d69106 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -408,6 +408,11 @@ Changes in existing checks
   Because it only sees one file at a time, the check can't be sure
   such entities aren't referenced in any other files of that module.
 
+- Improved :doc:`modernize-deprecated-headers
+  <clang-tidy/checks/modernize/deprecated-headers>` check by avoiding false
+  positives on project headers that use the same name as a standard library
+  header.
+
 - Improved :doc:`modernize-pass-by-value
   <clang-tidy/checks/modernize/pass-by-value>` check by adding `IgnoreMacros`
   option to suppress warnings in macros.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/deprecated-headers/user/assert.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/deprecated-headers/user/assert.h
new file mode 100644
index 0000000000000..1038e93a26f11
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/deprecated-headers/user/assert.h
@@ -0,0 +1 @@
+#define USER_ASSERT_H 1
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/deprecated-headers-user.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/deprecated-headers-user.cpp
new file mode 100644
index 0000000000000..4fa74a63b9c37
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/deprecated-headers-user.cpp
@@ -0,0 +1,9 @@
+// RUN: %check_clang_tidy -std=c++11 %s modernize-deprecated-headers %t -- -extra-arg-before=-iquote%S/Inputs/deprecated-headers/user -extra-arg-before=-isystem%S/Inputs/deprecated-headers
+
+#include "assert.h"
+
+#include <assert.h>
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: inclusion of deprecated C++ header 'assert.h'; consider using 'cassert' instead [modernize-deprecated-headers]
+// CHECK-FIXES: #include <cassert>
+
+int user_header = USER_ASSERT_H;

``````````

</details>


https://github.com/llvm/llvm-project/pull/195507