njames93 created this revision.
njames93 added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, mgorny.
Herald added a project: clang.
njames93 requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
First step of implementing clang-tidy configuration into clangd config.
This is just getting a few of the basics working, like reading the
configuration and a simple implementation to get clang-tidy to use those values.
However right now its achieving the latter in a much less than ideal way so
definitely need to clean that up.
Also need to add some tests here and there also.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D90531
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigTidyProvider.cpp
clang-tools-extra/clangd/ConfigTidyProvider.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -8,6 +8,7 @@
#include "ClangdLSPServer.h"
#include "CodeComplete.h"
+#include "ConfigTidyProvider.h"
#include "Features.inc"
#include "PathMapping.h"
#include "Protocol.h"
@@ -811,10 +812,18 @@
tidy::ClangTidyOptions OverrideClangTidyOptions;
if (!ClangTidyChecks.empty())
OverrideClangTidyOptions.Checks = ClangTidyChecks;
- ClangTidyOptProvider = std::make_unique<tidy::FileOptionsProvider>(
- tidy::ClangTidyGlobalOptions(),
- /* Default */ EmptyDefaults,
- /* Override */ OverrideClangTidyOptions, TFS.view(/*CWD=*/llvm::None));
+ if (Opts.ConfigProvider)
+ ClangTidyOptProvider = std::make_unique<ConfigTidyOptionsProvider>(
+ tidy::ClangTidyGlobalOptions(),
+ /* Default */ EmptyDefaults, *Opts.ConfigProvider,
+ /* Override */ OverrideClangTidyOptions,
+ TFS.view(/*CWD=*/llvm::None));
+ else
+ ClangTidyOptProvider = std::make_unique<tidy::FileOptionsProvider>(
+ tidy::ClangTidyGlobalOptions(),
+ /* Default */ EmptyDefaults,
+ /* Override */ OverrideClangTidyOptions,
+ TFS.view(/*CWD=*/llvm::None));
Opts.GetClangTidyOptions = [&](llvm::vfs::FileSystem &,
llvm::StringRef File) {
// This function must be thread-safe and tidy option providers are not.
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -40,6 +40,7 @@
Dict.handle("CompileFlags", [&](Node &N) { parse(F.CompileFlags, N); });
Dict.handle("Index", [&](Node &N) { parse(F.Index, N); });
Dict.handle("Style", [&](Node &N) { parse(F.Style, N); });
+ Dict.handle("Tidy", [&](Node &N) { parse(F.Tidy, N); });
Dict.parse(N);
return !(N.failed() || HadError);
}
@@ -82,6 +83,51 @@
Dict.parse(N);
}
+ void parse(Fragment::TidyBlock &F, Node &N) {
+ DictParser Dict("Tidy", this);
+ Dict.handle("Enable", [&](Node &N) {
+ if (auto Value = scalarBool(N, "Enable"))
+ F.Enable = *Value;
+ });
+ Dict.handle("Add", [&](Node &N) {
+ if (auto Values = scalarValues(N))
+ F.Add = std::move(*Values);
+ });
+ Dict.handle("Remove", [&](Node &N) {
+ if (auto Values = scalarValues(N))
+ F.Remove = std::move(*Values);
+ });
+ Dict.handle("CheckOptions", [&](Node &N) {
+ if (N.getType() != Node::NK_Mapping) {
+ this->error("CheckOptions should be a dictionary", N);
+ return;
+ }
+ llvm::SmallSet<std::string, 8> Seen;
+
+ for (auto &KV : llvm::cast<MappingNode>(N)) {
+ auto *K = KV.getKey();
+ if (!K) // YAMLParser emitted an error.
+ continue;
+ auto Key = this->scalarValue(*K, "CheckOption Key");
+ if (!Key)
+ continue;
+ if (!Seen.insert(**Key).second) {
+ this->warning("Duplicate CheckOption " + **Key + " is ignored", *K);
+ continue;
+ }
+ auto *V = KV.getValue();
+ if (!V) // YAMLParser emitted an error.
+ continue;
+ auto Value = this->scalarValue(*V, **Key);
+ if (!Value) // Couldn't parse V as a String
+ continue;
+ F.CheckOptions.emplace_back(
+ std::make_pair(std::move(**Key), std::move(**Value)),
+ N.getSourceRange());
+ }
+ });
+ }
+
void parse(Fragment::IndexBlock &F, Node &N) {
DictParser Dict("Index", this);
Dict.handle("Background",
@@ -157,6 +203,29 @@
}
};
+ // Try to parse a single boolean value from the node, warn on failure.
+ llvm::Optional<Located<bool>> scalarBool(Node &N, llvm::StringRef Desc) {
+ llvm::SmallString<256> Buf;
+ llvm::StringRef Str;
+ if (auto *S = llvm::dyn_cast<ScalarNode>(&N))
+ Str = S->getValue(Buf).trim();
+ else if (auto *BS = llvm::dyn_cast<BlockScalarNode>(&N))
+ Str = BS->getValue().trim();
+ else {
+ warning(Desc + " should be a boolean", N);
+ return llvm::None;
+ }
+ if (Str.equals_lower("true"))
+ return Located<bool>(true, N.getSourceRange());
+ if (Str.equals_lower("false"))
+ return Located<bool>(false, N.getSourceRange());
+ bool Result;
+ if (!Str.getAsInteger(10, Result))
+ return Located<bool>(Result, N.getSourceRange());
+ warning("Couldn't parse '" + Str + "' as a boolean for " + Desc, N);
+ return llvm::None;
+ }
+
// Try to parse a single scalar value from the node, warn on failure.
llvm::Optional<Located<std::string>> scalarValue(Node &N,
llvm::StringRef Desc) {
Index: clang-tools-extra/clangd/ConfigTidyProvider.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/ConfigTidyProvider.h
@@ -0,0 +1,43 @@
+//===--- ConfigTidyProvider.h ------------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Creates a ClangTidyOptionsProvider that merges a users clangd config with
+// .clang-tidy files
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGTIDYPROVIDER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGTIDYPROVIDER_H
+
+#include "../clang-tidy/ClangTidyOptions.h"
+
+namespace clang {
+namespace clangd {
+namespace config {
+class Provider;
+} // namespace config
+
+class ConfigTidyOptionsProvider : public tidy::FileOptionsBaseProvider {
+public:
+ ConfigTidyOptionsProvider(
+ const tidy::ClangTidyGlobalOptions &GlobalOptions,
+ const tidy::ClangTidyOptions &DefaultOptions,
+ config::Provider &ConfigProvider,
+ const tidy::ClangTidyOptions &OverrideOptions,
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr);
+
+ std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+
+private:
+ config::Provider &ConfigProvider;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGTIDYPROVIDER_H
\ No newline at end of file
Index: clang-tools-extra/clangd/ConfigTidyProvider.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/ConfigTidyProvider.cpp
@@ -0,0 +1,57 @@
+//===--- ConfigTidyProvider.cpp ----------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConfigTidyProvider.h"
+#include "../clang-tidy/ClangTidyOptions.h"
+#include "Config.h"
+#include "ConfigProvider.h"
+
+namespace clang {
+namespace clangd {
+ConfigTidyOptionsProvider::ConfigTidyOptionsProvider(
+ const tidy::ClangTidyGlobalOptions &GlobalOptions,
+ const tidy::ClangTidyOptions &DefaultOptions,
+ config::Provider &ConfigProvider,
+ const tidy::ClangTidyOptions &OverrideOptions,
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS)
+ : tidy::FileOptionsBaseProvider(GlobalOptions, DefaultOptions,
+ OverrideOptions, FS),
+ ConfigProvider(ConfigProvider) {}
+
+std::vector<tidy::ClangTidyOptionsProvider::OptionsSource>
+ConfigTidyOptionsProvider::getRawOptions(llvm::StringRef FileName) {
+ std::vector<OptionsSource> RawOptions =
+ DefaultOptionsProvider::getRawOptions(FileName);
+ assert(FS && "FS must be set.");
+
+ llvm::SmallString<128> AbsoluteFilePath(FileName);
+
+ if (!FS->makeAbsolute(AbsoluteFilePath)) {
+ addRawFileOptions(AbsoluteFilePath, RawOptions);
+ }
+
+ config::Params P;
+ P.Path = AbsoluteFilePath;
+ auto IgnoreDiag = [](const llvm::SMDiagnostic &) {};
+ Config C = ConfigProvider.getConfig(P, IgnoreDiag);
+
+ tidy::ClangTidyOptions ConfigOptions;
+
+ if (!C.ClangTidy.Checks.empty())
+ ConfigOptions.Checks = C.ClangTidy.Checks;
+ for (auto &Item : C.ClangTidy.CheckOptions)
+ ConfigOptions.CheckOptions.try_emplace(Item.first, Item.second);
+
+ RawOptions.emplace_back(ConfigOptions, ".clangd config");
+
+ RawOptions.emplace_back(OverrideOptions,
+ "command-line option '-clang-tidy-checks'");
+ return RawOptions;
+}
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -171,6 +171,32 @@
std::vector<Located<std::string>> FullyQualifiedNamespaces;
};
StyleBlock Style;
+
+ /// This section controls how clang-tidy will run over the code base
+ ///
+ /// The settings are merged with any settings found in .clang-tidy
+ /// configiration files with these ones taking precedence.
+ struct TidyBlock {
+ llvm::Optional<Located<bool>> Enable;
+ /// List of checks to enable or disable, can use wildcards.
+ /// \ref Add checks get ran first, then \ref Remove checks. This lets you
+ /// enable all checks from a module apart from a few specific checks,
+ /// Example:
+ /// Add: llvm-
+ /// Remove: llvm-include-order
+ /// This will enable all checks from the llvm module apart from
+ /// llvm-include-order.
+ std::vector<Located<std::string>> Add;
+ std::vector<Located<std::string>> Remove;
+
+ /// A Key-Value pair list of options to pass to clang-tidy checks
+ /// These take precedence over options specified in clang-tidy configuration
+ /// files. Example:
+ /// CheckOptions:
+ /// readability-braces-around-statements.ShortStatementLines: 2
+ std::vector<Located<std::pair<std::string, std::string>>> CheckOptions;
+ };
+ TidyBlock Tidy;
};
} // namespace config
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -132,6 +132,7 @@
compile(std::move(F.If));
compile(std::move(F.CompileFlags));
compile(std::move(F.Index));
+ compile(std::move(F.Tidy));
}
void compile(Fragment::IfBlock &&F) {
@@ -227,6 +228,62 @@
}
}
+ void checkAdjuster(std::string &Adjuster, const Located<std::string> &Arg,
+ bool IsPositive) {
+ SmallString<64> DiagBuffer;
+ StringRef Str = *Arg;
+ StringRef Type(IsPositive ? "Add" : "Remove");
+ // Don't support negating here, its handled if the item is in the Add or
+ // Remove list.
+ if (Str[0] == '-') {
+ diag(Error,
+ (Type + " check item can't start with '-'").toStringRef(DiagBuffer),
+ Arg.Range);
+ return;
+ }
+ if (Str.contains(',')) {
+ diag(Error,
+ ("Invalid character ',' found in " + Type + " check item")
+ .toStringRef(DiagBuffer),
+ Arg.Range);
+ return;
+ }
+ if (!IsPositive)
+ Adjuster += '-';
+ Adjuster += Str;
+ }
+
+ void compile(Fragment::TidyBlock &&F) {
+ if (F.Enable)
+ Out.Apply.push_back(
+ [Enable = **F.Enable](Config &C) { C.ClangTidy.Enable = Enable; });
+
+ std::string Checks;
+ if (!F.Add.empty())
+ llvm::for_each(F.Add, std::bind(&FragmentCompiler::checkAdjuster, this,
+ Checks, std::placeholders::_1, true));
+
+ if (!F.Remove.empty())
+ llvm::for_each(F.Remove, std::bind(&FragmentCompiler::checkAdjuster, this,
+ Checks, std::placeholders::_1, false));
+
+ if (!Checks.empty())
+ Out.Apply.push_back([Checks = std::move(Checks)](Config &C) {
+ C.ClangTidy.Checks = std::move(Checks);
+ });
+ if (!F.CheckOptions.empty()) {
+ std::vector<std::pair<std::string, std::string>> CheckOptions;
+ for (auto &Opt : F.CheckOptions)
+ CheckOptions.push_back(std::move(*Opt));
+ Out.Apply.push_back([CheckOptions = std::move(CheckOptions)](Config &C) {
+ C.ClangTidy.CheckOptions.insert(
+ C.ClangTidy.CheckOptions.end(),
+ std::make_move_iterator(CheckOptions.begin()),
+ std::make_move_iterator(CheckOptions.end()));
+ });
+ }
+ }
+
constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
constexpr static llvm::SourceMgr::DiagKind Warning =
llvm::SourceMgr::DK_Warning;
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -70,6 +70,13 @@
// ::). All nested namespaces are affected as well.
std::vector<std::string> FullyQualifiedNamespaces;
} Style;
+
+ // Configures what clang-tidy checks to run and options to use with them.
+ struct {
+ bool Enable;
+ std::string Checks;
+ std::vector<std::pair<std::string, std::string>> CheckOptions;
+ } ClangTidy;
};
} // namespace clangd
Index: clang-tools-extra/clangd/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -49,6 +49,7 @@
Compiler.cpp
Config.cpp
ConfigCompile.cpp
+ ConfigTidyProvider.cpp
ConfigProvider.cpp
ConfigYAML.cpp
Diagnostics.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
