Fully implement fix-its reusing designated initializer computation from Clangd

Author: SimplyDanny

clang-tools-extra/clang-tidy/modernize/UseDesignatedInitializersCheck.cpp

@@ -14,9 +14,9 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchersMacros.h"
-#include <algorithm>
-#include <iterator>
-#include <vector>
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/DesignatedInitializers.h"
 
 using namespace clang::ast_matchers;
 
@@ -29,15 +29,6 @@ static const bool IgnoreSingleElementAggregatesDefault = true;
 static const char *RestrictToPODTypesName = "RestrictToPODTypes";
 static const bool RestrictToPODTypesDefault = false;
 
-static std::vector<Stmt *>
-getUndesignatedComponents(const InitListExpr *SyntacticInitList) {
-  std::vector<Stmt *> Result;
-  std::copy_if(SyntacticInitList->begin(), SyntacticInitList->end(),
-               std::back_inserter(Result),
-               [](auto S) { return !isa<DesignatedInitExpr>(S); });
-  return Result;
-}
-
 UseDesignatedInitializersCheck::UseDesignatedInitializersCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), IgnoreSingleElementAggregates(Options.get(
@@ -51,7 +42,9 @@ AST_MATCHER(CXXRecordDecl, isAggregate) { return Node.isAggregate(); }
 AST_MATCHER(CXXRecordDecl, isPOD) { return Node.isPOD(); }
 
 AST_MATCHER(InitListExpr, isFullyDesignated) {
-  return getUndesignatedComponents(&Node).empty();
+  return std::all_of(Node.begin(), Node.end(), [](auto *InitExpr) {
+    return isa<DesignatedInitExpr>(InitExpr);
+  });
 }
 
 AST_MATCHER(InitListExpr, hasSingleElement) { return Node.getNumInits() == 1; }
@@ -82,31 +75,43 @@ void UseDesignatedInitializersCheck::registerMatchers(MatchFinder *Finder) {
       this);
 }
 
+static bool isFullyUndesignated(const InitListExpr *SyntacticInitList) {
+  return std::all_of(
+      SyntacticInitList->begin(), SyntacticInitList->end(),
+      [](auto *InitExpr) { return !isa<DesignatedInitExpr>(InitExpr); });
+}
+
 void UseDesignatedInitializersCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *InitList = Result.Nodes.getNodeAs<InitListExpr>("init");
   const auto *Type = Result.Nodes.getNodeAs<CXXRecordDecl>("type");
   if (!Type || !InitList)
     return;
   if (const auto *SyntacticInitList = InitList->getSyntacticForm()) {
-    const auto UndesignatedComponents =
-        getUndesignatedComponents(SyntacticInitList);
-    if (UndesignatedComponents.size() == SyntacticInitList->getNumInits()) {
-      diag(InitList->getLBraceLoc(), "use designated initializer list");
-      return;
-    }
-    const auto FieldIterator = Type->fields().begin();
-    for (const auto *InitExpr : *SyntacticInitList) {
-      const auto Field = std::next(FieldIterator);
-      if (std::find(UndesignatedComponents.begin(),
-                    UndesignatedComponents.end(),
-                    InitExpr) == UndesignatedComponents.end())
-        continue;
-      if (const auto *FieldID = Field->getIdentifier()) {
-        const auto FieldName = FieldID->getName();
-        diag(InitExpr->getBeginLoc(), "use designated init expression")
-            << FixItHint::CreateInsertion(InitExpr->getBeginLoc(),
-                                          "." + FieldName.str() + "=");
+    const llvm::DenseMap<clang::SourceLocation, std::string> Designators =
+        clang::tooling::getDesignators(SyntacticInitList);
+    if (isFullyUndesignated(SyntacticInitList)) {
+      std::string NewList = "{";
+      for (const Stmt *InitExpr : *SyntacticInitList) {
+        if (InitExpr != *SyntacticInitList->begin())
+          NewList += ", ";
+        NewList += Designators.at(InitExpr->getBeginLoc());
+        NewList += "=";
+        NewList += Lexer::getSourceText(
+            CharSourceRange::getTokenRange(InitExpr->getSourceRange()),
+            *Result.SourceManager, getLangOpts());
+      }
+      NewList += "}";
+      diag(InitList->getLBraceLoc(), "use designated initializer list")
+          << FixItHint::CreateReplacement(InitList->getSourceRange(), NewList);
+    } else {
+      for (const auto *InitExpr : *SyntacticInitList) {
+        if (!isa<DesignatedInitExpr>(InitExpr)) {
+          diag(InitExpr->getBeginLoc(), "use designated init expression")
+              << FixItHint::CreateInsertion(
+                     InitExpr->getBeginLoc(),
+                     Designators.at(InitExpr->getBeginLoc()) + "=");
+        }
       }
     }
   }

clang-tools-extra/test/clang-tidy/checkers/modernize/use-designated-initializers.cpp

@@ -20,16 +20,19 @@ S2 s21{.i=1, .j =2};
 S2 s22 = {1, 2};
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers]
 // CHECK-MESSAGES-POD: :[[@LINE-2]]:10: warning: use designated initializer list [modernize-use-designated-initializers]
+// CHECK-FIXES: S2 s22 = {.i=1, .j=2};
 
 S2 s23{1};
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use designated initializer list [modernize-use-designated-initializers]
 // CHECK-MESSAGES-POD: :[[@LINE-2]]:7: warning: use designated initializer list [modernize-use-designated-initializers]
+// CHECK-FIXES: S2 s23{.i=1};
 
 S2 s24{.i = 1};
 
 S2 s25 = {.i=1, 2};
 // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use designated init expression [modernize-use-designated-initializers]
 // CHECK-MESSAGES-POD: :[[@LINE-2]]:17: warning: use designated init expression [modernize-use-designated-initializers]
+// CHECK-FIXES: S2 s25 = {.i=1, .j=2};
 
 class S3 {
   public:
@@ -42,12 +45,14 @@ S3 s31 = {.s2 = 1, 2, 3.1};
 // CHECK-MESSAGES: :[[@LINE-2]]:23: warning: use designated init expression [modernize-use-designated-initializers]
 // CHECK-MESSAGES-POD: :[[@LINE-3]]:20: warning: use designated init expression [modernize-use-designated-initializers]
 // CHECK-MESSAGES-POD: :[[@LINE-4]]:23: warning: use designated init expression [modernize-use-designated-initializers]
+// CHECK-FIXES: S3 s31 = {.s2 = 1, .s2.j=2, .d=3.1};
 
 S3 s32 = {{.i = 1, 2}};
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers]
 // CHECK-MESSAGES: :[[@LINE-2]]:20: warning: use designated init expression [modernize-use-designated-initializers]
 // CHECK-MESSAGES-POD: :[[@LINE-3]]:10: warning: use designated initializer list [modernize-use-designated-initializers]
 // CHECK-MESSAGES-POD: :[[@LINE-4]]:20: warning: use designated init expression [modernize-use-designated-initializers]
+// CHECK-FIXES: S3 s32 = {.s2={.i = 1, 2}};
 
 struct S4 {
     double d;
@@ -56,9 +61,11 @@ struct S4 {
 
 S4 s41 {2.2};
 // CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:8: warning: use designated initializer list [modernize-use-designated-initializers]
+// CHECK-FIXES-SINGLE-ELEMENT: S4 s41 {.d=2.2};
 
 S4 s42 = {{}};
 // CHECK-MESSAGES-SINGLE-ELEMENT: :[[@LINE-1]]:10: warning: use designated initializer list [modernize-use-designated-initializers]
+// CHECK-FIXES-SINGLE-ELEMENT: S4 s42 = {.d={}};
 
 template<typename S> S template1() { return {10, 11}; }
 
@@ -99,3 +106,4 @@ struct S9 {
 
 S9 s91{1, 2};
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use designated initializer list [modernize-use-designated-initializers]
+// CHECK-FIXES: S9 s91{.i=1, .j=2};