Index: change-namespace/ChangeNamespace.cpp =================================================================== --- change-namespace/ChangeNamespace.cpp +++ change-namespace/ChangeNamespace.cpp @@ -256,9 +256,10 @@ auto DeclMatcher = namedDecl( hasAncestor(namespaceDecl()), unless(anyOf( - hasAncestor(namespaceDecl(isAnonymous())), + isImplicit(), hasAncestor(namespaceDecl(isAnonymous())), hasAncestor(cxxRecordDecl()), allOf(IsInMovedNs, unless(cxxRecordDecl(unless(isDefinition()))))))); + // Match TypeLocs on the declaration. Carefully match only the outermost // TypeLoc that's directly linked to the old class and don't handle nested // name specifier locs. @@ -271,10 +272,13 @@ hasAncestor(decl().bind("dc"))) .bind("type"), this); + // Types in `UsingShadowDecl` is not matched by `typeLoc` above, so we need to // special case it. Finder->addMatcher( - usingDecl(hasAnyUsingShadowDecl(IsInMovedNs)).bind("using_decl"), this); + usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl())).bind("using_decl"), + this); + // Handle types in nested name specifier. Finder->addMatcher(nestedNameSpecifierLoc( hasAncestor(decl(IsInMovedNs).bind("dc")), @@ -284,18 +288,19 @@ this); // Handle function. - // Only handle functions that are defined in a namespace excluding static - // methods (qualified by nested specifier) and functions defined in the global - // namespace. + // Only handle functions that are defined in a namespace excluding member + // function, static methods (qualified by nested specifier), and functions + // defined in the global namespace. // Note that the matcher does not exclude calls to out-of-line static method // definitions, so we need to exclude them in the callback handler. - auto FuncMatcher = functionDecl( - hasParent(namespaceDecl()), - unless(anyOf(IsInMovedNs, hasAncestor(namespaceDecl(isAnonymous())), - hasAncestor(cxxRecordDecl())))); + auto FuncMatcher = + functionDecl(unless(anyOf(cxxMethodDecl(), IsInMovedNs, + hasAncestor(namespaceDecl(isAnonymous())), + hasAncestor(cxxRecordDecl()))), + hasParent(namespaceDecl())); Finder->addMatcher( decl(forEachDescendant(callExpr(callee(FuncMatcher)).bind("call")), - IsInMovedNs) + IsInMovedNs, unless(isImplicit())) .bind("dc"), this); } @@ -432,6 +437,8 @@ // Replaces a qualified symbol that refers to a declaration `DeclName` with the // shortest qualified name possible when the reference is in `NewNamespace`. +// FIXME: don't need to add redundant namespace qualifier when there is +// UsingShadowDecl or using namespace decl. void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext( const ast_matchers::MatchFinder::MatchResult &Result, const Decl *DeclCtx, SourceLocation Start, SourceLocation End, llvm::StringRef DeclName) { Index: unittests/change-namespace/ChangeNamespaceTests.cpp =================================================================== --- unittests/change-namespace/ChangeNamespaceTests.cpp +++ unittests/change-namespace/ChangeNamespaceTests.cpp @@ -229,8 +229,23 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, DoNotCrashWithLambdaAsParameter) { + std::string Code = + "#include \n" + "void f(std::function func, int param) { func(param); } " + "void g() { f([](int x) {}, 1); }"; + + std::string Expected = + "#include \n" + "void f(std::function func, int param) { func(param); } " + "void g() { f([](int x) {}, 1); }"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + TEST_F(ChangeNamespaceTest, FixUsingShadowDecl) { - std::string Code = "namespace na {\n" + std::string Code = "class GLOB {};\n" + "using BLOG = GLOB;\n" + "namespace na {\n" "namespace nc {\n" "class SAME {};\n" "}\n" @@ -245,7 +260,9 @@ "} // namespace nb\n" "} // namespace na\n"; - std::string Expected = "namespace na {\n" + std::string Expected = "class GLOB {};\n" + "using BLOG = GLOB;\n" + "namespace na {\n" "namespace nc {\n" "class SAME {};\n" "}\n" @@ -265,6 +282,85 @@ EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); } +TEST_F(ChangeNamespaceTest, UsingShadowDeclInFunction) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "namespace na {\n" + "namespace nb {\n" + "void f() {\n" + " using glob::Glob;\n" + " Glob g;\n" + "}\n" + "} // namespace nb\n" + "} // namespace na\n"; + + // FIXME: don't add namespace qualifier when there is UsingShadowDecl. + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "void f() {\n" + " using ::glob::Glob;\n" + " glob::Glob g;\n" + "}\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "using glob::Glob;\n" + "namespace na {\n" + "namespace nb {\n" + "void f() { Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + // FIXME: don't add namespace qualifier when there is UsingShadowDecl. + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "using glob::Glob;\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "void f() { glob::Glob g; }\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + +TEST_F(ChangeNamespaceTest, UsingNamespace) { + std::string Code = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "using namespace glob;\n" + "namespace na {\n" + "namespace nb {\n" + "void f() { Glob g; }\n" + "} // namespace nb\n" + "} // namespace na\n"; + + // FIXME: don't add namespace qualifier when there is "using namespace" decl. + std::string Expected = "namespace glob {\n" + "class Glob {};\n" + "}\n" + "using namespace glob;\n" + "\n" + "namespace x {\n" + "namespace y {\n" + "void f() { glob::Glob g; }\n" + "} // namespace y\n" + "} // namespace x\n"; + EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code)); +} + TEST_F(ChangeNamespaceTest, TypeInNestedNameSpecifier) { std::string Code = "namespace na {\n"