From 00c3a3defe134abf6c8d532ef5f99b9a960a14a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 20:38:35 +0100 Subject: [PATCH 01/16] nixos-option: add clang-tidy --- pkgs/tools/nix/nixos-option/.clang-tidy | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 pkgs/tools/nix/nixos-option/.clang-tidy diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy new file mode 100644 index 0000000000000..a59bd8c73d7ec --- /dev/null +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -0,0 +1,21 @@ +Checks: +# TODO +# - bugprone-* +# - performance-* +# - modernize-* +# - readability-* +# - misc-* +# - portability-* +# - concurrency-* +# - google-* +# - -google-readability-todo +# +# # don't find them too problematic +# - -readability-identifier-length +# - -readability-magic-numbers +# +# - cppcoreguidelines-* +# - -cppcoreguidelines-avoid-magic-numbers +UseColor: true +CheckOptions: + misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic: True From 7e9e33e79c7003088af7bd7ee100d12df8c20017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 20:44:54 +0100 Subject: [PATCH 02/16] nixos-option: add devShell --- pkgs/tools/nix/nixos-option/.envrc | 1 + pkgs/tools/nix/nixos-option/shell.nix | 7 +++++++ 2 files changed, 8 insertions(+) create mode 100644 pkgs/tools/nix/nixos-option/.envrc create mode 100644 pkgs/tools/nix/nixos-option/shell.nix diff --git a/pkgs/tools/nix/nixos-option/.envrc b/pkgs/tools/nix/nixos-option/.envrc new file mode 100644 index 0000000000000..1d953f4bd7359 --- /dev/null +++ b/pkgs/tools/nix/nixos-option/.envrc @@ -0,0 +1 @@ +use nix diff --git a/pkgs/tools/nix/nixos-option/shell.nix b/pkgs/tools/nix/nixos-option/shell.nix new file mode 100644 index 0000000000000..c779679a9b7ce --- /dev/null +++ b/pkgs/tools/nix/nixos-option/shell.nix @@ -0,0 +1,7 @@ +with import ../../../.. { }; +nixos-option.overrideAttrs (old: { + nativeBuildInputs = old.nativeBuildInputs ++ [ + # hiprio so that it has a higher priority than the default unwrapped clang tools from clang if our stdenv is based on clang + (lib.hiPrio pkgs.buildPackages.clang-tools) + ]; +}) From 226981490b9eb849ace075cb9a68ff0f5e1779a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 21:06:52 +0100 Subject: [PATCH 03/16] nixos-option: clean up includes based on clang-tidy suggestions Before the code was using include-what-you-means, but it's a bit tricky to integrate this in nix. I had some trouble with nix-eval-jobs when using include-what-you-means. clang-tidy on the other hand proved more robust to me and is useful for linting in general, so less tooling overall for this little project. --- pkgs/tools/nix/nixos-option/.clang-tidy | 2 + .../nix/nixos-option/src/libnix-copy-paste.cc | 3 +- .../nix/nixos-option/src/libnix-copy-paste.hh | 2 +- .../nix/nixos-option/src/nixos-option.cc | 50 ++++++++++++------- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index a59bd8c73d7ec..bccf7072bccdb 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -1,4 +1,6 @@ Checks: +- -* +- misc-include-cleaner # TODO # - bugprone-* # - performance-* diff --git a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc index b5ae1b4958840..22532c71788c7 100644 --- a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc +++ b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc @@ -2,8 +2,9 @@ // Since they are not, copy/paste them here. // TODO: Delete these and use the ones in the library as they become available. +#include #include "libnix-copy-paste.hh" -#include // for Strings +#include // From nix/src/nix/repl.cc bool isVarName(const std::string_view & s) diff --git a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh index f680b4c1a7185..20a1f4b5817e0 100644 --- a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh +++ b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh @@ -1,5 +1,5 @@ #pragma once -#include +#include bool isVarName(const std::string_view & s); diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index 977ea83d1973b..c32ce09a4661c 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -1,20 +1,36 @@ -#include // for argvToStrings, UsageError -#include // for findAlongAttrPath, parseAttrPath -#include // for Attr, Bindings, Bindings::iterator -#include // for MixEvalArgs -#include // for initGC, initNix -#include // for EvalState::forceValue -#include // for EvalState, initGC, operator<< -#include // for initPlugins, Settings, settings -#include // for Pos -#include // for getArg, LegacyArgs, printVersion -#include // for openStore -#include // for Symbol, SymbolTable -#include // for Error, Path, Strings, PathSet -#include // for absPath, baseNameOf -#include // for Value, Value::(anonymous), Value:... -#include // for string, operator+, operator== -#include // for move +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include "libnix-copy-paste.hh" From 553461adc754ff9f7f5bf47c18f3f198261a8c28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 22:27:06 +0100 Subject: [PATCH 04/16] nixos-option: use c++20 starts_with --- pkgs/tools/nix/nixos-option/src/nixos-option.cc | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index c32ce09a4661c..87056765435b1 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -1,4 +1,3 @@ -#include #include #include #include @@ -498,20 +497,13 @@ void printConfigValue(Context & ctx, Out & out, const std::string & path, std::v out << ";\n"; } -// Replace with std::starts_with when C++20 is available -bool starts_with(const std::string & s, const std::string & prefix) -{ - return s.size() >= prefix.size() && - std::equal(s.begin(), std::next(s.begin(), prefix.size()), prefix.begin(), prefix.end()); -} - void printRecursive(Context & ctx, Out & out, const std::string & path) { mapOptions( [&ctx, &out, &path](const std::string & optionPath) { mapConfigValuesInOption( [&ctx, &out, &path](const std::string & configPath, std::variant v) { - if (starts_with(configPath, path)) { + if (configPath.starts_with(path)) { printConfigValue(ctx, out, configPath, v); } }, From e8d76d62a2871f14b6162c4f464bffd3c041f29e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 22:30:25 +0100 Subject: [PATCH 05/16] nixos-option: explicitly cast to int from size_t to make the linter happy. --- pkgs/tools/nix/nixos-option/src/nixos-option.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index 87056765435b1..6361a6d414a71 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -397,7 +397,7 @@ void printValue(Context & ctx, Out & out, std::variant(v.listSize())); for (unsigned int n = 0; n < v.listSize(); ++n) { printValue(ctx, listOut, *v.listElems()[n], ""); listOut << Out::sep; @@ -406,7 +406,7 @@ void printList(Context & ctx, Out & out, Value & v) void printAttrs(Context & ctx, Out & out, Value & v, const std::string & path) { - Out attrsOut(out, "{", "}", v.attrs()->size()); + Out attrsOut(out, "{", "}", static_cast(v.attrs()->size())); for (const auto & a : v.attrs()->lexicographicOrder(ctx.state.symbols)) { if (!forbiddenRecursionName(a->name, ctx.state.symbols)) { std::string_view name = ctx.state.symbols[a->name]; From 2cfcb6f44242c7789d481b57351ca5f9c2dc9b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 22:30:42 +0100 Subject: [PATCH 06/16] nixos-option: don't throw errors in main https://clang.llvm.org/extra/clang-tidy/checks/bugprone/exception-escape.html --- .../nix/nixos-option/src/nixos-option.cc | 108 +++++++++--------- 1 file changed, 55 insertions(+), 53 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index 6361a6d414a71..0c8a0be72329d 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -593,65 +593,67 @@ void printOne(Context & ctx, Out & out, const std::string & path) int main(int argc, char ** argv) { - bool recursive = false; - std::string path = "."; - std::string optionsExpr = "(import {}).options"; - std::string configExpr = "(import {}).config"; - std::vector args; - - struct MyArgs : nix::LegacyArgs, nix::MixEvalArgs - { - using nix::LegacyArgs::LegacyArgs; - }; - - MyArgs myArgs(std::string(nix::baseNameOf(argv[0])), [&](Strings::iterator & arg, const Strings::iterator & end) { - if (*arg == "--help") { - nix::showManPage("nixos-option"); - } else if (*arg == "--version") { - nix::printVersion("nixos-option"); - } else if (*arg == "-r" || *arg == "--recursive") { - recursive = true; - } else if (*arg == "--path") { - path = nix::getArg(*arg, arg, end); - } else if (*arg == "--options_expr") { - optionsExpr = nix::getArg(*arg, arg, end); - } else if (*arg == "--config_expr") { - configExpr = nix::getArg(*arg, arg, end); - } else if (!arg->empty() && arg->at(0) == '-') { - return false; - } else { - args.push_back(*arg); - } - return true; - }); + return nix::handleExceptions(argv[0], [&]() { + bool recursive = false; + std::string path = "."; + std::string optionsExpr = "(import {}).options"; + std::string configExpr = "(import {}).config"; + std::vector args; + + struct MyArgs : nix::LegacyArgs, nix::MixEvalArgs + { + using nix::LegacyArgs::LegacyArgs; + }; + + MyArgs myArgs(std::string(nix::baseNameOf(argv[0])), [&](Strings::iterator & arg, const Strings::iterator & end) { + if (*arg == "--help") { + nix::showManPage("nixos-option"); + } else if (*arg == "--version") { + nix::printVersion("nixos-option"); + } else if (*arg == "-r" || *arg == "--recursive") { + recursive = true; + } else if (*arg == "--path") { + path = nix::getArg(*arg, arg, end); + } else if (*arg == "--options_expr") { + optionsExpr = nix::getArg(*arg, arg, end); + } else if (*arg == "--config_expr") { + configExpr = nix::getArg(*arg, arg, end); + } else if (!arg->empty() && arg->at(0) == '-') { + return false; + } else { + args.push_back(*arg); + } + return true; + }); - myArgs.parseCmdline(nix::argvToStrings(argc, argv)); + myArgs.parseCmdline(nix::argvToStrings(argc, argv)); - nix::initNix(); - nix::initGC(); - nix::settings.readOnlyMode = true; - auto store = nix::openStore(); + nix::initNix(); + nix::initGC(); + nix::settings.readOnlyMode = true; + auto store = nix::openStore(); - auto evalStore = myArgs.evalStoreUrl ? nix::openStore(*myArgs.evalStoreUrl) - : nix::openStore(); - auto state = nix::make_ref( - myArgs.lookupPath, evalStore, nix::fetchSettings, nix::evalSettings); + auto evalStore = myArgs.evalStoreUrl ? nix::openStore(*myArgs.evalStoreUrl) + : nix::openStore(); + auto state = nix::make_ref( + myArgs.lookupPath, evalStore, nix::fetchSettings, nix::evalSettings); - Value optionsRoot = parseAndEval(*state, optionsExpr, path); - Value configRoot = parseAndEval(*state, configExpr, path); + Value optionsRoot = parseAndEval(*state, optionsExpr, path); + Value configRoot = parseAndEval(*state, configExpr, path); - Context ctx{*state, *myArgs.getAutoArgs(*state), optionsRoot, configRoot}; - Out out(std::cout); + Context ctx{*state, *myArgs.getAutoArgs(*state), optionsRoot, configRoot}; + Out out(std::cout); - auto print = recursive ? printRecursive : printOne; - if (args.empty()) { - print(ctx, out, ""); - } - for (const auto & arg : args) { - print(ctx, out, arg); - } + auto print = recursive ? printRecursive : printOne; + if (args.empty()) { + print(ctx, out, ""); + } + for (const auto & arg : args) { + print(ctx, out, arg); + } - ctx.state.maybePrintStats(); + ctx.state.maybePrintStats(); - return 0; + return 0; + }); } From 01580b19ae9ae74688c499350922eda42fea7e3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 22:30:58 +0100 Subject: [PATCH 07/16] nixos-option: enable bugprone lints --- pkgs/tools/nix/nixos-option/.clang-tidy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index bccf7072bccdb..b66579e5c0bc2 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -1,8 +1,10 @@ Checks: - -* - misc-include-cleaner +- bugprone-* +# don't find them too problematic +- -bugprone-easily-swappable-parameters # TODO -# - bugprone-* # - performance-* # - modernize-* # - readability-* From c3e48526e0a53207f6421838c89d099f5d711a42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 22:35:22 +0100 Subject: [PATCH 08/16] nixos-option: enable performance lint --- pkgs/tools/nix/nixos-option/.clang-tidy | 2 +- pkgs/tools/nix/nixos-option/src/nixos-option.cc | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index b66579e5c0bc2..cdc32d73afdff 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -4,8 +4,8 @@ Checks: - bugprone-* # don't find them too problematic - -bugprone-easily-swappable-parameters +- performance-* # TODO -# - performance-* # - modernize-* # - readability-* # - misc-* diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index 0c8a0be72329d..cc285b0d24ad1 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -69,7 +70,7 @@ class Out class Separator {}; const static Separator sep; - enum LinePolicy + enum LinePolicy : std::uint8_t { ONE_LINE, MULTI_LINE @@ -504,7 +505,7 @@ void printRecursive(Context & ctx, Out & out, const std::string & path) mapConfigValuesInOption( [&ctx, &out, &path](const std::string & configPath, std::variant v) { if (configPath.starts_with(path)) { - printConfigValue(ctx, out, configPath, v); + printConfigValue(ctx, out, configPath, std::move(v)); } }, optionPath, ctx); From 9c4e940564af9aa61c3f57f309115cc0b874a3d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 22:53:42 +0100 Subject: [PATCH 09/16] nixos-option: enable modernize-* checks in clang-tidy --- pkgs/tools/nix/nixos-option/.clang-tidy | 2 +- pkgs/tools/nix/nixos-option/src/nixos-option.cc | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index cdc32d73afdff..6818a5afbe4cc 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -5,8 +5,8 @@ Checks: # don't find them too problematic - -bugprone-easily-swappable-parameters - performance-* +- modernize-* # TODO -# - modernize-* # - readability-* # - misc-* # - portability-* diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index cc285b0d24ad1..34acb78c66b64 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -82,8 +82,8 @@ class Out {} Out(const Out &) = delete; Out(Out &&) = default; - Out & operator=(const Out &) = delete; - Out & operator=(Out &&) = delete; + auto operator=(const Out &) -> Out & = delete; + auto operator=(Out &&) -> Out & = delete; ~Out() { ostream << end; } private: @@ -92,9 +92,9 @@ class Out std::string end; LinePolicy policy; bool writeSinceSep; - template friend Out & operator<<(Out & o, T thing); + template friend auto operator<<(Out & o, T thing) -> Out &; - friend void printValue(Context & ctx, Out & out, std::variant maybeValue, const std::string & path); + friend auto printValue(Context & ctx, Out & out, std::variant maybeValue, const std::string & path) -> void; }; template Out & operator<<(Out & o, T thing) From e5705ba07b6bdbbad63ae938cd170ad312589021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 23:24:04 +0100 Subject: [PATCH 10/16] nixos-option: enable readability lint --- pkgs/tools/nix/nixos-option/.clang-tidy | 8 ++++---- pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc | 2 ++ pkgs/tools/nix/nixos-option/src/nixos-option.cc | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index 6818a5afbe4cc..71c13839afe5a 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -6,17 +6,17 @@ Checks: - -bugprone-easily-swappable-parameters - performance-* - modernize-* +- readability-* +# don't find them too problematic +- -readability-identifier-length +- -readability-magic-numbers # TODO -# - readability-* # - misc-* # - portability-* # - concurrency-* # - google-* # - -google-readability-todo # -# # don't find them too problematic -# - -readability-identifier-length -# - -readability-magic-numbers # # - cppcoreguidelines-* # - -cppcoreguidelines-avoid-magic-numbers diff --git a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc index 22532c71788c7..15a411258b933 100644 --- a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc +++ b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc @@ -6,6 +6,7 @@ #include "libnix-copy-paste.hh" #include +// NOLINTBEGIN // From nix/src/nix/repl.cc bool isVarName(const std::string_view & s) { @@ -21,3 +22,4 @@ bool isVarName(const std::string_view & s) return false; return true; } +// NOLINTEND diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index 34acb78c66b64..8b553bae8bbf2 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -168,7 +168,7 @@ std::string quoteAttribute(const std::string_view & attribute) return buf.str(); } -const std::string appendPath(const std::string & prefix, const std::string_view & suffix) +std::string appendPath(const std::string & prefix, const std::string_view & suffix) { if (prefix.empty()) { return quoteAttribute(suffix); @@ -376,7 +376,7 @@ void describeDerivation(Context & ctx, Out & out, Value v) { // Copy-pasted from nix/src/nix/repl.cc printDerivation() :( std::optional storePath = std::nullopt; - if (auto i = v.attrs()->get(ctx.state.sDrvPath)) { + if (const auto *i = v.attrs()->get(ctx.state.sDrvPath)) { nix::NixStringContext context; storePath = ctx.state.coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation"); } @@ -457,7 +457,7 @@ void printMultiLineString(Out & out, const Value & v) void printValue(Context & ctx, Out & out, std::variant maybeValue, const std::string & path) { try { - if (auto ex = std::get_if(&maybeValue)) { + if (auto *ex = std::get_if(&maybeValue)) { std::rethrow_exception(*ex); } Value v = evaluateValue(ctx, std::get(maybeValue)); From 1e59fb41f0a669621979161ea4857c9d32e979d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 23:26:30 +0100 Subject: [PATCH 11/16] nixos-option: enable more lints --- pkgs/tools/nix/nixos-option/.clang-tidy | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index 71c13839afe5a..8468b33e2b80d 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -10,14 +10,12 @@ Checks: # don't find them too problematic - -readability-identifier-length - -readability-magic-numbers +- portability-* +- concurrency-* +- google-* +- -google-readability-todo # TODO # - misc-* -# - portability-* -# - concurrency-* -# - google-* -# - -google-readability-todo -# -# # - cppcoreguidelines-* # - -cppcoreguidelines-avoid-magic-numbers UseColor: true From e7367a6eabb559839a370dc82ecb22b237ab6515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Wed, 13 Nov 2024 23:35:23 +0100 Subject: [PATCH 12/16] nixos-option: enable misc lints --- pkgs/tools/nix/nixos-option/.clang-tidy | 5 ++-- .../nix/nixos-option/src/nixos-option.cc | 29 +++++++++---------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index 8468b33e2b80d..1041f79bdece6 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -1,6 +1,5 @@ Checks: - -* -- misc-include-cleaner - bugprone-* # don't find them too problematic - -bugprone-easily-swappable-parameters @@ -14,8 +13,10 @@ Checks: - concurrency-* - google-* - -google-readability-todo +- misc-* +# we maybe want to address this? +- -misc-no-recursion # TODO -# - misc-* # - cppcoreguidelines-* # - -cppcoreguidelines-avoid-magic-numbers UseColor: true diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index 8b553bae8bbf2..bd7d3b5ae631f 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -34,7 +34,6 @@ #include "libnix-copy-paste.hh" -using nix::absPath; using nix::Bindings; using nix::Error; using nix::EvalError; @@ -45,9 +44,7 @@ using nix::Strings; using nix::Symbol; using nix::nAttrs; using nix::ThrownError; -using nix::tLambda; using nix::nString; -using nix::UsageError; using nix::Value; struct Context @@ -144,7 +141,7 @@ bool isOption(Context & ctx, const Value & v) return false; } try { - Value evaluatedType = evaluateValue(ctx, *actualType->value); + const Value evaluatedType = evaluateValue(ctx, *actualType->value); if (evaluatedType.type() != nString) { return false; } @@ -208,7 +205,7 @@ void recurse(const std::functionname, ctx.state.symbols)) { continue; } - std::string_view name = ctx.state.symbols[child->name]; + const std::string_view name = ctx.state.symbols[child->name]; recurse(f, ctx, *child->value, appendPath(path, name)); } } @@ -220,7 +217,7 @@ bool optionTypeIs(Context & ctx, Value & v, const std::string & soughtType) if (typeLookup == v.attrs()->end()) { return false; } - Value type = evaluateValue(ctx, *typeLookup->value); + const Value type = evaluateValue(ctx, *typeLookup->value); if (type.type() != nAttrs) { return false; } @@ -228,7 +225,7 @@ bool optionTypeIs(Context & ctx, Value & v, const std::string & soughtType) if (nameLookup == type.attrs()->end()) { return false; } - Value name = evaluateValue(ctx, *nameLookup->value); + const Value name = evaluateValue(ctx, *nameLookup->value); if (name.type() != nString) { return false; } @@ -273,7 +270,7 @@ FindAlongOptionPathRet findAlongOptionPath(Context & ctx, const std::string & pa for (auto i = tokens.begin(); i != tokens.end(); i++) { const std::string_view attr = ctx.state.symbols[*i]; try { - bool lastAttribute = std::next(i) == tokens.end(); + const bool lastAttribute = std::next(i) == tokens.end(); v = evaluateValue(ctx, v); if (attr.empty()) { throw OptionPathError(ctx.state, "empty attribute name"); @@ -316,7 +313,7 @@ void mapOptions(const std::function & f, Context auto root = findAlongOptionPath(ctx, path); recurse( [f, &ctx](const std::string & path, std::variant v) { - bool isOpt = std::holds_alternative(v) || isOption(ctx, std::get(v)); + const bool isOpt = std::holds_alternative(v) || isOption(ctx, std::get(v)); if (isOpt) { f(path); } @@ -359,7 +356,7 @@ void mapConfigValuesInOption( } recurse( [f, ctx](const std::string & path, std::variant v) { - bool leaf = std::holds_alternative(v) || std::get(v).type() != nAttrs || + const bool leaf = std::holds_alternative(v) || std::get(v).type() != nAttrs || ctx.state.isDerivation(std::get(v)); if (!leaf) { return true; // Keep digging @@ -387,7 +384,7 @@ void describeDerivation(Context & ctx, Out & out, Value v) out << "ยป"; } -Value parseAndEval(EvalState & state, const std::string & expression, const std::string & path) +Value parseAndEval(EvalState & state, const std::string & expression) { Value v{}; state.eval(state.parseExprFromString(expression, state.rootPath(".")), v); @@ -410,7 +407,7 @@ void printAttrs(Context & ctx, Out & out, Value & v, const std::string & path) Out attrsOut(out, "{", "}", static_cast(v.attrs()->size())); for (const auto & a : v.attrs()->lexicographicOrder(ctx.state.symbols)) { if (!forbiddenRecursionName(a->name, ctx.state.symbols)) { - std::string_view name = ctx.state.symbols[a->name]; + const std::string_view name = ctx.state.symbols[a->name]; attrsOut << name << " = "; printValue(ctx, attrsOut, *a->value, appendPath(path, name)); attrsOut << ";" << Out::sep; @@ -439,11 +436,11 @@ void multiLineStringEscape(Out & out, const std::string_view & s) void printMultiLineString(Out & out, const Value & v) { - std::string_view s = v.string_view(); + const std::string_view s = v.string_view(); Out strOut(out, "''", "''", Out::MULTI_LINE); std::string::size_type begin = 0; while (begin < s.size()) { - std::string::size_type end = s.find('\n', begin); + const std::string::size_type end = s.find('\n', begin); if (end == std::string::npos) { multiLineStringEscape(strOut, s.substr(begin, s.size() - begin)); break; @@ -639,8 +636,8 @@ int main(int argc, char ** argv) auto state = nix::make_ref( myArgs.lookupPath, evalStore, nix::fetchSettings, nix::evalSettings); - Value optionsRoot = parseAndEval(*state, optionsExpr, path); - Value configRoot = parseAndEval(*state, configExpr, path); + const Value optionsRoot = parseAndEval(*state, optionsExpr); + const Value configRoot = parseAndEval(*state, configExpr); Context ctx{*state, *myArgs.getAutoArgs(*state), optionsRoot, configRoot}; Out out(std::cout); From 6fe4fb2b95a813c1dd86cedc383fe9e9dfa5530c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Thu, 14 Nov 2024 00:12:50 +0100 Subject: [PATCH 13/16] nixos-option: enable cppcoreguidelines lints --- pkgs/tools/nix/nixos-option/.clang-tidy | 7 ++++--- .../nix/nixos-option/src/nixos-option.cc | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index 1041f79bdece6..1d92756771f3f 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -16,9 +16,10 @@ Checks: - misc-* # we maybe want to address this? - -misc-no-recursion -# TODO -# - cppcoreguidelines-* -# - -cppcoreguidelines-avoid-magic-numbers +- cppcoreguidelines-* +- -cppcoreguidelines-avoid-magic-numbers +# We could use std::reference_wrapper, but it's not super important +- -cppcoreguidelines-avoid-const-or-ref-data-members UseColor: true CheckOptions: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic: True diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index bd7d3b5ae631f..6846cc7e5fbbd 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -347,7 +348,7 @@ void mapConfigValuesInOption( const std::function v)> & f, const std::string & path, Context & ctx) { - Value * option; + Value * option = nullptr; try { option = findAlongAttrPath(ctx.state, path, ctx.autoArgs, ctx.configRoot).first; } catch (Error &) { @@ -396,8 +397,8 @@ void printValue(Context & ctx, Out & out, std::variant(v.listSize())); - for (unsigned int n = 0; n < v.listSize(); ++n) { - printValue(ctx, listOut, *v.listElems()[n], ""); + for (auto *elem : v.listItems()) { + printValue(ctx, listOut, *elem, ""); listOut << Out::sep; } } @@ -417,7 +418,7 @@ void printAttrs(Context & ctx, Out & out, Value & v, const std::string & path) void multiLineStringEscape(Out & out, const std::string_view & s) { - size_t i; + size_t i = 0; for (i = 1; i < s.size(); i++) { if (s[i - 1] == '$' && s[i] == '{') { out << "''${"; @@ -591,7 +592,8 @@ void printOne(Context & ctx, Out & out, const std::string & path) int main(int argc, char ** argv) { - return nix::handleExceptions(argv[0], [&]() { + auto args = std::span(argv, argc); + return nix::handleExceptions(args[0], [&]() { bool recursive = false; std::string path = "."; std::string optionsExpr = "(import {}).options"; @@ -600,10 +602,15 @@ int main(int argc, char ** argv) struct MyArgs : nix::LegacyArgs, nix::MixEvalArgs { + MyArgs(const MyArgs& other) = default; + MyArgs(MyArgs&& other) noexcept = default; + auto operator=(const MyArgs& other) noexcept -> MyArgs& = default; + auto operator=(MyArgs&& other) noexcept -> MyArgs& = default; + virtual ~MyArgs() = default; using nix::LegacyArgs::LegacyArgs; }; - MyArgs myArgs(std::string(nix::baseNameOf(argv[0])), [&](Strings::iterator & arg, const Strings::iterator & end) { + MyArgs myArgs(std::string(nix::baseNameOf(args[0])), [&](Strings::iterator & arg, const Strings::iterator & end) { if (*arg == "--help") { nix::showManPage("nixos-option"); } else if (*arg == "--version") { From 7a8407e477f5892104f9ccd00972e18245be8a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Thu, 14 Nov 2024 00:23:00 +0100 Subject: [PATCH 14/16] nixos-option: fix potential memory leak in mapOptions --- pkgs/tools/nix/nixos-option/.clang-tidy | 1 - pkgs/tools/nix/nixos-option/src/nixos-option.cc | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index 1d92756771f3f..4627a18c71613 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -1,5 +1,4 @@ Checks: -- -* - bugprone-* # don't find them too problematic - -bugprone-easily-swappable-parameters diff --git a/pkgs/tools/nix/nixos-option/src/nixos-option.cc b/pkgs/tools/nix/nixos-option/src/nixos-option.cc index 6846cc7e5fbbd..99997afb17c1d 100644 --- a/pkgs/tools/nix/nixos-option/src/nixos-option.cc +++ b/pkgs/tools/nix/nixos-option/src/nixos-option.cc @@ -313,7 +313,7 @@ void mapOptions(const std::function & f, Context { auto root = findAlongOptionPath(ctx, path); recurse( - [f, &ctx](const std::string & path, std::variant v) { + [&f, &ctx](const std::string & path, std::variant v) { const bool isOpt = std::holds_alternative(v) || isOption(ctx, std::get(v)); if (isOpt) { f(path); From cf117192587887bfc3a9a68504887814845bf07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Mon, 18 Nov 2024 17:10:05 +0100 Subject: [PATCH 15/16] nixos-option: disable modernize-use-trailing-return-type --- pkgs/tools/nix/nixos-option/.clang-tidy | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkgs/tools/nix/nixos-option/.clang-tidy b/pkgs/tools/nix/nixos-option/.clang-tidy index 4627a18c71613..ef187ed77fb56 100644 --- a/pkgs/tools/nix/nixos-option/.clang-tidy +++ b/pkgs/tools/nix/nixos-option/.clang-tidy @@ -4,6 +4,8 @@ Checks: - -bugprone-easily-swappable-parameters - performance-* - modernize-* +# doesn't improve readability much in this project +- -modernize-use-trailing-return-type - readability-* # don't find them too problematic - -readability-identifier-length From b7ea84f90078a065b5072434aa044caff0b17a42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Mon, 18 Nov 2024 17:29:10 +0100 Subject: [PATCH 16/16] nixos-options: don't use references for string_views --- pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc | 2 +- pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc index 15a411258b933..e00418eb99e02 100644 --- a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc +++ b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.cc @@ -8,7 +8,7 @@ // NOLINTBEGIN // From nix/src/nix/repl.cc -bool isVarName(const std::string_view & s) +bool isVarName(std::string_view s) { if (s.size() == 0) return false; if (nix::isReservedKeyword(s)) return false; diff --git a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh index 20a1f4b5817e0..e26e57b0ea5cc 100644 --- a/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh +++ b/pkgs/tools/nix/nixos-option/src/libnix-copy-paste.hh @@ -2,4 +2,4 @@ #include -bool isVarName(const std::string_view & s); +bool isVarName(std::string_view s);