From 6b0020749d4b1711b669ba68fe74474f8241f084 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Sun, 17 Mar 2024 19:54:58 -0700 Subject: [PATCH] clang-tidy check infrastructure This brings in infrastructure for developing new custom clang-tidy lints and refactors for Lix. Change-Id: I3df5f5855712ab4f97d4e84d771e5e818f81f881 --- clang-tidy/.clang-format | 1 + clang-tidy/HasPrefixSuffix.cc | 80 ++++++++++++++++++++++++++++++++ clang-tidy/HasPrefixSuffix.hh | 25 ++++++++++ clang-tidy/NixClangTidyChecks.cc | 17 +++++++ clang-tidy/README.md | 56 ++++++++++++++++++++++ clang-tidy/meson.build | 8 ++++ flake.nix | 9 +++- 7 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 clang-tidy/.clang-format create mode 100644 clang-tidy/HasPrefixSuffix.cc create mode 100644 clang-tidy/HasPrefixSuffix.hh create mode 100644 clang-tidy/NixClangTidyChecks.cc create mode 100644 clang-tidy/README.md create mode 100644 clang-tidy/meson.build diff --git a/clang-tidy/.clang-format b/clang-tidy/.clang-format new file mode 100644 index 000000000..cd8995543 --- /dev/null +++ b/clang-tidy/.clang-format @@ -0,0 +1 @@ +BasedOnStyle: llvm diff --git a/clang-tidy/HasPrefixSuffix.cc b/clang-tidy/HasPrefixSuffix.cc new file mode 100644 index 000000000..e29b67e7c --- /dev/null +++ b/clang-tidy/HasPrefixSuffix.cc @@ -0,0 +1,80 @@ +#include "HasPrefixSuffix.hh" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace nix::clang_tidy { +using namespace clang::ast_matchers; +using namespace clang; + +void HasPrefixSuffixCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { + Finder->addMatcher( + traverse(clang::TK_AsIs, + callExpr(callee(functionDecl(anyOf(hasName("hasPrefix"), + hasName("hasSuffix"))) + .bind("callee-decl")), + optionally(hasArgument( + 0, cxxConstructExpr( + hasDeclaration(functionDecl(hasParameter( + 0, parmVarDecl(hasType( + asString("const char *"))))))) + .bind("implicit-cast")))) + .bind("call")), + this); +} + +void HasPrefixSuffixCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + + const auto *CalleeDecl = Result.Nodes.getNodeAs("callee-decl"); + auto FuncName = std::string(CalleeDecl->getName()); + std::string NewName; + if (FuncName == "hasPrefix") { + NewName = "starts_with"; + } else if (FuncName == "hasSuffix") { + NewName = "ends_with"; + } else { + llvm_unreachable("nix-has-prefix: invalid callee"); + } + + const auto *MatchedDecl = Result.Nodes.getNodeAs("call"); + const auto *ImplicitConvertArg = + Result.Nodes.getNodeAs("implicit-cast"); + + const auto *Lhs = MatchedDecl->getArg(0); + const auto *Rhs = MatchedDecl->getArg(1); + auto Diag = diag(MatchedDecl->getExprLoc(), FuncName + " is deprecated"); + + std::string Text = ""; + + // Form possible cast to string_view, or nothing. + if (ImplicitConvertArg) { + Text = "std::string_view("; + Text.append(tooling::getText(*Lhs, *Result.Context)); + Text.append(")."); + } else { + Text.append(*tooling::buildAccess(*Lhs, *Result.Context)); + } + + // Call .starts_with. + Text.append(NewName); + Text.push_back('('); + Text.append(tooling::getText(*Rhs, *Result.Context)); + Text.push_back(')'); + + Diag << FixItHint::CreateReplacement(MatchedDecl->getSourceRange(), Text); + + // for (const auto *arg : MatchedDecl->arguments()) { + // arg->dumpColor(); + // arg->getType().dump(); + // } +} +}; // namespace nix::clang_tidy diff --git a/clang-tidy/HasPrefixSuffix.hh b/clang-tidy/HasPrefixSuffix.hh new file mode 100644 index 000000000..8693b6767 --- /dev/null +++ b/clang-tidy/HasPrefixSuffix.hh @@ -0,0 +1,25 @@ +#pragma once +///@file +/// This is an example of a clang-tidy automated refactoring against the Nix +/// codebase. The refactoring has been completed in +/// https://gerrit.lix.systems/c/lix/+/565 so this code is around as +/// an example. + +#include +#include +#include + +namespace nix::clang_tidy { + +using namespace clang; +using namespace clang::tidy; +using namespace llvm; + +class HasPrefixSuffixCheck : public ClangTidyCheck { +public: + HasPrefixSuffixCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; +}; // namespace nix::clang_tidy diff --git a/clang-tidy/NixClangTidyChecks.cc b/clang-tidy/NixClangTidyChecks.cc new file mode 100644 index 000000000..8f0309107 --- /dev/null +++ b/clang-tidy/NixClangTidyChecks.cc @@ -0,0 +1,17 @@ +#include +#include +#include "HasPrefixSuffix.hh" + +namespace nix::clang_tidy { +using namespace clang; +using namespace clang::tidy; + +class NixClangTidyChecks : public ClangTidyModule { + public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck("nix-hasprefixsuffix"); + } +}; + +static ClangTidyModuleRegistry::Add X("nix-module", "Adds nix specific checks"); +}; diff --git a/clang-tidy/README.md b/clang-tidy/README.md new file mode 100644 index 000000000..cf46c71ec --- /dev/null +++ b/clang-tidy/README.md @@ -0,0 +1,56 @@ +# Clang tidy lints for Nix + +This is a skeleton of a clang-tidy lints library for Nix. + +Currently there is one check (which is already obsolete as it has served its +goal and is there as an example), `HasPrefixSuffixCheck`. + +## Running fixes/checks + +One file: + +``` +ninja -C build && clang-tidy --checks='-*,nix-*' --load=build/libnix-clang-tidy.so -p ../compile_commands.json --fix ../src/libcmd/installables.cc +``` + +Several files, in parallel: + +``` +ninja -C build && run-clang-tidy -checks='-*,nix-*' -load=build/libnix-clang-tidy.so -p .. -fix ../src | tee -a clang-tidy-result +``` + +## Resources + +* https://firefox-source-docs.mozilla.org/code-quality/static-analysis/writing-new/clang-query.html +* https://clang.llvm.org/docs/LibASTMatchersReference.html +* https://devblogs.microsoft.com/cppblog/exploring-clang-tooling-part-3-rewriting-code-with-clang-tidy/ + +## Developing new checks + +Put something like so in `myquery.txt`: + +``` +set traversal IgnoreUnlessSpelledInSource +# ^ Ignore implicit AST nodes. May need to use AsIs depending on how you are +# working. +set bind-root true +# ^ true unless you use any .bind("foo") commands +set print-matcher true +enable output dump +match callExpr(callee(functionDecl(hasName("hasPrefix"))), optionally(hasArgument( 0, cxxConstructExpr(hasDeclaration(functionDecl(hasParameter(0, parmVarDecl(hasType(asString("const char *"))).bind("meow2")))))))) +``` + +Then run, e.g. `clang-query --preload hasprefix.query -p compile_commands.json src/libcmd/installables.cc`. + +With this you can iterate a query before writing it in C++ and suffering from +C++. + +### Tips and tricks for the C++ + +There is a function `dump()` on many things that will dump to stderr. Also +`llvm::errs()` lets you print to stderr. + +When I wrote `HasPrefixSuffixCheck`, I was not really able to figure out how +the structured replacement system was supposed to work. In principle you can +describe the replacement with a nice DSL. Look up the Stencil system in Clang +for details. diff --git a/clang-tidy/meson.build b/clang-tidy/meson.build new file mode 100644 index 000000000..48770e39f --- /dev/null +++ b/clang-tidy/meson.build @@ -0,0 +1,8 @@ +project('nix-clang-tidy', ['cpp', 'c'], + version : '0.1', + default_options : ['warning_level=3', 'cpp_std=c++20']) + +llvm = dependency('Clang', version: '>= 14', modules: ['libclang']) +sources = ['HasPrefixSuffix.cc', 'NixClangTidyChecks.cc'] +shared_module('nix-clang-tidy', sources, + dependencies: llvm) diff --git a/flake.nix b/flake.nix index 11c461896..3ee9a4994 100644 --- a/flake.nix +++ b/flake.nix @@ -317,11 +317,18 @@ }; in nix.overrideAttrs (prev: { + # Required for clang-tidy checks + buildInputs = prev.buildInputs ++ lib.optionals (stdenv.cc.isClang) [ pkgs.llvmPackages.llvm pkgs.llvmPackages.clang-unwrapped.dev ]; nativeBuildInputs = prev.nativeBuildInputs ++ lib.optional (stdenv.cc.isClang && !stdenv.buildPlatform.isDarwin) pkgs.buildPackages.bear + # Required for clang-tidy checks + ++ lib.optionals (stdenv.cc.isClang) [ pkgs.buildPackages.cmake pkgs.buildPackages.ninja pkgs.buildPackages.llvmPackages.llvm.dev ] ++ lib.optional (stdenv.cc.isClang && stdenv.hostPlatform == stdenv.buildPlatform) - pkgs.buildPackages.clang-tools; + # for some reason that seems accidental and was changed in + # NixOS 24.05-pre, clang-tools is pinned to LLVM 14 when + # default LLVM is newer. + (pkgs.buildPackages.clang-tools.override { inherit (pkgs.buildPackages) llvmPackages; }); src = null;