[clang-format] Fix regression when getStyle() called with empty filename

[benhamilton]

Summary

D43522 caused an assertion failure when getStyle() was called with
an empty filename:

P8065

This adds a test to reproduce the failure and fixes the issue by
ensuring we never pass an empty filename to
Environment::CreateVirtualEnvironment().

Test Plan: New test added. Ran test with:

% make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests
Before diff, test failed with P8065. Now, test passes.

[vsapsai]

I'm not familiar with this code, so post-commit review by someone more knowledgeable might be a good idea.

Code-wise it looks good. I feel uneasy about using fake file name, but it is exactly the case being tested, so that's not a problem.

[bjope]

unittests/Format/FormatTest.cpp

11727

Do you really want to try to find a ".clang-format" file here, with fallback to "Google" if no such file is found?

When I'm building I usually end up having my build directory inside the llvm repo. And since there is a .clang-format file checked in to llvm that file is found, as it searches for a .clang-format file somewhere in the directory structure above the current dir when running the test (if I remember correctly?). We have had such problem before.

Can't you just as well do

auto Style1 = getStyle("Google", "", "Google");

or is that not triggering the original bug?

Right now our build bots ends up like this (I guess it has found the .clang-format in my llvm/clang repo and decided to use "LLVM" as format for "Style1"):

FAIL: Clang-Unit :: Format/./FormatTests/FormatStyle.GetStyleWithEmptyFileName (14009 of 36611)
******************** TEST 'Clang-Unit :: Format/./FormatTests/FormatStyle.GetStyleWithEmptyFileName' FAILED ********************
Note: Google Test filter = FormatStyle.GetStyleWithEmptyFileName
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from FormatStyle
[ RUN      ] FormatStyle.GetStyleWithEmptyFileName
../tools/clang/unittests/Format/FormatTest.cpp:11729: Failure
      Expected: *Style1
      Which is: 456-byte object <FE-FF FF-FF 00-00 00-00 00-00 00-00 02-00 00-00 01-01 01-00 00-00 00-00 04-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 01-01 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 ... 00-00 00-00 00-00 00-00 01-01 01-00 01-01 6E-53 01-00 00-00 00-00 00-00 01-00 00-00 00-01 00-00 00-00 00-00 01-00 00-00 08-00 00-00 00-00 00-00 10-73 77-01 00-00 00-00 50-73 77-01 00-00 00-00>
To be equal to: getGoogleStyle()
      Which is: 456-byte object <FF-FF FF-FF 00-00 00-00 00-00 00-00 01-00 00-00 01-01 01-00 00-00 00-00 04-00 00-00 01-01 00-00 00-00 00-00 00-00 00-00 01-01 01-01 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 ... 38-4A 77-01 00-00 00-00 01-01 01-00 01-01 00-00 01-00 00-00 00-00 00-00 02-00 00-00 00-01 00-00 00-73 77-01 02-00 00-00 08-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00>
[  FAILED  ] FormatStyle.GetStyleWithEmptyFileName (1 ms)
[----------] 1 test from FormatStyle (1 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (1 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] FormatStyle.GetStyleWithEmptyFileName

[bjope]

unittests/Format/FormatTest.cpp

11727

I've proposed a fix here: https://reviews.llvm.org/D43732
By using an InMemoryFileSystem to avoid relying on no .clang-format present in the path of the real file system when running the test.

lib/Format/Format.cpp

static FormatStyle::LanguageKind getLanguageByFileName(StringRef FileName) {
   if (FileName.endswith_lower(".td"))
     return FormatStyle::LK_TableGen;
   return FormatStyle::LK_Cpp;
 }
 
 FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
   FormatStyle::LanguageKind result = getLanguageByFileName(FileName);
   if (result == FormatStyle::LK_Cpp) {
-    auto extension = llvm::sys::path::extension(FileName);
+    auto Extension = llvm::sys::path::extension(FileName);
     // If there's no file extension (or it's .h), we need to check the contents
     // of the code to see if it contains Objective-C.
-    if (extension.empty() || extension == ".h") {
+    if (Extension.empty() || Extension == ".h") {
+      auto NonEmptyFileName = FileName.empty() ? "guess.h" : FileName;
       std::unique_ptr<Environment> Env =
-          Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{});
+          Environment::CreateVirtualEnvironment(Code, NonEmptyFileName, /*Ranges=*/{});
       ObjCHeaderStyleGuesser Guesser(*Env, getLLVMStyle());
       Guesser.process();
       if (Guesser.isObjC()) {
         result = FormatStyle::LK_ObjC;
       }
     }
   }
   return result;

unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ArrayAsTemplateType) {
   EXPECT_EQ("auto a = unique_ptr<Foo<Bar>[ 10 ]>;",
             format("auto a = unique_ptr < Foo < Bar>[10]> ;", Spaces));
 }
 
 TEST_F(FormatTest, NoSpaceAfterSuper) {
     verifyFormat("__super::FooBar();");
 }
 
+TEST(FormatStyle, GetStyleWithEmptyFileName) {
+  auto Style1 = getStyle("file", "", "Google");
+  ASSERT_TRUE((bool)Style1);
+  ASSERT_EQ(*Style1, getGoogleStyle());
+}
+
 TEST(FormatStyle, GetStyleOfFile) {
   vfs::InMemoryFileSystem FS;
   // Test 1: format file in the same directory.
   ASSERT_TRUE(
       FS.addFile("/a/.clang-format", 0,
                  llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM")));
   ASSERT_TRUE(
       FS.addFile("/a/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;")));