[clang] Support BasedOnStyle referencing an arbitrary file (PR #110634)
Ryan Saunders via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 8 14:45:49 PDT 2024
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 <ryansaun at 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 <ryansaun at 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) {
More information about the cfe-commits
mailing list