[Clangd] Fix the code action `RemoveUsingNamespace`

[v1nh1shungry]

Summary

Avoid adding qualifiers before C++ operators declared in a non-class context

[tom-anders]

Did a quick test on my machine, seems to work! Can you add a regression test to clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp though?

[v1nh1shungry]

Add a test and rewrite the comment

[v1nh1shungry]

Correct the test

[v1nh1shungry]

@tom-anders Thank you for reviewing!

I tried adding a test but I don't know how to run the tests.

And I just found that operators declared in a class context are already handled in the existing version, but user-defined literals are not, so I rewrote the comments.

[v1nh1shungry]

Correct the test

[junaire]

In D137494#3910500, @v1nh1shungry wrote:

@tom-anders Thank you for reviewing!

I tried adding a test but I don't know how to run the tests.

And I just found that operators declared in a class context are already handled in the existing version, but user-defined literals are not, so I rewrote the comments.

I guess that's ninja check-clangd?

clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp

259

is this a typo?

[v1nh1shungry]

In D137494#3910500, @v1nh1shungry wrote:

@tom-anders Thank you for reviewing!

I tried adding a test but I don't know how to run the tests.

And I just found that operators declared in a class context are already handled in the existing version, but user-defined literals are not, so I rewrote the comments.

Sorry, I found tips about running tests in the document. Feel like a fool :)

Please check this test is correct and suitable. Thanks!

[v1nh1shungry]

clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp

259

I guess not. I refer to the tests above and I think this represents the cursor. But I'm not sure though.

[junaire]

clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp

259

I guess not. I refer to the tests above and I think this represents the cursor. But I'm not sure though.

OK, thanks for the clarification!

[v1nh1shungry]

@junaire Thank you for reviewing!

[v1nh1shungry]

Format codes

[tom-anders]

Thanks for the fix, LGTM! Do you want me to commit this for you?

[v1nh1shungry]

In D137494#3910598, @tom-anders wrote:

Thanks for the fix, LGTM! Do you want me to commit this for you?

Yes! Thanks a lot!

clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp

findExplicitReferences(
         [&](ReferenceLoc Ref) {
           if (Ref.Qualifier)
             return; // This reference is already qualified.
 
           for (auto *T : Ref.Targets) {
             if (!visibleContext(T->getDeclContext())
                      ->Equals(TargetDirective->getNominatedNamespace()))
               return;
+            // Avoid adding qualifiers before operators, e.g.
+            //   using namespace std;
+            //   cout << "foo"; // Must not changed to std::cout std:: << "foo"
+            // FIXME: User-defined literals are not handled
+            if (T->isInIdentifierNamespace(
+                    Decl::IdentifierNamespace::IDNS_NonMemberOperator))
+              return;
           }
           SourceLocation Loc = Ref.NameLoc;
           if (Loc.isMacroID()) {
             // Avoid adding qualifiers before macro expansions, it's probably
             // incorrect, e.g.
             //   namespace std { int foo(); }
             //   #define FOO 1 + foo()
             //   using namespace foo; // provides matrix

clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp

)cpp"},
       }
     )cpp",
        R"cpp(
       namespace std { inline namespace ns1 { inline namespace ns2 { struct vector {}; }}}
       
       int main() {
         std::vector V;
       }
+    )cpp"},
+      {// Does not qualify operators declared in a non-class context
+       R"cpp(
+      namespace ns {
+      struct Foo {};
+      void operator+(const Foo &, int) {}
+      }
+      using namespace n^s;
+      int main() {
+        Foo foo;
+        foo + 10;
+      }
+    )cpp",
+       R"cpp(
+      namespace ns {
+      struct Foo {};
+      void operator+(const Foo &, int) {}
+      }
+      
+      int main() {
+        ns::Foo foo;
+        foo + 10;
+      }
     )cpp"}};
   for (auto C : Cases)
     EXPECT_EQ(C.second, apply(C.first)) << C.first;
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang