zwliew updated this revision to Diff 396458. zwliew added a comment. Support inheritance in a chain of more than 1 parents
I made the following changes: 1. Refactor the code for loading and parsing configs into a separate function 2. Add a new test case (9.1.2) to test the case mentioned in https://reviews.llvm.org/D93844#inline-1112285. 3. Fix the test case failure for the newly added test 9.1.2. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72326/new/ https://reviews.llvm.org/D72326 Files: 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
Index: clang/unittests/Format/FormatTest.cpp =================================================================== --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -21480,7 +21480,7 @@ ASSERT_TRUE((bool)StyleTd); ASSERT_EQ(*StyleTd, getLLVMStyle(FormatStyle::LK_TableGen)); - // Test 9.1: overwriting a file style, when parent no file exists with no + // Test 9.1.1: overwriting a file style, when parent no file exists with no // fallback style ASSERT_TRUE(FS.addFile( "/e/sub/.clang-format", 0, @@ -21496,6 +21496,25 @@ return Style; }()); + // Test 9.1.2: propagate more than one level with no parent file + ASSERT_TRUE(FS.addFile("/e/sub/sub/code.cpp", 0, + llvm::MemoryBuffer::getMemBuffer("int i;"))); + ASSERT_TRUE(FS.addFile("/e/sub/sub/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: InheritParentConfig\n" + "WhitespaceSensitiveMacros: ['FOO', 'BAR']"))); + std::vector<std::string> NonDefaultWhiteSpaceMacros{"FOO", "BAR"}; + + ASSERT_NE(Style9->WhitespaceSensitiveMacros, NonDefaultWhiteSpaceMacros); + Style9 = getStyle("file", "/e/sub/sub/code.cpp", "none", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style9)); + ASSERT_EQ(*Style9, [&NonDefaultWhiteSpaceMacros] { + auto Style = getNoStyle(); + Style.ColumnLimit = 20; + Style.WhitespaceSensitiveMacros = NonDefaultWhiteSpaceMacros; + return Style; + }()); + // Test 9.2: with LLVM fallback style Style9 = getStyle("file", "/e/sub/code.cpp", "LLVM", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); @@ -21519,15 +21538,7 @@ return Style; }()); - // Test 9.4: propagate more than one level - ASSERT_TRUE(FS.addFile("/e/sub/sub/code.cpp", 0, - llvm::MemoryBuffer::getMemBuffer("int i;"))); - ASSERT_TRUE(FS.addFile("/e/sub/sub/.clang-format", 0, - llvm::MemoryBuffer::getMemBuffer( - "BasedOnStyle: InheritParentConfig\n" - "WhitespaceSensitiveMacros: ['FOO', 'BAR']"))); - std::vector<std::string> NonDefaultWhiteSpaceMacros{"FOO", "BAR"}; - + // Test 9.4: propagate more than one level with a parent file const auto SubSubStyle = [&NonDefaultWhiteSpaceMacros] { auto Style = getGoogleStyle(); Style.ColumnLimit = 20; @@ -21584,6 +21595,70 @@ Style.IndentWidth = 7; return Style; }()); + + // Test 9.9: use inheritance from a specific config file + Style9 = getStyle("file:/e/sub/sub/.clang-format", "/e/sub/sub/code.cpp", + "none", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style9)); + ASSERT_EQ(*Style9, SubSubStyle); +} + +TEST(FormatStyle, GetStyleFromExternalFile) { + llvm::vfs::InMemoryFileSystem FS; + // 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 Style = getStyle("file:/e/explicit.clang-format", + "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style)); + ASSERT_EQ(*Style, getGoogleStyle()); + + // Relative path to a format file + ASSERT_TRUE( + FS.addFile("../../e/explicit.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google"))); + Style = getStyle("file:../../e/explicit.clang-format", + "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style)); + ASSERT_EQ(*Style, getGoogleStyle()); + + // Missing explicit format file + Style = getStyle("file:/e/missing.clang-format", "/e/sub/sub/sub/test.cpp", + "LLVM", "", &FS); + ASSERT_FALSE(static_cast<bool>(Style)); + llvm::consumeError(Style.takeError()); + + // 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(); + Style = getStyle(format_file_arg, TestFilePath, "LLVM", "", nullptr); + + llvm::sys::fs::remove(FormatFilePath.c_str()); + llvm::sys::fs::remove(TestFilePath.c_str()); + ASSERT_TRUE(static_cast<bool>(Style)); + ASSERT_EQ(*Style, getGoogleStyle()); } TEST_F(ReplacementTest, FormatCodeAfterReplacements) { Index: clang/lib/Format/Format.cpp =================================================================== --- clang/lib/Format/Format.cpp +++ clang/lib/Format/Format.cpp @@ -3181,6 +3181,8 @@ ".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:<format_file_path> to explicitly specify" + "the configuration file.\n" "Use -style=\"{key: value, ...}\" to set specific\n" "parameters, e.g.:\n" " -style=\"{BasedOnStyle: llvm, IndentWidth: 8}\""; @@ -3233,6 +3235,18 @@ const char *DefaultFallbackStyle = "LLVM"; +llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> +loadAndParseConfigFile(StringRef ConfigFile, llvm::vfs::FileSystem *FS, + FormatStyle *Style, bool AllowUnknownOptions) { + llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = + FS->getBufferForFile(ConfigFile.str()); + if (auto EC = Text.getError()) + return EC; + if (auto EC = parseConfiguration(*Text.get(), Style, AllowUnknownOptions)) + return EC; + return Text; +} + llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, StringRef FallbackStyleName, StringRef Code, llvm::vfs::FileSystem *FS, @@ -3263,6 +3277,29 @@ return Style; } + // User provided clang-format file using -style=file:path/to/format/file + // Check for explicit config filename + if (!Style.InheritsParentConfig && + StyleName.startswith_insensitive("file:")) { + auto ConfigFile = StyleName.substr(5); + llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = + loadAndParseConfigFile(ConfigFile, FS, &Style, AllowUnknownOptions); + if (auto EC = Text.getError()) + return make_string_error("Error reading " + ConfigFile + ": " + + EC.message()); + + LLVM_DEBUG(llvm::dbgs() + << "Using configuration file " << ConfigFile << "\n"); + + if (!Style.InheritsParentConfig) + return Style; + + // Search for parent configs starting from the parent directory of + // ConfigFile + FileName = ConfigFile; + ChildFormatTextToApply.emplace_back(std::move(*Text)); + } + // If the style inherits the parent configuration it is a command line // configuration, which wants to inherit, so we have to skip the check of the // StyleName. @@ -3288,6 +3325,16 @@ auto dropDiagnosticHandler = [](const llvm::SMDiagnostic &, void *) {}; + auto applyChildFormatTexts = [&](FormatStyle *Style) { + for (const auto &MemBuf : llvm::reverse(ChildFormatTextToApply)) { + auto EC = parseConfiguration(*MemBuf, Style, AllowUnknownOptions, + dropDiagnosticHandler); + // It was already correctly parsed. + assert(!EC); + static_cast<void>(EC); + } + }; + for (StringRef Directory = Path; !Directory.empty(); Directory = llvm::sys::path::parent_path(Directory)) { @@ -3308,19 +3355,16 @@ if (Status && (Status->getType() == llvm::sys::fs::file_type::regular_file)) { 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(), &Style, AllowUnknownOptions)) { - if (ec == ParseError::Unsuitable) { + loadAndParseConfigFile(ConfigFile, FS, &Style, AllowUnknownOptions); + if (auto EC = Text.getError()) { + if (EC == ParseError::Unsuitable) { if (!UnsuitableConfigFiles.empty()) UnsuitableConfigFiles.append(", "); UnsuitableConfigFiles.append(ConfigFile); continue; } return make_string_error("Error reading " + ConfigFile + ": " + - ec.message()); + EC.message()); } LLVM_DEBUG(llvm::dbgs() << "Using configuration file " << ConfigFile << "\n"); @@ -3330,14 +3374,7 @@ return Style; LLVM_DEBUG(llvm::dbgs() << "Applying child configurations\n"); - - for (const auto &MemBuf : llvm::reverse(ChildFormatTextToApply)) { - auto Ec = parseConfiguration(*MemBuf, &Style, AllowUnknownOptions, - dropDiagnosticHandler); - // It was already correctly parsed. - assert(!Ec); - static_cast<void>(Ec); - } + applyChildFormatTexts(&Style); return Style; } @@ -3363,17 +3400,9 @@ UnsuitableConfigFiles); if (!ChildFormatTextToApply.empty()) { - assert(ChildFormatTextToApply.size() == 1); - LLVM_DEBUG(llvm::dbgs() - << "Applying child configuration on fallback style\n"); - - auto Ec = - parseConfiguration(*ChildFormatTextToApply.front(), &FallbackStyle, - AllowUnknownOptions, dropDiagnosticHandler); - // It was already correctly parsed. - assert(!Ec); - static_cast<void>(Ec); + << "Applying child configurations on fallback style\n"); + applyChildFormatTexts(&FallbackStyle); } return FallbackStyle; Index: clang/include/clang/Format/Format.h =================================================================== --- clang/include/clang/Format/Format.h +++ clang/include/clang/Format/Format.h @@ -4066,6 +4066,8 @@ /// * "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:<format_file_path>" to explicitly specify the configuration file to +/// use. /// /// \param[in] StyleName Style name to interpret according to the description /// above. Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -298,6 +298,10 @@ space before parentheses. The custom options can be set using ``SpaceBeforeParensOptions``. +- 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>`. + - Improved C++20 Modules and Coroutines support. libclang Index: clang/docs/ClangFormatStyleOptions.rst =================================================================== --- clang/docs/ClangFormatStyleOptions.rst +++ clang/docs/ClangFormatStyleOptions.rst @@ -32,6 +32,10 @@ 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 Index: clang/docs/ClangFormat.rst =================================================================== --- clang/docs/ClangFormat.rst +++ clang/docs/ClangFormat.rst @@ -82,6 +82,10 @@ .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}"
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits