[clang-tools-extra] [clang] Fixed clang-tidy rewriter not properly handling symlinks (PR #76350)

[Félix-Antoine Constantin via cfe-commits]

https://github.com/felix642 created https://github.com/llvm/llvm-project/pull/76350

Rewriter would not properly fix files if they were symlinked and using --fix with clang-tidy would overwrite the symlink with the corrections rather than the file.

With these changes the Rewriter now properly resolves the symlink before applying the fixes.

Fixes #60845

>From 37bde4bedaa6a80fb5c855a06d790cb0180fa5bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 <felix-antoine.constantin at polymtl.ca>
Date: Sun, 24 Dec 2023 21:01:32 -0500
Subject: [PATCH] Fixed clang-tidy rewriter not properly handling symlinks

Rewriter would not properly fix files if they were symlinked and using
--fix with clang-tidy would overwrite the symlink with the corrections
rather than the file.

With these changes the Rewriter now properly resolves the symlink before
applying the fixes.

Fixes #60845
---
 clang-tools-extra/docs/ReleaseNotes.rst   |  2 ++
 clang/lib/Rewrite/Rewriter.cpp            | 24 +++++++++++++---------
 clang/unittests/libclang/LibclangTest.cpp | 25 +++++++++++++++++++++++
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d91748e4cef18..8a16ce1a7988a2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,8 @@ Improvements to clang-tidy
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Improved `--fix` to properly apply corrections on files that are symlinked.
+
 - Improved :program:`clang-tidy-diff.py` script. It now returns exit code `1`
   if any :program:`clang-tidy` subprocess exits with a non-zero code or if
   exporting fixes fails. It now accepts a directory as a value for
diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp
index 0e6ae365064463..edc147ec304801 100644
--- a/clang/lib/Rewrite/Rewriter.cpp
+++ b/clang/lib/Rewrite/Rewriter.cpp
@@ -14,6 +14,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
@@ -412,16 +413,19 @@ 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) {
-    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;
+    if (OptionalFileEntryRef fileEntry =
+            getSourceMgr().getFileEntryRefForID(I->first)) {
+      llvm::StringRef FileName =
+          getSourceMgr().getFileManager().getCanonicalName(*fileEntry);
+      if (auto Error =
+              llvm::writeToOutput(FileName, [&](llvm::raw_ostream &OS) {
+                I->second.write(OS);
+                return llvm::Error::success();
+              })) {
+        Diag.Report(OverwriteFailure)
+            << FileName << llvm::toString(std::move(Error));
+        AllWritten = false;
+      }
     }
   }
   return !AllWritten;
diff --git a/clang/unittests/libclang/LibclangTest.cpp b/clang/unittests/libclang/LibclangTest.cpp
index 87075a46d75187..cde19d9004cd81 100644
--- a/clang/unittests/libclang/LibclangTest.cpp
+++ b/clang/unittests/libclang/LibclangTest.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include <cstring>
+#include <filesystem>
 #include <fstream>
 #include <functional>
 #include <map>
@@ -1379,3 +1380,27 @@ TEST_F(LibclangRewriteTest, RewriteRemove) {
   ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(Rew), 0);
   EXPECT_EQ(getFileContent(Filename), "int () { return 0; }");
 }
+
+TEST_F(LibclangRewriteTest, Symlink) {
+  std::filesystem::path Symlink = "link.cpp";
+  std::filesystem::create_symlink(Filename, Symlink);
+  ASSERT_TRUE(std::filesystem::exists(Symlink));
+  FilesAndDirsToRemove.emplace(Symlink);
+
+  CXTranslationUnit SymTu = clang_parseTranslationUnit(
+      Index, Symlink.c_str(), nullptr, 0, nullptr, 0, TUFlags);
+  CXFile SymlinkFile = clang_getFile(SymTu, Symlink.c_str());
+  CXRewriter SymRew = clang_CXRewriter_create(SymTu);
+
+  CXSourceLocation B = clang_getLocation(SymTu, SymlinkFile, 1, 5);
+  CXSourceLocation E = clang_getLocation(SymTu, SymlinkFile, 1, 9);
+  CXSourceRange Rng = clang_getRange(B, E);
+
+  clang_CXRewriter_removeText(SymRew, Rng);
+
+  ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(SymRew), 0);
+  EXPECT_EQ(getFileContent(Filename), "int () { return 0; }");
+  EXPECT_TRUE(std::filesystem::is_symlink(Symlink));
+
+  clang_CXRewriter_dispose(SymRew);
+}