tree-wide: fix a pile of lints

Mostly these are bugprone-unused-local-non-trivial-variable.

Also fix instances of:
- bugprone-optional-value-conversion
- bugprone-inc-dec-in-conditions (please check this loop is correct, it
  is the only non trivial code change in here)
- bugprone-unused-return-value (well, by fixing the lint config)

There are three notable changes relating to undefined vars:
- openLogFile ignoring the result. This is because openLogFile does a
  whole bunch of mutation of member variables
- hiliteMatches: i am guessing this is because showing the derivation
  name was unhelpful and it just got changed
- canonPath in NarAccessor: canonPath inside of a thing that is supposed
  to be vfs based cannot possibly be correct, so let's delete it given
  it is unused.

Fixes: https://git.lix.systems/lix-project/lix/issues/584

Change-Id: I887adc9ff28b61f726dcfed197e6796b414c2fcf

.clang-tidy

@@ -16,6 +16,8 @@ Checks:
   - -bugprone-unchecked-optional-access
     # many warnings, seems like a questionable lint
   - -bugprone-branch-clone
+    # extremely noisy before clang 19: https://github.com/llvm/llvm-project/issues/93959
+  - -bugprone-multi-level-implicit-pointer-conversion
     # all thrown exceptions must derive from std::exception
   - hicpp-exception-baseclass
     # capturing async lambdas are dangerous
@@ -33,3 +35,4 @@ Checks:
 
 CheckOptions:
   bugprone-reserved-identifier.AllowedIdentifiers: '__asan_default_options'
+  bugprone-unused-return-value.AllowCastToVoid: true

src/libexpr/attr-path.cc

@@ -129,7 +129,6 @@ std::pair<SourcePath, uint32_t> findPackageFilename(EvalState & state, Value & v
     try {
         auto colon = fn.rfind(':');
         if (colon == std::string::npos) fail();
-        std::string filename(fn, 0, colon);
         auto lineno = std::stoi(std::string(fn, colon + 1, std::string::npos));
         return {CanonPath(fn.substr(0, colon)), lineno};
     } catch (std::invalid_argument & e) {

src/libfetchers/fetchers.cc

@@ -250,9 +250,7 @@ std::optional<Hash> Input::getNarHash() const
 
 std::optional<std::string> Input::getRef() const
 {
-    if (auto s = maybeGetStrAttr(attrs, "ref"))
-        return *s;
-    return {};
+    return maybeGetStrAttr(attrs, "ref");
 }
 
 std::optional<Hash> Input::getRev() const
@@ -273,16 +271,12 @@ std::optional<Hash> Input::getRev() const
 
 std::optional<uint64_t> Input::getRevCount() const
 {
-    if (auto n = maybeGetIntAttr(attrs, "revCount"))
-        return *n;
-    return {};
+    return maybeGetIntAttr(attrs, "revCount");
 }
 
 std::optional<time_t> Input::getLastModified() const
 {
-    if (auto n = maybeGetIntAttr(attrs, "lastModified"))
-        return *n;
-    return {};
+    return maybeGetIntAttr(attrs, "lastModified");
 }
 
 ParsedURL InputScheme::toURL(const Input & input) const

src/libfetchers/git.cc

@@ -130,7 +130,7 @@ std::optional<std::string> readHeadCached(const std::string & actualUrl)
         // This function must behave the same way, so we return the expired
         // cached ref here.
         warn("could not get HEAD ref for repository '%s'; using expired cached ref '%s'", actualUrl, *cachedRef);
-        return *cachedRef;
+        return cachedRef;
     }
 
     return std::nullopt;

src/libstore/build/derivation-goal.cc

@@ -1212,7 +1212,7 @@ HookReply DerivationGoal::tryBuildHook()
     hook->toHook.writeSide.reset();
 
     /* Create the log file and pipe. */
-    Path logFile = openLogFile();
+    openLogFile();
 
     std::set<int> fds;
     fds.insert(hook->fromHook.readSide.get());

src/libstore/build/local-derivation-goal.cc

@@ -778,7 +778,7 @@ void LocalDerivationGoal::startBuilder()
         printMsg(lvlVomit, "setting builder env variable '%1%'='%2%'", i.first, i.second);
 
     /* Create the log file. */
-    Path logFile = openLogFile();
+    openLogFile();
 
     /* Create a pseudoterminal to get the output of the builder. */
     builderOut = AutoCloseFD{posix_openpt(O_RDWR | O_NOCTTY)};

src/libstore/common-protocol.cc

@@ -93,7 +93,7 @@ WireFormatGenerator CommonProto::Serialise<std::optional<ContentAddress>>::write
 {
     return [](std::string s) -> WireFormatGenerator {
         co_yield s;
-    }(caOpt ? renderContentAddress(*caOpt) : "");
+    }(caOpt ? renderContentAddress(caOpt) : "");
 }
 
 }

src/libstore/export-import.cc

@@ -13,14 +13,9 @@ void Store::exportPaths(const StorePathSet & paths, Sink & sink)
     auto sorted = topoSortPaths(paths);
     std::reverse(sorted.begin(), sorted.end());
 
-    std::string doneLabel("paths exported");
-    //logger->incExpected(doneLabel, sorted.size());
-
     for (auto & path : sorted) {
-        //Activity act(*logger, lvlInfo, "exporting path '%s'", path);
         sink << 1;
         exportPath(path, sink);
-        //logger->incProgress(doneLabel);
     }
 
     sink << 0;

