[clang][tidy] Ensure rewriter has the correct CWD (#67839)

This patch replaces use of the deprecated `FileEntry::getName()` with
`FileEntryRef::getName()`. This means the code now uses the path that
was used to register file entry in `SourceManager` instead of the
absolute path that happened to be used in the last call to
`FileManager::getFile()` some place else.

This caused some test failures due to the fact that some paths can be
relative and thus rely on the VFS CWD. The CWD can change for each TU,
so when we run `clang-tidy` on a compilation database and try to perform
all the replacements at the end, relative paths won't resolve the same.
This patch takes care to reinstate the correct CWD and make the path
reported by `FileEntryRef` absolute before passing it to
`llvm::writeToOutput()`.

Author: jansvoboda11

clang-tools-extra/clang-tidy/ClangTidy.cpp

@@ -147,7 +147,8 @@ class ErrorReporter {
             Files.makeAbsolutePath(FixAbsoluteFilePath);
             tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(),
                                    Repl.getLength(), Repl.getReplacementText());
-            Replacements &Replacements = FileReplacements[R.getFilePath()];
+            auto &Entry = FileReplacements[R.getFilePath()];
+            Replacements &Replacements = Entry.Replacements;
             llvm::Error Err = Replacements.add(R);
             if (Err) {
               // FIXME: Implement better conflict handling.
@@ -174,6 +175,7 @@ class ErrorReporter {
             }
             FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset());
             FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied));
+            Entry.BuildDir = Error.BuildDirectory;
           }
         }
       }
