[PATCH] D159263: [clang-tidy] misc-include-cleaner: remove duplicated includes & fixes

[Ding Fei via Phabricator via cfe-commits]

danix800 created this revision.
danix800 added reviewers: VitaNuo, kadircet, sammccall.
danix800 added a project: clang-tools-extra.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
danix800 requested review of this revision.
Herald added a subscriber: cfe-commits.

1. Duplicated includes could be removed:

  #include "bar.h"
  #include "bar.h" # could be eliminated



2. FixItHint should not be applied (multiple insertions) for different symbols from same header.

  // baz.h
  #pragma once
  #define BAZ 10
  int baz();
  
  // include-cleaner.cpp
  // only one '#include "baz.h"' should be inserted.
  int BazResult = baz();
  int BazResultAgain = BAZ;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159263

Files:
  clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/baz.h
  clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/include-cleaner.cpp
@@ -1,5 +1,8 @@
 // RUN: %check_clang_tidy %s misc-include-cleaner %t -- -- -I%S/Inputs -isystem%S/Inputs/system
 #include "bar.h"
+#include "bar.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header bar.h is not used directly [misc-include-cleaner]
+// CHECK-FIXES: {{^}}
 // CHECK-FIXES: {{^}}#include "baz.h"{{$}}
 #include "foo.h"
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: included header foo.h is not used directly [misc-include-cleaner]
@@ -11,6 +14,8 @@
 int BarResult = bar();
 int BazResult = baz();
 // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: no header providing "baz" is directly included [misc-include-cleaner]
+int BazResultAgain = BAZ; // Header should not be inserted more than once
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: no header providing "BAZ" is directly included [misc-include-cleaner]
 std::string HelloString;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: no header providing "std::string" is directly included [misc-include-cleaner]
 int FooBarResult = foobar();
Index: clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/baz.h
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/baz.h
+++ clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/baz.h
@@ -1,2 +1,3 @@
 #pragma once
+#define BAZ 10
 int baz();
Index: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -33,6 +33,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
@@ -156,10 +157,18 @@
            });
 
   std::vector<const include_cleaner::Include *> Unused;
+  llvm::StringSet<> AlreadyIncludedHeaders;
   for (const include_cleaner::Include &I :
        RecordedPreprocessor.Includes.all()) {
-    if (Used.contains(&I) || !I.Resolved)
+    if (!I.Resolved)
       continue;
+    if (Used.contains(&I)) {
+      if (!AlreadyIncludedHeaders.contains(I.Spelled))
+        AlreadyIncludedHeaders.insert(I.Spelled);
+      else
+        Unused.push_back(&I);
+      continue;
+    }
     if (RecordedPI.shouldKeep(*I.Resolved))
       continue;
     // Check if main file is the public interface for a private header. If so
@@ -199,6 +208,7 @@
 
   tooling::HeaderIncludes HeaderIncludes(getCurrentMainFile(), Code,
                                          FileStyle->IncludeStyle);
+  llvm::StringSet<> AlreadyInserted;
   for (const auto &Inc : Missing) {
     std::string Spelling = include_cleaner::spellHeader(
         {Inc.Missing, PP->getHeaderSearchInfo(), MainFile});
@@ -209,14 +219,18 @@
     // main file.
     if (auto Replacement =
             HeaderIncludes.insert(llvm::StringRef{Spelling}.trim("\"<>"),
-                                  Angled, tooling::IncludeDirective::Include))
-      diag(SM->getSpellingLoc(Inc.SymRef.RefLocation),
-           "no header providing \"%0\" is directly included")
-          << Inc.SymRef.Target.name()
-          << FixItHint::CreateInsertion(
-                 SM->getComposedLoc(SM->getMainFileID(),
-                                    Replacement->getOffset()),
-                 Replacement->getReplacementText());
+                                  Angled, tooling::IncludeDirective::Include)) {
+      DiagnosticBuilder DB =
+          diag(SM->getSpellingLoc(Inc.SymRef.RefLocation),
+               "no header providing \"%0\" is directly included")
+          << Inc.SymRef.Target.name();
+      if (!AlreadyInserted.contains(Inc.Missing.resolvedPath())) {
+        DB << FixItHint::CreateInsertion(
+            SM->getComposedLoc(SM->getMainFileID(), Replacement->getOffset()),
+            Replacement->getReplacementText());
+        AlreadyInserted.insert(Inc.Missing.resolvedPath());
+      }
+    }
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D159263.554947.patch
Type: text/x-patch
Size: 4355 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230831/17953493/attachment.bin>