From 34d2688a50f23b4b15bdeab054e28e033ece9363 Mon Sep 17 00:00:00 2001 From: Nathan James Date: Thu, 10 Dec 2020 14:52:44 +0000 Subject: [PATCH] [clang-tidy] Use a MemoryBufferRef when parsing configuration files. Using a MemoryBufferRef, If there is an error parsing, we can point the user to the location of the file. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D93024 --- clang-tools-extra/clang-tidy/ClangTidyOptions.cpp | 5 +++-- clang-tools-extra/clang-tidy/ClangTidyOptions.h | 5 +++-- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp | 13 +++++++------ clang-tools-extra/clangd/TidyProvider.cpp | 3 ++- .../unittests/clang-tidy/ClangTidyOptionsTest.cpp | 21 +++++++++++++-------- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp index 1de1b1b..f17ef71 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp @@ -360,7 +360,7 @@ FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) { if ((*Text)->getBuffer().empty()) continue; llvm::ErrorOr ParsedOptions = - ConfigHandler.second((*Text)->getBuffer()); + ConfigHandler.second({(*Text)->getBuffer(), ConfigFile}); if (!ParsedOptions) { if (ParsedOptions.getError()) llvm::errs() << "Error parsing " << ConfigFile << ": " @@ -380,7 +380,8 @@ std::error_code parseLineFilter(StringRef LineFilter, return Input.error(); } -llvm::ErrorOr parseConfiguration(StringRef Config) { +llvm::ErrorOr +parseConfiguration(llvm::MemoryBufferRef Config) { llvm::yaml::Input Input(Config); ClangTidyOptions Options; Input >> Options; diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h index b4a00f6..e7cd899 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h @@ -192,7 +192,7 @@ protected: // A pair of configuration file base name and a function parsing // configuration from text in the corresponding format. typedef std::pair( - llvm::StringRef)>> + llvm::MemoryBufferRef)>> ConfigFileHandler; /// Configuration file handlers listed in the order of priority. @@ -308,7 +308,8 @@ std::error_code parseLineFilter(llvm::StringRef LineFilter, /// Parses configuration from JSON and returns \c ClangTidyOptions or an /// error. -llvm::ErrorOr parseConfiguration(llvm::StringRef Config); +llvm::ErrorOr +parseConfiguration(llvm::MemoryBufferRef Config); /// Serializes configuration to a YAML-encoded string. std::string configurationAsText(const ClangTidyOptions &Options); diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index beef79e..2748fd9 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -312,10 +312,11 @@ static std::unique_ptr createOptionsProvider( if (UseColor.getNumOccurrences() > 0) OverrideOptions.UseColor = UseColor; - auto LoadConfig = [&](StringRef Configuration) - -> std::unique_ptr { + auto LoadConfig = + [&](StringRef Configuration, + StringRef Source) -> std::unique_ptr { llvm::ErrorOr ParsedConfig = - parseConfiguration(Configuration); + parseConfiguration(MemoryBufferRef(Configuration, Source)); if (ParsedConfig) return std::make_unique( std::move(GlobalOptions), @@ -334,18 +335,18 @@ static std::unique_ptr createOptionsProvider( } llvm::ErrorOr> Text = - llvm::MemoryBuffer::getFile(ConfigFile.c_str()); + llvm::MemoryBuffer::getFile(ConfigFile); if (std::error_code EC = Text.getError()) { llvm::errs() << "Error: can't read config-file '" << ConfigFile << "': " << EC.message() << "\n"; return nullptr; } - return LoadConfig((*Text)->getBuffer()); + return LoadConfig((*Text)->getBuffer(), ConfigFile); } if (Config.getNumOccurrences() > 0) - return LoadConfig(Config); + return LoadConfig(Config, ""); return std::make_unique( std::move(GlobalOptions), std::move(DefaultOptions), diff --git a/clang-tools-extra/clangd/TidyProvider.cpp b/clang-tools-extra/clangd/TidyProvider.cpp index f51ac91..730a402 100644 --- a/clang-tools-extra/clangd/TidyProvider.cpp +++ b/clang-tools-extra/clangd/TidyProvider.cpp @@ -41,7 +41,8 @@ public: [this](llvm::Optional Data) { Value.reset(); if (Data && !Data->empty()) { - if (auto Parsed = tidy::parseConfiguration(*Data)) + if (auto Parsed = tidy::parseConfiguration( + llvm::MemoryBufferRef(*Data, path()))) Value = std::make_shared( std::move(*Parsed)); else diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp index 7e71f28..40cd9a5 100644 --- a/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp @@ -72,10 +72,11 @@ TEST(ParseLineFilter, ValidFilter) { TEST(ParseConfiguration, ValidConfiguration) { llvm::ErrorOr Options = - parseConfiguration("Checks: \"-*,misc-*\"\n" - "HeaderFilterRegex: \".*\"\n" - "AnalyzeTemporaryDtors: true\n" - "User: some.user"); + parseConfiguration(llvm::MemoryBufferRef("Checks: \"-*,misc-*\"\n" + "HeaderFilterRegex: \".*\"\n" + "AnalyzeTemporaryDtors: true\n" + "User: some.user", + "Options")); EXPECT_TRUE(!!Options); EXPECT_EQ("-*,misc-*", *Options->Checks); EXPECT_EQ(".*", *Options->HeaderFilterRegex); @@ -83,7 +84,8 @@ TEST(ParseConfiguration, ValidConfiguration) { } TEST(ParseConfiguration, MergeConfigurations) { - llvm::ErrorOr Options1 = parseConfiguration(R"( + llvm::ErrorOr Options1 = + parseConfiguration(llvm::MemoryBufferRef(R"( Checks: "check1,check2" HeaderFilterRegex: "filter1" AnalyzeTemporaryDtors: true @@ -91,9 +93,11 @@ TEST(ParseConfiguration, MergeConfigurations) { ExtraArgs: ['arg1', 'arg2'] ExtraArgsBefore: ['arg-before1', 'arg-before2'] UseColor: false - )"); + )", + "Options1")); ASSERT_TRUE(!!Options1); - llvm::ErrorOr Options2 = parseConfiguration(R"( + llvm::ErrorOr Options2 = + parseConfiguration(llvm::MemoryBufferRef(R"( Checks: "check3,check4" HeaderFilterRegex: "filter2" AnalyzeTemporaryDtors: false @@ -101,7 +105,8 @@ TEST(ParseConfiguration, MergeConfigurations) { ExtraArgs: ['arg3', 'arg4'] ExtraArgsBefore: ['arg-before3', 'arg-before4'] UseColor: true - )"); + )", + "Options2")); ASSERT_TRUE(!!Options2); ClangTidyOptions Options = Options1->merge(*Options2, 0); EXPECT_EQ("check1,check2,check3,check4", *Options.Checks); -- 2.7.4