Fix replacements for files with relative paths are not applied.

[arielbernal]

Summary

When using a compilation database where paths in the command are specified relative to the directory then replacements for these files are not applied since there is no path associated to the files.

[klimek]

lib/Tooling/Refactoring.cpp

108–118

1. was it intentional that you removed the Entry != NULL check?
2. please add a regression test

[arielbernal]

lib/Tooling/Refactoring.cpp

108–118

No it wasn't, it was a mistake, I fixed it and added a regression test. The problem is that this patch breaks many other tests in c-index-test and also some unit tests.

[klimek]

lib/Tooling/Refactoring.cpp

108–118

Which ones and why? The easiest way to review that would be if you included the fixes for all breaking tests in the patch :D

[arielbernal]

Addressed comments and fixed the case where virtual file paths cannot be made absolute. Also added a regression test.

Note that make_absolute already checks for the file to exist so it should be equivalent to check whether the file is virtual or not but if this introduces a performance issue then maybe we would want to have a function in the FileManager to query if a FileEntry is a virtual file (in the VirtualFileEntries vector).

[klimek]

lg

lib/Tooling/Refactoring.cpp

116

I think the common practice is to use braces if one part of the if-else needs braces.

[arielbernal]

Fixed braces and committed r191766

lib/Tooling/Refactoring.cpp

 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/Refactoring.h"
 #include "llvm/Support/raw_os_ostream.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 
 namespace clang {
 namespace tooling {
 
 static const char * const InvalidLocation = "";
 
 Replacement::Replacement()
   : FilePath(InvalidLocation) {}

 }
 
 void Replacement::setFromSourceLocation(SourceManager &Sources,
                                         SourceLocation Start, unsigned Length,
                                         StringRef ReplacementText) {
   const std::pair<FileID, unsigned> DecomposedLocation =
       Sources.getDecomposedLoc(Start);
   const FileEntry *Entry = Sources.getFileEntryForID(DecomposedLocation.first);
-  this->FilePath = Entry != NULL ? Entry->getName() : InvalidLocation;
+
+  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
+    this->FilePath = InvalidLocation;
   this->ReplacementRange = Range(DecomposedLocation.second, Length);
   this->ReplacementText = ReplacementText;
 }
 
 // FIXME: This should go into the Lexer, but we need to figure out how
 // to handle ranges for refactoring in general first - there is no obvious
 // good way how to integrate this into the Lexer yet.
 static int getRangeSize(SourceManager &Sources, const CharSourceRange &Range) {
   SourceLocation SpellingBegin = Sources.getSpellingLoc(Range.getBegin());

test/Tooling/clang-check-rel-path.cpp

+// This block test a compilation database with files relative to the directory
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo '[{"directory":"%t","command":"clang++ -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: not clang-check -p "%t" "%t/test.cpp" 2>&1|FileCheck %s
+// FIXME: Make the above easier.
+
+// CHECK: C++ requires
+invalid;