[change-namespace] don't generate replacements for files that don't match file pattern.

[hokein]

LGTM with two nits.

change-namespace/ChangeNamespace.cpp

786 ↗

(On Diff #79909)

How about creating FilePatternRE from FilePattern here? So that you don't need save FilePatternRE as a member variable.

test/change-namespace/Inputs/macro.h

1 ↗

(On Diff #79909)

For the simple file like this, I would use echo "#define USING using na::nc::X" > macro.h in macro.cc instead of creating a new file.

[ioeric]

• Addressed comments.

clang-tools-extra/trunk/change-namespace/ChangeNamespace.h

 //===-- ChangeNamespace.h -- Change namespace  ------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CHANGE_NAMESPACE_CHANGENAMESPACE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CHANGE_NAMESPACE_CHANGENAMESPACE_H
 
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/Support/Regex.h"
 #include <string>
 
 namespace clang {
 namespace change_namespace {
 
 // This tool can be used to change the surrounding namespaces of class/function
 // definitions. Classes/functions in the moved namespace will have new
 // namespaces while references to symbols (e.g. types, functions) which are not
private:
   std::string DiffOldNamespace;
   // The longest suffix in the new namespace that does not overlap the old
   // namespace.
   // For example, if `OldNamespace` is "a::b::c" and `NewNamespace` is
   // "a::x::y", then `DiffNewNamespace` will be "x::y".
   std::string DiffNewNamespace;
   // A regex pattern that matches files to be processed.
   std::string FilePattern;
+  llvm::Regex FilePatternRE;
   // Information about moved namespaces grouped by file.
   // Since we are modifying code in old namespaces (e.g. add namespace
   // spedifiers) as well as moving them, we store information about namespaces
   // to be moved and only move them after all modifications are finished (i.e.
   // in `onEndOfTranslationUnit`).
   std::map<std::string, std::vector<MoveNamespace>> MoveNamespaces;
   // Information about forward declaration insertions grouped by files.
   // A class forward declaration is not moved, so it will be deleted from the

clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp

NestedNameSpecifierLoc NestedNameSpecifier =
         TLoc.castAs<ElaboratedTypeLoc>().getQualifierLoc();
     if (NestedNameSpecifier.getNestedNameSpecifier())
       return NestedNameSpecifier.getBeginLoc();
     TLoc = TLoc.getNextTypeLoc();
   }
   return TLoc.getLocStart();
 }
 
-SourceLocation EndLocationForType(TypeLoc TLoc) {
+SourceLocation endLocationForType(TypeLoc TLoc) {
   // Dig past any namespace or keyword qualifications.
   while (TLoc.getTypeLocClass() == TypeLoc::Elaborated ||
          TLoc.getTypeLocClass() == TypeLoc::Qualified)
     TLoc = TLoc.getNextTypeLoc();
 
   // The location for template specializations (e.g. Foo<int>) includes the
   // templated types in its location range.  We want to restrict this to just
   // before the `<` character.

 } // anonymous namespace
 
 ChangeNamespaceTool::ChangeNamespaceTool(
     llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern,
     std::map<std::string, tooling::Replacements> *FileToReplacements,
     llvm::StringRef FallbackStyle)
     : FallbackStyle(FallbackStyle), FileToReplacements(*FileToReplacements),
       OldNamespace(OldNs.ltrim(':')), NewNamespace(NewNs.ltrim(':')),
-      FilePattern(FilePattern) {
+      FilePattern(FilePattern), FilePatternRE(FilePattern) {
   FileToReplacements->clear();
   llvm::SmallVector<llvm::StringRef, 4> OldNsSplitted;
   llvm::SmallVector<llvm::StringRef, 4> NewNsSplitted;
   llvm::StringRef(OldNamespace).split(OldNsSplitted, "::");
   llvm::StringRef(NewNamespace).split(NewNsSplitted, "::");
   // Calculates `DiffOldNamespace` and `DiffNewNamespace`.
   while (!OldNsSplitted.empty() && !NewNsSplitted.empty() &&
          OldNsSplitted.front() == NewNsSplitted.front()) {
if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("using")) {
     moveClassForwardDeclaration(Result, FwdDecl);
   } else if (const auto *UsingWithShadow =
                  Result.Nodes.getNodeAs<UsingDecl>("using_with_shadow")) {
     fixUsingShadowDecl(Result, UsingWithShadow);
   } else if (const auto *Specifier =
                  Result.Nodes.getNodeAs<NestedNameSpecifierLoc>(
                      "nested_specifier_loc")) {
     SourceLocation Start = Specifier->getBeginLoc();
-    SourceLocation End = EndLocationForType(Specifier->getTypeLoc());
+    SourceLocation End = endLocationForType(Specifier->getTypeLoc());
     fixTypeLoc(Result, Start, End, Specifier->getTypeLoc());
   } else if (const auto *BaseInitializer =
                  Result.Nodes.getNodeAs<CXXCtorInitializer>(
                      "base_initializer")) {
     BaseCtorInitializerTypeLocs.push_back(
         BaseInitializer->getTypeSourceInfo()->getTypeLoc());
   } else if (const auto *TLoc = Result.Nodes.getNodeAs<TypeLoc>("type")) {
-    fixTypeLoc(Result, startLocationForType(*TLoc), EndLocationForType(*TLoc),
+    fixTypeLoc(Result, startLocationForType(*TLoc), endLocationForType(*TLoc),
                *TLoc);
   } else if (const auto *VarRef =
                  Result.Nodes.getNodeAs<DeclRefExpr>("var_ref")) {
     const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var_decl");
     assert(Var);
     if (Var->getCanonicalDecl()->isStaticDataMember())
       return;
     const auto *Context = Result.Nodes.getNodeAs<Decl>("dc");
auto IsInMovedNs = [&](const NamedDecl *D) {
                .startswith(OldNamespace + "::"))
         return false;
       auto ExpansionLoc =
           Result.SourceManager->getExpansionLoc(D->getLocStart());
       if (ExpansionLoc.isInvalid())
         return false;
       llvm::StringRef Filename =
           Result.SourceManager->getFilename(ExpansionLoc);
-      llvm::Regex RE(FilePattern);
-      return RE.match(Filename);
+      return FilePatternRE.match(Filename);
     };
     // Don't fix the \p Type if it refers to a type alias decl in the moved
     // namespace since the alias decl will be moved along with the type
     // reference.
     if (IsInMovedNs(FromDecl))
       return;
   }
 
for (const auto &FileAndNsMoves : MoveNamespaces) {
     auto CleanReplacements =
         format::cleanupAroundReplacements(Code, Replaces, Style);
     if (!CleanReplacements) {
       llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
       continue;
     }
     FileToReplacements[FilePath] = *CleanReplacements;
   }
+
+  // Make sure we don't generate replacements for files that do not match
+  // FilePattern.
+  for (auto &Entry : FileToReplacements)
+    if (!FilePatternRE.match(Entry.first))
+      Entry.second.clear();
 }
 
 } // namespace change_namespace
 } // namespace clang

clang-tools-extra/trunk/test/change-namespace/macro.cpp

+// RUN: cp %S/macro.cpp %T/macro.cpp
+// RUN: echo "#define USING using na::nc::X" > %T/macro.h
+//
+// RUN: clang-change-namespace -old_namespace "na::nb" -new_namespace "x::y" --file_pattern "macro.cpp" --i %T/macro.cpp --
+// RUN: FileCheck -input-file=%T/macro.cpp -check-prefix=CHECK-CC %s
+// RUN: FileCheck -input-file=%T/macro.h -check-prefix=CHECK-HEADER %s
+//
+// RUN: cp %S/macro.cpp %T/macro.cpp
+// RUN: echo "#define USING using na::nc::X" > %T/macro.h
+// RUN: clang-change-namespace -old_namespace "na::nb" -new_namespace "x::y" --file_pattern ".*" --i %T/macro.cpp --
+// RUN: FileCheck -input-file=%T/macro.cpp -check-prefix=CHECK-CC %s
+// RUN: FileCheck -input-file=%T/macro.h -check-prefix=CHECK-CHANGED-HEADER %s
+#include "macro.h"
+namespace na { namespace nc { class X{}; } }
+
+namespace na {
+namespace nb {
+USING;
+}
+}
+// CHECK-CC: namespace x {
+// CHECK-CC: namespace y {
+// CHECK-CC: USING;
+// CHECK-CC: } // namespace y
+// CHECK-CC: } // namespace x
+
+// CHECK-HEADER: #define USING using na::nc::X
+
+// CHECK-CHANGED-HEADER: #define USING using ::na::nc::X