[PATCH] D147843: [clang-tidy] fix hint use correct range to replace last NamespaceDecl

[Congcong Cai via Phabricator via cfe-commits]

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

range of replacing last namespace decl should be from last non nested namespace 
to last namespace


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147843

Files:
  clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
===================================================================
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -105,7 +105,7 @@
 namespace n28 {
 namespace n29::n30 {
 // CHECK-MESSAGES-DAG: :[[@LINE-3]]:1: warning: nested namespaces can be 
concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n26::n27::n28::n29::n30
+// CHECK-FIXES: namespace n26::n27::n28::n29::n30 {
 void t() {}
 } // namespace n29::n30
 } // namespace n28
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -124,12 +124,12 @@
   SmallVector<SourceRange, 6> Backs;
   Backs.reserve(Namespaces.size());
 
-  NamespaceDecl const *LastND = nullptr;
+  NamespaceDecl const *LastNonNestND = nullptr;
 
   for (const NamespaceDecl *ND : Namespaces) {
     if (ND->isNested())
       continue;
-    LastND = ND;
+    LastNonNestND = ND;
     std::optional<SourceRange> SR =
         getCleanedNamespaceFrontRange(ND, SM, LangOpts);
     if (!SR.has_value())
@@ -137,7 +137,7 @@
     Fronts.push_back(SR.value());
     Backs.push_back(getCleanedNamespaceBackRange(ND, SM, LangOpts));
   }
-  if (LastND == nullptr || Fronts.empty() || Backs.empty())
+  if (LastNonNestND == nullptr || Fronts.empty() || Backs.empty())
     return;
   // the last one should be handled specially
   Fronts.pop_back();
@@ -147,9 +147,11 @@
   for (SourceRange const &Front : Fronts)
     DB << FixItHint::CreateRemoval(Front);
   DB << FixItHint::CreateReplacement(
-      SourceRange{LastND->getBeginLoc(), LastND->getLocation()},
+      SourceRange{LastNonNestND->getBeginLoc(),
+                  Namespaces.back()->getLocation()},
       ConcatNameSpace);
-  if (LastRBrace != SourceRange{LastND->getRBraceLoc(), 
LastND->getRBraceLoc()})
+  if (LastRBrace !=
+      SourceRange{LastNonNestND->getRBraceLoc(), 
LastNonNestND->getRBraceLoc()})
     DB << FixItHint::CreateReplacement(LastRBrace,
                                        ("} // " + ConcatNameSpace).str());
   for (SourceRange const &Back : llvm::reverse(Backs))



Index: clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/concat-nested-namespaces.cpp
@@ -105,7 +105,7 @@
 namespace n28 {
 namespace n29::n30 {
 // CHECK-MESSAGES-DAG: :[[@LINE-3]]:1: warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
-// CHECK-FIXES: namespace n26::n27::n28::n29::n30
+// CHECK-FIXES: namespace n26::n27::n28::n29::n30 {
 void t() {}
 } // namespace n29::n30
 } // namespace n28
Index: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
@@ -124,12 +124,12 @@
   SmallVector<SourceRange, 6> Backs;
   Backs.reserve(Namespaces.size());
 
-  NamespaceDecl const *LastND = nullptr;
+  NamespaceDecl const *LastNonNestND = nullptr;
 
   for (const NamespaceDecl *ND : Namespaces) {
     if (ND->isNested())
       continue;
-    LastND = ND;
+    LastNonNestND = ND;
     std::optional<SourceRange> SR =
         getCleanedNamespaceFrontRange(ND, SM, LangOpts);
     if (!SR.has_value())
@@ -137,7 +137,7 @@
     Fronts.push_back(SR.value());
     Backs.push_back(getCleanedNamespaceBackRange(ND, SM, LangOpts));
   }
-  if (LastND == nullptr || Fronts.empty() || Backs.empty())
+  if (LastNonNestND == nullptr || Fronts.empty() || Backs.empty())
     return;
   // the last one should be handled specially
   Fronts.pop_back();
@@ -147,9 +147,11 @@
   for (SourceRange const &Front : Fronts)
     DB << FixItHint::CreateRemoval(Front);
   DB << FixItHint::CreateReplacement(
-      SourceRange{LastND->getBeginLoc(), LastND->getLocation()},
+      SourceRange{LastNonNestND->getBeginLoc(),
+                  Namespaces.back()->getLocation()},
       ConcatNameSpace);
-  if (LastRBrace != SourceRange{LastND->getRBraceLoc(), LastND->getRBraceLoc()})
+  if (LastRBrace !=
+      SourceRange{LastNonNestND->getRBraceLoc(), LastNonNestND->getRBraceLoc()})
     DB << FixItHint::CreateReplacement(LastRBrace,
                                        ("} // " + ConcatNameSpace).str());
   for (SourceRange const &Back : llvm::reverse(Backs))

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits