r192299 - This patch fixes replacements that are not applied when relative paths are

[Rafael Espíndola]

Tooling/ToolingTests/FlushRewrittenFilesTest.GetFilePath is failing
for me after this patch.

On 9 October 2013 12:09, Ariel J. Bernal <ariel.j.bernal at intel.com> wrote:
> Author: ajbernal
> Date: Wed Oct  9 11:09:23 2013
> New Revision: 192299
>
> URL: http://llvm.org/viewvc/llvm-project?rev=192299&view=rev
> Log:
> 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.
>
> Modified:
>     cfe/trunk/lib/Tooling/Refactoring.cpp
>     cfe/trunk/unittests/Tooling/RefactoringTest.cpp
>
> Modified: cfe/trunk/lib/Tooling/Refactoring.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring.cpp?rev=192299&r1=192298&r2=192299&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Tooling/Refactoring.cpp (original)
> +++ cfe/trunk/lib/Tooling/Refactoring.cpp Wed Oct  9 11:09:23 2013
> @@ -105,16 +105,21 @@ void Replacement::setFromSourceLocation(
>    const std::pair<FileID, unsigned> 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;
>  }
>
> Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=192299&r1=192298&r2=192299&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original)
> +++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Wed Oct  9 11:09:23 2013
> @@ -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, StoresCh
>              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 <typename T>
>  class TestVisitor : public clang::RecursiveASTVisitor<T> {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits