From b71aa7af7f4f9140c7bbcccca30aeecc671d767f Mon Sep 17 00:00:00 2001 From: "Ariel J. Bernal" Date: Wed, 9 Oct 2013 16:09:23 +0000 Subject: [PATCH] This patch fixes replacements that are not applied when relative paths are specified. In particular it makes sure that relative paths for non-virtual files aren't made absolute. Added unittest test. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@192299 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Tooling/Refactoring.cpp | 19 ++++++---- unittests/Tooling/RefactoringTest.cpp | 53 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/lib/Tooling/Refactoring.cpp b/lib/Tooling/Refactoring.cpp index 13811648ee..175dbd42ac 100644 --- a/lib/Tooling/Refactoring.cpp +++ b/lib/Tooling/Refactoring.cpp @@ -105,16 +105,21 @@ void Replacement::setFromSourceLocation(SourceManager &Sources, const std::pair DecomposedLocation = Sources.getDecomposedLoc(Start); const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first); - if (Entry != NULL) { // Make FilePath absolute so replacements can be applied correctly when - // relative paths for files are used. - llvm::SmallString<256> FilePath(Entry->getName()); - llvm::error_code EC = llvm::sys::fs::make_absolute(FilePath); - // Don't change the FilePath if the file is a virtual file. - this->FilePath = EC ? FilePath.c_str() : Entry->getName(); - } else + // relative paths for files are used. But we don't want to change virtual + // files. + if (llvm::sys::fs::exists(Entry->getName())) { + llvm::SmallString<256> FilePath(Entry->getName()); + llvm::sys::fs::make_absolute(FilePath); + this->FilePath = FilePath.c_str(); + } + else { + this->FilePath = Entry->getName(); + } + } else { this->FilePath = InvalidLocation; + } this->ReplacementRange = Range(DecomposedLocation.second, Length); this->ReplacementText = ReplacementText; } diff --git a/unittests/Tooling/RefactoringTest.cpp b/unittests/Tooling/RefactoringTest.cpp index 8c7bfa1c76..9dce26fb51 100644 --- a/unittests/Tooling/RefactoringTest.cpp +++ b/unittests/Tooling/RefactoringTest.cpp @@ -244,6 +244,10 @@ public: return Context.Sources.createFileID(File, SourceLocation(), SrcMgr::C_User); } + StringRef getFilePath(llvm::StringRef Name) { + return TemporaryFiles.lookup(Name); + } + std::string getFileContentFromDisk(llvm::StringRef Name) { std::string Path = TemporaryFiles.lookup(Name); assert(!Path.empty()); @@ -270,6 +274,55 @@ TEST_F(FlushRewrittenFilesTest, StoresChangesOnDisk) { getFileContentFromDisk("input.cpp")); } +TEST_F(FlushRewrittenFilesTest, GetFilePath) { + // Create a temporary file. + createFile("input.cpp", "line1\nline2\nline3\nline4"); + + StringRef FilePath = getFilePath("input.cpp"); + StringRef TempPath = llvm::sys::path::parent_path(FilePath); + StringRef TempFile = llvm::sys::path::filename(FilePath); + + // Save current path. + SmallString<512> CurrentPath; + llvm::sys::fs::current_path(CurrentPath); + + // Change directory to the temporary directory. + EXPECT_FALSE(chdir(TempPath.str().c_str())); + llvm::sys::fs::current_path(CurrentPath); + + // Get a FileEntry from the current directory. + FileManager Files((FileSystemOptions())); + const FileEntry *Entry = Files.getFile(TempFile); + ASSERT_TRUE(Entry != NULL); + + // We expect to get just the filename and "." in the FileEntry + // since paths are relative to the current directory. + EXPECT_EQ(TempFile, Entry->getName()); + EXPECT_STREQ(".", Entry->getDir()->getName()); + + FileID ID = Context.Sources.createFileID(Entry, SourceLocation(), + SrcMgr::C_User); + + Replacement R = Replacement(Context.Sources, Context.getLocation(ID, 2, 1), + 5, "replaced"); + + // Change back to the original path so we can verify that replacements + // are being applied independently of the location. + EXPECT_EQ(0, chdir(CurrentPath.c_str())); + + // We expect that the file path of the replacement is using the absolute + // path. + EXPECT_EQ(R.getFilePath(), getFilePath("input.cpp")); + + // Apply replacements. + Replacements Replaces; + Replaces.insert(R); + EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite)); + EXPECT_FALSE(Context.Rewrite.overwriteChangedFiles()); + EXPECT_EQ("line1\nreplaced\nline3\nline4", + getFileContentFromDisk("input.cpp")); +} + namespace { template class TestVisitor : public clang::RecursiveASTVisitor { -- 2.40.0