[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 1 06:07:46 PDT 2024
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
More information about the cfe-commits
mailing list