fix cppcheck warnings

[antonvw]

No description provided.

[qodo-code-review]

PR Summary by Qodo

Fix cppcheck warnings in CI and modernize a few hot paths

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Update CI cppcheck suppressions to match current warning set.
• Tighten event/lambda parameter constness and avoid vector copies in dirctrl.
• Refactor diagnostics filtering to use ranges and reduce reallocations.

Diagram

graph TD
  CI(["CI: cppcheck job"]) --> CMake["ci/CMakeLists.txt"] --> Cpp{{"cppcheck"}} --> Dir["src/del/dirctrl.cpp"]
  CI(["CI: cppcheck job"]) --> CMake["ci/CMakeLists.txt"] --> Cpp{{"cppcheck"}} --> Env["src/ex/global-env.cpp"]
  CI(["CI: cppcheck job"]) --> CMake["ci/CMakeLists.txt"] --> Cpp{{"cppcheck"}} --> Diag["src/lsp/diagnostics.cpp"]
  subgraph Legend
    direction LR
    _svc(["Service/Job"]) ~~~ _cfg["Config"] ~~~ _tool{{"Tool"}} ~~~ _src["Source file"]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Move suppressions into a dedicated cppcheck suppressions file

• ➕ Keeps CMake invocation concise and easier to scan
• ➕ Enables per-warning comments/justifications near suppressions
• ➕ Often easier to share across local dev and CI
• ➖ Requires additional wiring/maintenance of a separate file
• ➖ May diverge if developers run cppcheck with different entrypoints

2. Prefer code fixes over new suppressions where feasible

• ➕ Reduces the risk of hiding real regressions under broad suppressions
• ➕ Improves code clarity (as done here with const refs / ranges)
• ➖ Not always practical for tool limitations or third-party patterns
• ➖ Can increase churn if warnings are noisy or style-based

Recommendation: The current approach is appropriate: it combines targeted code cleanups with narrowly scoped CI suppressions. If the suppression list continues to grow, consider moving it to a dedicated suppressions file (with brief justification comments) to keep ci/CMakeLists.txt readable and make it easier to audit why each suppression exists.

Files changed (4) +19 / -17

Bug fix (1) +7 / -7

dirctrl.cppFix const-correctness and avoid vector copies in callbacks +7/-7

Fix const-correctness and avoid vector copies in callbacks

• Makes several wx event handler lambdas take events as const references and changes selected-path callbacks to accept const std::vector<path>& rather than by value. This addresses cppcheck warnings and avoids unnecessary copies during UI interactions.

src/del/dirctrl.cpp

Refactor (2) +10 / -9

global-env.cppRename skip flag to avoid shadowing/clarity issues +3/-3

Rename skip flag to avoid shadowing/clarity issues

• Renames a local boolean from skip to skipped and updates its usage. This is a mechanical change intended to eliminate cppcheck warnings and improve readability.

src/ex/global-env.cpp

diagnostics.cppRefactor diagnostic filtering to ranges and pre-reserve output +7/-6

Refactor diagnostic filtering to ranges and pre-reserve output

• Replaces a manual loop/push_back with std::ranges::copy_if for collecting diagnostics for a line. Also reserves space when building the list of URIs to reduce reallocations.

src/lsp/diagnostics.cpp

Other (1) +2 / -1

CMakeLists.txtAdjust cppcheck suppression list for current warnings +2/-1

Adjust cppcheck suppression list for current warnings

• Updates the cppcheck invocation to suppress shadowFunction and functionStatic in addition to existing shadow-related suppressions. This aligns CI static analysis behavior with the warning set being produced in the repo.

ci/CMakeLists.txt

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Missing std includes ✓ Resolved 🐞 Bug ≡ Correctness

Description

src/lsp/diagnostics.cpp now uses std::ranges::copy_if and std::back_inserter but does not
include <algorithm> and <iterator>, so the file may fail to compile on toolchains that don’t
provide these via transitive includes.

Code

src/lsp/diagnostics.cpp[R56-62]

+  std::ranges::copy_if(
+    diags,
+    std::back_inserter(result),
+    [line](const diagnostic_item& diag)
    {
-      result.push_back(diag);
-    }
-  }
+      return diag.range.start.line == line || diag.range.end.line == line;
+    });

Relevance

⭐⭐⭐ High

Likely compile fix; team accepts include-hygiene changes (e.g., removing/adjusting includes in LSP
code).

PR-#1288

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The implementation file only includes wex/lsp/diagnostics.h, but the new code calls
std::ranges::copy_if and std::back_inserter. Neither diagnostics.cpp nor diagnostics.h
includes the standard headers that define those names, so compilation can fail when transitive
includes change or differ by toolchain.

src/lsp/diagnostics.cpp[8-9]
src/lsp/diagnostics.cpp[51-63]
include/wex/lsp/diagnostics.h[8-15]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`src/lsp/diagnostics.cpp` started using `std::ranges::copy_if` and `std::back_inserter`, but the translation unit does not directly include the standard headers that declare these APIs. Relying on transitive includes is not guaranteed and can break compilation depending on standard library / include ordering.

### Issue Context
- `std::ranges::copy_if` is declared in `<algorithm>`
- `std::back_inserter` is declared in `<iterator>`

### Fix Focus Areas
- src/lsp/diagnostics.cpp[1-10]
- src/lsp/diagnostics.cpp[51-63]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud