[clang-tools-extra] 4d90ff5 - [clangd] When inserting "using", add "::" in front if that's the style.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2020-08-25T14:07:49+02:00
New Revision: 4d90ff59ac453a67ac692ffdf8242e4cfbd2b34f

URL: https://github.com/llvm/llvm-project/commit/4d90ff59ac453a67ac692ffdf8242e4cfbd2b34f
DIFF: https://github.com/llvm/llvm-project/commit/4d90ff59ac453a67ac692ffdf8242e4cfbd2b34f.diff

LOG: [clangd] When inserting "using", add "::" in front if that's the style.

We guess the style based on the existing using declarations. If there
are any and they all start with ::, we add it to the newly added one
too.

Differential Revision: https://reviews.llvm.org/D86473

Added: 
    

Modified: 
    clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
    clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index 9e64ceeeaead..e4900041671a 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -86,6 +86,13 @@ class UsingFinder : public RecursiveASTVisitor<UsingFinder> {
   const SourceManager &SM;
 };
 
+bool isFullyQualified(const NestedNameSpecifier *NNS) {
+  if (!NNS)
+    return false;
+  return NNS->getKind() == NestedNameSpecifier::Global ||
+         isFullyQualified(NNS->getPrefix());
+}
+
 struct InsertionPointData {
   // Location to insert the "using" statement. If invalid then the statement
   // should not be inserted at all (it already exists).
@@ -94,6 +101,9 @@ struct InsertionPointData {
   // insertion point is anchored to, we may need one or more \n to ensure
   // proper formatting.
   std::string Suffix;
+  // Whether using should be fully qualified, even if what the user typed was
+  // not. This is based on our detection of the local style.
+  bool AlwaysFullyQualify = false;
 };
 
 // Finds the best place to insert the "using" statement. Returns invalid
@@ -118,7 +128,13 @@ findInsertionPoint(const Tweak::Selection &Inputs,
               SM)
       .TraverseAST(Inputs.AST->getASTContext());
 
+  bool AlwaysFullyQualify = true;
   for (auto &U : Usings) {
+    // Only "upgrade" to fully qualified is all relevant using decls are fully
+    // qualified. Otherwise trust what the user typed.
+    if (!isFullyQualified(U->getQualifier()))
+      AlwaysFullyQualify = false;
+
     if (SM.isBeforeInTranslationUnit(Inputs.Cursor, U->getUsingLoc()))
       // "Usings" is sorted, so we're done.
       break;
@@ -137,6 +153,7 @@ findInsertionPoint(const Tweak::Selection &Inputs,
   if (LastUsingLoc.isValid()) {
     InsertionPointData Out;
     Out.Loc = LastUsingLoc;
+    Out.AlwaysFullyQualify = AlwaysFullyQualify;
     return Out;
   }
 
@@ -278,6 +295,9 @@ Expected<Tweak::Effect> AddUsing::apply(const Selection &Inputs) {
     std::string UsingText;
     llvm::raw_string_ostream UsingTextStream(UsingText);
     UsingTextStream << "using ";
+    if (InsertionPoint->AlwaysFullyQualify &&
+        !isFullyQualified(QualifierToRemove.getNestedNameSpecifier()))
+      UsingTextStream << "::";
     QualifierToRemove.getNestedNameSpecifier()->print(
         UsingTextStream, Inputs.AST->getASTContext().getPrintingPolicy());
     UsingTextStream << Name << ";" << InsertionPoint->Suffix;

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index b4f135b3efe2..5626b7530599 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2723,6 +2723,63 @@ namespace foo { void fun(); }
 
 void foo::fun() {
   ff();
+})cpp"},
+            // If all other using are fully qualified, add ::
+            {R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ee;
+
+void fun() {
+  one::two::f^f();
+})cpp",
+             R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ff;using ::one::two::ee;
+
+void fun() {
+  ff();
+})cpp"},
+            // Make sure we don't add :: if it's already there
+            {R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ee;
+
+void fun() {
+  ::one::two::f^f();
+})cpp",
+             R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ff;using ::one::two::ee;
+
+void fun() {
+  ff();
+})cpp"},
+            // If even one using doesn't start with ::, do not add it
+            {R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using one::two::ee;
+
+void fun() {
+  one::two::f^f();
+})cpp",
+             R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using one::two::ff;using one::two::ee;
+
+void fun() {
+  ff();
 })cpp"}};
   llvm::StringMap<std::string> EditedFiles;
   for (const auto &Case : Cases) {