Skip to content

Conversation

@jediry
Copy link

@jediry jediry commented Sep 4, 2024

At present, the BasedOnStyle directive can reference a predefined style name, or InheritFromParent, instructing clang-format to search upward in the directory hierarchy for a .clang-format file. This works fine when variations in codebase formatting align with the directory hierarchy, but becomes problematic when it is desired to share formatting across portions of a repository with no common parent directory. For example, consider the case of a large, multi-team repository containing many projects as sub-directories of the repository root, where each of the various engineering teams owns multiple of these projects, and wants a common coding style across all of its owned projects.

In this PR, I'm extending BasedOnStyle to allow referencing an arbitrary file. While this could be an absolute path, that's not typically useful across machines. In typical usage, this will be a relative path (e.g., BasedOnStyle: format/team1.clang-format). For resolving relative paths, I've mimicked the "include path" model of C/C++ (and, in fact, I'm able to leverage SourceMgr's "include paths" mechanism to implement this). The list of style search paths is specified on the command-line via one or more --style-search-path <path> parameters.

The interesting stuff is in Format.cpp...most of the rest of this change is plumbing to get the StyleSearchPaths from the command-line to the call to getStyle().

Unfinished aspects of this change:

  • No unit tests. I am happy to write some, but I'm unsure how to do this in a harmonious way, seeing as my change inherently relies on external files on the filesystem. Most of the unit tests I've seen in Clang rely on in-code strings.
  • --style-search-path . does not seem to work for specifying the current directory as a search path. Evidently the SourceMgr include paths mechanism does not handle this natively. Can someone point me to the proper "Clang" way to resolve paths like . or .. into paths that SourceMgr can handle?
… relative to directories specified via --style-search-path Improve command-line help Clean up formatting Fix unit tests Fix other clang tools that leverage format::getStyle() Fix up clangd
@github-actions
Copy link

github-actions bot commented Sep 4, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2024

@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-format
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-llvm-support

Author: Ryan Saunders (jediry)

Changes

At present, the BasedOnStyle directive can reference a predefined style name, or InheritFromParent, instructing clang-format to search upward in the directory hierarchy for a .clang-format file. This works fine when variations in codebase formatting align with the directory hierarchy, but becomes problematic when it is desired to share formatting across portions of a repository with no common parent directory. For example, consider the case of a large, multi-team repository containing many projects as sub-directories of the repository root, where each of the various engineering teams owns multiple of these projects, and wants a common coding style across all of its owned projects.

In this PR, I'm extending BasedOnStyle to allow referencing an arbitrary file. While this could be an absolute path, that's not typically useful across machines. In typical usage, this will be a relative path (e.g., BasedOnStyle: format/team1.clang-format). For resolving relative paths, I've mimicked the "include path" model of C/C++ (and, in fact, I'm able to leverage SourceMgr's "include paths" mechanism to implement this). The list of style search paths is specified on the command-line via one or more --style-search-path &lt;path&gt; parameters.

The interesting stuff is in Format.cpp...most of the rest of this change is plumbing to get the StyleSearchPaths from the command-line to the call to getStyle().

Unfinished aspects of this change:

  • No unit tests. I am happy to write some, but I'm unsure how to do this in a harmonious way, seeing as my change inherently relies on external files on the filesystem. Most of the unit tests I've seen in Clang rely on in-code strings.
  • --style-search-path . does not seem to work for specifying the current directory as a search path. Evidently the SourceMgr include paths mechanism does not handle this natively. Can someone point me to the proper "Clang" way to resolve paths like . or .. into paths that SourceMgr can handle?

Patch is 80.87 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/107312.diff