src/libstore/nar-accessor.cc

@@ -152,7 +152,6 @@ struct NarAccessor : public FSAccessor
 
     NarMember * find(const Path & path)
     {
-        Path canon = path == "" ? "" : canonPath(path);
         NarMember * current = &root;
         auto end = path.end();
         for (auto it = path.begin(); it != end; ) {

src/libstore/nar-info.cc

@@ -120,7 +120,7 @@ std::string NarInfo::to_string(const Store & store) const
         res += "Sig: " + sig + "\n";
 
     if (ca)
-        res += "CA: " + renderContentAddress(*ca) + "\n";
+        res += "CA: " + renderContentAddress(ca) + "\n";
 
     return res;
 }

src/libstore/store-api.cc

@@ -1322,8 +1322,7 @@ std::optional<StorePath> Store::getBuildDerivationPath(const StorePath & path)
     if (!path.isDerivation()) {
         try {
             auto info = queryPathInfo(path);
-            if (!info->deriver) return std::nullopt;
-            return *info->deriver;
+            return info->deriver;
         } catch (InvalidPath &) {
             return std::nullopt;
         }

src/libutil/hilite.cc

@@ -25,7 +25,7 @@ std::string hiliteMatches(
         out.append(s.substr(last_end, m.position() - last_end));
         // Merge continous matches
         ssize_t end = start + m.length();
-        while (++it != matches.end() && (*it).position() <= end) {
+        for (++it; it != matches.end() && (*it).position() <= end; ++it) {
             auto n = *it;
             ssize_t nend = start + (n.position() - start + n.length());
             if (nend > end)

src/libutil/serialise.cc

@@ -75,7 +75,6 @@ void Source::operator () (char * data, size_t len)
 
 void Source::drainInto(Sink & sink)
 {
-    std::string s;
     std::array<char, 8192> buf;
     while (true) {
         size_t n;
@@ -251,7 +250,7 @@ Error readError(Source & source)
     auto type = readString(source);
     assert(type == "Error");
     auto level = (Verbosity) readInt(source);
-    auto name = readString(source); // removed
+    readString(source); // removed (name)
     auto msg = readString(source);
     ErrorInfo info {
         .level = level,

src/libutil/terminal.cc

@@ -34,7 +34,7 @@ bool shouldANSI(StandardOutputStream fileno)
 // FIXME(jade): replace with TerminalCodeEater. wowie this is evil code.
 std::string filterANSIEscapes(std::string_view s, bool filterAll, unsigned int width, bool eatTabs)
 {
-    std::string t, e;
+    std::string t;
     size_t w = 0;
     auto i = s.begin();
 

src/nix-env/nix-env.cc

@@ -1313,7 +1313,7 @@ static void opSwitchGeneration(Globals & globals, Strings opFlags, Strings opArg
         throw UsageError("exactly one argument expected");
 
     if (auto dstGen = string2Int<GenerationNumber>(opArgs.front()))
-        switchGeneration(globals.profile, *dstGen, globals.dryRun);
+        switchGeneration(globals.profile, dstGen, globals.dryRun);
     else
         throw UsageError("expected a generation number");
 }

src/nix/bundle.cc

@@ -116,8 +116,6 @@ struct CmdBundle : InstallableCommand
             },
         });
 
-        auto outPathS = store->printStorePath(outPath);
-
         if (!outLink) {
             auto * attr = vRes->attrs->get(evalState->sName);
             if (!attr)

src/nix/path-info.cc

@@ -114,7 +114,7 @@ struct CmdPathInfo : StorePathsCommand, MixJSON
                     std::cout << '\t';
                     Strings ss;
                     if (info->ultimate) ss.push_back("ultimate");
-                    if (info->ca) ss.push_back("ca:" + renderContentAddress(*info->ca));
+                    if (info->ca) ss.push_back("ca:" + renderContentAddress(info->ca));
                     for (auto & sig : info->sigs) ss.push_back(sig);
                     std::cout << concatStringsSep(" ", ss);
                 }

src/nix/search.cc

@@ -160,7 +160,6 @@ struct CmdSearch : InstallableCommand, MixJSON
                                 {"description", description},
                             };
                         } else {
-                            auto name2 = hiliteMatches(name.name, nameMatches, ANSI_GREEN, "\e[0;2m");
                             if (results > 1) logger->cout("");
                             logger->cout(
                                 "* %s%s",

src/nix/sigs.cc

@@ -40,13 +40,9 @@ struct CmdCopySigs : StorePathsCommand
 
         ThreadPool pool;
 
-        std::string doneLabel = "done";
         std::atomic<size_t> added{0};
 
-        //logger->setExpected(doneLabel, storePaths.size());
-
         auto doPath = [&](const Path & storePathS) {
-            //Activity act(*logger, lvlInfo, "getting signatures for '%s'", storePath);
 
             checkInterrupt();
 
@@ -78,8 +74,6 @@ struct CmdCopySigs : StorePathsCommand
                 store->addSignatures(storePath, newSigs);
                 added += newSigs.size();
             }
-
-            //logger->incProgress(doneLabel);
         };
 
         for (auto & storePath : storePaths)