[clang][Tooling] Make the filename behaviour consistent

[kadircet]

Summary

Dotdots were removed only when converting a relative path to absolute
one. This patch makes the behaviour consistent for absolute file paths by
removing them in that case as well. Also updates the documentation to mention
the behaviour.

Fixes https://github.com/clangd/clangd/issues/1317

[sammccall]

Code change LGTM, i think the doc change should be dropped or reshaped though.

clang/docs/JSONCompilationDatabase.rst

89 ↗

(On Diff #475189)

This doesn't fit with the rest of the spec text: the new text describing tool behavior, while everything else describes the data.

I'm not totally sure about documenting this (it's of pretty limited interest, doesn't cover symlink handling exhaustively, and seems more a limitation of the tools than a feature of the format). If you do want to I'd suggest separating it from the format description into a separate paragraph below (e.g. "Because compilation databases can be large, tools match filenames are matched against entries textually without IO. This means they generally don't understand symlinks.")

[kadircet]

• Drop documentation
• Call remove_dots on the common path

[thakis]

This breaks tests on windows: http://45.33.8.238/win/69993/step_7.txt

Please take a look and revert for now if it takes a while to fix.

[kadircet]

oops, should be fixed with eed1f337a1fbf8c1e7e53a6e1d0dfd4eb800aa08

clang/lib/Tooling/JSONCompilationDatabase.cpp

if (!Directory) {
       ErrorMessage = "Missing key: \"directory\".";
       return false;
     }
     SmallString<8> FileStorage;
     StringRef FileName = File->getValue(FileStorage);
     SmallString<128> NativeFilePath;
     if (llvm::sys::path::is_relative(FileName)) {
       SmallString<8> DirectoryStorage;
-      SmallString<128> AbsolutePath(
-          Directory->getValue(DirectoryStorage));
+      SmallString<128> AbsolutePath(Directory->getValue(DirectoryStorage));
       llvm::sys::path::append(AbsolutePath, FileName);
-      llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/ true);
       llvm::sys::path::native(AbsolutePath, NativeFilePath);
     } else {
       llvm::sys::path::native(FileName, NativeFilePath);
     }
+    llvm::sys::path::remove_dots(NativeFilePath, /*remove_dot_dot=*/true);
     auto Cmd = CompileCommandRef(Directory, File, *Command, Output);
     IndexByFile[NativeFilePath].push_back(Cmd);
     AllCommands.push_back(Cmd);
     MatchTrie.insert(NativeFilePath);
   }
   return true;
 }

clang/unittests/Tooling/CompilationDatabaseTest.cpp

TEST(JSONCompilationDatabase, GetAllFiles) {
   std::vector<std::string> expected_files;
   SmallString<16> PathStorage;
   llvm::sys::path::native("//net/dir/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/dir/file2", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
+  llvm::sys::path::native("//net/dir/file3", PathStorage);
+  expected_files.push_back(std::string(PathStorage.str()));
   EXPECT_EQ(expected_files,
-            getAllFiles("[{\"directory\":\"//net/dir\","
-                        "\"command\":\"command\","
-                        "\"file\":\"file1\"},"
-                        " {\"directory\":\"//net/dir\","
-                        "\"command\":\"command\","
-                        "\"file\":\"../file1\"},"
-                        " {\"directory\":\"//net/dir\","
-                        "\"command\":\"command\","
-                        "\"file\":\"file2\"}]",
+            getAllFiles(R"json(
+            [
+              {
+                "directory": "//net/dir",
+                "command": "command",
+                "file": "file1"
+              },
+              {
+                "directory": "//net/dir",
+                "command": "command",
+                "file": "../file1"
+              },
+              {
+                "directory": "//net/dir",
+                "command": "command",
+                "file": "file2"
+              },
+              {
+                "directory": "//net/dir",
+                "command": "command",
+                "file": "//net/dir/foo/../file3"
+              }
+            ])json",
                         ErrorMessage, JSONCommandLineSyntax::Gnu))
       << ErrorMessage;
 }
 
 TEST(JSONCompilationDatabase, GetAllCompileCommands) {
   std::string ErrorMessage;
   EXPECT_EQ(
       0u, getAllCompileCommands(JSONCommandLineSyntax::Gnu, "[]", ErrorMessage)