[clang] [include-cleaner]Support #import directives in HeaderIncludes (PR #212582)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: dmaclach (dmaclach)

<details>
<summary>Changes</summary>

This change updates HeaderIncludes to recognize #import directives in addition to #include when parsing existing headers. It also fixes up an existing issue parsing include/import statements that may have contained tabs.


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


2 Files Affected:

- (modified) clang/lib/Tooling/Inclusions/HeaderIncludes.cpp (+5-3) 
- (modified) clang/unittests/Tooling/HeaderIncludesTest.cpp (+41) 


``````````diff
diff --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
index 34f21faa8e342..62752122db2ac 100644
--- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -154,7 +154,7 @@ unsigned getMinHeaderInsertionOffset(StringRef FileName, StringRef Code,
 }
 
 // Check if a sequence of tokens is like
-//    "#include ("header.h" | <header.h>)".
+//    "#(include | import) ("header.h" | <header.h>)".
 // If it is, \p Tok will be the token after this directive; otherwise, it can be
 // any token after the given \p Tok (including \p Tok).
 bool checkAndConsumeInclusiveDirective(Lexer &Lex, Token &Tok) {
@@ -163,7 +163,9 @@ bool checkAndConsumeInclusiveDirective(Lexer &Lex, Token &Tok) {
     return true;
   };
   if (Tok.is(tok::hash) && !Lex.LexFromRawLexer(Tok) &&
-      Tok.is(tok::raw_identifier) && Tok.getRawIdentifier() == "include") {
+      Tok.is(tok::raw_identifier) &&
+      (Tok.getRawIdentifier() == "include" ||
+       Tok.getRawIdentifier() == "import")) {
     if (Lex.LexFromRawLexer(Tok))
       return false;
     if (Tok.is(tok::string_literal))
@@ -246,7 +248,7 @@ inline StringRef trimInclude(StringRef IncludeName) {
 }
 
 const char IncludeRegexPattern[] =
-    R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+    "^[\t ]*#[\t ]*(import|include)[^\"<]*([\"<][^\">]*[\">])";
 
 // The filename of Path excluding extension.
 // Used to match implementation with headers, this differs from sys::path::stem:
diff --git a/clang/unittests/Tooling/HeaderIncludesTest.cpp b/clang/unittests/Tooling/HeaderIncludesTest.cpp
index bc2ac3a97e97d..4c1848252df56 100644
--- a/clang/unittests/Tooling/HeaderIncludesTest.cpp
+++ b/clang/unittests/Tooling/HeaderIncludesTest.cpp
@@ -82,6 +82,47 @@ int x;)cpp";
   EXPECT_EQ("\nint x;", remove(Code, "<abc.h>"));
 }
 
+TEST_F(HeaderIncludesTest, DeleteMixedImportAndIncludeQuoted) {
+  std::string Code = R"cpp(
+#include "a.h"
+#import "a.h"
+int x;)cpp";
+  EXPECT_EQ("\nint x;", remove(Code, "\"a.h\""));
+}
+
+TEST_F(HeaderIncludesTest, ImportWithSpacesAndTabs) {
+  std::string Code = "int x;\n";
+  // The parser should detect these as existing imports if we had them,
+  // but here we are testing insertion/detection integration.
+  // Let's verify that a file with weird spacing is parsed correctly.
+  std::string CodeWithSpaces =
+      "#  import   \"a.h\"\n#\tinclude\t\"b.h\"\nint x;\n";
+
+  // Try inserting "a.h" again as import - should be blocked by the existing one
+  // if the regex captures it correctly.
+  EXPECT_EQ(CodeWithSpaces,
+            insert(CodeWithSpaces, "\"a.h\"", IncludeDirective::Import));
+
+  // Try inserting "b.h" again as include - should be blocked.
+  EXPECT_EQ(CodeWithSpaces,
+            insert(CodeWithSpaces, "\"b.h\"", IncludeDirective::Include));
+}
+
+TEST_F(HeaderIncludesTest, InsertIncludeWhenImportExists) {
+  std::string Code = "#import \"a.h\"\n";
+  std::string Expected = Code + "#include \"a.h\"\n";
+  // Currently, the logic allows inserting #include even if #import exists
+  // because the Directive differs. This test verifies this current behavior.
+  EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Include));
+}
+
+TEST_F(HeaderIncludesTest, InsertImportWhenIncludeExists) {
+  std::string Code = "#include \"a.h\"\n";
+  std::string Expected = Code + "#import \"a.h\"\n";
+  // Similarly, allows inserting #import even if #include exists.
+  EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Import));
+}
+
 TEST_F(HeaderIncludesTest, NoExistingIncludeWithDefine) {
   std::string Code = "#ifndef A_H\n"
                      "#define A_H\n"

``````````

</details>


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