[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

236 ↗

(On Diff #473477)

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

236 ↗

(On Diff #473477)

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

236 ↗

(On Diff #473477)

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: Operators declared in a class context 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