From 3daeeaefb115f47bf52ae96fa2b3f01842ec0f0f Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 31 Jul 2024 20:37:27 -0700 Subject: build: implement clang-tidy using our plugin The principle of this is that you can either externally build it with Nix (actual implementation will be in a future commit), or it can be built with meson if the Nix one is not passed in. The idea I have is that dev shells don't receive the one from Nix to avoid having to build it, but CI can use the one from Nix and save some gratuitous rebuilds. The design of this is that you can run `ninja -C build clang-tidy` and it will simply correctly clang-tidy the codebase in spite of PCH bullshit caused by the cc-wrapper. This is a truly horrendous number of hacks in a ball, caused by bugs in several pieces of software, and I am not even getting started. I don't consider this to fix the clang-tidy issue filing, since we still have a fair number of issues to fix even on the existing minimal configuration, and I have not yet implemented it in CI. Realistically we will need to do something like https://github.com/Ericsson/codechecker to be able to silence warnings without physically touching the code, or at least *diff* reports between versions. Also, the run-clang-tidy output design is rather atrocious and must not be inflicted upon anyone I have respect for, since it buries the diagnostics in a pile of invocation logs. We would do really well to integrate with the Gerrit SARIF stuff so we can dump the reports on people in a user-friendly manner. Related: https://git.lix.systems/lix-project/lix/issues/147 Change-Id: Ifefe533f3b56874795de231667046b2da6ff2461 --- clang-tidy/.clang-format | 1 - clang-tidy/.editorconfig | 4 - clang-tidy/FixIncludes.cc | 90 ---------------------- clang-tidy/FixIncludes.hh | 21 ----- clang-tidy/HasPrefixSuffix.cc | 80 ------------------- clang-tidy/HasPrefixSuffix.hh | 25 ------ clang-tidy/LixClangTidyChecks.cc | 19 ----- clang-tidy/README.md | 56 -------------- clang-tidy/meson.build | 13 ---- doc/manual/rl-next/clang-tidy-sorta.md | 10 +++ flake.nix | 4 + justfile | 10 +++ meson.build | 2 + meson.options | 8 +- meson/clang-tidy/build_required_targets.py | 21 +++++ meson/clang-tidy/clean_compdb.py | 53 +++++++++++++ meson/clang-tidy/fake.cc | 0 meson/clang-tidy/meson.build | 98 ++++++++++++++++++++++++ subprojects/lix-clang-tidy/.clang-format | 1 + subprojects/lix-clang-tidy/.editorconfig | 4 + subprojects/lix-clang-tidy/FixIncludes.cc | 90 ++++++++++++++++++++++ subprojects/lix-clang-tidy/FixIncludes.hh | 21 +++++ subprojects/lix-clang-tidy/HasPrefixSuffix.cc | 80 +++++++++++++++++++ subprojects/lix-clang-tidy/HasPrefixSuffix.hh | 25 ++++++ subprojects/lix-clang-tidy/LixClangTidyChecks.cc | 19 +++++ subprojects/lix-clang-tidy/README.md | 56 ++++++++++++++ subprojects/lix-clang-tidy/default.nix | 44 +++++++++++ subprojects/lix-clang-tidy/meson.build | 18 +++++ subprojects/lix-clang-tidy/meson.options | 3 + 29 files changed, 565 insertions(+), 311 deletions(-) delete mode 100644 clang-tidy/.clang-format delete mode 100644 clang-tidy/.editorconfig delete mode 100644 clang-tidy/FixIncludes.cc delete mode 100644 clang-tidy/FixIncludes.hh delete mode 100644 clang-tidy/HasPrefixSuffix.cc delete mode 100644 clang-tidy/HasPrefixSuffix.hh delete mode 100644 clang-tidy/LixClangTidyChecks.cc delete mode 100644 clang-tidy/README.md delete mode 100644 clang-tidy/meson.build create mode 100644 doc/manual/rl-next/clang-tidy-sorta.md create mode 100755 meson/clang-tidy/build_required_targets.py create mode 100755 meson/clang-tidy/clean_compdb.py create mode 100644 meson/clang-tidy/fake.cc create mode 100644 meson/clang-tidy/meson.build create mode 100644 subprojects/lix-clang-tidy/.clang-format create mode 100644 subprojects/lix-clang-tidy/.editorconfig create mode 100644 subprojects/lix-clang-tidy/FixIncludes.cc create mode 100644 subprojects/lix-clang-tidy/FixIncludes.hh create mode 100644 subprojects/lix-clang-tidy/HasPrefixSuffix.cc create mode 100644 subprojects/lix-clang-tidy/HasPrefixSuffix.hh create mode 100644 subprojects/lix-clang-tidy/LixClangTidyChecks.cc create mode 100644 subprojects/lix-clang-tidy/README.md create mode 100644 subprojects/lix-clang-tidy/default.nix create mode 100644 subprojects/lix-clang-tidy/meson.build create mode 100644 subprojects/lix-clang-tidy/meson.options diff --git a/clang-tidy/.clang-format b/clang-tidy/.clang-format deleted file mode 100644 index cd8995543..000000000 --- a/clang-tidy/.clang-format +++ /dev/null @@ -1 +0,0 @@ -BasedOnStyle: llvm diff --git a/clang-tidy/.editorconfig b/clang-tidy/.editorconfig deleted file mode 100644 index 19ee09eec..000000000 --- a/clang-tidy/.editorconfig +++ /dev/null @@ -1,4 +0,0 @@ -# LLVM style code is 2-space indented -[*.{cc,hh}] -indent_style = space -indent_size = 2 diff --git a/clang-tidy/FixIncludes.cc b/clang-tidy/FixIncludes.cc deleted file mode 100644 index 602d3d355..000000000 --- a/clang-tidy/FixIncludes.cc +++ /dev/null @@ -1,90 +0,0 @@ -#include "FixIncludes.hh" -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace nix::clang_tidy { - -using namespace clang; -using namespace clang::tidy; - -class FixIncludesCallbacks : public PPCallbacks { -public: - ClangTidyCheck &Check; - Preprocessor &PP; - FixIncludesCallbacks(ClangTidyCheck &Check, Preprocessor &PP) - : Check(Check), PP(PP) {} - -private: - bool Ignore = false; - virtual void LexedFileChanged(FileID FID, LexedFileChangeReason Reason, - SrcMgr::CharacteristicKind FileType, - FileID PrevFID, SourceLocation Loc) override; - - virtual void InclusionDirective(SourceLocation HashLoc, - const Token &IncludeTok, StringRef FileName, - bool IsAngled, CharSourceRange FilenameRange, - OptionalFileEntryRef File, - StringRef SearchPath, StringRef RelativePath, - const Module *Imported, - SrcMgr::CharacteristicKind FileType) override; -}; - -void FixIncludesCallbacks::LexedFileChanged(FileID, LexedFileChangeReason, - SrcMgr::CharacteristicKind FileType, - FileID, SourceLocation) { - Ignore = FileType != SrcMgr::C_User; -} - -void FixIncludesCallbacks::InclusionDirective( - SourceLocation, const Token &, StringRef FileName, bool IsAngled, - CharSourceRange FilenameRange, OptionalFileEntryRef File, StringRef, - StringRef, const Module *, SrcMgr::CharacteristicKind) { - if (Ignore) - return; - - // FIXME: this is kinda evil, but this is a one-time fixup - const std::vector SourceDirs = {"src/", "include/lix/"}; - - const auto Bracketize = [IsAngled](StringRef s) { - return IsAngled ? ("<" + s + ">").str() : ("\"" + s + "\"").str(); - }; - - for (const auto &SourceDir : SourceDirs) { - const bool IsAlreadyFixed = FileName.starts_with("lix/lib"); - if (File && File->getNameAsRequested().contains(SourceDir) && - !IsAlreadyFixed) { - StringRef Name = File->getNameAsRequested(); - auto Idx = Name.find(SourceDir); - assert(Idx != std::string::npos); - std::string Suffix = Name.drop_front(Idx + SourceDir.length()).str(); - - if (!Suffix.starts_with("lib")) { - llvm::dbgs() << "ignored: " << Suffix << "\n"; - return; - } - - Suffix = "lix/" + Suffix; - - auto Diag = Check.diag(FilenameRange.getBegin(), - "include needs to specify the source subdir"); - - Diag << FilenameRange - << FixItHint::CreateReplacement(FilenameRange, Bracketize(Suffix)); - } - } -} - -void FixIncludesCheck::registerPPCallbacks(const SourceManager &, - Preprocessor *PP, Preprocessor *) { - PP->addPPCallbacks(std::make_unique(*this, *PP)); -} - -}; // namespace nix::clang_tidy diff --git a/clang-tidy/FixIncludes.hh b/clang-tidy/FixIncludes.hh deleted file mode 100644 index ea890cd39..000000000 --- a/clang-tidy/FixIncludes.hh +++ /dev/null @@ -1,21 +0,0 @@ -#pragma once -///@file - -#include -#include -#include - -namespace nix::clang_tidy { - -using namespace clang; -using namespace clang::tidy; - -class FixIncludesCheck : public ClangTidyCheck { - public: - FixIncludesCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - - void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; -}; - -}; diff --git a/clang-tidy/HasPrefixSuffix.cc b/clang-tidy/HasPrefixSuffix.cc deleted file mode 100644 index e29b67e7c..000000000 --- a/clang-tidy/HasPrefixSuffix.cc +++ /dev/null @@ -1,80 +0,0 @@ -#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 deleted file mode 100644 index 8693b6767..000000000 --- a/clang-tidy/HasPrefixSuffix.hh +++ /dev/null @@ -1,25 +0,0 @@ -#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/LixClangTidyChecks.cc b/clang-tidy/LixClangTidyChecks.cc deleted file mode 100644 index b3503dd3a..000000000 --- a/clang-tidy/LixClangTidyChecks.cc +++ /dev/null @@ -1,19 +0,0 @@ -#include -#include -#include "FixIncludes.hh" -#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("lix-hasprefixsuffix"); - CheckFactories.registerCheck("lix-fixincludes"); - } -}; - -static ClangTidyModuleRegistry::Add X("lix-module", "Adds lix specific checks"); -}; diff --git a/clang-tidy/README.md b/clang-tidy/README.md deleted file mode 100644 index c2d1cb258..000000000 --- a/clang-tidy/README.md +++ /dev/null @@ -1,56 +0,0 @@ -# Clang tidy lints for Lix - -This is a skeleton of a clang-tidy lints library for Lix. - -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='-*,lix-*' --load=build/liblix-clang-tidy.so -p ../compile_commands.json -header-filter '\.\./src/.*\.h' --fix ../src/libcmd/installables.cc -``` - -Several files, in parallel: - -``` -ninja -C build && run-clang-tidy -checks='-*,lix-*' -load=build/liblix-clang-tidy.so -p .. -header-filter '\.\./src/.*\.h' -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 deleted file mode 100644 index c59164a72..000000000 --- a/clang-tidy/meson.build +++ /dev/null @@ -1,13 +0,0 @@ -project('lix-clang-tidy', ['cpp', 'c'], - version : '0.1', - default_options : ['warning_level=3', 'cpp_std=c++20']) - -llvm = dependency('Clang', version: '>= 14', modules: ['libclang']) -sources = files( - 'HasPrefixSuffix.cc', - 'LixClangTidyChecks.cc', - 'FixIncludes.cc', -) - -shared_module('lix-clang-tidy', sources, - dependencies: llvm) diff --git a/doc/manual/rl-next/clang-tidy-sorta.md b/doc/manual/rl-next/clang-tidy-sorta.md new file mode 100644 index 000000000..5f3f84348 --- /dev/null +++ b/doc/manual/rl-next/clang-tidy-sorta.md @@ -0,0 +1,10 @@ +--- +synopsis: "clang-tidy support" +cls: 1697 +issues: fj#147 +credits: jade +category: Development +--- + +`clang-tidy` can be used to lint Lix with a limited set of lints using `ninja -C build clang-tidy` and `ninja -C build clang-tidy-fix`. +In practice, this fixes the built-in meson rule that was used the same as above being broken ever since precompiled headers were introduced. diff --git a/flake.nix b/flake.nix index a1fc947b7..14fdf4147 100644 --- a/flake.nix +++ b/flake.nix @@ -197,6 +197,8 @@ busybox-sandbox-shell = final.busybox-sandbox-shell or final.default-busybox-sandbox-shell; }; + lix-clang-tidy = final.callPackage ./subprojects/lix-clang-tidy { }; + # Export the patched version of boehmgc that Lix uses into the overlay # for consumers of this flake. boehmgc-nix = final.nix.passthru.boehmgc-nix; @@ -384,6 +386,8 @@ rec { inherit (nixpkgsFor.${system}.native) nix; default = nix; + + inherit (nixpkgsFor.${system}.native) lix-clang-tidy; } // ( lib.optionalAttrs (builtins.elem system linux64BitSystems) { diff --git a/justfile b/justfile index 6a93fa63f..1576c759d 100644 --- a/justfile +++ b/justfile @@ -25,3 +25,13 @@ install *OPTIONS: (build OPTIONS) # Run tests test *OPTIONS: meson test -C build --print-errorlogs {{ OPTIONS }} + +alias clang-tidy := lint + +lint: + ninja -C build clang-tidy + +alias clang-tidy-fix := lint-fix + +lint-fix: + ninja -C build clang-tidy-fix diff --git a/meson.build b/meson.build index ed50dff78..8577657d9 100644 --- a/meson.build +++ b/meson.build @@ -548,3 +548,5 @@ if enable_tests subdir('tests/unit') subdir('tests/functional') endif + +subdir('meson/clang-tidy') diff --git a/meson.options b/meson.options index fc2050809..679d88347 100644 --- a/meson.options +++ b/meson.options @@ -1,7 +1,7 @@ # vim: filetype=meson option('enable-build', type : 'boolean', value : true, - description : 'Set to false to not actually build. Only really makes sense with -Dinternal-api-docs=true', + description : 'set to false to not actually build. Only really makes sense with -Dinternal-api-docs=true', ) option('gc', type : 'feature', @@ -37,7 +37,7 @@ option('tests-brief', type : 'boolean', value : false, ) option('profile-build', type : 'feature', value: 'disabled', - description : 'whether to enable -ftime-trace in clang builds, allowing for speeding up the build.' + description : 'whether to enable -ftime-trace in clang builds, allowing for diagnosing the cause of build time.' ) option('store-dir', type : 'string', value : '/nix/store', @@ -68,3 +68,7 @@ option('profile-dir', type : 'string', value : 'etc/profile.d', option('enable-pch-std', type : 'boolean', value : true, description : 'whether to use precompiled headers for C++\'s standard library (breaks clangd if you\'re using GCC)', ) + +option('lix-clang-tidy-checks-path', type : 'string', value : '', + description: 'path to lix-clang-tidy-checks library file, if providing it externally. Uses an internal one if this is not set', +) diff --git a/meson/clang-tidy/build_required_targets.py b/meson/clang-tidy/build_required_targets.py new file mode 100755 index 000000000..5c0e9641e --- /dev/null +++ b/meson/clang-tidy/build_required_targets.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python3 +import subprocess + +def get_targets_of_rule(build_root: str, rule_name: str) -> list[str]: + return subprocess.check_output(['ninja', '-C', build_root, '-t', 'targets', 'rule', rule_name]).decode().strip().splitlines() + +def ninja_build(build_root: str, targets: list[str]): + subprocess.check_call(['ninja', '-C', build_root, '--', *targets]) + +def main(): + import argparse + ap = argparse.ArgumentParser(description='Builds required targets for clang-tidy') + ap.add_argument('build_root', help='Ninja build root', type=str) + + args = ap.parse_args() + + targets = [t for t in get_targets_of_rule(args.build_root, 'CUSTOM_COMMAND') if t.endswith('gen.hh')] + ninja_build(args.build_root, targets) + +if __name__ == '__main__': + main() diff --git a/meson/clang-tidy/clean_compdb.py b/meson/clang-tidy/clean_compdb.py new file mode 100755 index 000000000..6736fe63a --- /dev/null +++ b/meson/clang-tidy/clean_compdb.py @@ -0,0 +1,53 @@ +#!/usr/bin/env python3 +# Deletes the PCH arguments from a compilation database, to workaround nixpkgs +# stdenv having a cc-wrapper that is impossible to use for anything except cc +# itself, for example, clang-tidy. + +import json +import shlex + + +def process_compdb(compdb: list[dict]) -> list[dict]: + + def munch_command(args: list[str]) -> list[str]: + out = [] + eat_next = False + for i, arg in enumerate(args): + if arg == '-fpch-preprocess': + # as used with gcc + continue + elif arg == '-include-pch' or (arg == '-include' and args[i + 1] == 'precompiled-headers.hh'): + # -include-pch some-pch (clang), or -include some-pch (gcc) + eat_next = True + continue + if not eat_next: + out.append(arg) + eat_next = False + return out + + def chomp(item: dict) -> dict: + item = item.copy() + item['command'] = shlex.join(munch_command(shlex.split(item['command']))) + return item + + return [chomp(x) for x in compdb if not x['file'].endswith('precompiled-headers.hh')] + + +def main(): + import argparse + ap = argparse.ArgumentParser( + description='Delete pch arguments from compilation database') + ap.add_argument('input', + type=argparse.FileType('r'), + help='Input json file') + ap.add_argument('output', + type=argparse.FileType('w'), + help='Output json file') + args = ap.parse_args() + + input_json = json.load(args.input) + json.dump(process_compdb(input_json), args.output, indent=2) + + +if __name__ == '__main__': + main() diff --git a/meson/clang-tidy/fake.cc b/meson/clang-tidy/fake.cc new file mode 100644 index 000000000..e69de29bb diff --git a/meson/clang-tidy/meson.build b/meson/clang-tidy/meson.build new file mode 100644 index 000000000..9aba5fbdc --- /dev/null +++ b/meson/clang-tidy/meson.build @@ -0,0 +1,98 @@ +# The clang-tidy target for Lix + +run_clang_tidy = find_program('run-clang-tidy', required : false) +# Although this looks like it wants to be pkg-config, pkg-config does not +# really work for *plugins*, which are executable-like .so files that also +# cannot be found via find_program. Fun! +if get_option('lix-clang-tidy-checks-path') != '' + lix_clang_tidy_so = get_option('lix-clang-tidy-checks-path') + lix_clang_tidy_so_found = true +else + lix_clang_tidy_subproj = subproject( + 'lix-clang-tidy', + required : false, + default_options : {'build-by-default': false} + ) + if lix_clang_tidy_subproj.found() + lix_clang_tidy_so = lix_clang_tidy_subproj.get_variable('lix_clang_tidy') + lix_clang_tidy_so_found = true + else + lix_clang_tidy_so_found = false + endif +endif + +# Due to numerous problems, such as: +# - Meson does not expose pch targets, but *fine*, I can just ask Ninja for +# them with `ninja -t targets rule cpp_PCH` and build them manually: +# https://github.com/mesonbuild/meson/issues/13499 +# - Nixpkgs stdenv buries the cc-wrapper under a giant pile of assumptions +# about the cc-wrapper actually being used on the cc of a stdenv, rather than +# independently for clang-tidy, and we need to use cc-wrapper to get the +# correct hardening flags so that clang-tidy can actually parse the PCH file +# +# I give up. I am going to delete the damn PCH args and then it will work. +meson.add_postconf_script( + python, + meson.current_source_dir() / 'clean_compdb.py', + meson.global_build_root() / 'compile_commands.json', + meson.current_build_dir() / 'compile_commands.json', +) + +# Horrible hack to get around not being able to depend on another target's +# generated headers in any way in the meson DSL +# https://github.com/mesonbuild/meson/issues/12817 which was incorrectly +# closed, if you *actually* need to generate the files once. +# Also related: https://github.com/mesonbuild/meson/issues/3667 +# +# Or we could ban meson generators because their design is broken. +build_all_generated_headers = custom_target( + command : [ + python, + meson.current_source_dir() / 'build_required_targets.py', + meson.global_build_root(), + ], + output : 'generated_headers.stamp', + build_by_default : false, + build_always_stale : true, +) + +if lix_clang_tidy_so_found + run_clang_tidy_args = [ + '-load', + lix_clang_tidy_so, + '-p', + # We have to workaround a run-clang-tidy bug too, so we must give the + # directory name rather than the actual compdb file. + # https://github.com/llvm/llvm-project/issues/101440 + meson.current_build_dir(), + '-quiet', + ] + run_target( + 'clang-tidy', + command : [ + # XXX: This explicitly invokes it with python because of a nixpkgs bug + # where clang-unwrapped does not patch interpreters in run-clang-tidy. + # However, making clang-unwrapped depend on python is also silly, so idk. + python, + run_clang_tidy, + run_clang_tidy_args, + '-warnings-as-errors', + '*', + ], + depends : [ + build_all_generated_headers, + ], + ) + run_target( + 'clang-tidy-fix', + command : [ + python, + run_clang_tidy, + run_clang_tidy_args, + '-fix', + ], + depends : [ + build_all_generated_headers, + ], + ) +endif diff --git a/subprojects/lix-clang-tidy/.clang-format b/subprojects/lix-clang-tidy/.clang-format new file mode 100644 index 000000000..cd8995543 --- /dev/null +++ b/subprojects/lix-clang-tidy/.clang-format @@ -0,0 +1 @@ +BasedOnStyle: llvm diff --git a/subprojects/lix-clang-tidy/.editorconfig b/subprojects/lix-clang-tidy/.editorconfig new file mode 100644 index 000000000..19ee09eec --- /dev/null +++ b/subprojects/lix-clang-tidy/.editorconfig @@ -0,0 +1,4 @@ +# LLVM style code is 2-space indented +[*.{cc,hh}] +indent_style = space +indent_size = 2 diff --git a/subprojects/lix-clang-tidy/FixIncludes.cc b/subprojects/lix-clang-tidy/FixIncludes.cc new file mode 100644 index 000000000..602d3d355 --- /dev/null +++ b/subprojects/lix-clang-tidy/FixIncludes.cc @@ -0,0 +1,90 @@ +#include "FixIncludes.hh" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace nix::clang_tidy { + +using namespace clang; +using namespace clang::tidy; + +class FixIncludesCallbacks : public PPCallbacks { +public: + ClangTidyCheck &Check; + Preprocessor &PP; + FixIncludesCallbacks(ClangTidyCheck &Check, Preprocessor &PP) + : Check(Check), PP(PP) {} + +private: + bool Ignore = false; + virtual void LexedFileChanged(FileID FID, LexedFileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID, SourceLocation Loc) override; + + virtual void InclusionDirective(SourceLocation HashLoc, + const Token &IncludeTok, StringRef FileName, + bool IsAngled, CharSourceRange FilenameRange, + OptionalFileEntryRef File, + StringRef SearchPath, StringRef RelativePath, + const Module *Imported, + SrcMgr::CharacteristicKind FileType) override; +}; + +void FixIncludesCallbacks::LexedFileChanged(FileID, LexedFileChangeReason, + SrcMgr::CharacteristicKind FileType, + FileID, SourceLocation) { + Ignore = FileType != SrcMgr::C_User; +} + +void FixIncludesCallbacks::InclusionDirective( + SourceLocation, const Token &, StringRef FileName, bool IsAngled, + CharSourceRange FilenameRange, OptionalFileEntryRef File, StringRef, + StringRef, const Module *, SrcMgr::CharacteristicKind) { + if (Ignore) + return; + + // FIXME: this is kinda evil, but this is a one-time fixup + const std::vector SourceDirs = {"src/", "include/lix/"}; + + const auto Bracketize = [IsAngled](StringRef s) { + return IsAngled ? ("<" + s + ">").str() : ("\"" + s + "\"").str(); + }; + + for (const auto &SourceDir : SourceDirs) { + const bool IsAlreadyFixed = FileName.starts_with("lix/lib"); + if (File && File->getNameAsRequested().contains(SourceDir) && + !IsAlreadyFixed) { + StringRef Name = File->getNameAsRequested(); + auto Idx = Name.find(SourceDir); + assert(Idx != std::string::npos); + std::string Suffix = Name.drop_front(Idx + SourceDir.length()).str(); + + if (!Suffix.starts_with("lib")) { + llvm::dbgs() << "ignored: " << Suffix << "\n"; + return; + } + + Suffix = "lix/" + Suffix; + + auto Diag = Check.diag(FilenameRange.getBegin(), + "include needs to specify the source subdir"); + + Diag << FilenameRange + << FixItHint::CreateReplacement(FilenameRange, Bracketize(Suffix)); + } + } +} + +void FixIncludesCheck::registerPPCallbacks(const SourceManager &, + Preprocessor *PP, Preprocessor *) { + PP->addPPCallbacks(std::make_unique(*this, *PP)); +} + +}; // namespace nix::clang_tidy diff --git a/subprojects/lix-clang-tidy/FixIncludes.hh b/subprojects/lix-clang-tidy/FixIncludes.hh new file mode 100644 index 000000000..ea890cd39 --- /dev/null +++ b/subprojects/lix-clang-tidy/FixIncludes.hh @@ -0,0 +1,21 @@ +#pragma once +///@file + +#include +#include +#include + +namespace nix::clang_tidy { + +using namespace clang; +using namespace clang::tidy; + +class FixIncludesCheck : public ClangTidyCheck { + public: + FixIncludesCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; +}; + +}; diff --git a/subprojects/lix-clang-tidy/HasPrefixSuffix.cc b/subprojects/lix-clang-tidy/HasPrefixSuffix.cc new file mode 100644 index 000000000..e29b67e7c --- /dev/null +++ b/subprojects/lix-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/subprojects/lix-clang-tidy/HasPrefixSuffix.hh b/subprojects/lix-clang-tidy/HasPrefixSuffix.hh new file mode 100644 index 000000000..8693b6767 --- /dev/null +++ b/subprojects/lix-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/subprojects/lix-clang-tidy/LixClangTidyChecks.cc b/subprojects/lix-clang-tidy/LixClangTidyChecks.cc new file mode 100644 index 000000000..b3503dd3a --- /dev/null +++ b/subprojects/lix-clang-tidy/LixClangTidyChecks.cc @@ -0,0 +1,19 @@ +#include +#include +#include "FixIncludes.hh" +#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("lix-hasprefixsuffix"); + CheckFactories.registerCheck("lix-fixincludes"); + } +}; + +static ClangTidyModuleRegistry::Add X("lix-module", "Adds lix specific checks"); +}; diff --git a/subprojects/lix-clang-tidy/README.md b/subprojects/lix-clang-tidy/README.md new file mode 100644 index 000000000..c2d1cb258 --- /dev/null +++ b/subprojects/lix-clang-tidy/README.md @@ -0,0 +1,56 @@ +# Clang tidy lints for Lix + +This is a skeleton of a clang-tidy lints library for Lix. + +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='-*,lix-*' --load=build/liblix-clang-tidy.so -p ../compile_commands.json -header-filter '\.\./src/.*\.h' --fix ../src/libcmd/installables.cc +``` + +Several files, in parallel: + +``` +ninja -C build && run-clang-tidy -checks='-*,lix-*' -load=build/liblix-clang-tidy.so -p .. -header-filter '\.\./src/.*\.h' -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/subprojects/lix-clang-tidy/default.nix b/subprojects/lix-clang-tidy/default.nix new file mode 100644 index 000000000..1bfc2d9a4 --- /dev/null +++ b/subprojects/lix-clang-tidy/default.nix @@ -0,0 +1,44 @@ +{ + lib, + stdenv, + cmake, + meson, + ninja, + llvmPackages, +}: +let + inherit (lib) fileset; +in +stdenv.mkDerivation { + pname = "lix-clang-tidy-checks"; + # Setting the version to the Lix version is just going to cause pointless + # rebuilds due to versionSuffix and similar, and I cannot conceive of a usage + # where we actually care about its version since this is internal-only. + version = "0.1"; + + src = fileset.toSource { + root = ./.; + fileset = fileset.unions [ + ./meson.build + ./meson.options + (fileset.fileFilter ( + { hasExt, ... }: + builtins.any hasExt [ + "cc" + "hh" + ] + ) ./.) + ]; + }; + + nativeBuildInputs = [ + meson + cmake + ninja + ]; + + buildInputs = [ + llvmPackages.llvm + llvmPackages.clang-unwrapped.dev + ]; +} diff --git a/subprojects/lix-clang-tidy/meson.build b/subprojects/lix-clang-tidy/meson.build new file mode 100644 index 000000000..ef0226420 --- /dev/null +++ b/subprojects/lix-clang-tidy/meson.build @@ -0,0 +1,18 @@ +project('lix-clang-tidy', ['cpp', 'c'], + version : '0.1', + default_options : ['warning_level=3', 'cpp_std=c++20'] +) + +llvm = dependency('Clang', version: '>= 17', modules: ['libclang']) +sources = files( + 'HasPrefixSuffix.cc', + 'LixClangTidyChecks.cc', + 'FixIncludes.cc', +) + +lix_clang_tidy = shared_module('lix-clang-tidy', sources, + dependencies: llvm, + # overrides build_by_default, see https://github.com/mesonbuild/meson/issues/13498 + install : get_option('build-by-default'), + build_by_default : get_option('build-by-default') +) diff --git a/subprojects/lix-clang-tidy/meson.options b/subprojects/lix-clang-tidy/meson.options new file mode 100644 index 000000000..57f0f713b --- /dev/null +++ b/subprojects/lix-clang-tidy/meson.options @@ -0,0 +1,3 @@ +option('build-by-default', type : 'boolean', value : true, + description : 'set to false to not actually build targets by default. This is a hack to deal with meson lacking a build_by_default default option and building subprojects by default' +) -- cgit v1.2.3