Author: dmaclach Date: 2026-08-11T10:47:18-04:00 New Revision: 81e8fe4480842d56821d2b09a757bdc4e26eeb0c
URL: https://github.com/llvm/llvm-project/commit/81e8fe4480842d56821d2b09a757bdc4e26eeb0c DIFF: https://github.com/llvm/llvm-project/commit/81e8fe4480842d56821d2b09a757bdc4e26eeb0c.diff LOG: [clang] Replace existing `#include` directives with `#import` when inserting an import (#213751) When inserting a header with an `#import` directive, if an existing `#include` directive for the same header and quotation style is found, replace it with the `#import` directive instead of adding a duplicate. This is based on two assumptions: 1. that `#import` outranks `#include` since headers that are included are assumed to have appropriate include guards to prevent multiple inclusions 2. that there is no good reason to have an include and import of the same header in a given source file. Note that this is intended for include-cleaner support for Objective-C. Added: Modified: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp clang/unittests/Tooling/HeaderIncludesTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp index eef06fb5e0517..c3bbf6b5f2e73 100644 --- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp +++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp @@ -450,15 +450,34 @@ HeaderIncludes::insert(llvm::StringRef Header, bool IsAngled, IncludeDirective Directive) const { assert(Header == trimInclude(Header)); // If a <header> ("header") already exists in code, "header" (<header>) with - // diff erent quotation and/or directive will still be inserted. + // diff erent quotation will still be inserted. // FIXME: figure out if this is the best behavior. auto It = ExistingIncludes.find(Header); if (It != ExistingIncludes.end()) { - for (const auto &Inc : It->second) - if (Inc.Directive == Directive && - ((IsAngled && StringRef(Inc.Name).starts_with("<")) || - (!IsAngled && StringRef(Inc.Name).starts_with("\"")))) - return std::nullopt; + for (const auto &Inc : It->second) { + bool SameQuotation = (IsAngled && StringRef(Inc.Name).starts_with("<")) || + (!IsAngled && StringRef(Inc.Name).starts_with("\"")); + if (SameQuotation) { + // If the directive is the same, or if the directive is an include and + // the existing directive is an import, then we don't need to insert + // the header. + if ((Inc.Directive == Directive) || + (Inc.Directive == IncludeDirective::Import && + Directive == IncludeDirective::Include)) { + return std::nullopt; + } + + // "import" outranks "include" with the assumption that includes are + // designed to handle multiple inclusions while import is not. + char Open = IsAngled ? '<' : '"'; + char Close = IsAngled ? '>' : '"'; + std::string NewInclude = + llvm::formatv("#import {0}{1}{2}\n", Open, Header, Close); + + return tooling::Replacement(FileName, Inc.R.getOffset(), + Inc.R.getLength(), NewInclude); + } + } } std::string Quoted = std::string(llvm::formatv(IsAngled ? "<{0}>" : "\"{0}\"", Header)); diff --git a/clang/unittests/Tooling/HeaderIncludesTest.cpp b/clang/unittests/Tooling/HeaderIncludesTest.cpp index 4c1848252df56..e697538c56cc7 100644 --- a/clang/unittests/Tooling/HeaderIncludesTest.cpp +++ b/clang/unittests/Tooling/HeaderIncludesTest.cpp @@ -7,12 +7,12 @@ //===----------------------------------------------------------------------===// #include "clang/Tooling/Inclusions/HeaderIncludes.h" -#include "../Tooling/ReplacementTest.h" -#include "../Tooling/RewriterTestContext.h" #include "clang/Format/Format.h" #include "clang/Tooling/Core/Replacement.h" +#include "llvm/ADT/StringRef.h" #include "gtest/gtest.h" +#include <cassert> namespace clang { namespace tooling { @@ -65,7 +65,7 @@ TEST_F(HeaderIncludesTest, RepeatedIncludes) { TEST_F(HeaderIncludesTest, InsertImportWithSameInclude) { std::string Code = "#include \"a.h\"\n"; - std::string Expected = Code + "#import \"a.h\"\n"; + std::string Expected = "#import \"a.h\"\n"; EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Import)); } @@ -106,20 +106,38 @@ TEST_F(HeaderIncludesTest, ImportWithSpacesAndTabs) { // Try inserting "b.h" again as include - should be blocked. EXPECT_EQ(CodeWithSpaces, insert(CodeWithSpaces, "\"b.h\"", IncludeDirective::Include)); + + // Try inserting "b.h" again as import - should replace. + std::string ExpectedAfterBImport = + "# import \"a.h\"\n#import \"b.h\"\nint x;\n"; + EXPECT_EQ(ExpectedAfterBImport, + insert(CodeWithSpaces, "\"b.h\"", IncludeDirective::Import)); } 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 diff ers. This test verifies this current behavior. - EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Include)); + EXPECT_EQ(Code, insert(Code, "\"a.h\"", IncludeDirective::Include)); +} + +TEST_F(HeaderIncludesTest, InsertImportWhenIncludeExistsAngled) { + std::string Code = "#include <a.h>\n"; + std::string Expected = "#import <a.h>\n"; + // Replaces #include with #import. + EXPECT_EQ(Expected, insert(Code, "<a.h>", IncludeDirective::Import)); } -TEST_F(HeaderIncludesTest, InsertImportWhenIncludeExists) { +TEST_F(HeaderIncludesTest, InsertImportAngledWhenIncludeQuotedExists) { std::string Code = "#include \"a.h\"\n"; - std::string Expected = Code + "#import \"a.h\"\n"; - // Similarly, allows inserting #import even if #include exists. + std::string Expected = Code + "#import <a.h>\n"; + // Different quotation, so it should insert alongside, not replace. + EXPECT_EQ(Expected, insert(Code, "<a.h>", IncludeDirective::Import)); +} + +TEST_F(HeaderIncludesTest, InsertImportQuotedWhenIncludeAngledExists) { + std::string Code = "#include <a.h>\n"; + std::string Expected = "#import \"a.h\"\n#include <a.h>\n"; + // Different quotation, so it should insert alongside, not replace. + // " comes before < in ASCII, so it is inserted before. EXPECT_EQ(Expected, insert(Code, "\"a.h\"", IncludeDirective::Import)); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
