https://github.com/jediry updated https://github.com/llvm/llvm-project/pull/110634
>From 82160d1e6de3f197c847bf8ed21ea1fc314b3cf4 Mon Sep 17 00:00:00 2001 From: Ryan Saunders <ryans...@microsoft.com> Date: Tue, 1 Oct 2024 00:03:50 -0700 Subject: [PATCH 1/2] Support BasedOnStyle: file:blah.clang-format --- clang/include/clang/Format/Format.h | 22 ++- clang/lib/Format/Format.cpp | 207 +++++++++++++++++++++------- 2 files changed, 174 insertions(+), 55 deletions(-) diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index d8b62c7652a0f6..1b833256500431 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -53,10 +53,24 @@ std::error_code make_error_code(ParseError e); /// The ``FormatStyle`` is used to configure the formatting to follow /// specific guidelines. struct FormatStyle { - // If the BasedOn: was InheritParentConfig and this style needs the file from - // the parent directories. It is not part of the actual style for formatting. - // Thus the // instead of ///. - bool InheritsParentConfig; + // If the BasedOnStyle: was InheritParentConfig, this is the string + // "<parent>", indicating to search upwards until a _clang-format or + // .clang-format file is found. + // + // Else, if the BasedOnStyle: was an explicit "file:" reference, this is + // that reference, verbatim, including the "file:" prefix. The string + // after "file:" may start with $(CLANG_FORMAT_DIR), in which case the value + // of the CLANG_FORMAT_DIR environment variable (which must be defined) is + // substituted; otherwise, the string after "file:" is interpreted as a + // path relative to the current config file. (Absolute paths are not + // permitted, for security reasons.) + // + // Else (i.e., if the BasedOnStyle is omitted or a predefined style), this is + // the empty string. + // + // This field is not part of the actual style for formatting, thus the // + // instead of ///. + std::string InheritsConfig; /// The extra indent or outdent of access modifiers, e.g. ``public:``. /// \version 3.3 diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index d2463b892fbb96..bfca01d1a67230 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1543,7 +1543,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.IndentRequiresClause = true; LLVMStyle.IndentWidth = 2; LLVMStyle.IndentWrappedFunctionNames = false; - LLVMStyle.InheritsParentConfig = false; + LLVMStyle.InheritsConfig.clear(); LLVMStyle.InsertBraces = false; LLVMStyle.InsertNewlineAtEOF = false; LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None; @@ -1992,7 +1992,9 @@ bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language, else if (Name.equals_insensitive("none")) *Style = getNoStyle(); else if (Name.equals_insensitive("inheritparentconfig")) - Style->InheritsParentConfig = true; + Style->InheritsConfig = "<parent>"; + else if (Name.starts_with_insensitive("file:")) + Style->InheritsConfig = Name; else return false; @@ -4004,6 +4006,45 @@ loadAndParseConfigFile(StringRef ConfigFile, llvm::vfs::FileSystem *FS, return Text; } +// Resolves an explicit file: reference in a BasedOnStyle directive to a +// canonical, absolute path, substituting the value of the CLANG_FORMAT_DIR +// environment variable if the path starts with $(CLANG_FORMAT_DIR), or treating +// BasedOnFile as relative to the current ConfigFile otherwise. +static Expected<SmallString<128>> +resolveExplicitParentConfigFile(StringRef ConfigFile, StringRef BasedOnFile, + llvm::vfs::FileSystem *FS) { + constexpr const char *EnvVar = "CLANG_FORMAT_DIR"; + constexpr const char *EnvVarExpansion = "$(CLANG_FORMAT_DIR)"; + if (BasedOnFile.starts_with(EnvVarExpansion)) { + const char *ClangFormatDir = getenv(EnvVar); + if (ClangFormatDir == nullptr) { + return make_string_error(ConfigFile + ": 'BasedOnStyle: " + BasedOnFile + + "' uses " + EnvVarExpansion + ", but the " + + EnvVar + " environment variable is not defined"); + } else { + auto Status = FS->status(ClangFormatDir); + if (!Status || + Status->getType() != llvm::sys::fs::file_type::directory_file) { + return make_string_error( + StringRef("Environment variable ") + EnvVar + " = " + + ClangFormatDir + "does not refer to a valid, readable directory"); + } + + SmallString<128> FileName{ClangFormatDir}; + llvm::sys::path::append(FileName, + BasedOnFile.substr(strlen(EnvVarExpansion))); + ; + llvm::sys::path::remove_dots(FileName, true /*remove_dot_dot*/); + return FileName; + } + } else { + SmallString<128> FileName = llvm::sys::path::parent_path(ConfigFile); + llvm::sys::path::append(FileName, BasedOnFile); + llvm::sys::path::remove_dots(FileName, true /*remove_dot_dot*/); + return FileName; + } +} + Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, StringRef FallbackStyleName, StringRef Code, llvm::vfs::FileSystem *FS, @@ -4025,7 +4066,7 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, return make_string_error("Error parsing -style: " + ec.message()); } - if (!Style.InheritsParentConfig) + if (Style.InheritsConfig.empty()) return Style; ChildFormatTextToApply.emplace_back( @@ -4037,7 +4078,7 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, assert(FS); // User provided clang-format file using -style=file:path/to/format/file. - if (!Style.InheritsParentConfig && + if (Style.InheritsConfig.empty() && StyleName.starts_with_insensitive("file:")) { auto ConfigFile = StyleName.substr(5); llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = @@ -4051,7 +4092,7 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, LLVM_DEBUG(llvm::dbgs() << "Using configuration file " << ConfigFile << "\n"); - if (!Style.InheritsParentConfig) + if (Style.InheritsConfig.empty()) return Style; // Search for parent configs starting from the parent directory of @@ -4063,10 +4104,10 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, // 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. - if (!Style.InheritsParentConfig && !StyleName.equals_insensitive("file")) { + if (Style.InheritsConfig.empty() && !StyleName.equals_insensitive("file")) { if (!getPredefinedStyle(StyleName, Style.Language, &Style)) return make_string_error("Invalid value for -style"); - if (!Style.InheritsParentConfig) + if (Style.InheritsConfig.empty()) return Style; } @@ -4075,7 +4116,7 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, return make_string_error(EC.message()); // Reset possible inheritance - Style.InheritsParentConfig = false; + Style.InheritsConfig.clear(); auto dropDiagnosticHandler = [](const llvm::SMDiagnostic &, void *) {}; @@ -4096,44 +4137,22 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, FilesToLookFor.push_back("_clang-format"); SmallString<128> UnsuitableConfigFiles; - for (StringRef Directory = Path; !Directory.empty(); - Directory = llvm::sys::path::parent_path(Directory)) { - auto Status = FS->status(Directory); - if (!Status || - Status->getType() != llvm::sys::fs::file_type::directory_file) { - continue; - } - - for (const auto &F : FilesToLookFor) { - SmallString<128> ConfigFile(Directory); - - llvm::sys::path::append(ConfigFile, F); - LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); - - Status = FS->status(ConfigFile); - if (!Status || - Status->getType() != llvm::sys::fs::file_type::regular_file) { - continue; - } - + SmallString<128> ExplicitlyInheritedConfigFile; + do { + if (!ExplicitlyInheritedConfigFile.empty()) { llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = - loadAndParseConfigFile(ConfigFile, FS, &Style, AllowUnknownOptions, - DiagHandler); + loadAndParseConfigFile(ExplicitlyInheritedConfigFile, FS, &Style, + AllowUnknownOptions, DiagHandler); if (auto EC = Text.getError()) { - if (EC != ParseError::Unsuitable) { - return make_string_error("Error reading " + ConfigFile + ": " + - EC.message()); - } - if (!UnsuitableConfigFiles.empty()) - UnsuitableConfigFiles.append(", "); - UnsuitableConfigFiles.append(ConfigFile); - continue; + return make_string_error("Error reading " + + ExplicitlyInheritedConfigFile + ": " + + EC.message()); } - LLVM_DEBUG(llvm::dbgs() - << "Using configuration file " << ConfigFile << "\n"); + LLVM_DEBUG(llvm::dbgs() << "Using configuration file " + << ExplicitlyInheritedConfigFile << "\n"); - if (!Style.InheritsParentConfig) { + if (Style.InheritsConfig.empty()) { if (!ChildFormatTextToApply.empty()) { LLVM_DEBUG(llvm::dbgs() << "Applying child configurations\n"); applyChildFormatTexts(&Style); @@ -4141,20 +4160,106 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, return Style; } - LLVM_DEBUG(llvm::dbgs() << "Inherits parent configuration\n"); + ChildFormatTextToApply.emplace_back(std::move(*Text)); - // Reset inheritance of style - Style.InheritsParentConfig = false; + if (Style.InheritsConfig == "<parent>") { + // Search for parent configs starting from the parent directory of + // ExplicitlyInheritedConfigFile. + Path = ExplicitlyInheritedConfigFile; + ExplicitlyInheritedConfigFile.clear(); // Don't process this file again + } else if (StringRef(Style.InheritsConfig) + .starts_with_insensitive("file:")) { + // This config file also inherits its parent with an explicit path + llvm::Expected<SmallString<128>> Parent = + resolveExplicitParentConfigFile(ExplicitlyInheritedConfigFile, + Style.InheritsConfig.substr(5), FS); + if (!Parent) + return Parent.takeError(); + + ExplicitlyInheritedConfigFile = *Parent; + continue; + } + } - ChildFormatTextToApply.emplace_back(std::move(*Text)); + for (StringRef Directory = Path; !Directory.empty(); + Directory = llvm::sys::path::parent_path(Directory)) { + auto Status = FS->status(Directory); + if (!Status || + Status->getType() != llvm::sys::fs::file_type::directory_file) { + continue; + } - // Breaking out of the inner loop, since we don't want to parse - // .clang-format AND _clang-format, if both exist. Then we continue the - // outer loop (parent directories) in search for the parent - // configuration. - break; + for (const auto &F : FilesToLookFor) { + SmallString<128> ConfigFile(Directory); + + llvm::sys::path::append(ConfigFile, F); + LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); + + Status = FS->status(ConfigFile); + if (!Status || + Status->getType() != llvm::sys::fs::file_type::regular_file) { + continue; + } + + llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = + loadAndParseConfigFile(ConfigFile, FS, &Style, AllowUnknownOptions, + DiagHandler); + if (auto EC = Text.getError()) { + if (EC != ParseError::Unsuitable) { + return make_string_error("Error reading " + ConfigFile + ": " + + EC.message()); + } + if (!UnsuitableConfigFiles.empty()) + UnsuitableConfigFiles.append(", "); + UnsuitableConfigFiles.append(ConfigFile); + continue; + } + + LLVM_DEBUG(llvm::dbgs() + << "Using configuration file " << ConfigFile << "\n"); + + if (Style.InheritsConfig.empty()) { + if (!ChildFormatTextToApply.empty()) { + LLVM_DEBUG(llvm::dbgs() << "Applying child configurations\n"); + applyChildFormatTexts(&Style); + } + return Style; + } + + if (Style.InheritsConfig == "<parent>") { + LLVM_DEBUG(llvm::dbgs() << "Inherits parent configuration\n"); + } else if (StringRef(Style.InheritsConfig) + .starts_with_insensitive("file:")) { + llvm::Expected<SmallString<128>> Parent = + resolveExplicitParentConfigFile( + ConfigFile, Style.InheritsConfig.substr(5), FS); + if (!Parent) + return Parent.takeError(); + + ExplicitlyInheritedConfigFile = *Parent; + LLVM_DEBUG(llvm::dbgs() << "Inherits configuration from " + << ExplicitlyInheritedConfigFile << "\n"); + } + + // Reset inheritance of style + Style.InheritsConfig.clear(); + + ChildFormatTextToApply.emplace_back(std::move(*Text)); + + // Breaking out of the inner loop, since we don't want to parse + // .clang-format AND _clang-format, if both exist. Then we continue the + // outer loop (parent directories) in search for the parent + // configuration. + break; + } + + // If processing a parent config file explicitly named via "BasedOnStyle: + // file:", break out of this loop as well, since any future parent + // directory checks will be relative to that file. + if (!ExplicitlyInheritedConfigFile.empty()) + break; } - } + } while (!ExplicitlyInheritedConfigFile.empty()); if (!UnsuitableConfigFiles.empty()) { return make_string_error("Configuration file(s) do(es) not support " + >From fb8c021742a651ab85221bb37d3164fc84fb00c6 Mon Sep 17 00:00:00 2001 From: Ryan Saunders <ryans...@microsoft.com> Date: Tue, 8 Oct 2024 14:45:36 -0700 Subject: [PATCH 2/2] Add docs and unit tests --- clang/docs/ClangFormatStyleOptions.rst | 24 ++ clang/docs/ReleaseNotes.rst | 1 + clang/lib/Format/Format.cpp | 137 +++++---- clang/unittests/Format/ConfigParseTest.cpp | 324 +++++++++++++++++++++ 4 files changed, 437 insertions(+), 49 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index a427d7cd40fcdd..1f43c291b173d0 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -184,6 +184,30 @@ the configuration (without a prefix: ``Auto``). With this option you can overwrite some parts of your main style for your subdirectories. This is also possible through the command line, e.g.: ``--style={BasedOnStyle: InheritParentConfig, ColumnLimit: 20}`` + * ``file:<format-file-path>`` + Allows to use an explicit path to another format file as the base style. + It is an error if this file cannot be read (e.g., if it does not exist, + or access is denied). ``<format-file-path>`` must have one of the + following forms: + + * a relative path. If used within a ``.clang-format`` file, the path is + relative to the directory containing the current format file; if used + within a command-line ``--style='{...}'`` argument, the path is + relative to the process's current working directory). E.g., + ``BasedOnStyle: file:../format/shared.clang-format``. + * a path beginning with ``$(CLANG_FORMAT_DIR)``. The string + ``$(CLANG_FORMAT_DIR)`` is replaced with the value of the environment + variable ``CLANG_FORMAT_DIR``, which must be defined to the absolute + path to a valid, readable directory. E.g., + ``BasedOnStyle: file:$(CLANG_FORMAT_DIR)/shared.clang-format`` + + With this option, you can maintain a shared base style in a centralized location + other than the root directory of your project. + + Note that the file at ``<format-file-path>`` may itself use + ``BasedOnStyle: InheritParentConfig`` or ``BasedOnStyle: file:<another-file-path>``, + and in these cases, the next base format is located relative to ``<format-file-path>``, + *not* relative to the original config that refers to ``<format-file-path>``. .. START_FORMAT_STYLE_OPTIONS diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8228055a1d861a..09792d5ff17226 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -512,6 +512,7 @@ clang-format ------------ - Adds ``BreakBinaryOperations`` option. +- Extends ``BasedOnStyle`` option to support inheriting styles from an arbitrary file path libclang -------- diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index bfca01d1a67230..0dc1db64c4029e 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -4006,43 +4006,62 @@ loadAndParseConfigFile(StringRef ConfigFile, llvm::vfs::FileSystem *FS, return Text; } -// Resolves an explicit file: reference in a BasedOnStyle directive to a +// Converts a path to canonical form, ensuring that it is an absolute path, with +// all .. and . components removed, and with Posix-style path separators. +static std::error_code makeCanonicalPath(SmallString<128> &Path, + llvm::vfs::FileSystem &FS) { + if (auto EC = FS.makeAbsolute(Path)) + return EC; + llvm::sys::path::remove_dots(Path, /*remove_dot_dot*/ true); + llvm::sys::path::native(Path, llvm::sys::path::Style::posix); + return {}; +} + +// Resolves an explicit "file:" reference in a BasedOnStyle directive to a // canonical, absolute path, substituting the value of the CLANG_FORMAT_DIR // environment variable if the path starts with $(CLANG_FORMAT_DIR), or treating -// BasedOnFile as relative to the current ConfigFile otherwise. +// BasedOnFile as relative to `Directory` otherwise. static Expected<SmallString<128>> -resolveExplicitParentConfigFile(StringRef ConfigFile, StringRef BasedOnFile, - llvm::vfs::FileSystem *FS) { +resolveExplicitBasedOnConfigFile(StringRef Source, StringRef Directory, + StringRef BasedOnFile, + llvm::vfs::FileSystem &FS) { constexpr const char *EnvVar = "CLANG_FORMAT_DIR"; constexpr const char *EnvVarExpansion = "$(CLANG_FORMAT_DIR)"; + + SmallString<128> FileName; if (BasedOnFile.starts_with(EnvVarExpansion)) { const char *ClangFormatDir = getenv(EnvVar); if (ClangFormatDir == nullptr) { - return make_string_error(ConfigFile + ": 'BasedOnStyle: " + BasedOnFile + + return make_string_error(Source + ": 'BasedOnStyle: file:" + BasedOnFile + "' uses " + EnvVarExpansion + ", but the " + EnvVar + " environment variable is not defined"); } else { - auto Status = FS->status(ClangFormatDir); - if (!Status || - Status->getType() != llvm::sys::fs::file_type::directory_file) { - return make_string_error( - StringRef("Environment variable ") + EnvVar + " = " + - ClangFormatDir + "does not refer to a valid, readable directory"); + auto Status = FS.status(ClangFormatDir); + if (!Status || !Status->isDirectory()) { + return make_string_error(StringRef("Environment variable ") + EnvVar + + " = '" + ClangFormatDir + + "' is not a valid, readable directory"); } - SmallString<128> FileName{ClangFormatDir}; + FileName = ClangFormatDir; llvm::sys::path::append(FileName, BasedOnFile.substr(strlen(EnvVarExpansion))); - ; - llvm::sys::path::remove_dots(FileName, true /*remove_dot_dot*/); - return FileName; } } else { - SmallString<128> FileName = llvm::sys::path::parent_path(ConfigFile); + FileName = Directory; llvm::sys::path::append(FileName, BasedOnFile); - llvm::sys::path::remove_dots(FileName, true /*remove_dot_dot*/); - return FileName; } + + // Canonize the filename and ensure that it exists + if (auto EC = makeCanonicalPath(FileName, FS)) + return make_string_error(EC.message()); + auto Status = FS.status(FileName); + if (!Status || !Status->isRegularFile()) { + return make_string_error(Source + ": 'BasedOnStyle: file:" + BasedOnFile + + "': '" + FileName + + "' is not a valid, readable file"); + } + return FileName; } Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, @@ -4055,6 +4074,7 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, if (!getPredefinedStyle(FallbackStyleName, Style.Language, &FallbackStyle)) return make_string_error("Invalid fallback style: " + FallbackStyleName); + SmallString<128> ExplicitlyInheritedConfigFile; SmallVector<std::unique_ptr<llvm::MemoryBuffer>, 1> ChildFormatTextToApply; if (StyleName.starts_with("{")) { @@ -4071,34 +4091,51 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, ChildFormatTextToApply.emplace_back( llvm::MemoryBuffer::getMemBuffer(StyleName, Source, false)); + + // In a command-line style block, "BasedOnStyle: file:..." is relative to + // the process's current working directory + if (StringRef(Style.InheritsConfig).starts_with_insensitive("file:")) { + llvm::ErrorOr<std::string> CWD = FS->getCurrentWorkingDirectory(); + if (auto EC = CWD.getError()) + return make_string_error(EC.message()); + llvm::Expected<SmallString<128>> BasedOnFile = + resolveExplicitBasedOnConfigFile(Source, *CWD, + Style.InheritsConfig.substr(5), *FS); + if (!BasedOnFile) + return BasedOnFile.takeError(); + + ExplicitlyInheritedConfigFile = *BasedOnFile; + } } if (!FS) FS = llvm::vfs::getRealFileSystem().get(); assert(FS); + SmallString<128> Path; + // User provided clang-format file using -style=file:path/to/format/file. if (Style.InheritsConfig.empty() && StyleName.starts_with_insensitive("file:")) { - auto ConfigFile = StyleName.substr(5); + Path = StyleName.substr(5); + if (auto EC = makeCanonicalPath(Path, *FS)) + return make_string_error(EC.message()); llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = - loadAndParseConfigFile(ConfigFile, FS, &Style, AllowUnknownOptions, + loadAndParseConfigFile(Path, FS, &Style, AllowUnknownOptions, DiagHandler); - if (auto EC = Text.getError()) { - return make_string_error("Error reading " + ConfigFile + ": " + - EC.message()); - } + if (auto EC = Text.getError()) + return make_string_error("Error reading " + Path + ": " + EC.message()); - LLVM_DEBUG(llvm::dbgs() - << "Using configuration file " << ConfigFile << "\n"); + LLVM_DEBUG(llvm::dbgs() << "Using configuration file " << Path << "\n"); if (Style.InheritsConfig.empty()) return Style; - // Search for parent configs starting from the parent directory of - // ConfigFile. - FileName = ConfigFile; ChildFormatTextToApply.emplace_back(std::move(*Text)); + } else { + Path = FileName; + if (auto EC = makeCanonicalPath(Path, *FS)) + return make_string_error(EC.message()); } // If the style inherits the parent configuration it is a command line @@ -4111,10 +4148,6 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, return Style; } - SmallString<128> Path(FileName); - if (std::error_code EC = FS->makeAbsolute(Path)) - return make_string_error(EC.message()); - // Reset possible inheritance Style.InheritsConfig.clear(); @@ -4137,7 +4170,6 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, FilesToLookFor.push_back("_clang-format"); SmallString<128> UnsuitableConfigFiles; - SmallString<128> ExplicitlyInheritedConfigFile; do { if (!ExplicitlyInheritedConfigFile.empty()) { llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = @@ -4170,13 +4202,16 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, } else if (StringRef(Style.InheritsConfig) .starts_with_insensitive("file:")) { // This config file also inherits its parent with an explicit path - llvm::Expected<SmallString<128>> Parent = - resolveExplicitParentConfigFile(ExplicitlyInheritedConfigFile, - Style.InheritsConfig.substr(5), FS); - if (!Parent) - return Parent.takeError(); - - ExplicitlyInheritedConfigFile = *Parent; + SmallString<128> ParentDirectory = + llvm::sys::path::parent_path(ExplicitlyInheritedConfigFile); + llvm::Expected<SmallString<128>> BasedOnFile = + resolveExplicitBasedOnConfigFile( + ExplicitlyInheritedConfigFile, ParentDirectory, + Style.InheritsConfig.substr(5), *FS); + if (!BasedOnFile) + return BasedOnFile.takeError(); + + ExplicitlyInheritedConfigFile = *BasedOnFile; continue; } } @@ -4191,8 +4226,9 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, for (const auto &F : FilesToLookFor) { SmallString<128> ConfigFile(Directory); - llvm::sys::path::append(ConfigFile, F); + if (auto EC = makeCanonicalPath(ConfigFile, *FS)) + return make_string_error(EC.message()); LLVM_DEBUG(llvm::dbgs() << "Trying " << ConfigFile << "...\n"); Status = FS->status(ConfigFile); @@ -4230,13 +4266,16 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, LLVM_DEBUG(llvm::dbgs() << "Inherits parent configuration\n"); } else if (StringRef(Style.InheritsConfig) .starts_with_insensitive("file:")) { - llvm::Expected<SmallString<128>> Parent = - resolveExplicitParentConfigFile( - ConfigFile, Style.InheritsConfig.substr(5), FS); - if (!Parent) - return Parent.takeError(); - - ExplicitlyInheritedConfigFile = *Parent; + SmallString<128> ParentDirectory = + llvm::sys::path::parent_path(ConfigFile); + llvm::Expected<SmallString<128>> BasedOnFile = + resolveExplicitBasedOnConfigFile(ConfigFile, ParentDirectory, + Style.InheritsConfig.substr(5), + *FS); + if (!BasedOnFile) + return BasedOnFile.takeError(); + + ExplicitlyInheritedConfigFile = *BasedOnFile; LLVM_DEBUG(llvm::dbgs() << "Inherits configuration from " << ExplicitlyInheritedConfigFile << "\n"); } diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index b8bdfaaa74e10e..44fd48fcc829b8 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -15,6 +15,22 @@ namespace clang { namespace format { namespace { +static bool UnsetEnv(const char *Name) noexcept { +#ifdef _WIN32 + return _putenv_s(Name, "") == 0; +#else + return setenv(Name, Value, 1 /*Overwrite*/) == 0; +#endif +} + +static bool SetEnv(const char *Name, const char *Value) noexcept { +#ifdef _WIN32 + return _putenv_s(Name, Value) == 0; +#else + return setenv(Name, Value, 1 /*Overwrite*/) == 0; +#endif +} + void dropDiagnosticHandler(const llvm::SMDiagnostic &, void *) {} FormatStyle getGoogleStyle() { return getGoogleStyle(FormatStyle::LK_Cpp); } @@ -1219,6 +1235,8 @@ TEST(ConfigParseTest, GetStyleWithEmptyFileName) { TEST(ConfigParseTest, GetStyleOfFile) { llvm::vfs::InMemoryFileSystem FS; + FS.setCurrentWorkingDirectory("/"); + // Test 1: format file in the same directory. ASSERT_TRUE( FS.addFile("/a/.clang-format", 0, @@ -1425,6 +1443,312 @@ TEST(ConfigParseTest, GetStyleOfFile) { "none", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, SubSubStyle); + + // Test 10: BasedOnStyle: file:... + ASSERT_TRUE(FS.addFile("/f/format/", 0, {}, std::nullopt, std::nullopt, + llvm::sys::fs::file_type::directory_file)); + ASSERT_TRUE(FS.addFile("/f/src_refs_nonexistent_env/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/" + "nonexistent.clang-format\nColumnLimit: 25"))); + ASSERT_TRUE(FS.addFile( + "/f/src_refs_nonexistent_rel/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: file:../format/nonexistent.clang-format\nColumnLimit: " + "25"))); + + // Test 10.1: inheritance based on $(CLANG_FORMAT_DIR) is an error if the + // CLANG_FORMAT_DIR environment variable is undefined + ASSERT_TRUE(UnsetEnv("CLANG_FORMAT_DIR")); + + // Test 10.1.1: test command-line + auto Style10 = getStyle( + "{BasedOnStyle: file:$(CLANG_FORMAT_DIR)/nonexistent.clang-format}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_FALSE(static_cast<bool>(Style10)); + ASSERT_EQ(llvm::toString(Style10.takeError()), + "<command-line>: 'BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/nonexistent.clang-format' uses " + "$(CLANG_FORMAT_DIR), but the CLANG_FORMAT_DIR environment " + "variable is not defined"); + + // Test 10.1.2: test file + Style10 = getStyle("file", "/f/src_refs_nonexistent_env/code.cpp", "google", + "", &FS); + ASSERT_FALSE(static_cast<bool>(Style10)); + ASSERT_EQ(llvm::toString(Style10.takeError()), + "/f/src_refs_nonexistent_env/.clang-format: 'BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/nonexistent.clang-format' uses " + "$(CLANG_FORMAT_DIR), but the CLANG_FORMAT_DIR environment " + "variable is not defined"); + + // Test 10.2: inheritance based on $(CLANG_FORMAT_DIR) is an error if the + // CLANG_FORMAT_DIR environment variable points to an invalid directory + ASSERT_TRUE(SetEnv("CLANG_FORMAT_DIR", "/f/nonexistent-directory")); + + // Test 10.2.1: test command-line + Style10 = getStyle( + "{BasedOnStyle: file:$(CLANG_FORMAT_DIR)/nonexistent.clang-format}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_FALSE(static_cast<bool>(Style10)); + ASSERT_EQ(llvm::toString(Style10.takeError()), + "Environment variable CLANG_FORMAT_DIR = " + "'/f/nonexistent-directory' is not a valid, readable directory"); + + // Test 10.2.2: test file + Style10 = getStyle("file", "/f/src_refs_nonexistent_env/code.cpp", "google", + "", &FS); + ASSERT_FALSE(static_cast<bool>(Style10)); + ASSERT_EQ(llvm::toString(Style10.takeError()), + "Environment variable CLANG_FORMAT_DIR = " + "'/f/nonexistent-directory' is not a valid, readable directory"); + + // Test 10.3: file inheritance is an error if the file pointed to via + // BasedOnStyle does not exist + ASSERT_TRUE(SetEnv("CLANG_FORMAT_DIR", "/f/format")); + + // Test 10.3.1: test command-line w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle( + "{BasedOnStyle: file:$(CLANG_FORMAT_DIR)/nonexistent.clang-format}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_FALSE(static_cast<bool>(Style10)); + ASSERT_EQ( + llvm::toString(Style10.takeError()), + "<command-line>: 'BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/nonexistent.clang-format': " + "'/f/format/nonexistent.clang-format' is not a valid, readable file"); + + // Test 10.3.2: test file w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle("file", "/f/src_refs_nonexistent_env/code.cpp", "google", + "", &FS); + ASSERT_FALSE(static_cast<bool>(Style10)); + ASSERT_EQ( + llvm::toString(Style10.takeError()), + "/f/src_refs_nonexistent_env/.clang-format: 'BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/nonexistent.clang-format': " + "'/f/format/nonexistent.clang-format' is not a valid, readable file"); + + // Test 10.3.3: test command-line w/ relative path + Style10 = getStyle("{BasedOnStyle: file:f/nonexistent.clang-format}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_FALSE(static_cast<bool>(Style10)); + ASSERT_EQ(llvm::toString(Style10.takeError()), + "<command-line>: 'BasedOnStyle: file:f/nonexistent.clang-format': " + "'/f/nonexistent.clang-format' is not a valid, readable file"); + + // Test 10.3.4: test file w/ relative path + Style10 = getStyle("file", "/f/src_refs_nonexistent_rel/code.cpp", "google", + "", &FS); + ASSERT_FALSE(static_cast<bool>(Style10)); + ASSERT_EQ( + llvm::toString(Style10.takeError()), + "/f/src_refs_nonexistent_rel/.clang-format: 'BasedOnStyle: " + "file:../format/nonexistent.clang-format': " + "'/f/format/nonexistent.clang-format' is not a valid, readable file"); + + // Test 10.4: file inheritance using a BasedOnStyle file w/ a predefined base + // style, applying child styles on top + ASSERT_TRUE(FS.addFile("/f/format/predefined.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: microsoft\nIndentWidth: 10"))); + ASSERT_TRUE(FS.addFile("/f/src_refs_predefined_env/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/" + "predefined.clang-format\nColumnLimit: 25"))); + ASSERT_TRUE(FS.addFile( + "/f/src_refs_predefined_rel/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: file:../format/predefined.clang-format\nColumnLimit: " + "25"))); + auto MergedStyle = getMicrosoftStyle(FormatStyle::LK_Cpp); + MergedStyle.IndentWidth = 10; + MergedStyle.ColumnLimit = 25; + + // Test 10.4.1: test command-line w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle( + "{BasedOnStyle: file:$(CLANG_FORMAT_DIR)/predefined.clang-format, " + "ColumnLimit: 25}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.4.2: test file w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle("file", "/f/src_refs_predefined_env/code.cpp", "google", + "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.4.3: test command-line w/ relative path + Style10 = getStyle( + "{BasedOnStyle: file:f/format/predefined.clang-format, ColumnLimit: 25}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.4.4: test file w/ relative path + Style10 = getStyle("file", "/f/src_refs_predefined_rel/code.cpp", "google", + "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.5: file inheritance using a BasedOnStyle file using + // InheritParentConfig, applying child styles on top + ASSERT_TRUE(FS.addFile("/f/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: Microsoft\nTabWidth: 100"))); + ASSERT_TRUE( + FS.addFile("/f/format/inherits_parent.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: InheritParentConfig\nIndentWidth: 10"))); + ASSERT_TRUE(FS.addFile("/f/src_refs_inherits_parent_env/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/" + "inherits_parent.clang-format\nColumnLimit: 25"))); + ASSERT_TRUE(FS.addFile( + "/f/src_refs_inherits_parent_rel/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:../format/inherits_parent.clang-format\nColumnLimit: 25"))); + MergedStyle = getMicrosoftStyle(FormatStyle::LK_Cpp); + MergedStyle.TabWidth = 100; + MergedStyle.IndentWidth = 10; + MergedStyle.ColumnLimit = 25; + + // Test 10.5.1: test command-line w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle( + "{BasedOnStyle: file:$(CLANG_FORMAT_DIR)/inherits_parent.clang-format, " + "ColumnLimit: 25}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.5.2: test file w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle("file", "/f/src_refs_inherits_parent_env/code.cpp", + "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.5.3: test command-line w/ relative path + Style10 = + getStyle("{BasedOnStyle: file:f/format/inherits_parent.clang-format, " + "ColumnLimit: 25}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.5.4: test file w/ relative path + Style10 = getStyle("file", "/f/src_refs_inherits_parent_rel/code.cpp", + "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.6: file inheritance using a BasedOnStyle file that uses + // file:$(CLANG_FORMAT_DIR) inheritance, applying child styles on top + ASSERT_TRUE(FS.addFile( + "/f/format/inherits_explicit_env.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/predefined.clang-format\nTabWidth: 100"))); + ASSERT_TRUE( + FS.addFile("/f/src_refs_inherits_explicit_env_env/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/" + "inherits_explicit_env.clang-format\nColumnLimit: 25"))); + ASSERT_TRUE( + FS.addFile("/f/src_refs_inherits_explicit_env_rel/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:../format/" + "inherits_explicit_env.clang-format\nColumnLimit: 25"))); + MergedStyle = getMicrosoftStyle(FormatStyle::LK_Cpp); + MergedStyle.TabWidth = 100; + MergedStyle.IndentWidth = 10; + MergedStyle.ColumnLimit = 25; + + // Test 10.6.1: test command-line w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle("{BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/" + "inherits_explicit_env.clang-format, ColumnLimit: 25}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.6.2: test file w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle("file", "/f/src_refs_inherits_explicit_env_env/code.cpp", + "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.6.3: test command-line w/ relative path + Style10 = getStyle( + "{BasedOnStyle: file:f/format/inherits_explicit_env.clang-format, " + "ColumnLimit: 25}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.6.4: test file w/ relative path + Style10 = getStyle("file", "/f/src_refs_inherits_explicit_env_rel/code.cpp", + "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.7: file inheritance using a BasedOnStyle file that uses + // file:<relative-path> inheritance, applying child styles on top + ASSERT_TRUE(FS.addFile( + "/f/format/inherits_explicit_rel.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: file:./predefined.clang-format\nTabWidth: 100"))); + ASSERT_TRUE( + FS.addFile("/f/src_refs_inherits_explicit_rel_env/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/" + "inherits_explicit_rel.clang-format\nColumnLimit: 25"))); + ASSERT_TRUE( + FS.addFile("/f/src_refs_inherits_explicit_rel_rel/.clang-format", 0, + llvm::MemoryBuffer::getMemBuffer( + "BasedOnStyle: " + "file:../format/" + "inherits_explicit_rel.clang-format\nColumnLimit: 25"))); + MergedStyle = getMicrosoftStyle(FormatStyle::LK_Cpp); + MergedStyle.TabWidth = 100; + MergedStyle.IndentWidth = 10; + MergedStyle.ColumnLimit = 25; + + // Test 10.7.1: test command-line w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle("{BasedOnStyle: " + "file:$(CLANG_FORMAT_DIR)/" + "inherits_explicit_rel.clang-format, ColumnLimit: 25}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.7.2: test file w/ $(CLANG_FORMAT_DIR) + Style10 = getStyle("file", "/f/src_refs_inherits_explicit_rel_env/code.cpp", + "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.7.3: test command-line w/ relative path + Style10 = getStyle( + "{BasedOnStyle: file:f/format/inherits_explicit_rel.clang-format, " + "ColumnLimit: 25}", + "/f/src/code.cpp", "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + // Test 10.7.4: test file w/ relative path + Style10 = getStyle("file", "/f/src_refs_inherits_explicit_rel_rel/code.cpp", + "google", "", &FS); + ASSERT_TRUE(static_cast<bool>(Style10)); + ASSERT_EQ(*Style10, MergedStyle); + + UnsetEnv("CLANG_FORMAT_DIR"); } TEST(ConfigParseTest, GetStyleOfSpecificFile) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits