[clang] c5c487f - Revert "[clang-format] Add option to specify explicit config file"

Mitchell Balan via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 11 13:15:04 PDT 2020


Author: Mitchell Balan
Date: 2020-03-11T16:14:42-04:00
New Revision: c5c487f0d4c6720f4384f670490086e723f5fe32

URL: https://github.com/llvm/llvm-project/commit/c5c487f0d4c6720f4384f670490086e723f5fe32
DIFF: https://github.com/llvm/llvm-project/commit/c5c487f0d4c6720f4384f670490086e723f5fe32.diff

LOG: Revert "[clang-format] Add option to specify explicit config file"
There were a number of unexpected test failures.

This reverts commit 10b1a87ba35d386b718f0e83c1d750631705b220.

Added: 
    

Modified: 
    clang/docs/ClangFormat.rst
    clang/docs/ClangFormatStyleOptions.rst
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Format/Format.h
    clang/lib/Format/Format.cpp
    clang/unittests/Format/FormatTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ClangFormat.rst b/clang/docs/ClangFormat.rst
index 633cffa55537..1138b2332670 100644
--- a/clang/docs/ClangFormat.rst
+++ b/clang/docs/ClangFormat.rst
@@ -75,10 +75,6 @@ to format C/C++/Java/JavaScript/Objective-C/Protobuf/C# code.
                                  .clang-format file located in one of the parent
                                  directories of the source file (or current
                                  directory for stdin).
-                                 Use -style=file:<format_file_path> to load style
-                                 configuration from a format file located at
-                                 <format_file_path>. This path can be absolute or
-                                 relative to the working directory.
                                  Use -style="{key: value, ...}" to set specific
                                  parameters, e.g.:
                                    -style="{BasedOnStyle: llvm, IndentWidth: 8}"

diff  --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 99a6a7df790b..50b4ff5d9010 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -24,10 +24,6 @@ try to find the ``.clang-format`` file located in the closest parent directory
 of the input file. When the standard input is used, the search is started from
 the current directory.
 
-When using ``-style=file:<format_file_path>, :program:`clang-format` for 
-each input file will use the format file located at `<format_file_path>`.
-The path may be absolute or relative to the working directory.
-
 The ``.clang-format`` file uses YAML format:
 
 .. code-block:: yaml

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ce15513c1c8c..664ae4e4167c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -253,10 +253,6 @@ clang-format
           bar();
         });
 
-- The command line argument `-style=<string>` has been extended so that a specific
-  format file at location <format_file_path> can be selected. This is supported
-  via the syntax: `-style=file:<format_file_path>`.
-
 libclang
 --------
 

diff  --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 8c1cb6ea3986..d4f76c87c14e 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2476,7 +2476,6 @@ extern const char *DefaultFallbackStyle;
 /// * "file" - Load style configuration from a file called ``.clang-format``
 /// located in one of the parent directories of ``FileName`` or the current
 /// directory if ``FileName`` is empty.
-/// * "file:<configfile>" to explicitly specify the configuration file to use.
 ///
 /// \param[in] StyleName Style name to interpret according to the description
 /// above.

diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 4c20eb2941ba..031312bd16d8 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2640,8 +2640,6 @@ const char *StyleOptionHelpDescription =
     ".clang-format file located in one of the parent\n"
     "directories of the source file (or current\n"
     "directory for stdin).\n"
-    "Use -style=file:<configfile> to explicitly specify"
-    "the configuration file.\n"
     "Use -style=\"{key: value, ...}\" to set specific\n"
     "parameters, e.g.:\n"
     "  -style=\"{BasedOnStyle: llvm, IndentWidth: 8}\"";
@@ -2687,29 +2685,6 @@ FormatStyle::LanguageKind guessLanguage(StringRef FileName, StringRef Code) {
   return GuessedLanguage;
 }
 
-/// Attempts to load a format file
-llvm::Expected<FormatStyle> LoadConfigFile(StringRef ConfigFile,
-                                           llvm::vfs::FileSystem *FS,
-                                           bool *IsSuitable) {
-  FormatStyle Style = getLLVMStyle();
-  *IsSuitable = true;
-
-  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
-      FS->getBufferForFile(ConfigFile.str());
-  if (std::error_code EC = Text.getError())
-    return make_string_error(EC.message());
-  std::error_code ParserErrorCode =
-      parseConfiguration(Text.get()->getBuffer(), &Style);
-  if (ParserErrorCode == ParseError::Unsuitable) {
-    *IsSuitable = false;
-    return Style;
-  } else if (ParserErrorCode != ParseError::Success) {
-    return make_string_error("Error reading " + ConfigFile + ": " +
-                             ParserErrorCode.message());
-  }
-  return Style;
-}
-
 const char *DefaultFormatStyle = "file";
 
 const char *DefaultFallbackStyle = "LLVM";
@@ -2734,21 +2709,6 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
     return Style;
   }
 
