llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-format Author: Ryan Saunders (jediry) <details> <summary>Changes</summary> This change adds support for clang-format's BasedOnStyle to reference an explicit, arbitrary file, using syntax like: ```BasedOnStyle: file:../../format/my-team.clang-format``` or ```BasedOnStyle: file:$(CLANG_FORMAT_DIR)/my-team.clang-format``` In the first form, the string after ```file:``` is treated as a path relative to the current .clang-format file. In the second, when the path starts with the string ```$(CLANG_FORMAT_DIR)```, the value of the CLANG_FORMAT_DIR environment variable (which must be defined and must identify a readable directory) is substituted. For security reasons (and because it's likely not that useful anyway), absolute paths are not supported. The .clang-format file referenced in this way may itself use ```BasedOnStyle: file:``` to include another file, or ```BasedOnStyle: InheritParentConfig```, in which case the parent config file is searched upward, starting from the included file. --- Full diff: https://github.com/llvm/llvm-project/pull/110634.diff 2 Files Affected: - (modified) clang/include/clang/Format/Format.h (+18-4) - (modified) clang/lib/Format/Format.cpp (+156-51) ``````````diff 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 " + `````````` </details> https://github.com/llvm/llvm-project/pull/110634 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
