From 8188484daa4195a2c8b5253765036fa2c6da7263 Mon Sep 17 00:00:00 2001 From: Adrian Vogelsgesang Date: Thu, 11 Nov 2021 18:19:54 +0000 Subject: [PATCH] [clang-apply-replacements] Correctly handle relative paths The fixes from the YAML file can refer to relative paths. Those relative paths are meant to be resolved relative to the corresponding `build directory`. However, `clang-apply-replacements` currently interprets all paths relative to its own working directory. This causes issues, e.g., when `clang-apply-replacements` is run from outside of the original build directory. This commit adjusts `clang-apply-replacements` to take the build directory into account when resolving relative file paths. Reviewed By: ymandel Differential Revision: https://reviews.llvm.org/D112647 --- .../lib/Tooling/ApplyReplacements.cpp | 12 +++++-- .../Inputs/relative-paths/basic.h | 32 +++++++++++++++++++ .../Inputs/relative-paths/file1.yaml | 27 ++++++++++++++++ .../Inputs/relative-paths/file2.yaml | 15 +++++++++ .../relative-paths.cpp | 7 ++++ 5 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml create mode 100644 clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml create mode 100644 clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp diff --git a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp index 1e5012b9891b..b13b83cf3f00 100644 --- a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp +++ b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp @@ -23,6 +23,7 @@ #include "clang/Tooling/DiagnosticsYaml.h" #include "clang/Tooling/ReplacementsYaml.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Optional.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" @@ -152,9 +153,13 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs, DiagReplacements; auto AddToGroup = [&](const tooling::Replacement &R, - const tooling::TranslationUnitDiagnostics *SourceTU) { + const tooling::TranslationUnitDiagnostics *SourceTU, + const llvm::Optional BuildDir) { // Use the file manager to deduplicate paths. FileEntries are // automatically canonicalized. + auto PrevWorkingDir = SM.getFileManager().getFileSystemOpts().WorkingDir; + if (BuildDir) + SM.getFileManager().getFileSystemOpts().WorkingDir = std::move(*BuildDir); if (auto Entry = SM.getFileManager().getFile(R.getFilePath())) { if (SourceTU) { auto &Replaces = DiagReplacements[*Entry]; @@ -170,18 +175,19 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs, errs() << "Described file '" << R.getFilePath() << "' doesn't exist. Ignoring...\n"; } + SM.getFileManager().getFileSystemOpts().WorkingDir = PrevWorkingDir; }; for (const auto &TU : TUs) for (const tooling::Replacement &R : TU.Replacements) - AddToGroup(R, nullptr); + AddToGroup(R, nullptr, {}); for (const auto &TU : TUDs) for (const auto &D : TU.Diagnostics) if (const auto *ChoosenFix = tooling::selectFirstFix(D)) { for (const auto &Fix : *ChoosenFix) for (const tooling::Replacement &R : Fix.second) - AddToGroup(R, &TU); + AddToGroup(R, &TU, D.BuildDirectory); } // Sort replacements per file to keep consistent behavior when diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h new file mode 100644 index 000000000000..48509684b772 --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/basic.h @@ -0,0 +1,32 @@ +#ifndef BASIC_H +#define BASIC_H + + +class Parent { +public: + virtual void func() {} +}; + +class Derived : public Parent { +public: + virtual void func() {} + // CHECK: virtual void func() override {} +}; + +extern void ext(int (&)[5], const Parent &); + +void func(int t) { + int ints[5]; + for (unsigned i = 0; i < 5; ++i) { + int &e = ints[i]; + e = t; + // CHECK: for (auto & elem : ints) { + // CHECK-NEXT: elem = t; + } + + Derived d; + + ext(ints, d); +} + +#endif // BASIC_H diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml new file mode 100644 index 000000000000..7219ae8ab00e --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file1.yaml @@ -0,0 +1,27 @@ +--- +MainSourceFile: source1.cpp +Diagnostics: + - BuildDirectory: $(path)/subdir/ + DiagnosticName: test-relative-path + DiagnosticMessage: + Message: Fix + FilePath: ../relative-path.h + FileOffset: 242 + Replacements: + - FilePath: ../basic.h + Offset: 242 + Length: 26 + ReplacementText: 'auto & elem : ints' + - FilePath: $(path)/basic.h + Offset: 276 + Length: 22 + ReplacementText: '' + - FilePath: ../basic.h + Offset: 298 + Length: 1 + ReplacementText: elem + - FilePath: ../../relative-paths/basic.h + Offset: 148 + Length: 0 + ReplacementText: 'override ' +... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml new file mode 100644 index 000000000000..b93942c269b7 --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/relative-paths/file2.yaml @@ -0,0 +1,15 @@ +--- +MainSourceFile: source2.cpp +Diagnostics: + - BuildDirectory: $(path) + DiagnosticName: test-relative-path + DiagnosticMessage: + Message: Fix + FilePath: ./basic.h + FileOffset: 148 + Replacements: + - FilePath: ./basic.h + Offset: 298 + Length: 1 + ReplacementText: elem +... diff --git a/clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp b/clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp new file mode 100644 index 000000000000..92cde8413447 --- /dev/null +++ b/clang-tools-extra/test/clang-apply-replacements/relative-paths.cpp @@ -0,0 +1,7 @@ +// RUN: mkdir -p %T/Inputs/relative-paths +// RUN: mkdir -p %T/Inputs/relative-paths/subdir +// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/relative-paths/basic.h > %T/Inputs/relative-paths/basic.h +// RUN: sed "s#\$(path)#%/T/Inputs/relative-paths#" %S/Inputs/relative-paths/file1.yaml > %T/Inputs/relative-paths/file1.yaml +// RUN: sed "s#\$(path)#%/T/Inputs/relative-paths#" %S/Inputs/relative-paths/file2.yaml > %T/Inputs/relative-paths/file2.yaml +// RUN: clang-apply-replacements %T/Inputs/relative-paths +// RUN: FileCheck -input-file=%T/Inputs/relative-paths/basic.h %S/Inputs/relative-paths/basic.h