@@ -189,9 +191,14 @@ class ErrorReporter {
 
   void finish() {
     if (TotalFixes > 0) {
-      Rewriter Rewrite(SourceMgr, LangOpts);
+      auto &VFS = Files.getVirtualFileSystem();
+      auto OriginalCWD = VFS.getCurrentWorkingDirectory();
+      bool AnyNotWritten = false;
+
       for (const auto &FileAndReplacements : FileReplacements) {
+        Rewriter Rewrite(SourceMgr, LangOpts);
         StringRef File = FileAndReplacements.first();
+        VFS.setCurrentWorkingDirectory(FileAndReplacements.second.BuildDir);
         llvm::ErrorOr<std::unique_ptr<MemoryBuffer>> Buffer =
             SourceMgr.getFileManager().getBufferForFile(File);
         if (!Buffer) {
@@ -208,8 +215,8 @@ class ErrorReporter {
           continue;
         }
         llvm::Expected<tooling::Replacements> Replacements =
-            format::cleanupAroundReplacements(Code, FileAndReplacements.second,
-                                              *Style);
+            format::cleanupAroundReplacements(
+                Code, FileAndReplacements.second.Replacements, *Style);
         if (!Replacements) {
           llvm::errs() << llvm::toString(Replacements.takeError()) << "\n";
           continue;
@@ -226,13 +233,18 @@ class ErrorReporter {
         if (!tooling::applyAllReplacements(Replacements.get(), Rewrite)) {
           llvm::errs() << "Can't apply replacements for file " << File << "\n";
         }
+        AnyNotWritten &= Rewrite.overwriteChangedFiles();
       }
-      if (Rewrite.overwriteChangedFiles()) {
+
+      if (AnyNotWritten) {
         llvm::errs() << "clang-tidy failed to apply suggested fixes.\n";
       } else {
         llvm::errs() << "clang-tidy applied " << AppliedFixes << " of "
                      << TotalFixes << " suggested fixes.\n";
       }
+
+      if (OriginalCWD)
+        VFS.setCurrentWorkingDirectory(*OriginalCWD);
     }
   }
 
@@ -289,13 +301,18 @@ class ErrorReporter {
     return CharSourceRange::getCharRange(BeginLoc, EndLoc);
   }
 
+  struct ReplacementsWithBuildDir {
+    StringRef BuildDir;
+    Replacements Replacements;
+  };
+
   FileManager Files;
   LangOptions LangOpts; // FIXME: use langopts from each original file
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
   DiagnosticConsumer *DiagPrinter;
   DiagnosticsEngine Diags;
   SourceManager SourceMgr;
-  llvm::StringMap<Replacements> FileReplacements;
+  llvm::StringMap<ReplacementsWithBuildDir> FileReplacements;
   ClangTidyContext &Context;
   FixBehaviour ApplyFixes;
   unsigned TotalFixes = 0U;

clang-tools-extra/test/clang-tidy/infrastructure/Inputs/compilation-database/template.json

@@ -20,10 +20,15 @@
   "file": "test_dir/b/c.cpp"
 },
 {
-  "directory": "test_dir/b",
-  "command": "clang++ -I../include -o test.o ../b/d.cpp",
+  "directory": "test_dir/",
+  "command": "clang++ -o test.o ./b/d.cpp",
   "file": "test_dir/b/d.cpp"
 },
+{
+  "directory": "test_dir/b",
+  "command": "clang++ -I../include -o test.o ../include.cpp",
+  "file": "test_dir/include.cpp"
+},
 {
   "directory": "test_dir/",
   "command": "clang++ -o test.o test_dir/b/not-exist.cpp",

clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-run-with-database.cpp

@@ -5,8 +5,9 @@
 // RUN: echo 'int *AB = 0;' > %T/compilation-database-test/a/b.cpp
 // RUN: echo 'int *BB = 0;' > %T/compilation-database-test/b/b.cpp
 // RUN: echo 'int *BC = 0;' > %T/compilation-database-test/b/c.cpp
+// RUN: echo 'int *BD = 0;' > %T/compilation-database-test/b/d.cpp
 // RUN: echo 'int *HP = 0;' > %T/compilation-database-test/include/header.h
-// RUN: echo '#include "header.h"' > %T/compilation-database-test/b/d.cpp
+// RUN: echo '#include "header.h"' > %T/compilation-database-test/include.cpp
 // RUN: sed 's|test_dir|%/T/compilation-database-test|g' %S/Inputs/compilation-database/template.json > %T/compile_commands.json
 
 // Regression test: shouldn't crash.
@@ -15,15 +16,17 @@
 // CHECK-NOT-EXIST: unable to handle compilation
 // CHECK-NOT-EXIST: Found compiler error
 
-// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T %T/compilation-database-test/a/a.cpp %T/compilation-database-test/a/b.cpp %T/compilation-database-test/b/b.cpp %T/compilation-database-test/b/c.cpp %T/compilation-database-test/b/d.cpp -header-filter=.* -fix
+// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T %T/compilation-database-test/a/a.cpp %T/compilation-database-test/a/b.cpp %T/compilation-database-test/b/b.cpp %T/compilation-database-test/b/c.cpp %T/compilation-database-test/b/d.cpp %T/compilation-database-test/include.cpp -header-filter=.* -fix
 // RUN: FileCheck -input-file=%T/compilation-database-test/a/a.cpp %s -check-prefix=CHECK-FIX1
 // RUN: FileCheck -input-file=%T/compilation-database-test/a/b.cpp %s -check-prefix=CHECK-FIX2
 // RUN: FileCheck -input-file=%T/compilation-database-test/b/b.cpp %s -check-prefix=CHECK-FIX3
 // RUN: FileCheck -input-file=%T/compilation-database-test/b/c.cpp %s -check-prefix=CHECK-FIX4
-// RUN: FileCheck -input-file=%T/compilation-database-test/include/header.h %s -check-prefix=CHECK-FIX5
+// RUN: FileCheck -input-file=%T/compilation-database-test/b/d.cpp %s -check-prefix=CHECK-FIX5
+// RUN: FileCheck -input-file=%T/compilation-database-test/include/header.h %s -check-prefix=CHECK-FIX6
 
 // CHECK-FIX1: int *AA = nullptr;
 // CHECK-FIX2: int *AB = nullptr;
 // CHECK-FIX3: int *BB = nullptr;
 // CHECK-FIX4: int *BC = nullptr;
-// CHECK-FIX5: int *HP = nullptr;
+// CHECK-FIX5: int *BD = nullptr;
+// CHECK-FIX6: int *HP = nullptr;

clang/lib/Rewrite/Rewriter.cpp

@@ -412,12 +412,13 @@ bool Rewriter::overwriteChangedFiles() {
   unsigned OverwriteFailure = Diag.getCustomDiagID(
       DiagnosticsEngine::Error, "unable to overwrite file %0: %1");
   for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) {
-    const FileEntry *Entry = getSourceMgr().getFileEntryForID(I->first);
-    if (auto Error =
-            llvm::writeToOutput(Entry->getName(), [&](llvm::raw_ostream &OS) {
-              I->second.write(OS);
-              return llvm::Error::success();
-            })) {
+    OptionalFileEntryRef Entry = getSourceMgr().getFileEntryRefForID(I->first);
+    llvm::SmallString<128> Path(Entry->getName());
+    getSourceMgr().getFileManager().makeAbsolutePath(Path);
+    if (auto Error = llvm::writeToOutput(Path, [&](llvm::raw_ostream &OS) {
+          I->second.write(OS);
+          return llvm::Error::success();
+        })) {
       Diag.Report(OverwriteFailure)
           << Entry->getName() << llvm::toString(std::move(Error));
       AllWritten = false;