39 Files Affected:

  • (modified) clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp (+19)
  • (modified) clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp (+4-3)
  • (modified) clang-tools-extra/clang-change-namespace/ChangeNamespace.h (+5-1)
  • (modified) clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp (+17-1)
  • (modified) clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp (+20-2)
  • (modified) clang-tools-extra/clang-move/Move.cpp (+1)
  • (modified) clang-tools-extra/clang-move/Move.h (+3)
  • (modified) clang-tools-extra/clang-move/tool/ClangMove.cpp (+19-3)
  • (modified) clang-tools-extra/clang-tidy/ClangTidy.cpp (+2-1)
  • (modified) clang-tools-extra/clang-tidy/ClangTidyOptions.h (+4)
  • (modified) clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp (+3-2)
  • (modified) clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h (+1)
  • (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+15)
  • (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/ClangdServer.cpp (+9-4)
  • (modified) clang-tools-extra/clangd/ClangdServer.h (+10-1)
  • (modified) clang-tools-extra/clangd/Config.h (+4)
  • (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+1)
  • (modified) clang-tools-extra/clangd/SourceCode.cpp (+3)
  • (modified) clang-tools-extra/clangd/tool/Check.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/tool/ClangdMain.cpp (+23)
  • (modified) clang-tools-extra/clangd/unittests/ClangdTests.cpp (+1-1)
  • (modified) clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp (+14-1)
  • (modified) clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp (+1-1)
  • (modified) clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp (+2-2)
  • (modified) clang/include/clang/Format/Format.h (+27-2)
  • (modified) clang/include/clang/Tooling/Refactoring.h (+3-1)
  • (modified) clang/lib/Format/Format.cpp (+41-10)
  • (modified) clang/lib/Tooling/Refactoring.cpp (+2-2)
  • (modified) clang/tools/clang-format/ClangFormat.cpp (+20-1)
  • (modified) clang/unittests/Format/ConfigParseTest.cpp (+57-49)
  • (modified) clang/unittests/Format/FormatTestObjC.cpp (+28-28)
  • (modified) clang/unittests/Format/FormatTestRawStrings.cpp (+1-1)
  • (modified) clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp (+4-2)
  • (modified) clang/unittests/Format/QualifierFixerTest.cpp (+4-2)
  • (modified) clang/unittests/Rename/ClangRenameTest.h (+1-1)
  • (modified) clang/unittests/Tooling/RefactoringTest.cpp (+2-1)
  • (modified) llvm/include/llvm/Support/YAMLTraits.h (+6)
  • (modified) llvm/lib/Support/YAMLTraits.cpp (+12)
diff --git a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp index 68b5743c6540f8..52e1251cb1482a 100644 --- a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp +++ b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp @@ -69,6 +69,24 @@ static cl::opt<std::string> FormatStyleOpt("style", cl::desc(format::StyleOptionHelpDescription), cl::init("LLVM"), cl::cat(FormattingCategory)); +static cl::list<std::string> +StyleSearchPaths( + "style-search-path", + cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), + cl::cat(FormattingCategory)); + +static cl::alias +StyleSearchPathShort( + "S", + cl::desc("Alias for --style-search-path"), + cl::cat(FormattingCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); + namespace { // Helper object to remove the TUReplacement and TUDiagnostic (triggered by // "remove-change-desc-files" command line option) when exiting current scope. @@ -102,6 +120,7 @@ int main(int argc, char **argv) { // Determine a formatting style from options. auto FormatStyleOrError = format::getStyle(FormatStyleOpt, FormatStyleConfig, + StyleSearchPaths, format::DefaultFallbackStyle); if (!FormatStyleOrError) { llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n"; diff --git a/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp b/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp index 879c0d26d472a8..f1a4716e2c45ea 100644 --- a/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp +++ b/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp @@ -339,8 +339,9 @@ ChangeNamespaceTool::ChangeNamespaceTool( llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern, llvm::ArrayRef<std::string> AllowedSymbolPatterns, std::map<std::string, tooling::Replacements> *FileToReplacements, - llvm::StringRef FallbackStyle) - : FallbackStyle(FallbackStyle), FileToReplacements(*FileToReplacements), + llvm::StringRef FallbackStyle, const std::vector<std::string>& StyleSearchPaths) + : FallbackStyle(FallbackStyle), StyleSearchPaths(StyleSearchPaths), + FileToReplacements(*FileToReplacements), OldNamespace(OldNs.ltrim(':')), NewNamespace(NewNs.ltrim(':')), FilePattern(FilePattern), FilePatternRE(FilePattern) { FileToReplacements->clear(); @@ -1004,7 +1005,7 @@ void ChangeNamespaceTool::onEndOfTranslationUnit() { // which refers to the original code. Replaces = Replaces.merge(NewReplacements); auto Style = - format::getStyle(format::DefaultFormatStyle, FilePath, FallbackStyle); + format::getStyle(format::DefaultFormatStyle, FilePath, StyleSearchPaths, FallbackStyle); if (!Style) { llvm::errs() << llvm::toString(Style.takeError()) << "\n"; continue; diff --git a/clang-tools-extra/clang-change-namespace/ChangeNamespace.h b/clang-tools-extra/clang-change-namespace/ChangeNamespace.h index d35119b70a69c4..969d983681ebd5 100644 --- a/clang-tools-extra/clang-change-namespace/ChangeNamespace.h +++ b/clang-tools-extra/clang-change-namespace/ChangeNamespace.h @@ -51,7 +51,8 @@ class ChangeNamespaceTool : public ast_matchers::MatchFinder::MatchCallback { llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern, llvm::ArrayRef<std::string> AllowedSymbolPatterns, std::map<std::string, tooling::Replacements> *FileToReplacements, - llvm::StringRef FallbackStyle = "LLVM"); + llvm::StringRef FallbackStyle = "LLVM", + const std::vector<std::string>& StyleSearchPaths = {}); void registerMatchers(ast_matchers::MatchFinder *Finder); @@ -109,6 +110,9 @@ class ChangeNamespaceTool : public ast_matchers::MatchFinder::MatchCallback { }; std::string FallbackStyle; + // Specifies the list of paths to be searched when FormatStyle or a BasedOnStyle + // in a .clang-format file specifies an arbitrary file to include + std::vector<std::string> StyleSearchPaths; // In match callbacks, this contains replacements for replacing `typeLoc`s in // and deleting forward declarations in the moved namespace blocks. // In `onEndOfTranslationUnit` callback, the previous added replacements are diff --git a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp index 22d26db0c11bcf..b78dcfc713a037 100644 --- a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp +++ b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp @@ -72,6 +72,22 @@ cl::opt<std::string> Style("style", cl::desc("The style name used for reformatting."), cl::init("LLVM"), cl::cat(ChangeNamespaceCategory)); +cl::list<std::string> StyleSearchPaths( + "style-search-path", + cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), + cl::cat(ChangeNamespaceCategory)); + +cl::alias StyleSearchPathShort( + "S", + cl::desc("Alias for --style-search-path"), + cl::cat(ChangeNamespaceCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); + cl::opt<std::string> AllowedFile( "allowed_file", cl::desc("A file containing regexes of symbol names that are not expected " @@ -135,7 +151,7 @@ int main(int argc, const char **argv) { SourceManager Sources(Diagnostics, FileMgr); Rewriter Rewrite(Sources, DefaultLangOptions); - if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style)) { + if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style, StyleSearchPaths)) { llvm::errs() << "Failed applying all replacements.\n"; return 1; } diff --git a/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp index 3a11a22def1946..febbd4021ac4b7 100644 --- a/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp +++ b/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp @@ -157,6 +157,24 @@ cl::opt<std::string> "headers if there is no clang-format config file found."), cl::init("llvm"), cl::cat(IncludeFixerCategory)); +static cl::list<std::string> +StyleSearchPaths( + "style-search-path", + cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), + cl::cat(IncludeFixerCategory)); + +static cl::alias +StyleSearchPathShort( + "S", + cl::desc("Alias for --style-search-path"), + cl::cat(IncludeFixerCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden ); + std::unique_ptr<include_fixer::SymbolIndexManager> createSymbolIndexManager(StringRef FilePath) { using find_all_symbols::SymbolInfo; @@ -330,7 +348,7 @@ int includeFixerMain(int argc, const char **argv) { return LHS.QualifiedName == RHS.QualifiedName; }); auto InsertStyle = format::getStyle(format::DefaultFormatStyle, - Context.getFilePath(), Style); + Context.getFilePath(), StyleSearchPaths, Style); if (!InsertStyle) { llvm::errs() << llvm::toString(InsertStyle.takeError()) << "\n"; return 1; @@ -410,7 +428,7 @@ int includeFixerMain(int argc, const char **argv) { for (const auto &Context : Contexts) { StringRef FilePath = Context.getFilePath(); auto InsertStyle = - format::getStyle(format::DefaultFormatStyle, FilePath, Style); + format::getStyle(format::DefaultFormatStyle, FilePath, StyleSearchPaths, Style); if (!InsertStyle) { llvm::errs() << llvm::toString(InsertStyle.takeError()) << "\n"; return 1; diff --git a/clang-tools-extra/clang-move/Move.cpp b/clang-tools-extra/clang-move/Move.cpp index ac16803b46783e..d5068d3087b44f 100644 --- a/clang-tools-extra/clang-move/Move.cpp +++ b/clang-tools-extra/clang-move/Move.cpp @@ -781,6 +781,7 @@ void ClangMoveTool::removeDeclsInOldFiles() { if (SI == FilePathToFileID.end()) continue; llvm::StringRef Code = SM.getBufferData(SI->second); auto Style = format::getStyle(format::DefaultFormatStyle, FilePath, + Context->StyleSearchPaths, Context->FallbackStyle); if (!Style) { llvm::errs() << llvm::toString(Style.takeError()) << "\n"; diff --git a/clang-tools-extra/clang-move/Move.h b/clang-tools-extra/clang-move/Move.h index ea241bbbc4f8a0..a368fac3645dbf 100644 --- a/clang-tools-extra/clang-move/Move.h +++ b/clang-tools-extra/clang-move/Move.h @@ -89,6 +89,9 @@ struct ClangMoveContext { // directory when analyzing the source file. We save the original working // directory in order to get the absolute file path for the fields in Spec. std::string OriginalRunningDirectory; + // Specifies the list of paths to be searched when BasedOnStyle + // in a .clang-format file specifies an arbitrary file to include + std::vector<std::string> StyleSearchPaths; // The name of a predefined code style. std::string FallbackStyle; // Whether dump all declarations in old header. diff --git a/clang-tools-extra/clang-move/tool/ClangMove.cpp b/clang-tools-extra/clang-move/tool/ClangMove.cpp index 1560dcaad67793..5ea3ef218ed0f3 100644 --- a/clang-tools-extra/clang-move/tool/ClangMove.cpp +++ b/clang-tools-extra/clang-move/tool/ClangMove.cpp @@ -76,6 +76,22 @@ cl::opt<bool> "add #include of old header to new header."), cl::init(false), cl::cat(ClangMoveCategory)); +cl::list<std::string> + StyleSearchPaths("style-search-path", + cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), + cl::cat(ClangMoveCategory)); + +cl::alias + StyleSearchPathShort("S", + cl::desc("Alias for --style-search-path"), + cl::cat(ClangMoveCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); + cl::opt<std::string> Style("style", cl::desc("The style name used for reformatting. Default is \"llvm\""), @@ -131,8 +147,8 @@ int main(int argc, const char **argv) { Twine(EC.message())); move::ClangMoveContext Context{Spec, Tool.getReplacements(), - std::string(InitialDirectory), Style, - DumpDecls}; + std::string(InitialDirectory), StyleSearchPaths, + Style, DumpDecls}; move::DeclarationReporter Reporter; move::ClangMoveActionFactory Factory(&Context, &Reporter); @@ -185,7 +201,7 @@ int main(int argc, const char **argv) { SourceManager SM(Diagnostics, FileMgr); Rewriter Rewrite(SM, LangOptions()); - if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style)) { + if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style, StyleSearchPaths)) { llvm::errs() << "Failed applying all replacements.\n"; return 1; } diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 1cd7cdd10bc25f..32b0de29291ce4 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -209,7 +209,8 @@ class ErrorReporter { } StringRef Code = Buffer.get()->getBuffer(); auto Style = format::getStyle( - *Context.getOptionsForFile(File).FormatStyle, File, "none"); + *Context.getOptionsForFile(File).FormatStyle, File, + Context.getGlobalOptions().StyleSearchPaths, "none"); if (!Style) { llvm::errs() << llvm::toString(Style.takeError()) << "\n"; continue; diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h index 85d5a02ebbc1bc..e71299c3278361 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h @@ -42,6 +42,10 @@ struct ClangTidyGlobalOptions { /// Output warnings from certain line ranges of certain files only. /// If empty, no warnings will be filtered. std::vector<FileFilter> LineFilter; + + /// Specifies the list of paths to be searched when BasedOnStyle + /// in a .clang-format file specifies an arbitrary file to include + std::vector<std::string> StyleSearchPaths; }; /// Contains options for clang-tidy. These options may be read from diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp index 5e7a0e65690b7a..7d76b13f0735f4 100644 --- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp @@ -60,7 +60,8 @@ IncludeCleanerCheck::IncludeCleanerCheck(StringRef Name, IgnoreHeaders(utils::options::parseStringList( Options.getLocalOrGlobal("IgnoreHeaders", ""))), DeduplicateFindings( - Options.getLocalOrGlobal("DeduplicateFindings", true)) { + Options.getLocalOrGlobal("DeduplicateFindings", true)), + StyleSearchPaths(Context->getGlobalOptions().StyleSearchPaths) { for (const auto &Header : IgnoreHeaders) { if (!llvm::Regex{Header}.isValid()) configurationDiag("Invalid ignore headers regex '%0'") << Header; @@ -196,7 +197,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) { llvm::StringRef Code = SM->getBufferData(SM->getMainFileID()); auto FileStyle = format::getStyle(format::DefaultFormatStyle, getCurrentMainFile(), - format::DefaultFallbackStyle, Code, + StyleSearchPaths, format::DefaultFallbackStyle, Code, &SM->getFileManager().getVirtualFileSystem()); if (!FileStyle) FileStyle = format::getLLVMStyle(); diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h index b46e409bd6f6a0..229c8b7127cbc9 100644 --- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h +++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h @@ -47,6 +47,7 @@ class IncludeCleanerCheck : public ClangTidyCheck { std::vector<StringRef> IgnoreHeaders; // Whether emit only one finding per usage of a symbol. const bool DeduplicateFindings; + std::vector<std::string> StyleSearchPaths; llvm::SmallVector<llvm::Regex> IgnoreHeadersRegex; bool shouldIgnore(const include_cleaner::Header &H); }; diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index d42dafa8ffc362..4148c3a8d9d514 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -208,6 +208,20 @@ This option overrides the 'FormatStyle` option in cl::init("none"), cl::cat(ClangTidyCategory)); +static cl::list<std::string> StyleSearchPaths("style-search-path", desc(R"( +Directory to search for BasedOnStyle files, when the value of the +BasedOnStyle directive is not one of the predefined styles, nor +InheritFromParent. Multiple style search paths may be specified, +and will be searched in order, stopping at the first file found. +)"), + cl::value_desc("directory"), + cl::cat(ClangTidyCategory)); + +static cl::alias StyleSearchPathShort("S", cl::desc("Alias for --style-search-path"), + cl::cat(ClangTidyCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); + static cl::opt<bool> ListChecks("list-checks", desc(R"( List all enabled checks and exit. Use with -checks=* to list all available checks. @@ -366,6 +380,7 @@ static void printStats(const ClangTidyStats &Stats) { static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider( llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) { ClangTidyGlobalOptions GlobalOptions; + GlobalOptions.StyleSearchPaths = StyleSearchPaths; if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) { llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n"; llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true); diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 06573a57554245..a169a7885c0679 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1650,7 +1650,7 @@ ClangdLSPServer::ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS, assert(!Opts.ContextProvider && "Only one of ConfigProvider and ContextProvider allowed!"); this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider( - Opts.ConfigProvider, this); + Opts.ConfigProvider, this, Opts.StyleSearchPaths); } LSPBinder Bind(this->Handlers, *this); Bind.method("initialize", this, &ClangdLSPServer::onInitialize); diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index e910a80ba0bae9..cfd60bb8279077 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -223,6 +223,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions), ImportInsertions(Opts.ImportInsertions), PublishInactiveRegions(Opts.PublishInactiveRegions), + StyleSearchPaths(Opts.StyleSearchPaths), WorkspaceRoot(Opts.WorkspaceRoot), Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate : TUScheduler::NoInvalidation), @@ -335,17 +336,20 @@ std::shared_ptr<const std::string> ClangdServer::getDraft(PathRef File) const { std::function<Context(PathRef)> ClangdServer::createConfiguredContextProvider(const config::Provider *Provider, - Callbacks *Publish) { + Callbacks *Publish, + const std::vector<std::string> &StyleSearchPaths) { if (!Provider) return [](llvm::StringRef) { return Context::current().clone(); }; struct Impl { const config::Provider *Provider; ClangdServer::Callbacks *Publish; + std::vector<std::string> StyleSearchPaths; std::mutex PublishMu; - Impl(const config::Provider *Provider, ClangdServer::Callbacks *Publish) - : Provider(Provider), Publish(Publish) {} + Impl(const config::Provider *Provide... [truncated] 
@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2024

@llvm/pr-subscribers-clang-tidy

Author: Ryan Saunders (jediry)

Changes

At present, the BasedOnStyle directive can reference a predefined style name, or InheritFromParent, instructing clang-format to search upward in the directory hierarchy for a .clang-format file. This works fine when variations in codebase formatting align with the directory hierarchy, but becomes problematic when it is desired to share formatting across portions of a repository with no common parent directory. For example, consider the case of a large, multi-team repository containing many projects as sub-directories of the repository root, where each of the various engineering teams owns multiple of these projects, and wants a common coding style across all of its owned projects.

In this PR, I'm extending BasedOnStyle to allow referencing an arbitrary file. While this could be an absolute path, that's not typically useful across machines. In typical usage, this will be a relative path (e.g., BasedOnStyle: format/team1.clang-format). For resolving relative paths, I've mimicked the "include path" model of C/C++ (and, in fact, I'm able to leverage SourceMgr's "include paths" mechanism to implement this). The list of style search paths is specified on the command-line via one or more --style-search-path &lt;path&gt; parameters.

The interesting stuff is in Format.cpp...most of the rest of this change is plumbing to get the StyleSearchPaths from the command-line to the call to getStyle().

Unfinished aspects of this change:

  • No unit tests. I am happy to write some, but I'm unsure how to do this in a harmonious way, seeing as my change inherently relies on external files on the filesystem. Most of the unit tests I've seen in Clang rely on in-code strings.
  • --style-search-path . does not seem to work for specifying the current directory as a search path. Evidently the SourceMgr include paths mechanism does not handle this natively. Can someone point me to the proper "Clang" way to resolve paths like . or .. into paths that SourceMgr can handle?

Patch is 80.87 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/107312.diff

39 Files Affected:

  • (modified) clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp (+19)
  • (modified) clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp (+4-3)
  • (modified) clang-tools-extra/clang-change-namespace/ChangeNamespace.h (+5-1)
  • (modified) clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp (+17-1)
  • (modified) clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp (+20-2)
  • (modified) clang-tools-extra/clang-move/Move.cpp (+1)
  • (modified) clang-tools-extra/clang-move/Move.h (+3)
  • (modified) clang-tools-extra/clang-move/tool/ClangMove.cpp (+19-3)
  • (modified) clang-tools-extra/clang-tidy/ClangTidy.cpp (+2-1)
  • (modified) clang-tools-extra/clang-tidy/ClangTidyOptions.h (+4)
  • (modified) clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp (+3-2)
  • (modified) clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h (+1)
  • (modified) clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp (+15)
  • (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/ClangdServer.cpp (+9-4)
  • (modified) clang-tools-extra/clangd/ClangdServer.h (+10-1)
  • (modified) clang-tools-extra/clangd/Config.h (+4)
  • (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+1)
  • (modified) clang-tools-extra/clangd/SourceCode.cpp (+3)
  • (modified) clang-tools-extra/clangd/tool/Check.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/tool/ClangdMain.cpp (+23)
  • (modified) clang-tools-extra/clangd/unittests/ClangdTests.cpp (+1-1)
  • (modified) clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp (+14-1)
  • (modified) clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp (+1-1)
  • (modified) clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp (+2-2)
  • (modified) clang/include/clang/Format/Format.h (+27-2)
  • (modified) clang/include/clang/Tooling/Refactoring.h (+3-1)
  • (modified) clang/lib/Format/Format.cpp (+41-10)
  • (modified) clang/lib/Tooling/Refactoring.cpp (+2-2)
  • (modified) clang/tools/clang-format/ClangFormat.cpp (+20-1)
  • (modified) clang/unittests/Format/ConfigParseTest.cpp (+57-49)
  • (modified) clang/unittests/Format/FormatTestObjC.cpp (+28-28)
  • (modified) clang/unittests/Format/FormatTestRawStrings.cpp (+1-1)
  • (modified) clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp (+4-2)
  • (modified) clang/unittests/Format/QualifierFixerTest.cpp (+4-2)
  • (modified) clang/unittests/Rename/ClangRenameTest.h (+1-1)
  • (modified) clang/unittests/Tooling/RefactoringTest.cpp (+2-1)
  • (modified) llvm/include/llvm/Support/YAMLTraits.h (+6)
  • (modified) llvm/lib/Support/YAMLTraits.cpp (+12)
diff --git a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp index 68b5743c6540f8..52e1251cb1482a 100644 --- a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp +++ b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp @@ -69,6 +69,24 @@ static cl::opt<std::string> FormatStyleOpt("style", cl::desc(format::StyleOptionHelpDescription), cl::init("LLVM"), cl::cat(FormattingCategory)); +static cl::list<std::string> +StyleSearchPaths( + "style-search-path", + cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), + cl::cat(FormattingCategory)); + +static cl::alias +StyleSearchPathShort( + "S", + cl::desc("Alias for --style-search-path"), + cl::cat(FormattingCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); + namespace { // Helper object to remove the TUReplacement and TUDiagnostic (triggered by // "remove-change-desc-files" command line option) when exiting current scope. @@ -102,6 +120,7 @@ int main(int argc, char **argv) { // Determine a formatting style from options. auto FormatStyleOrError = format::getStyle(FormatStyleOpt, FormatStyleConfig, + StyleSearchPaths, format::DefaultFallbackStyle); if (!FormatStyleOrError) { llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n"; diff --git a/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp b/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp index 879c0d26d472a8..f1a4716e2c45ea 100644 --- a/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp +++ b/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp @@ -339,8 +339,9 @@ ChangeNamespaceTool::ChangeNamespaceTool( llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern, llvm::ArrayRef<std::string> AllowedSymbolPatterns, std::map<std::string, tooling::Replacements> *FileToReplacements, - llvm::StringRef FallbackStyle) - : FallbackStyle(FallbackStyle), FileToReplacements(*FileToReplacements), + llvm::StringRef FallbackStyle, const std::vector<std::string>& StyleSearchPaths) + : FallbackStyle(FallbackStyle), StyleSearchPaths(StyleSearchPaths), + FileToReplacements(*FileToReplacements), OldNamespace(OldNs.ltrim(':')), NewNamespace(NewNs.ltrim(':')), FilePattern(FilePattern), FilePatternRE(FilePattern) { FileToReplacements->clear(); @@ -1004,7 +1005,7 @@ void ChangeNamespaceTool::onEndOfTranslationUnit() { // which refers to the original code. Replaces = Replaces.merge(NewReplacements); auto Style = - format::getStyle(format::DefaultFormatStyle, FilePath, FallbackStyle); + format::getStyle(format::DefaultFormatStyle, FilePath, StyleSearchPaths, FallbackStyle); if (!Style) { llvm::errs() << llvm::toString(Style.takeError()) << "\n"; continue; diff --git a/clang-tools-extra/clang-change-namespace/ChangeNamespace.h b/clang-tools-extra/clang-change-namespace/ChangeNamespace.h index d35119b70a69c4..969d983681ebd5 100644 --- a/clang-tools-extra/clang-change-namespace/ChangeNamespace.h +++ b/clang-tools-extra/clang-change-namespace/ChangeNamespace.h @@ -51,7 +51,8 @@ class ChangeNamespaceTool : public ast_matchers::MatchFinder::MatchCallback { llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern, llvm::ArrayRef<std::string> AllowedSymbolPatterns, std::map<std::string, tooling::Replacements> *FileToReplacements, - llvm::StringRef FallbackStyle = "LLVM"); + llvm::StringRef FallbackStyle = "LLVM", + const std::vector<std::string>& StyleSearchPaths = {}); void registerMatchers(ast_matchers::MatchFinder *Finder); @@ -109,6 +110,9 @@ class ChangeNamespaceTool : public ast_matchers::MatchFinder::MatchCallback { }; std::string FallbackStyle; + // Specifies the list of paths to be searched when FormatStyle or a BasedOnStyle + // in a .clang-format file specifies an arbitrary file to include + std::vector<std::string> StyleSearchPaths; // In match callbacks, this contains replacements for replacing `typeLoc`s in // and deleting forward declarations in the moved namespace blocks. // In `onEndOfTranslationUnit` callback, the previous added replacements are diff --git a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp index 22d26db0c11bcf..b78dcfc713a037 100644 --- a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp +++ b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp @@ -72,6 +72,22 @@ cl::opt<std::string> Style("style", cl::desc("The style name used for reformatting."), cl::init("LLVM"), cl::cat(ChangeNamespaceCategory)); +cl::list<std::string> StyleSearchPaths( + "style-search-path", + cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), + cl::cat(ChangeNamespaceCategory)); + +cl::alias StyleSearchPathShort( + "S", + cl::desc("Alias for --style-search-path"), + cl::cat(ChangeNamespaceCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); + cl::opt<std::string> AllowedFile( "allowed_file", cl::desc("A file containing regexes of symbol names that are not expected " @@ -135,7 +151,7 @@ int main(int argc, const char **argv) { SourceManager Sources(Diagnostics, FileMgr); Rewriter Rewrite(Sources, DefaultLangOptions); - if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style)) { + if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style, StyleSearchPaths)) { llvm::errs() << "Failed applying all replacements.\n"; return 1; } diff --git a/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp index 3a11a22def1946..febbd4021ac4b7 100644 --- a/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp +++ b/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp @@ -157,6 +157,24 @@ cl::opt<std::string> "headers if there is no clang-format config file found."), cl::init("llvm"), cl::cat(IncludeFixerCategory)); +static cl::list<std::string> +StyleSearchPaths( + "style-search-path", + cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), + cl::cat(IncludeFixerCategory)); + +static cl::alias +StyleSearchPathShort( + "S", + cl::desc("Alias for --style-search-path"), + cl::cat(IncludeFixerCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden ); + std::unique_ptr<include_fixer::SymbolIndexManager> createSymbolIndexManager(StringRef FilePath) { using find_all_symbols::SymbolInfo; @@ -330,7 +348,7 @@ int includeFixerMain(int argc, const char **argv) { return LHS.QualifiedName == RHS.QualifiedName; }); auto InsertStyle = format::getStyle(format::DefaultFormatStyle, - Context.getFilePath(), Style); + Context.getFilePath(), StyleSearchPaths, Style); if (!InsertStyle) { llvm::errs() << llvm::toString(InsertStyle.takeError()) << "\n"; return 1; @@ -410,7 +428,7 @@ int includeFixerMain(int argc, const char **argv) { for (const auto &Context : Contexts) { StringRef FilePath = Context.getFilePath(); auto InsertStyle = - format::getStyle(format::DefaultFormatStyle, FilePath, Style); + format::getStyle(format::DefaultFormatStyle, FilePath, StyleSearchPaths, Style); if (!InsertStyle) { llvm::errs() << llvm::toString(InsertStyle.takeError()) << "\n"; return 1; diff --git a/clang-tools-extra/clang-move/Move.cpp b/clang-tools-extra/clang-move/Move.cpp index ac16803b46783e..d5068d3087b44f 100644 --- a/clang-tools-extra/clang-move/Move.cpp +++ b/clang-tools-extra/clang-move/Move.cpp @@ -781,6 +781,7 @@ void ClangMoveTool::removeDeclsInOldFiles() { if (SI == FilePathToFileID.end()) continue; llvm::StringRef Code = SM.getBufferData(SI->second); auto Style = format::getStyle(format::DefaultFormatStyle, FilePath, + Context->StyleSearchPaths, Context->FallbackStyle); if (!Style) { llvm::errs() << llvm::toString(Style.takeError()) << "\n"; diff --git a/clang-tools-extra/clang-move/Move.h b/clang-tools-extra/clang-move/Move.h index ea241bbbc4f8a0..a368fac3645dbf 100644 --- a/clang-tools-extra/clang-move/Move.h +++ b/clang-tools-extra/clang-move/Move.h @@ -89,6 +89,9 @@ struct ClangMoveContext { // directory when analyzing the source file. We save the original working // directory in order to get the absolute file path for the fields in Spec. std::string OriginalRunningDirectory; + // Specifies the list of paths to be searched when BasedOnStyle + // in a .clang-format file specifies an arbitrary file to include + std::vector<std::string> StyleSearchPaths; // The name of a predefined code style. std::string FallbackStyle; // Whether dump all declarations in old header. diff --git a/clang-tools-extra/clang-move/tool/ClangMove.cpp b/clang-tools-extra/clang-move/tool/ClangMove.cpp index 1560dcaad67793..5ea3ef218ed0f3 100644 --- a/clang-tools-extra/clang-move/tool/ClangMove.cpp +++ b/clang-tools-extra/clang-move/tool/ClangMove.cpp @@ -76,6 +76,22 @@ cl::opt<bool> "add #include of old header to new header."), cl::init(false), cl::cat(ClangMoveCategory)); +cl::list<std::string> + StyleSearchPaths("style-search-path", + cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), + cl::cat(ClangMoveCategory)); + +cl::alias + StyleSearchPathShort("S", + cl::desc("Alias for --style-search-path"), + cl::cat(ClangMoveCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); + cl::opt<std::string> Style("style", cl::desc("The style name used for reformatting. Default is \"llvm\""), @@ -131,8 +147,8 @@ int main(int argc, const char **argv) { Twine(EC.message())); move::ClangMoveContext Context{Spec, Tool.getReplacements(), - std::string(InitialDirectory), Style, - DumpDecls}; + std::string(InitialDirectory), StyleSearchPaths, + Style, DumpDecls}; move::DeclarationReporter Reporter; move::ClangMoveActionFactory Factory(&Context, &Reporter); @@ -185,7 +201,7 @@ int main(int argc, const char **argv) { SourceManager SM(Diagnostics, FileMgr); Rewriter Rewrite(SM, LangOptions()); - if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style)) { + if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style, StyleSearchPaths)) { llvm::errs() << "Failed applying all replacements.\n"; return 1; } diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 1cd7cdd10bc25f..32b0de29291ce4 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -209,7 +209,8 @@ class ErrorReporter { } StringRef Code = Buffer.get()->getBuffer(); auto Style = format::getStyle( - *Context.getOptionsForFile(File).FormatStyle, File, "none"); + *Context.getOptionsForFile(File).FormatStyle, File, + Context.getGlobalOptions().StyleSearchPaths, "none"); if (!Style) { llvm::errs() << llvm::toString(Style.takeError()) << "\n"; continue; diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h index 85d5a02ebbc1bc..e71299c3278361 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h @@ -42,6 +42,10 @@ struct ClangTidyGlobalOptions { /// Output warnings from certain line ranges of certain files only. /// If empty, no warnings will be filtered. std::vector<FileFilter> LineFilter; + + /// Specifies the list of paths to be searched when BasedOnStyle + /// in a .clang-format file specifies an arbitrary file to include + std::vector<std::string> StyleSearchPaths; }; /// Contains options for clang-tidy. These options may be read from diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp index 5e7a0e65690b7a..7d76b13f0735f4 100644 --- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp @@ -60,7 +60,8 @@ IncludeCleanerCheck::IncludeCleanerCheck(StringRef Name, IgnoreHeaders(utils::options::parseStringList( Options.getLocalOrGlobal("IgnoreHeaders", ""))), DeduplicateFindings( - Options.getLocalOrGlobal("DeduplicateFindings", true)) { + Options.getLocalOrGlobal("DeduplicateFindings", true)), + StyleSearchPaths(Context->getGlobalOptions().StyleSearchPaths) { for (const auto &Header : IgnoreHeaders) { if (!llvm::Regex{Header}.isValid()) configurationDiag("Invalid ignore headers regex '%0'") << Header; @@ -196,7 +197,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) { llvm::StringRef Code = SM->getBufferData(SM->getMainFileID()); auto FileStyle = format::getStyle(format::DefaultFormatStyle, getCurrentMainFile(), - format::DefaultFallbackStyle, Code, + StyleSearchPaths, format::DefaultFallbackStyle, Code, &SM->getFileManager().getVirtualFileSystem()); if (!FileStyle) FileStyle = format::getLLVMStyle(); diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h index b46e409bd6f6a0..229c8b7127cbc9 100644 --- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h +++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h @@ -47,6 +47,7 @@ class IncludeCleanerCheck : public ClangTidyCheck { std::vector<StringRef> IgnoreHeaders; // Whether emit only one finding per usage of a symbol. const bool DeduplicateFindings; + std::vector<std::string> StyleSearchPaths; llvm::SmallVector<llvm::Regex> IgnoreHeadersRegex; bool shouldIgnore(const include_cleaner::Header &H); }; diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index d42dafa8ffc362..4148c3a8d9d514 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -208,6 +208,20 @@ This option overrides the 'FormatStyle` option in cl::init("none"), cl::cat(ClangTidyCategory)); +static cl::list<std::string> StyleSearchPaths("style-search-path", desc(R"( +Directory to search for BasedOnStyle files, when the value of the +BasedOnStyle directive is not one of the predefined styles, nor +InheritFromParent. Multiple style search paths may be specified, +and will be searched in order, stopping at the first file found. +)"), + cl::value_desc("directory"), + cl::cat(ClangTidyCategory)); + +static cl::alias StyleSearchPathShort("S", cl::desc("Alias for --style-search-path"), + cl::cat(ClangTidyCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); + static cl::opt<bool> ListChecks("list-checks", desc(R"( List all enabled checks and exit. Use with -checks=* to list all available checks. @@ -366,6 +380,7 @@ static void printStats(const ClangTidyStats &Stats) { static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider( llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) { ClangTidyGlobalOptions GlobalOptions; + GlobalOptions.StyleSearchPaths = StyleSearchPaths; if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) { llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n"; llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true); diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 06573a57554245..a169a7885c0679 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1650,7 +1650,7 @@ ClangdLSPServer::ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS, assert(!Opts.ContextProvider && "Only one of ConfigProvider and ContextProvider allowed!"); this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider( - Opts.ConfigProvider, this); + Opts.ConfigProvider, this, Opts.StyleSearchPaths); } LSPBinder Bind(this->Handlers, *this); Bind.method("initialize", this, &ClangdLSPServer::onInitialize); diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index e910a80ba0bae9..cfd60bb8279077 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -223,6 +223,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions), ImportInsertions(Opts.ImportInsertions), PublishInactiveRegions(Opts.PublishInactiveRegions), + StyleSearchPaths(Opts.StyleSearchPaths), WorkspaceRoot(Opts.WorkspaceRoot), Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate : TUScheduler::NoInvalidation), @@ -335,17 +336,20 @@ std::shared_ptr<const std::string> ClangdServer::getDraft(PathRef File) const { std::function<Context(PathRef)> ClangdServer::createConfiguredContextProvider(const config::Provider *Provider, - Callbacks *Publish) { + Callbacks *Publish, + const std::vector<std::string> &StyleSearchPaths) { if (!Provider) return [](llvm::StringRef) { return Context::current().clone(); }; struct Impl { const config::Provider *Provider; ClangdServer::Callbacks *Publish; + std::vector<std::string> StyleSearchPaths; std::mutex PublishMu; - Impl(const config::Provider *Provider, ClangdServer::Callbacks *Publish) - : Provider(Provider), Publish(Publish) {} + Impl(const config::Provider *Provide... [truncated] 
Copy link
Contributor

@HazardyKnusperkeks HazardyKnusperkeks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are touching a lot of projects. I don't know how others feel about that, but my preference would be to tackle one project per PR.


std::string FallbackStyle;
// Specifies the list of paths to be searched when FormatStyle or a BasedOnStyle
// in a .clang-format file specifies an arbitrary file to include
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// in a .clang-format file specifies an arbitrary file to include
// in a .clang-format file specifies an arbitrary file to include.

Comments ending in full stop.

Copy link
Contributor

@mydeveloperday mydeveloperday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm with @HazardyKnusperkeks you are changing too much in one go, get this established in clang-format first and then look to rolling out to other tools that use lib/format

YAML changes should be separate and agreed with that code owner we don't own that


IO::~IO() = default;

SourceMgr& IO::getSourceMgr() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like an unrelated changes or change that could be done in isolation first

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can break this up.


friend std::error_code
parseConfiguration(llvm::MemoryBufferRef Config, FormatStyle *Style,
const std::vector<std::string> &StyleSearchPaths,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by putting your argument 3rd you made every instance of this function need to change..

you could have added it last and used a default argument, or made an overloaded function and a wrapper for the old one to call the new with the empty argument this would cut down the flux

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can certainly do that if that is preferred. And, if I'm to do this as multiple PRs as several have requested, then a default param (or an overload) is needed. FWIW, though, I did this intentionally, to flush out all callers of lib/format: once this feature is in and used by actual users, this needs to be pushed through all the tools that leverage lib/format, or else those tools will choke on peoples' valid .clang-format files.

@mydeveloperday
Copy link
Contributor

You are touching a lot of projects. I don't know how others feel about that, but my preference would be to tackle one project per PR.

+1

@kadircet
Copy link
Member

kadircet commented Sep 5, 2024

I am not sure if it's best to push a new command line flag to all other tools that use clang-format as a library.

Have you considered any other alternatives that can self-contain this in clang-format? e.g can we just treat paths as relative to current .clang-format file ?

@github-actions
Copy link

github-actions bot commented Sep 6, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 776495987272294de6aafbe73dab3e9ab445227a 163657095741b2292540fce7ff424cb4600391da --extensions cpp,h -- clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp clang-tools-extra/clang-change-namespace/ChangeNamespace.h clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp clang-tools-extra/clang-move/Move.cpp clang-tools-extra/clang-move/Move.h clang-tools-extra/clang-move/tool/ClangMove.cpp clang-tools-extra/clang-tidy/ClangTidy.cpp clang-tools-extra/clang-tidy/ClangTidyOptions.h clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/ClangdServer.h clang-tools-extra/clangd/Config.h clang-tools-extra/clangd/IncludeCleaner.cpp clang-tools-extra/clangd/SourceCode.cpp clang-tools-extra/clangd/tool/Check.cpp clang-tools-extra/clangd/tool/ClangdMain.cpp clang-tools-extra/clangd/unittests/ClangdTests.cpp clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp clang/include/clang/Format/Format.h clang/include/clang/Tooling/Refactoring.h clang/lib/Format/Format.cpp clang/lib/Tooling/Refactoring.cpp clang/tools/clang-format/ClangFormat.cpp clang/unittests/Format/ConfigParseTest.cpp clang/unittests/Format/FormatTestObjC.cpp clang/unittests/Format/FormatTestRawStrings.cpp clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp clang/unittests/Format/QualifierFixerTest.cpp clang/unittests/Rename/ClangRenameTest.h clang/unittests/Tooling/RefactoringTest.cpp llvm/include/llvm/Support/YAMLTraits.h llvm/lib/Support/YAMLTraits.cpp
View the diff from clang-format here.
diff --git a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp index 52e1251cb1..34b8b69992 100644 --- a/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp +++ b/clang-tools-extra/clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp @@ -69,23 +69,20 @@ static cl::opt<std::string> FormatStyleOpt("style", cl::desc(format::StyleOptionHelpDescription), cl::init("LLVM"), cl::cat(FormattingCategory)); -static cl::list<std::string> -StyleSearchPaths( +static cl::list<std::string> StyleSearchPaths( "style-search-path", - cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" - "BasedOnStyle directive is not one of the predefined styles, nor\n" - "InheritFromParent. Multiple style search paths may be specified,\n" - "and will be searched in order, stopping at the first file found."), - cl::value_desc("directory"), - cl::cat(FormattingCategory)); - -static cl::alias -StyleSearchPathShort( - "S", - cl::desc("Alias for --style-search-path"), - cl::cat(FormattingCategory), - cl::aliasopt(StyleSearchPaths), - cl::NotHidden); + cl::desc( + "Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), cl::cat(FormattingCategory)); + +static cl::alias StyleSearchPathShort("S", + cl::desc("Alias for --style-search-path"), + cl::cat(FormattingCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); namespace { // Helper object to remove the TUReplacement and TUDiagnostic (triggered by @@ -119,9 +116,9 @@ int main(int argc, char **argv) { IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs()), DiagOpts.get()); // Determine a formatting style from options. - auto FormatStyleOrError = format::getStyle(FormatStyleOpt, FormatStyleConfig, - StyleSearchPaths, - format::DefaultFallbackStyle); + auto FormatStyleOrError = + format::getStyle(FormatStyleOpt, FormatStyleConfig, StyleSearchPaths, + format::DefaultFallbackStyle); if (!FormatStyleOrError) { llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n"; return 1; diff --git a/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp b/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp index f1a4716e2c..743a77b404 100644 --- a/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp +++ b/clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp @@ -339,11 +339,12 @@ ChangeNamespaceTool::ChangeNamespaceTool( llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern, llvm::ArrayRef<std::string> AllowedSymbolPatterns, std::map<std::string, tooling::Replacements> *FileToReplacements, - llvm::StringRef FallbackStyle, const std::vector<std::string>& StyleSearchPaths) + llvm::StringRef FallbackStyle, + const std::vector<std::string> &StyleSearchPaths) : FallbackStyle(FallbackStyle), StyleSearchPaths(StyleSearchPaths), - FileToReplacements(*FileToReplacements), - OldNamespace(OldNs.ltrim(':')), NewNamespace(NewNs.ltrim(':')), - FilePattern(FilePattern), FilePatternRE(FilePattern) { + FileToReplacements(*FileToReplacements), OldNamespace(OldNs.ltrim(':')), + NewNamespace(NewNs.ltrim(':')), FilePattern(FilePattern), + FilePatternRE(FilePattern) { FileToReplacements->clear(); auto OldNsSplitted = splitSymbolName(OldNamespace); auto NewNsSplitted = splitSymbolName(NewNamespace); @@ -1004,8 +1005,8 @@ void ChangeNamespaceTool::onEndOfTranslationUnit() { // Add replacements referring to the changed code to existing replacements, // which refers to the original code. Replaces = Replaces.merge(NewReplacements); - auto Style = - format::getStyle(format::DefaultFormatStyle, FilePath, StyleSearchPaths, FallbackStyle); + auto Style = format::getStyle(format::DefaultFormatStyle, FilePath, + StyleSearchPaths, FallbackStyle); if (!Style) { llvm::errs() << llvm::toString(Style.takeError()) << "\n"; continue; diff --git a/clang-tools-extra/clang-change-namespace/ChangeNamespace.h b/clang-tools-extra/clang-change-namespace/ChangeNamespace.h index 969d983681..6d51df3211 100644 --- a/clang-tools-extra/clang-change-namespace/ChangeNamespace.h +++ b/clang-tools-extra/clang-change-namespace/ChangeNamespace.h @@ -52,7 +52,7 @@ public: llvm::ArrayRef<std::string> AllowedSymbolPatterns, std::map<std::string, tooling::Replacements> *FileToReplacements, llvm::StringRef FallbackStyle = "LLVM", - const std::vector<std::string>& StyleSearchPaths = {}); + const std::vector<std::string> &StyleSearchPaths = {}); void registerMatchers(ast_matchers::MatchFinder *Finder); @@ -110,8 +110,8 @@ private: }; std::string FallbackStyle; - // Specifies the list of paths to be searched when FormatStyle or a BasedOnStyle - // in a .clang-format file specifies an arbitrary file to include + // Specifies the list of paths to be searched when FormatStyle or a + // BasedOnStyle in a .clang-format file specifies an arbitrary file to include std::vector<std::string> StyleSearchPaths; // In match callbacks, this contains replacements for replacing `typeLoc`s in // and deleting forward declarations in the moved namespace blocks. diff --git a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp index b78dcfc713..4148cb4633 100644 --- a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp +++ b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp @@ -74,19 +74,16 @@ cl::opt<std::string> Style("style", cl::list<std::string> StyleSearchPaths( "style-search-path", - cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" - "BasedOnStyle directive is not one of the predefined styles, nor\n" - "InheritFromParent. Multiple style search paths may be specified,\n" - "and will be searched in order, stopping at the first file found."), - cl::value_desc("directory"), - cl::cat(ChangeNamespaceCategory)); + cl::desc( + "Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), cl::cat(ChangeNamespaceCategory)); -cl::alias StyleSearchPathShort( - "S", - cl::desc("Alias for --style-search-path"), - cl::cat(ChangeNamespaceCategory), - cl::aliasopt(StyleSearchPaths), - cl::NotHidden); +cl::alias StyleSearchPathShort("S", cl::desc("Alias for --style-search-path"), + cl::cat(ChangeNamespaceCategory), + cl::aliasopt(StyleSearchPaths), cl::NotHidden); cl::opt<std::string> AllowedFile( "allowed_file", @@ -151,7 +148,8 @@ int main(int argc, const char **argv) { SourceManager Sources(Diagnostics, FileMgr); Rewriter Rewrite(Sources, DefaultLangOptions); - if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style, StyleSearchPaths)) { + if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style, + StyleSearchPaths)) { llvm::errs() << "Failed applying all replacements.\n"; return 1; } diff --git a/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp index febbd4021a..c4be516035 100644 --- a/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp +++ b/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp @@ -157,23 +157,20 @@ cl::opt<std::string> "headers if there is no clang-format config file found."), cl::init("llvm"), cl::cat(IncludeFixerCategory)); -static cl::list<std::string> -StyleSearchPaths( +static cl::list<std::string> StyleSearchPaths( "style-search-path", - cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" - "BasedOnStyle directive is not one of the predefined styles, nor\n" - "InheritFromParent. Multiple style search paths may be specified,\n" - "and will be searched in order, stopping at the first file found."), - cl::value_desc("directory"), - cl::cat(IncludeFixerCategory)); - -static cl::alias -StyleSearchPathShort( - "S", - cl::desc("Alias for --style-search-path"), - cl::cat(IncludeFixerCategory), - cl::aliasopt(StyleSearchPaths), - cl::NotHidden ); + cl::desc( + "Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), cl::cat(IncludeFixerCategory)); + +static cl::alias StyleSearchPathShort("S", + cl::desc("Alias for --style-search-path"), + cl::cat(IncludeFixerCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); std::unique_ptr<include_fixer::SymbolIndexManager> createSymbolIndexManager(StringRef FilePath) { @@ -347,8 +344,9 @@ int includeFixerMain(int argc, const char **argv) { const IncludeFixerContext::HeaderInfo &RHS) { return LHS.QualifiedName == RHS.QualifiedName; }); - auto InsertStyle = format::getStyle(format::DefaultFormatStyle, - Context.getFilePath(), StyleSearchPaths, Style); + auto InsertStyle = + format::getStyle(format::DefaultFormatStyle, Context.getFilePath(), + StyleSearchPaths, Style); if (!InsertStyle) { llvm::errs() << llvm::toString(InsertStyle.takeError()) << "\n"; return 1; @@ -427,8 +425,8 @@ int includeFixerMain(int argc, const char **argv) { std::vector<tooling::Replacements> FixerReplacements; for (const auto &Context : Contexts) { StringRef FilePath = Context.getFilePath(); - auto InsertStyle = - format::getStyle(format::DefaultFormatStyle, FilePath, StyleSearchPaths, Style); + auto InsertStyle = format::getStyle(format::DefaultFormatStyle, FilePath, + StyleSearchPaths, Style); if (!InsertStyle) { llvm::errs() << llvm::toString(InsertStyle.takeError()) << "\n"; return 1; diff --git a/clang-tools-extra/clang-move/Move.cpp b/clang-tools-extra/clang-move/Move.cpp index d5068d3087..59625a7fa2 100644 --- a/clang-tools-extra/clang-move/Move.cpp +++ b/clang-tools-extra/clang-move/Move.cpp @@ -780,9 +780,9 @@ void ClangMoveTool::removeDeclsInOldFiles() { // Ignore replacements for new.h/cc. if (SI == FilePathToFileID.end()) continue; llvm::StringRef Code = SM.getBufferData(SI->second); - auto Style = format::getStyle(format::DefaultFormatStyle, FilePath, - Context->StyleSearchPaths, - Context->FallbackStyle); + auto Style = + format::getStyle(format::DefaultFormatStyle, FilePath, + Context->StyleSearchPaths, Context->FallbackStyle); if (!Style) { llvm::errs() << llvm::toString(Style.takeError()) << "\n"; continue; diff --git a/clang-tools-extra/clang-move/tool/ClangMove.cpp b/clang-tools-extra/clang-move/tool/ClangMove.cpp index 5ea3ef218e..6a8396012b 100644 --- a/clang-tools-extra/clang-move/tool/ClangMove.cpp +++ b/clang-tools-extra/clang-move/tool/ClangMove.cpp @@ -76,21 +76,18 @@ cl::opt<bool> "add #include of old header to new header."), cl::init(false), cl::cat(ClangMoveCategory)); -cl::list<std::string> - StyleSearchPaths("style-search-path", - cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" - "BasedOnStyle directive is not one of the predefined styles, nor\n" - "InheritFromParent. Multiple style search paths may be specified,\n" - "and will be searched in order, stopping at the first file found."), - cl::value_desc("directory"), - cl::cat(ClangMoveCategory)); - -cl::alias - StyleSearchPathShort("S", - cl::desc("Alias for --style-search-path"), - cl::cat(ClangMoveCategory), - cl::aliasopt(StyleSearchPaths), - cl::NotHidden); +cl::list<std::string> StyleSearchPaths( + "style-search-path", + cl::desc( + "Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), cl::cat(ClangMoveCategory)); + +cl::alias StyleSearchPathShort("S", cl::desc("Alias for --style-search-path"), + cl::cat(ClangMoveCategory), + cl::aliasopt(StyleSearchPaths), cl::NotHidden); cl::opt<std::string> Style("style", @@ -146,9 +143,12 @@ int main(int argc, const char **argv) { llvm::report_fatal_error("Cannot detect current path: " + Twine(EC.message())); - move::ClangMoveContext Context{Spec, Tool.getReplacements(), - std::string(InitialDirectory), StyleSearchPaths, - Style, DumpDecls}; + move::ClangMoveContext Context{Spec, + Tool.getReplacements(), + std::string(InitialDirectory), + StyleSearchPaths, + Style, + DumpDecls}; move::DeclarationReporter Reporter; move::ClangMoveActionFactory Factory(&Context, &Reporter); @@ -201,7 +201,8 @@ int main(int argc, const char **argv) { SourceManager SM(Diagnostics, FileMgr); Rewriter Rewrite(SM, LangOptions()); - if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style, StyleSearchPaths)) { + if (!formatAndApplyAllReplacements(Tool.getReplacements(), Rewrite, Style, + StyleSearchPaths)) { llvm::errs() << "Failed applying all replacements.\n"; return 1; } diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index 4148c3a8d9..b2df52f166 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -217,7 +217,8 @@ and will be searched in order, stopping at the first file found. cl::value_desc("directory"), cl::cat(ClangTidyCategory)); -static cl::alias StyleSearchPathShort("S", cl::desc("Alias for --style-search-path"), +static cl::alias StyleSearchPathShort("S", + cl::desc("Alias for --style-search-path"), cl::cat(ClangTidyCategory), cl::aliasopt(StyleSearchPaths), cl::NotHidden); diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index cfd60bb827..41a2a69b67 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -334,10 +334,9 @@ std::shared_ptr<const std::string> ClangdServer::getDraft(PathRef File) const { return std::move(Draft->Contents); } -std::function<Context(PathRef)> -ClangdServer::createConfiguredContextProvider(const config::Provider *Provider, - Callbacks *Publish, - const std::vector<std::string> &StyleSearchPaths) { +std::function<Context(PathRef)> ClangdServer::createConfiguredContextProvider( + const config::Provider *Provider, Callbacks *Publish, + const std::vector<std::string> &StyleSearchPaths) { if (!Provider) return [](llvm::StringRef) { return Context::current().clone(); }; @@ -349,7 +348,8 @@ ClangdServer::createConfiguredContextProvider(const config::Provider *Provider, Impl(const config::Provider *Provider, ClangdServer::Callbacks *Publish, const std::vector<std::string> &StyleSearchPaths) - : Provider(Provider), Publish(Publish), StyleSearchPaths(StyleSearchPaths) {} + : Provider(Provider), Publish(Publish), + StyleSearchPaths(StyleSearchPaths) {} Context operator()(llvm::StringRef File) { config::Params Params; @@ -410,9 +410,8 @@ ClangdServer::createConfiguredContextProvider(const config::Provider *Provider, }; // Copyable wrapper. - return [I(std::make_shared<Impl>(Provider, Publish, StyleSearchPaths))](llvm::StringRef Path) { - return (*I)(Path); - }; + return [I(std::make_shared<Impl>(Provider, Publish, StyleSearchPaths))]( + llvm::StringRef Path) { return (*I)(Path); }; } void ClangdServer::removeDocument(PathRef File) { diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 58d5bd6f01..4605c7d014 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -94,10 +94,9 @@ public: /// Creates a context provider that loads and installs config. /// Errors in loading config are reported as diagnostics via Callbacks. /// (This is typically used as ClangdServer::Options::ContextProvider). - static std::function<Context(PathRef)> - createConfiguredContextProvider(const config::Provider *Provider, - ClangdServer::Callbacks *, - const std::vector<std::string> &StyleSearchPaths); + static std::function<Context(PathRef)> createConfiguredContextProvider( + const config::Provider *Provider, ClangdServer::Callbacks *, + const std::vector<std::string> &StyleSearchPaths); struct Options { /// To process requests asynchronously, ClangdServer spawns worker threads. diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp index 3f08159fc6..27576b7eb1 100644 --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -599,10 +599,10 @@ format::FormatStyle getFormatStyleForFile(llvm::StringRef File, if (!FormatFile) Content = {}; auto &Cfg = Config::current(); - auto Style = format::getStyle(format::DefaultFormatStyle, File, - Cfg.Style.StyleSearchPaths, - format::DefaultFallbackStyle, Content, - TFS.view(/*CWD=*/std::nullopt).get()); + auto Style = + format::getStyle(format::DefaultFormatStyle, File, + Cfg.Style.StyleSearchPaths, format::DefaultFallbackStyle, + Content, TFS.view(/*CWD=*/std::nullopt).get()); if (!Style) { log("getStyle() failed for file {0}: {1}. Fallback is LLVM style.", File, Style.takeError()); diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index e27e9ef0e4..4466e9e794 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -80,9 +80,9 @@ using llvm::cl::list; using llvm::cl::NotHidden; using llvm::cl::opt; using llvm::cl::OptionCategory; +using llvm::cl::value_desc; using llvm::cl::ValueOptional; using llvm::cl::values; -using llvm::cl::value_desc; // All flags must be placed in a category, or they will be shown neither in // --help, nor --help-hidden! @@ -252,17 +252,11 @@ list<std::string> StyleSearchPaths{ "BasedOnStyle directive is not one of the predefined styles, nor " "InheritFromParent. Multiple style search paths may be specified, " "and will be searched in order, stopping at the first file found."), - value_desc("directory"), - cat(Features) -}; + value_desc("directory"), cat(Features)}; -alias StyleSearchPathShort{ - "S", - desc("Alias for --style-search-path"), - cat(Features), - aliasopt(StyleSearchPaths), - NotHidden -}; +alias StyleSearchPathShort{"S", desc("Alias for --style-search-path"), + cat(Features), aliasopt(StyleSearchPaths), + NotHidden}; opt<bool> EnableFunctionArgSnippets{ "function-arg-placeholders", diff --git a/clang-tools-extra/clangd/unittests/ClangdTests.cpp b/clang-tools-extra/clangd/unittests/ClangdTests.cpp index 2ea9f06495..8a39844fd5 100644 --- a/clang-tools-extra/clangd/unittests/ClangdTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdTests.cpp @@ -347,8 +347,8 @@ TEST(ClangdServerTest, RespectsConfig) { } CfgProvider; auto Opts = ClangdServer::optsForTest(); - Opts.ContextProvider = - ClangdServer::createConfiguredContextProvider(&CfgProvider, nullptr, Opts.StyleSearchPaths); + Opts.ContextProvider = ClangdServer::createConfiguredContextProvider( + &CfgProvider, nullptr, Opts.StyleSearchPaths); OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{}, CommandMangler::forTests()); MockFS FS; diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp index 1b14183540..1cec0e6646 100644 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -113,15 +113,17 @@ and will be searched in order, stopping at the first file found. cl::value_desc("directory"), cl::cat(IncludeCleaner)); -static cl::alias StyleSearchPathShort("S", cl::desc("Alias for --style-search-path"), - cl::cat(IncludeCleaner), cl::aliasopt(StyleSearchPaths), +static cl::alias StyleSearchPathShort("S", + cl::desc("Alias for --style-search-path"), + cl::cat(IncludeCleaner), + cl::aliasopt(StyleSearchPaths), cl::NotHidden); std::atomic<unsigned> Errors = ATOMIC_VAR_INIT(0); format::FormatStyle getStyle(llvm::StringRef Filename) { - auto S = format::getStyle(format::DefaultFormatStyle, Filename, StyleSearchPaths, - format::DefaultFallbackStyle); + auto S = format::getStyle(format::DefaultFormatStyle, Filename, + StyleSearchPaths, format::DefaultFallbackStyle); if (!S || !S->isCpp()) { consumeError(S.takeError()); return format::getLLVMStyle(); diff --git a/clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp b/clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp index 08db97a4e9..fb7f8fe1f6 100644 --- a/clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp +++ b/clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp @@ -46,7 +46,8 @@ public: if (!tooling::runToolOnCodeWithArgs(Factory->create(), Code, {"-std=c++11"}, FileName)) return ""; - formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "file", {} /*StyleSearchPatsh*/); + formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "file", + {} /*StyleSearchPatsh*/); return format(Context.getRewrittenText(ID)); } diff --git a/clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp b/clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp index 06ec26bc23..bb39ede79e 100644 --- a/clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp +++ b/clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp @@ -226,8 +226,9 @@ runClangMoveOnCode(const move::MoveDefinitionSpec &Spec, CreateFiles(TestCCName, CC); std::map<std::string, tooling::Replacements> FileToReplacements; - ClangMoveContext MoveContext = {Spec, FileToReplacements, Dir.c_str(), /*StyleSearchPaths*/{}, "LLVM", - Reporter != nullptr}; + ClangMoveContext MoveContext = {Spec, FileToReplacements, + Dir.c_str(), /*StyleSearchPaths*/ {}, + "LLVM", Reporter != nullptr}; auto Factory = std::make_unique<clang::move::ClangMoveActionFactory>( &MoveContext, Reporter); @@ -236,7 +237,8 @@ runClangMoveOnCode(const move::MoveDefinitionSpec &Spec, Factory->create(), CC, Context.InMemoryFileSystem, {"-std=c++11", "-fparse-all-comments", "-I."}, TestCCName, "clang-move", std::make_shared<PCHContainerOperations>()); - formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm", /*StyleSearchPaths*/{}); + formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm", + /*StyleSearchPaths*/ {}); // The Key is file name, value is the new code after moving the class. std::map<std::string, std::string> Results; for (const auto &It : FileToReplacements) { diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 79b0313bb6..e54eb10a54 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -5354,23 +5354,23 @@ parseConfiguration(llvm::MemoryBufferRef Config, FormatStyle *Style, void *DiagHandlerCtx = nullptr); /// Like above but accepts an unnamed buffer. -inline std::error_code parseConfiguration(StringRef Config, FormatStyle *Style, - const std::vector<std::string> &StyleSearchPaths, - bool AllowUnknownOptions = false) { +inline std::error_code +parseConfiguration(StringRef Config, FormatStyle *Style, + const std::vector<std::string> &StyleSearchPaths, + bool AllowUnknownOptions = false) { return parseConfiguration(llvm::MemoryBufferRef(Config, "YAML"), Style, StyleSearchPaths, AllowUnknownOptions); } /// Like above but discards Style->StyleSet after resolving the desired style. -/// This allows a BasedOnStyle that references a YAML file included via StyleSearchPaths -/// (which might contain styles for multiple langages) to be consumed while maintaining -/// the invariant that a FormatStyle may belong to only one StyleSet. -std::error_code -parseNestedConfiguration(llvm::MemoryBufferRef Config, FormatStyle *Style, - const std::vector<std::string> &StyleSearchPaths, - bool AllowUnknownOptions, - llvm::SourceMgr::DiagHandlerTy DiagHandler, - void *DiagHandlerCtxt); +/// This allows a BasedOnStyle that references a YAML file included via +/// StyleSearchPaths (which might contain styles for multiple langages) to be +/// consumed while maintaining the invariant that a FormatStyle may belong to +/// only one StyleSet. +std::error_code parseNestedConfiguration( + llvm::MemoryBufferRef Config, FormatStyle *Style, + const std::vector<std::string> &StyleSearchPaths, bool AllowUnknownOptions, + llvm::SourceMgr::DiagHandlerTy DiagHandler, void *DiagHandlerCtxt); /// Gets configuration in a YAML string. std::string configurationAsText(const FormatStyle &Style); @@ -5532,9 +5532,8 @@ extern const char *DefaultFallbackStyle; Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, const std::vector<std::string> &StyleSearchPaths, - StringRef FallbackStyle, - StringRef Code = "", llvm::vfs::FileSystem *FS = nullptr, - bool AllowUnknownOptions = false, + StringRef FallbackStyle, StringRef Code = "", + llvm::vfs::FileSystem *FS = nullptr, bool AllowUnknownOptions = false, llvm::SourceMgr::DiagHandlerTy DiagHandler = nullptr); // Guesses the language from the ``FileName`` and ``Code`` to be formatted. diff --git a/clang/include/clang/Tooling/Refactoring.h b/clang/include/clang/Tooling/Refactoring.h index f505300f4d..ca62462681 100644 --- a/clang/include/clang/Tooling/Refactoring.h +++ b/clang/include/clang/Tooling/Refactoring.h @@ -93,7 +93,8 @@ private: /// \returns true if all replacements applied and formatted. false otherwise. bool formatAndApplyAllReplacements( const std::map<std::string, Replacements> &FileToReplaces, - Rewriter &Rewrite, StringRef Style, const std::vector<std::string>& StyleSearchPaths); + Rewriter &Rewrite, StringRef Style, + const std::vector<std::string> &StyleSearchPaths); } // end namespace tooling } // end namespace clang diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index d6bc46ffab..8121f5595b 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -854,17 +854,21 @@ template <> struct MappingTraits<FormatStyle> { ((FormatStyle *)IO.getContext())->Language; if (!getPredefinedStyle(BasedOnStyle, Language, &Style)) { // OK, not a predefined style. See if this is an includable file. - SourceMgr& SrcMgr = IO.getSourceMgr(); + SourceMgr &SrcMgr = IO.getSourceMgr(); std::string IncludedFile; - ErrorOr<std::unique_ptr<MemoryBuffer>> IncFileOrError = SrcMgr.OpenIncludeFile(BasedOnStyle.str(), IncludedFile); + ErrorOr<std::unique_ptr<MemoryBuffer>> IncFileOrError = + SrcMgr.OpenIncludeFile(BasedOnStyle.str(), IncludedFile); if (!IncFileOrError) { - IO.setError(Twine("BasedOnStyle value is not a predefined style nor a file relative to the style search path: ", BasedOnStyle)); + IO.setError(Twine("BasedOnStyle value is not a predefined style " + "nor a file relative to the style search path: ", + BasedOnStyle)); return; } Style.Language = Language; - if (auto EC = parseNestedConfiguration((*IncFileOrError)->getMemBufferRef(), &Style, - SrcMgr.getIncludeDirs(), IO.allowUnknownKeys(), - SrcMgr.getDiagHandler(), SrcMgr.getDiagContext())) { + if (auto EC = parseNestedConfiguration( + (*IncFileOrError)->getMemBufferRef(), &Style, + SrcMgr.getIncludeDirs(), IO.allowUnknownKeys(), + SrcMgr.getDiagHandler(), SrcMgr.getDiagContext())) { IO.setError(Twine(EC.message())); return; } @@ -2045,12 +2049,10 @@ ParseError validateQualifierOrder(FormatStyle *Style) { return ParseError::Success; } -std::error_code parseConfiguration(llvm::MemoryBufferRef Config, - FormatStyle *Style, - const std::vector<std::string> &StyleSearchPaths, - bool AllowUnknownOptions, - llvm::SourceMgr::DiagHandlerTy DiagHandler, - void *DiagHandlerCtxt) { +std::error_code parseConfiguration( + llvm::MemoryBufferRef Config, FormatStyle *Style, + const std::vector<std::string> &StyleSearchPaths, bool AllowUnknownOptions, + llvm::SourceMgr::DiagHandlerTy DiagHandler, void *DiagHandlerCtxt) { assert(Style); FormatStyle::LanguageKind Language = Style->Language; assert(Language != FormatStyle::LK_None); @@ -2114,13 +2116,13 @@ std::error_code parseConfiguration(llvm::MemoryBufferRef Config, return make_error_code(ParseError::Success); } -std::error_code parseNestedConfiguration(llvm::MemoryBufferRef Config, - FormatStyle *Style, - const std::vector<std::string> &StyleSearchPaths, - bool AllowUnknownOptions, - llvm::SourceMgr::DiagHandlerTy DiagHandler, - void *DiagHandlerCtxt) { - auto EC = parseConfiguration(Config, Style, StyleSearchPaths, AllowUnknownOptions, DiagHandler, DiagHandlerCtxt); +std::error_code parseNestedConfiguration( + llvm::MemoryBufferRef Config, FormatStyle *Style, + const std::vector<std::string> &StyleSearchPaths, bool AllowUnknownOptions, + llvm::SourceMgr::DiagHandlerTy DiagHandler, void *DiagHandlerCtxt) { + auto EC = + parseConfiguration(Config, Style, StyleSearchPaths, AllowUnknownOptions, + DiagHandler, DiagHandlerCtxt); if (!EC) Style->StyleSet.Clear(); return EC; @@ -4017,18 +4019,16 @@ const char *DefaultFormatStyle = "file"; const char *DefaultFallbackStyle = "LLVM"; -llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> -loadAndParseConfigFile(StringRef ConfigFile, llvm::vfs::FileSystem *FS, - FormatStyle *Style, - const std::vector<std::string> &StyleSearchPaths, - bool AllowUnknownOptions, - llvm::SourceMgr::DiagHandlerTy DiagHandler) { +llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> loadAndParseConfigFile( + StringRef ConfigFile, llvm::vfs::FileSystem *FS, FormatStyle *Style, + const std::vector<std::string> &StyleSearchPaths, bool AllowUnknownOptions, + llvm::SourceMgr::DiagHandlerTy DiagHandler) { llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = FS->getBufferForFile(ConfigFile.str()); if (auto EC = Text.getError()) return EC; - if (auto EC = parseConfiguration(*Text.get(), Style, StyleSearchPaths, AllowUnknownOptions, - DiagHandler)) { + if (auto EC = parseConfiguration(*Text.get(), Style, StyleSearchPaths, + AllowUnknownOptions, DiagHandler)) { return EC; } return Text; @@ -4050,9 +4050,9 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, if (StyleName.starts_with("{")) { // Parse YAML/JSON style from the command line. StringRef Source = "<command-line>"; - if (std::error_code ec = - parseConfiguration(llvm::MemoryBufferRef(StyleName, Source), &Style, - StyleSearchPaths, AllowUnknownOptions, DiagHandler)) { + if (std::error_code ec = parseConfiguration( + llvm::MemoryBufferRef(StyleName, Source), &Style, StyleSearchPaths, + AllowUnknownOptions, DiagHandler)) { return make_string_error("Error parsing -style: " + ec.message()); } @@ -4112,9 +4112,9 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, auto applyChildFormatTexts = [&](FormatStyle *Style) { for (const auto &MemBuf : llvm::reverse(ChildFormatTextToApply)) { - auto EC = - parseConfiguration(*MemBuf, Style, StyleSearchPaths, AllowUnknownOptions, - DiagHandler ? DiagHandler : dropDiagnosticHandler); + auto EC = parseConfiguration( + *MemBuf, Style, StyleSearchPaths, AllowUnknownOptions, + DiagHandler ? DiagHandler : dropDiagnosticHandler); // It was already correctly parsed. assert(!EC); static_cast<void>(EC); @@ -4148,8 +4148,8 @@ Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName, } llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text = - loadAndParseConfigFile(ConfigFile, FS, &Style, StyleSearchPaths, AllowUnknownOptions, - DiagHandler); + loadAndParseConfigFile(ConfigFile, FS, &Style, StyleSearchPaths, + AllowUnknownOptions, DiagHandler); if (auto EC = Text.getError()) { if (EC != ParseError::Unsuitable) { return make_string_error("Error reading " + ConfigFile + ": " + diff --git a/clang/lib/Tooling/Refactoring.cpp b/clang/lib/Tooling/Refactoring.cpp index 8221957648..6bfaed2932 100644 --- a/clang/lib/Tooling/Refactoring.cpp +++ b/clang/lib/Tooling/Refactoring.cpp @@ -68,7 +68,8 @@ int RefactoringTool::saveRewrittenFiles(Rewriter &Rewrite) { bool formatAndApplyAllReplacements( const std::map<std::string, Replacements> &FileToReplaces, - Rewriter &Rewrite, StringRef Style, const std::vector<std::string>& StyleSearchPaths) { + Rewriter &Rewrite, StringRef Style, + const std::vector<std::string> &StyleSearchPaths) { SourceManager &SM = Rewrite.getSourceMgr(); FileManager &Files = SM.getFileManager(); diff --git a/clang/tools/clang-format/ClangFormat.cpp b/clang/tools/clang-format/ClangFormat.cpp index 0cca1e1c4c..80d2947240 100644 --- a/clang/tools/clang-format/ClangFormat.cpp +++ b/clang/tools/clang-format/ClangFormat.cpp @@ -172,23 +172,20 @@ static cl::opt<bool> cl::desc("If set, changes formatting warnings to errors"), cl::cat(ClangFormatCategory)); -static cl::list<std::string> - StyleSearchPaths( +static cl::list<std::string> StyleSearchPaths( "style-search-path", - cl::desc("Directory to search for BasedOnStyle files, when the value of the\n" - "BasedOnStyle directive is not one of the predefined styles, nor\n" - "InheritFromParent. Multiple style search paths may be specified,\n" - "and will be searched in order, stopping at the first file found."), - cl::value_desc("directory"), - cl::cat(ClangFormatCategory)); - -static cl::alias - StyleSearchPathShort( - "S", - cl::desc("Alias for --style-search-path"), - cl::cat(ClangFormatCategory), - cl::aliasopt(StyleSearchPaths), - cl::NotHidden); + cl::desc( + "Directory to search for BasedOnStyle files, when the value of the\n" + "BasedOnStyle directive is not one of the predefined styles, nor\n" + "InheritFromParent. Multiple style search paths may be specified,\n" + "and will be searched in order, stopping at the first file found."), + cl::value_desc("directory"), cl::cat(ClangFormatCategory)); + +static cl::alias StyleSearchPathShort("S", + cl::desc("Alias for --style-search-path"), + cl::cat(ClangFormatCategory), + cl::aliasopt(StyleSearchPaths), + cl::NotHidden); namespace { enum class WNoError { Unknown }; @@ -469,9 +466,9 @@ static bool format(StringRef FileName, bool ErrorOnIncompleteFormat = false) { return true; } - Expected<FormatStyle> FormatStyle = - getStyle(Style, AssumedFileName, StyleSearchPaths, FallbackStyle, Code->getBuffer(), - nullptr, WNoErrorList.isSet(WNoError::Unknown)); + Expected<FormatStyle> FormatStyle = getStyle( + Style, AssumedFileName, StyleSearchPaths, FallbackStyle, + Code->getBuffer(), nullptr, WNoErrorList.isSet(WNoError::Unknown)); if (!FormatStyle) { llvm::errs() << toString(FormatStyle.takeError()) << "\n"; return true; @@ -591,8 +588,7 @@ static int dumpConfig() { Expected<clang::format::FormatStyle> FormatStyle = clang::format::getStyle( Style, FileNames.empty() || FileNames[0] == "-" ? AssumeFileName : FileNames[0], - StyleSearchPaths, - FallbackStyle, Code ? Code->getBuffer() : ""); + StyleSearchPaths, FallbackStyle, Code ? Code->getBuffer() : ""); if (!FormatStyle) { llvm::errs() << toString(FormatStyle.takeError()) << "\n"; return 1; diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 31b99b640f..3db8699773 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -80,27 +80,31 @@ TEST(ConfigParseTest, GetsCorrectBasedOnStyle) { Styles[0] = getGoogleStyle(); Styles[1] = getLLVMStyle(); - EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google", &Styles[1], /*StyleSearchPaths*/{}).value()); + EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google", &Styles[1], + /*StyleSearchPaths*/ {}) + .value()); EXPECT_ALL_STYLES_EQUAL(Styles); Styles.resize(5); Styles[0] = getGoogleStyle(FormatStyle::LK_JavaScript); Styles[1] = getLLVMStyle(); Styles[1].Language = FormatStyle::LK_JavaScript; - EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google", &Styles[1], /*StyleSearchPaths*/{}).value()); + EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google", &Styles[1], + /*StyleSearchPaths*/ {}) + .value()); Styles[2] = getLLVMStyle(); Styles[2].Language = FormatStyle::LK_JavaScript; EXPECT_EQ(0, parseConfiguration("Language: JavaScript\n" "BasedOnStyle: Google", - &Styles[2], /*StyleSearchPaths*/{}) + &Styles[2], /*StyleSearchPaths*/ {}) .value()); Styles[3] = getLLVMStyle(); Styles[3].Language = FormatStyle::LK_JavaScript; EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google\n" "Language: JavaScript", - &Styles[3], /*StyleSearchPaths*/{}) + &Styles[3], /*StyleSearchPaths*/ {}) .value()); Styles[4] = getLLVMStyle(); @@ -111,7 +115,7 @@ TEST(ConfigParseTest, GetsCorrectBasedOnStyle) { "---\n" "BasedOnStyle: Google\n" "Language: JavaScript", - &Styles[4], /*StyleSearchPaths*/{}) + &Styles[4], /*StyleSearchPaths*/ {}) .value()); EXPECT_ALL_STYLES_EQUAL(Styles); } @@ -119,25 +123,25 @@ TEST(ConfigParseTest, GetsCorrectBasedOnStyle) { #define CHECK_PARSE_BOOL_FIELD(FIELD, CONFIG_NAME) \ Style.FIELD = false; \ EXPECT_EQ(0, parseConfiguration(CONFIG_NAME ": true", &Style, \ - /*StyleSearchPaths*/{}).value()); \ + /*StyleSearchPaths*/ {}) \ + .value()); \ EXPECT_TRUE(Style.FIELD); \ EXPECT_EQ(0, parseConfiguration(CONFIG_NAME ": false", &Style, \ - /*StyleSearchPaths*/{}).value()); \ + /*StyleSearchPaths*/ {}) \ + .value()); \ EXPECT_FALSE(Style.FIELD) #define CHECK_PARSE_BOOL(FIELD) CHECK_PARSE_BOOL_FIELD(FIELD, #FIELD) #define CHECK_PARSE_NESTED_BOOL_FIELD(STRUCT, FIELD, CONFIG_NAME) \ Style.STRUCT.FIELD = false; \ - EXPECT_EQ(0, \ - parseConfiguration(#STRUCT ":\n " CONFIG_NAME ": true", \ - &Style, /*StyleSearchPaths*/{}) \ - .value()); \ + EXPECT_EQ(0, parseConfiguration(#STRUCT ":\n " CONFIG_NAME ": true", \ + &Style, /*StyleSearchPaths*/ {}) \ + .value()); \ EXPECT_TRUE(Style.STRUCT.FIELD); \ - EXPECT_EQ(0, \ - parseConfiguration(#STRUCT ":\n " CONFIG_NAME ": false", \ - &Style, /*StyleSearchPaths*/{}) \ - .value()); \ + EXPECT_EQ(0, parseConfiguration(#STRUCT ":\n " CONFIG_NAME ": false", \ + &Style, /*StyleSearchPaths*/ {}) \ + .value()); \ EXPECT_FALSE(Style.STRUCT.FIELD) #define CHECK_PARSE_NESTED_BOOL(STRUCT, FIELD) \ @@ -145,13 +149,14 @@ TEST(ConfigParseTest, GetsCorrectBasedOnStyle) { #define CHECK_PARSE(TEXT, FIELD, VALUE) \ EXPECT_NE(VALUE, Style.FIELD) << "Initial value already the same!"; \ - EXPECT_EQ(0, parseConfiguration(TEXT, &Style, \ - /*StyleSearchPaths*/{}).value()); \ + EXPECT_EQ( \ + 0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/ {}).value()); \ EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!" #define CHECK_PARSE_NESTED_VALUE(TEXT, STRUCT, FIELD, VALUE) \ EXPECT_NE(VALUE, Style.STRUCT.FIELD) << "Initial value already the same!"; \ EXPECT_EQ(0, parseConfiguration(#STRUCT ":\n " TEXT, &Style, \ - /*StyleSearchPaths*/{}).value()); \ + /*StyleSearchPaths*/ {}) \ + .value()); \ EXPECT_EQ(VALUE, Style.STRUCT.FIELD) << "Unexpected value after parsing!" TEST(ConfigParseTest, ParsesConfigurationBools) { @@ -1070,13 +1075,13 @@ TEST(ConfigParseTest, ParsesConfigurationWithLanguages) { IndentWidth, 12u); EXPECT_EQ(parseConfiguration("Language: JavaScript\n" "IndentWidth: 34", - &Style, /*StyleSearchPaths*/{}), + &Style, /*StyleSearchPaths*/ {}), ParseError::Unsuitable); FormatStyle BinPackedTCS = {}; BinPackedTCS.Language = FormatStyle::LK_JavaScript; EXPECT_EQ(parseConfiguration("BinPackArguments: true\n" "InsertTrailingCommas: Wrapped", - &BinPackedTCS, /*StyleSearchPaths*/{}), + &BinPackedTCS, /*StyleSearchPaths*/ {}), ParseError::BinPackTrailingCommaConflict); EXPECT_EQ(12u, Style.IndentWidth); CHECK_PARSE("IndentWidth: 56", IndentWidth, 56u); @@ -1089,7 +1094,7 @@ TEST(ConfigParseTest, ParsesConfigurationWithLanguages) { CHECK_PARSE("IndentWidth: 23", IndentWidth, 23u); EXPECT_EQ(parseConfiguration("Language: Cpp\n" "IndentWidth: 34", - &Style, /*StyleSearchPaths*/{}), + &Style, /*StyleSearchPaths*/ {}), ParseError::Unsuitable); EXPECT_EQ(23u, Style.IndentWidth); CHECK_PARSE("IndentWidth: 56", IndentWidth, 56u); @@ -1141,7 +1146,7 @@ TEST(ConfigParseTest, ParsesConfigurationWithLanguages) { "BreakBeforeBraces: Stroustrup\n" "TabWidth: 789\n" "...\n", - &Style, /*StyleSearchPaths*/{})); + &Style, /*StyleSearchPaths*/ {})); EXPECT_EQ(123u, Style.ColumnLimit); EXPECT_EQ(456u, Style.IndentWidth); EXPECT_EQ(FormatStyle::BS_Stroustrup, Style.BreakBeforeBraces); @@ -1153,7 +1158,7 @@ TEST(ConfigParseTest, ParsesConfigurationWithLanguages) { "---\n" "IndentWidth: 78\n" "...\n", - &Style, /*StyleSearchPaths*/{}), + &Style, /*StyleSearchPaths*/ {}), ParseError::Error); EXPECT_EQ(parseConfiguration("---\n" "Language: JavaScript\n" @@ -1162,7 +1167,7 @@ TEST(ConfigParseTest, ParsesConfigurationWithLanguages) { "Language: JavaScript\n" "IndentWidth: 78\n" "...\n", - &Style, /*StyleSearchPaths*/{}), + &Style, /*StyleSearchPaths*/ {}), ParseError::Error); EXPECT_EQ(FormatStyle::LK_Cpp, Style.Language); @@ -1189,7 +1194,9 @@ TEST(ConfigParseTest, UsesLanguageForBasedOnStyle) { FormatStyle Style = {}; Style.Language = FormatStyle::LK_JavaScript; Style.BreakBeforeTernaryOperators = true; - EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google", &Style, /*StyleSearchPaths*/{}).value()); + EXPECT_EQ(0, parseConfiguration("BasedOnStyle: Google", &Style, + /*StyleSearchPaths*/ {}) + .value()); EXPECT_FALSE(Style.BreakBeforeTernaryOperators); Style.BreakBeforeTernaryOperators = true; @@ -1199,7 +1206,7 @@ TEST(ConfigParseTest, UsesLanguageForBasedOnStyle) { "Language: JavaScript\n" "IndentWidth: 76\n" "...\n", - &Style, /*StyleSearchPaths*/{}) + &Style, /*StyleSearchPaths*/ {}) .value()); EXPECT_FALSE(Style.BreakBeforeTernaryOperators); EXPECT_EQ(76u, Style.IndentWidth); @@ -1211,13 +1218,16 @@ TEST(ConfigParseTest, ConfigurationRoundTripTest) { std::string YAML = configurationAsText(Style); FormatStyle ParsedStyle = {}; ParsedStyle.Language = FormatStyle::LK_Cpp; - EXPECT_EQ(0, parseConfiguration(YAML, &ParsedStyle, /*StyleSearchPaths*/{}).value()); + EXPECT_EQ( + 0, + parseConfiguration(YAML, &ParsedStyle, /*StyleSearchPaths*/ {}).value()); EXPECT_EQ(Style, ParsedStyle); } TEST(ConfigParseTest, GetStyleWithEmptyFileName) { llvm::vfs::InMemoryFileSystem FS; - auto Style1 = getStyle("file", "", /*StyleSearchPaths*/{}, "Google", "", &FS); + auto Style1 = + getStyle("file", "", /*StyleSearchPaths*/ {}, "Google", "", &FS); ASSERT_TRUE((bool)Style1); ASSERT_EQ(*Style1, getGoogleStyle()); } @@ -1230,19 +1240,22 @@ TEST(ConfigParseTest, GetStyleOfFile) { llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM"))); ASSERT_TRUE( FS.addFile("/a/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); - auto Style1 = getStyle("file", "/a/.clang-format", /*StyleSearchPaths*/{}, "Google", "", &FS); + auto Style1 = getStyle("file", "/a/.clang-format", /*StyleSearchPaths*/ {}, + "Google", "", &FS); ASSERT_TRUE((bool)Style1); ASSERT_EQ(*Style1, getLLVMStyle()); // Test 2.1: fallback to default. ASSERT_TRUE( FS.addFile("/b/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); - auto Style2 = getStyle("file", "/b/test.cpp", /*StyleSearchPaths*/{}, "Mozilla", "", &FS); + auto Style2 = getStyle("file", "/b/test.cpp", /*StyleSearchPaths*/ {}, + "Mozilla", "", &FS); ASSERT_TRUE((bool)Style2); ASSERT_EQ(*Style2, getMozillaStyle()); // Test 2.2: no format on 'none' fallback style. - Style2 = getStyle("file", "/b/test.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + Style2 = + getStyle("file", "/b/test.cpp", /*StyleSearchPaths*/ {}, "none", "", &FS); ASSERT_TRUE((bool)Style2); ASSERT_EQ(*Style2, getNoStyle()); @@ -1250,12 +1263,13 @@ TEST(ConfigParseTest, GetStyleOfFile) { // 'none'. ASSERT_TRUE(FS.addFile("/b/.clang-format", 0, llvm::MemoryBuffer::getMemBuffer("IndentWidth: 2"))); - Style2 = getStyle("file", "/b/test.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + Style2 = + getStyle("file", "/b/test.cpp", /*StyleSearchPaths*/ {}, "none", "", &FS); ASSERT_TRUE((bool)Style2); ASSERT_EQ(*Style2, getLLVMStyle()); // Test 2.4: format if yaml with no based style, while fallback is 'none'. - Style2 = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "", &FS); + Style2 = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", "", &FS); ASSERT_TRUE((bool)Style2); ASSERT_EQ(*Style2, getLLVMStyle()); @@ -1265,24 +1279,27 @@ TEST(ConfigParseTest, GetStyleOfFile) { llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google"))); ASSERT_TRUE(FS.addFile("/c/sub/sub/sub/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); - auto Style3 = getStyle("file", "/c/sub/sub/sub/test.cpp", /*StyleSearchPaths*/{}, "LLVM", "", &FS); + auto Style3 = getStyle("file", "/c/sub/sub/sub/test.cpp", + /*StyleSearchPaths*/ {}, "LLVM", "", &FS); ASSERT_TRUE((bool)Style3); ASSERT_EQ(*Style3, getGoogleStyle()); // Test 4: error on invalid fallback style - auto Style4 = getStyle("file", "a.h", /*StyleSearchPaths*/{}, "KungFu", "", &FS); + auto Style4 = + getStyle("file", "a.h", /*StyleSearchPaths*/ {}, "KungFu", "", &FS); ASSERT_FALSE((bool)Style4); llvm::consumeError(Style4.takeError()); // Test 5: error on invalid yaml on command line auto Style5 = getStyle("{invalid_key=invalid_value}", "a.h", "LLVM", "", &FS, - /*StyleSearchPaths*/{}, /*AllowUnknownOptions=*/false, + /*StyleSearchPaths*/ {}, /*AllowUnknownOptions=*/false, dropDiagnosticHandler); ASSERT_FALSE((bool)Style5); llvm::consumeError(Style5.takeError()); // Test 6: error on invalid style - auto Style6 = getStyle("KungFu", "a.h", /*StyleSearchPaths*/{}, "LLVM", "", &FS); + auto Style6 = + getStyle("KungFu", "a.h", /*StyleSearchPaths*/ {}, "LLVM", "", &FS); ASSERT_FALSE((bool)Style6); llvm::consumeError(Style6.takeError()); @@ -1294,18 +1311,19 @@ TEST(ConfigParseTest, GetStyleOfFile) { ASSERT_TRUE( FS.addFile("/d/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); auto Style7a = getStyle("file", "/d/.clang-format", "LLVM", "", &FS, - /*StyleSearchPaths*/{}, /*AllowUnknownOptions=*/false, - dropDiagnosticHandler); + /*StyleSearchPaths*/ {}, + /*AllowUnknownOptions=*/false, dropDiagnosticHandler); ASSERT_FALSE((bool)Style7a); llvm::consumeError(Style7a.takeError()); auto Style7b = getStyle("file", "/d/.clang-format", "LLVM", "", &FS, - /*StyleSearchPaths*/{}, /*AllowUnknownOptions=*/true, + /*StyleSearchPaths*/ {}, /*AllowUnknownOptions=*/true, dropDiagnosticHandler); ASSERT_TRUE((bool)Style7b); // Test 8: inferred per-language defaults apply. - auto StyleTd = getStyle("file", "x.td", /*StyleSearchPaths*/{}, "llvm", "", &FS); + auto StyleTd = + getStyle("file", "x.td", /*StyleSearchPaths*/ {}, "llvm", "", &FS); ASSERT_TRUE((bool)StyleTd); ASSERT_EQ(*StyleTd, getLLVMStyle(FormatStyle::LK_TableGen)); @@ -1317,7 +1335,8 @@ TEST(ConfigParseTest, GetStyleOfFile) { "ColumnLimit: 20"))); ASSERT_TRUE(FS.addFile("/e/sub/code.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); - auto Style9 = getStyle("file", "/e/sub/code.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + auto Style9 = getStyle("file", "/e/sub/code.cpp", /*StyleSearchPaths*/ {}, + "none", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, [] { auto Style = getNoStyle(); @@ -1338,7 +1357,8 @@ TEST(ConfigParseTest, GetStyleOfFile) { NonDefaultWhiteSpaceMacros[1] = "BAR"; ASSERT_NE(Style9->WhitespaceSensitiveMacros, NonDefaultWhiteSpaceMacros); - Style9 = getStyle("file", "/e/sub/sub/code.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + Style9 = getStyle("file", "/e/sub/sub/code.cpp", /*StyleSearchPaths*/ {}, + "none", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, [&NonDefaultWhiteSpaceMacros] { auto Style = getNoStyle(); @@ -1348,7 +1368,8 @@ TEST(ConfigParseTest, GetStyleOfFile) { }()); // Test 9.2: with LLVM fallback style - Style9 = getStyle("file", "/e/sub/code.cpp", /*StyleSearchPaths*/{}, "LLVM", "", &FS); + Style9 = getStyle("file", "/e/sub/code.cpp", /*StyleSearchPaths*/ {}, "LLVM", + "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, [] { auto Style = getLLVMStyle(); @@ -1361,7 +1382,8 @@ TEST(ConfigParseTest, GetStyleOfFile) { FS.addFile("/e/.clang-format", 0, llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google\n" "UseTab: Always"))); - Style9 = getStyle("file", "/e/sub/code.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + Style9 = getStyle("file", "/e/sub/code.cpp", /*StyleSearchPaths*/ {}, "none", + "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, [] { auto Style = getGoogleStyle(); @@ -1380,26 +1402,29 @@ TEST(ConfigParseTest, GetStyleOfFile) { }(); ASSERT_NE(Style9->WhitespaceSensitiveMacros, NonDefaultWhiteSpaceMacros); - Style9 = getStyle("file", "/e/sub/sub/code.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + Style9 = getStyle("file", "/e/sub/sub/code.cpp", /*StyleSearchPaths*/ {}, + "none", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, SubSubStyle); // Test 9.5: use InheritParentConfig as style name - Style9 = - getStyle("inheritparentconfig", "/e/sub/sub/code.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + Style9 = getStyle("inheritparentconfig", "/e/sub/sub/code.cpp", + /*StyleSearchPaths*/ {}, "none", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, SubSubStyle); // Test 9.6: use command line style with inheritance - Style9 = getStyle("{BasedOnStyle: InheritParentConfig}", - "/e/sub/sub/code.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + Style9 = + getStyle("{BasedOnStyle: InheritParentConfig}", "/e/sub/sub/code.cpp", + /*StyleSearchPaths*/ {}, "none", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, SubSubStyle); // Test 9.7: use command line style with inheritance and own config - Style9 = getStyle("{BasedOnStyle: InheritParentConfig, " - "WhitespaceSensitiveMacros: ['FOO', 'BAR']}", - "/e/sub/code.cpp", /*StyleSearchPaths*/{}, "none", "", &FS); + Style9 = + getStyle("{BasedOnStyle: InheritParentConfig, " + "WhitespaceSensitiveMacros: ['FOO', 'BAR']}", + "/e/sub/code.cpp", /*StyleSearchPaths*/ {}, "none", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, SubSubStyle); @@ -1411,7 +1436,8 @@ TEST(ConfigParseTest, GetStyleOfFile) { llvm::MemoryBuffer::getMemBuffer( "BasedOnStyle: InheritParentConfig\nIndentWidth: 7"))); // Make sure we do not use the fallback style - Style9 = getStyle("file", "/e/withoutbase/code.cpp", /*StyleSearchPaths*/{}, "google", "", &FS); + Style9 = getStyle("file", "/e/withoutbase/code.cpp", /*StyleSearchPaths*/ {}, + "google", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, [] { auto Style = getLLVMStyle(); @@ -1419,7 +1445,8 @@ TEST(ConfigParseTest, GetStyleOfFile) { return Style; }()); - Style9 = getStyle("file", "/e/withoutbase/sub/code.cpp", /*StyleSearchPaths*/{}, "google", "", &FS); + Style9 = getStyle("file", "/e/withoutbase/sub/code.cpp", + /*StyleSearchPaths*/ {}, "google", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, [] { auto Style = getLLVMStyle(); @@ -1429,8 +1456,8 @@ TEST(ConfigParseTest, GetStyleOfFile) { }()); // Test 9.9: use inheritance from a specific config file. - Style9 = getStyle("file:/e/sub/sub/.clang-format", "/e/sub/sub/code.cpp", /*StyleSearchPaths*/{}, - "none", "", &FS); + Style9 = getStyle("file:/e/sub/sub/.clang-format", "/e/sub/sub/code.cpp", + /*StyleSearchPaths*/ {}, "none", "", &FS); ASSERT_TRUE(static_cast<bool>(Style9)); ASSERT_EQ(*Style9, SubSubStyle); } @@ -1446,8 +1473,9 @@ TEST(ConfigParseTest, GetStyleOfSpecificFile) { llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google"))); ASSERT_TRUE(FS.addFile("/e/sub/sub/sub/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;"))); - auto Style = getStyle("file:/e/explicit.clang-format", - "/e/sub/sub/sub/test.cpp", /*StyleSearchPaths*/{}, "LLVM", "", &FS); + auto Style = + getStyle("file:/e/explicit.clang-format", "/e/sub/sub/sub/test.cpp", + /*StyleSearchPaths*/ {}, "LLVM", "", &FS); ASSERT_TRUE(static_cast<bool>(Style)); ASSERT_EQ(*Style, getGoogleStyle()); @@ -1455,14 +1483,15 @@ TEST(ConfigParseTest, GetStyleOfSpecificFile) { ASSERT_TRUE( FS.addFile("../../e/explicit.clang-format", 0, llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google"))); - Style = getStyle("file:../../e/explicit.clang-format", - "/e/sub/sub/sub/test.cpp", /*StyleSearchPaths*/{}, "LLVM", "", &FS); + Style = + getStyle("file:../../e/explicit.clang-format", "/e/sub/sub/sub/test.cpp", + /*StyleSearchPaths*/ {}, "LLVM", "", &FS); ASSERT_TRUE(static_cast<bool>(Style)); ASSERT_EQ(*Style, getGoogleStyle()); // Specify path to a format file that does not exist. - Style = getStyle("file:/e/missing.clang-format", "/e/sub/sub/sub/test.cpp", /*StyleSearchPaths*/{}, - "LLVM", "", &FS); + Style = getStyle("file:/e/missing.clang-format", "/e/sub/sub/sub/test.cpp", + /*StyleSearchPaths*/ {}, "LLVM", "", &FS); ASSERT_FALSE(static_cast<bool>(Style)); llvm::consumeError(Style.takeError()); @@ -1485,7 +1514,8 @@ TEST(ConfigParseTest, GetStyleOfSpecificFile) { CodeFileTest.close(); std::string format_file_arg = std::string("file:") + FormatFilePath.c_str(); - Style = getStyle(format_file_arg, TestFilePath, /*StyleSearchPaths*/{}, "LLVM", "", nullptr); + Style = getStyle(format_file_arg, TestFilePath, /*StyleSearchPaths*/ {}, + "LLVM", "", nullptr); llvm::sys::fs::remove(FormatFilePath.c_str()); llvm::sys::fs::remove(TestFilePath.c_str()); diff --git a/clang/unittests/Format/FormatTestObjC.cpp b/clang/unittests/Format/FormatTestObjC.cpp index c692dc0cd8..f7ad87c008 100644 --- a/clang/unittests/Format/FormatTestObjC.cpp +++ b/clang/unittests/Format/FormatTestObjC.cpp @@ -32,7 +32,7 @@ protected: #define verifyFormat(...) _verifyFormat(__FILE__, __LINE__, __VA_ARGS__) TEST(FormatTestObjCStyle, DetectsObjCInStdin) { - auto Style = getStyle("LLVM", "<stdin>", /*StyleSearchPaths*/{}, "none", + auto Style = getStyle("LLVM", "<stdin>", /*StyleSearchPaths*/ {}, "none", "@interface\n" "- (id)init;"); ASSERT_TRUE((bool)Style); @@ -40,67 +40,73 @@ TEST(FormatTestObjCStyle, DetectsObjCInStdin) { } TEST(FormatTestObjCStyle, DetectsObjCInHeaders) { - auto Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/{}, "none", + auto Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/ {}, "none", "@interface\n" "- (id)init;"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/{}, "none", + Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/ {}, "none", "@interface\n" "+ (id)init;"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/{}, "none", + Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/ {}, "none", "@interface\n" "@end\n" "//comment"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/{}, "none", + Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/ {}, "none", "@interface\n" "@end //comment"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); // No recognizable ObjC. - Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/{}, "none", "void f() {}"); + Style = + getStyle("LLVM", "a.h", /*StyleSearchPaths*/ {}, "none", "void f() {}"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "@interface Foo\n@end"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "@interface Foo\n@end"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", "const int interface = 1;\nconst int end = 2;"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "@protocol Foo\n@end"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "@protocol Foo\n@end"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", "const int protocol = 1;\nconst int end = 2;"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "typedef NS_ENUM(int, Foo) {};"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "typedef NS_ENUM(int, Foo) {};"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "typedef NS_CLOSED_ENUM(int, Foo) {};"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "typedef NS_CLOSED_ENUM(int, Foo) {};"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "typedef NS_ERROR_ENUM(int, Foo) {};"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "typedef NS_ERROR_ENUM(int, Foo) {};"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", R"objc( + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", R"objc( NS_ASSUME_NONNULL_BEGIN extern int i; NS_ASSUME_NONNULL_END @@ -108,71 +114,78 @@ NS_ASSUME_NONNULL_END ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", R"objc( + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", R"objc( FOUNDATION_EXTERN void DoStuff(void); )objc"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", R"objc( + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", R"objc( FOUNDATION_EXPORT void DoStuff(void); )objc"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "enum Foo {};"); + Style = + getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", "enum Foo {};"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "inline void Foo() { Log(@\"Foo\"); }"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "inline void Foo() { Log(@\"Foo\"); }"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "inline void Foo() { Log(\"Foo\"); }"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "inline void Foo() { Log(\"Foo\"); }"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language); - Style = - getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "inline void Foo() { id = @[1, 2, 3]; }"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "inline void Foo() { id = @[1, 2, 3]; }"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", "inline void Foo() { id foo = @{1: 2, 3: 4, 5: 6}; }"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", "inline void Foo() { int foo[] = {1, 2, 3}; }"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language); // ObjC characteristic types. - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "extern NSString *kFoo;"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "extern NSString *kFoo;"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "extern NSInteger Foo();"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "extern NSInteger Foo();"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "NSObject *Foo();"); + Style = getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", + "NSObject *Foo();"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); - Style = getStyle("{}", "a.h", /*StyleSearchPaths*/{}, "none", "NSSet *Foo();"); + Style = + getStyle("{}", "a.h", /*StyleSearchPaths*/ {}, "none", "NSSet *Foo();"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language); } TEST(FormatTestObjCStyle, AvoidDetectingDesignatedInitializersAsObjCInHeaders) { - auto Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/{}, "none", + auto Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/ {}, "none", "static const char *names[] = {[0] = \"foo\",\n" "[kBar] = \"bar\"};"); ASSERT_TRUE((bool)Style); EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language); - Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/{}, "none", + Style = getStyle("LLVM", "a.h", /*StyleSearchPaths*/ {}, "none", "static const char *names[] = {[0] EQ \"foo\",\n" "[kBar] EQ \"bar\"};"); ASSERT_TRUE((bool)Style); diff --git a/clang/unittests/Format/FormatTestRawStrings.cpp b/clang/unittests/Format/FormatTestRawStrings.cpp index d2d4773df7..ed72b83592 100644 --- a/clang/unittests/Format/FormatTestRawStrings.cpp +++ b/clang/unittests/Format/FormatTestRawStrings.cpp @@ -136,7 +136,7 @@ TEST_F(FormatTestRawStrings, UsesConfigurationOverBaseStyle) { EXPECT_EQ(0, parseConfiguration("---\n" "Language: Cpp\n" "BasedOnStyle: Google", - &Style, /*StyleSearchPaths*/{}) + &Style, /*StyleSearchPaths*/ {}) .value()); Style.RawStringFormats = {{ FormatStyle::LK_Cpp, diff --git a/clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp b/clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp index 5fda3b9817..8d02931f3e 100644 --- a/clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp +++ b/clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp @@ -19,13 +19,13 @@ namespace { #define CHECK_PARSE(TEXT, FIELD, VALUE) \ EXPECT_NE(VALUE, Style.FIELD) << "Initial value already the same!"; \ - EXPECT_EQ(0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/{}) \ - .value()); \ + EXPECT_EQ( \ + 0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/ {}).value()); \ EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!" #define FAIL_PARSE(TEXT, FIELD, VALUE) \ - EXPECT_NE(0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/{}) \ - .value()); \ + EXPECT_NE( \ + 0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/ {}).value()); \ EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!" class ObjCPropertyAttributeOrderFixerTest : public FormatTestBase { diff --git a/clang/unittests/Format/QualifierFixerTest.cpp b/clang/unittests/Format/QualifierFixerTest.cpp index 3ed1e55799..9498e7ce1c 100644 --- a/clang/unittests/Format/QualifierFixerTest.cpp +++ b/clang/unittests/Format/QualifierFixerTest.cpp @@ -19,13 +19,13 @@ namespace { #define CHECK_PARSE(TEXT, FIELD, VALUE) \ EXPECT_NE(VALUE, Style.FIELD) << "Initial value already the same!"; \ - EXPECT_EQ(0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/{}) \ - .value()); \ + EXPECT_EQ( \ + 0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/ {}).value()); \ EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!" #define FAIL_PARSE(TEXT, FIELD, VALUE) \ - EXPECT_NE(0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/{}) \ - .value()); \ + EXPECT_NE( \ + 0, parseConfiguration(TEXT, &Style, /*StyleSearchPaths*/ {}).value()); \ EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!" class QualifierFixerTest : public FormatTestBase { diff --git a/clang/unittests/Rename/ClangRenameTest.h b/clang/unittests/Rename/ClangRenameTest.h index 7b242c2638..431d3e7409 100644 --- a/clang/unittests/Rename/ClangRenameTest.h +++ b/clang/unittests/Rename/ClangRenameTest.h @@ -81,7 +81,8 @@ protected: FileContents)) return ""; - formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm", /*StyleSearchPaths*/{}); + formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm", + /*StyleSearchPaths*/ {}); return Context.getRewrittenText(InputFileID); } diff --git a/clang/unittests/Tooling/RefactoringTest.cpp b/clang/unittests/Tooling/RefactoringTest.cpp index cf3c7c7fa1..321db0861f 100644 --- a/clang/unittests/Tooling/RefactoringTest.cpp +++ b/clang/unittests/Tooling/RefactoringTest.cpp @@ -539,10 +539,9 @@ TEST_F(ReplacementTest, MultipleFilesReplaceAndFormat) { "4567890123"), tooling::Replacement(Context.Sources, Context.getLocation(ID2, 2, 9), 1, "10")}); - EXPECT_TRUE( - formatAndApplyAllReplacements(FileToReplaces, Context.Rewrite, - "{BasedOnStyle: LLVM, ColumnLimit: 20}", - /*StyleSearchPaths*/{})); + EXPECT_TRUE(formatAndApplyAllReplacements( + FileToReplaces, Context.Rewrite, "{BasedOnStyle: LLVM, ColumnLimit: 20}", + /*StyleSearchPaths*/ {})); EXPECT_EQ(Expected1, Context.getRewrittenText(ID1)); EXPECT_EQ(Expected2, Context.getRewrittenText(ID2)); } diff --git a/llvm/include/llvm/Support/YAMLTraits.h b/llvm/include/llvm/Support/YAMLTraits.h index 04dc9ccca3..ba5b06b53b 100644 --- a/llvm/include/llvm/Support/YAMLTraits.h +++ b/llvm/include/llvm/Support/YAMLTraits.h @@ -779,7 +779,7 @@ public: IO(void *Ctxt = nullptr); virtual ~IO(); - virtual SourceMgr& getSourceMgr(); + virtual SourceMgr &getSourceMgr(); virtual bool outputting() const = 0; @@ -1452,7 +1452,7 @@ public: // Check if there was an syntax or semantic error during parsing. std::error_code error(); - SourceMgr& getSourceMgr() override; + SourceMgr &getSourceMgr() override; private: bool outputting() const override; diff --git a/llvm/lib/Support/YAMLTraits.cpp b/llvm/lib/Support/YAMLTraits.cpp index d7bc0255fe..d737a37d33 100644 --- a/llvm/lib/Support/YAMLTraits.cpp +++ b/llvm/lib/Support/YAMLTraits.cpp @@ -39,9 +39,7 @@ IO::IO(void *Context) : Ctxt(Context) {} IO::~IO() = default; -SourceMgr& IO::getSourceMgr() { - llvm_unreachable("Only supported for Input"); -} +SourceMgr &IO::getSourceMgr() { llvm_unreachable("Only supported for Input"); } void *IO::getContext() const { return Ctxt; @@ -83,7 +81,7 @@ Input::~Input() = default; std::error_code Input::error() { return EC; } -SourceMgr& Input::getSourceMgr() { return SrcMgr; } +SourceMgr &Input::getSourceMgr() { return SrcMgr; } bool Input::outputting() const { return false; 
@jediry
Copy link
Author

jediry commented Sep 6, 2024

I am not sure if it's best to push a new command line flag to all other tools that use clang-format as a library.

Have you considered any other alternatives that can self-contain this in clang-format? e.g can we just treat paths as relative to current .clang-format file ?

@kadircet @mydeveloperday @HazardyKnusperkeks I would love to make this more self-contained, as this certainly does hit a lot of layers of code. I specifically selected this approach because of its similarity to the familiar -I mechanism for specifying include paths for C/C++. Two other ideas I did consider are:

  1. Allow the use of environment variables in the path (e.g., BasedOnStyle: $(FORMATDIR)\team1.clang-format). Not sure if this is better or worse than the current approach, though it's certainly a smaller code change. (That said, I find that most command-line tools that find things via environment variable also support command-line overrides for this, since it's sometimes cumbersome to have to set the environment variable in order to use the tool.)
  2. As you say, do this relative to the current .clang-format file. This is also more self-contained, though it has the downside of being easily broken by code moves (e.g., if I "git mv" a directory to a different path, expressions like BasedOnStyle: ../../../format/team1.clang-format become invalid).

I'm happy to go a different way with this if that is preferred.

@owenca
Copy link
Contributor

owenca commented Sep 7, 2024

@jediry, can you open a github issue for this?

We probably should continue the discussion on your RFC before getting into the details of the implementation. At the very least, I need to understand what exactly you are proposing to extend BasedOnStyle from the user's perspective. (See here for an example.)

Some questions off the top of my head:

  • Should both absolute and relative file paths be supported?
  • How would the other configs in the .clang-format file work with the imported ones?
  • What happens if the imported config file also imports another config file, possibly the one that imports it?
  • Is the search path (or environment variable) mechanism necessary?
@jediry
Copy link
Author

jediry commented Sep 9, 2024

@owenca I've opened an issue here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6 participants