-  llvm::SmallVector<std::string, 2> FilesToLookFor;
-  // User provided clang-format file using -style=file:/path/to/format/file
-  // Check for explicit config filename
-  if (StyleName.startswith_lower("file:")) {
-    auto StyleNameFile = StyleName.substr(5);
-    bool IsSuitable = true;
-    auto Style = LoadConfigFile(StyleNameFile, FS, &IsSuitable);
-    if (Style && !IsSuitable) {
-      return make_string_error("Configuration file(s) do(es) not support " +
-                               getLanguageName((*Style).Language) + ": " +
-                               StyleNameFile);
-    }
-    return Style;
-  }
-
   if (!StyleName.equals_lower("file")) {
     if (!getPredefinedStyle(StyleName, Style.Language, &Style))
       return make_string_error("Invalid value for -style");
@@ -2761,6 +2721,7 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
   if (std::error_code EC = FS->makeAbsolute(Path))
     return make_string_error(EC.message());
 
+  llvm::SmallVector<std::string, 2> FilesToLookFor;
   FilesToLookFor.push_back(".clang-format");
   FilesToLookFor.push_back("_clang-format");
 
@@ -2778,22 +2739,29 @@ llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
 
       llvm::sys::path::append(ConfigFile, F);
       LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n");
+
       Status = FS->status(ConfigFile.str());
+
       if (Status &&
           (Status->getType() == llvm::sys::fs::file_type::regular_file)) {
-        bool IsSuitable;
-        if (auto ConfigStyle = LoadConfigFile(ConfigFile, FS, &IsSuitable)) {
-          if (!IsSuitable) {
+        llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
+            FS->getBufferForFile(ConfigFile.str());
+        if (std::error_code EC = Text.getError())
+          return make_string_error(EC.message());
+        if (std::error_code ec =
+                parseConfiguration(Text.get()->getBuffer(), &Style)) {
+          if (ec == ParseError::Unsuitable) {
             if (!UnsuitableConfigFiles.empty())
               UnsuitableConfigFiles.append(", ");
             UnsuitableConfigFiles.append(ConfigFile);
             continue;
-          } else {
-            return *ConfigStyle;
           }
-        } else {
-          return ConfigStyle.takeError();
+          return make_string_error("Error reading " + ConfigFile + ": " +
+                                   ec.message());
         }
+        LLVM_DEBUG(llvm::dbgs()
+                   << "Using configuration file " << ConfigFile << "\n");
+        return Style;
       }
     }
   }

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 2aac72ced6bd..a6f35ba27069 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -14912,61 +14912,6 @@ TEST(FormatStyle, GetStyleOfFile) {
   auto StyleTd = getStyle("file", "x.td", "llvm", "", &FS);
   ASSERT_TRUE((bool)StyleTd);
   ASSERT_EQ(*StyleTd, getLLVMStyle(FormatStyle::LK_TableGen));
-
-  // Test 9: explicit format file in parent directory.
-  ASSERT_TRUE(
-      FS.addFile("/e/.clang-format", 0,
-                 llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM")));
-  ASSERT_TRUE(
-      FS.addFile("/e/explicit.clang-format", 0,
-                 llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
-  ASSERT_TRUE(FS.addFile("/e/sub/sub/sub/test.cpp", 0,
-                         llvm::MemoryBuffer::getMemBuffer("int i;")));
-  auto Style8 = getStyle("file:/e/explicit.clang-format",
-                         "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS);
-  ASSERT_TRUE((bool)Style8);
-  ASSERT_EQ(*Style8, getGoogleStyle());
-
-  // Test 10: relative pah to a format file
-  ASSERT_TRUE(
-      FS.addFile("../../e/explicit.clang-format", 0,
-                 llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
-  auto Style9 = getStyle("file:../../e/explicit.clang-format",
-                         "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS);
-  ASSERT_TRUE((bool)Style9);
-  ASSERT_EQ(*Style9, getGoogleStyle());
-
-  // Test 11: missing explicit format file
-  auto Style10 = getStyle("file:/e/missing.clang-format",
-                          "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS);
-  ASSERT_FALSE((bool)Style10);
-  llvm::consumeError(Style10.takeError());
-
-  // Test 12: format file from the filesystem
-  SmallString<128> FormatFilePath;
-  std::error_code ECF = llvm::sys::fs::createTemporaryFile(
-      "FormatFileTest", "tpl", FormatFilePath);
-  EXPECT_FALSE((bool)ECF);
-  llvm::raw_fd_ostream FormatFileTest(FormatFilePath, ECF);
-  EXPECT_FALSE((bool)ECF);
-  FormatFileTest << "BasedOnStyle: Google\n";
-  FormatFileTest.close();
-
-  SmallString<128> TestFilePath;
-  std::error_code ECT =
-      llvm::sys::fs::createTemporaryFile("CodeFileTest", "cc", TestFilePath);
-  EXPECT_FALSE((bool)ECT);
-  llvm::raw_fd_ostream CodeFileTest(TestFilePath, ECT);
-  CodeFileTest << "int i;\n";
-  CodeFileTest.close();
-
-  std::string format_file_arg = std::string("file:") + FormatFilePath.c_str();
-  auto Style11 = getStyle(format_file_arg, TestFilePath, "LLVM", "", nullptr);
-
-  llvm::sys::fs::remove(FormatFilePath.c_str());
-  llvm::sys::fs::remove(TestFilePath.c_str());
-  ASSERT_TRUE((bool)Style11);
-  ASSERT_EQ(*Style11, getGoogleStyle());
 }
 
 TEST_F(ReplacementTest, FormatCodeAfterReplacements) {


        


More information about the cfe-commits mailing list