Lint tooling#57
Merged
Merged
Conversation
cppcheck's unusedFunction check sees every first-party source in one pass, so it can flag functions with no caller anywhere in the project, which the per-TU clang-tidy pass cannot. Fix what it found: - get_equalizer() and get_bytes_per_sample() had no caller anywhere; the whole EQ feature had no unit-test coverage (only the fuzzer toggles presets). Add an equalizer_presets test covering the default, the set/get round-trip on every preset, a full decode under a non-flat preset, and the format constants. - Drop a dead total_decoded accumulation in the fuzz harness's EOS flush loop (it is a real cap in the main loop; the flush-loop increment was never read). - Default-initialize TaskParams (benchmark example) and Case (tests), replace two C-style casts in the benchmark, pass the decoder as const ref in mp3_to_wav, narrow a variable scope.
script/check-includes.sh runs clang-include-cleaner over every source and header in two passes: a host pass, and an ESP pass with -DESP_PLATFORM plus stub ESP-IDF headers (materialized from script/esp_stubs.py) so guarded ESP32 branches are analyzed without a cross toolchain. A foo.cpp may rely on its matching header's direct includes; IWYU pragmas are honored. The vendored src/opencore-mp3dec fork is excluded, mirroring cppcheck. Fixes it surfaced: mp3_decoder.cpp used opencore error codes reached only transitively (now includes pvmp3_audio_type_defs.h/pvmp3_dec_defs.h); decode_benchmark called esp_get_free_heap_size() without esp_system.h; missing <cstdint> in three files.
…ary conventions - clang-tidy.sh: honor a pre-set $CLANG_TIDY (CI pins clang-tidy-18) instead of clobbering it, and validate the resolved binary up front - ci.yml: pass CLANG_TIDY=clang-tidy-18 to the lint step; build a pinned cppcheck 2.21.0 from source with a cache step instead of the unpinned apt package, so CI findings match local runs - cppcheck.sh: scan the src directory and exclude the vendored fork with -i, replacing the hand-listed source that could silently omit new files - check_includes.py: add the gather_files prune overlap fix to the shared engine - publish.yml: add timeout-minutes to all four jobs - release-drafter.yml: drop the workflow permissions to the empty set since everything routes through the GitHub App token - .clang-format / .clang-tidy: correct the declared standard to C++14, which is the real floor (relaxed-constexpr loop in is_ape_prefix) and what every host and test build enforces
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds two new static-analysis gates (cppcheck whole-program analysis and clang-include-cleaner–based include checking) and aligns existing lint/CI/config to the stated “microCodec library conventions”, while also adding decoder test coverage for the equalizer accessor API and exercising an EQ preset during a full decode.
Changes:
- Add CI jobs + scripts for whole-program cppcheck and include-what-you-use checking (with ESP-IDF stub headers to analyze
ESP_PLATFORMbranches). - Add an equalizer presets test (round-trip set/get across presets, plus decode under a non-flat preset) and minor test/fuzz cleanups.
- Align tooling/config details (C++14 floor in
.clang-format/.clang-tidy,clang-tidy.shhonoring$CLANG_TIDY, tightened workflow permissions/timeouts, missing include fixes).
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_mp3_decoder.cpp | Adds equalizer_presets test and minor initialization/flow tweaks in existing tests. |
| tests/fuzz/fuzz_mp3_decode.cpp | Removes a dead accumulator update during fuzz flush path. |
| tests/conformance/conformance.cpp | Adds a cppcheck suppression for main signature const-parameter warning. |
| tests/CMakeLists.txt | Registers the new equalizer_presets decoder test. |
| src/mp3_decoder.cpp | Adds explicit OpenCore header includes and <cstdint> for include completeness. |
| script/esp_stubs.py | Introduces ESP-IDF stub-header materialization data for include analysis on host. |
| script/cppcheck.sh | Adds a whole-program cppcheck runner with exclusions/suppressions aligned to repo layout. |
| script/clang-tidy.sh | Allows CI to pin clang-tidy via $CLANG_TIDY and validates the resolved binary. |
| script/check-includes.sh | Adds a thin wrapper to run the include checker. |
| script/check_includes.py | Implements clang-include-cleaner–based include checking with host+ESP passes and optional --fix removals. |
| host_examples/mp3_to_wav/mp3_to_wav.cpp | Adds <cstdint> and makes a helper take const Mp3Decoder&. |
| examples/decode_benchmark/main/decode_benchmark.cpp | Fixes missing includes, removes unused <vector>, and replaces C-style casts / uninitialized fields. |
| .gitignore | Ignores .conventions-version marker file. |
| .github/workflows/release-drafter.yml | Drops default token permissions to {} and relies on a GitHub App token for operations. |
| .github/workflows/publish.yml | Adds timeouts and clarifies/adjusts job permissions (notably re-granting contents: read where needed). |
| .github/workflows/ci.yml | Adds include-check and cppcheck jobs and wires them into the overall CI “all jobs” gate. |
| .clang-tidy | Updates stated language floor to C++14 and removes a trailing comma in the checks list. |
| .clang-format | Updates configured standard from C++11 to C++14. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kahrendt
added a commit
that referenced
this pull request
Jul 7, 2026
Tightens the first-party warning sets and cleans up the C++ standard story. No decoder behavior changes; this is compile-time enforcement plus documentation. Builds on the lint-tooling gates from #57. - **Static internal linkage:** adds `-Wmissing-prototypes` (clang) / `-Wmissing-declarations` (GCC): any function not declared in a header must be static, which keeps `-Wunused-function` able to see dead internal functions. Marked the five `mp3_to_wav` helpers it flagged. - **No C-style casts:** adds `-Wold-style-cast`, requiring `static_cast`/`reinterpret_cast` in first-party C++. The vendored OpenCore include dir is made SYSTEM so its headers don't leak warnings into first-party compiles (the header-side counterpart of the existing `-w` suppression). - **decode() marked `MICRO_MP3_NODISCARD`:** so now silently ignoring the decoder's only error channel warns at every call site. - **C++ standard hygiene:** documents why the fuzz target builds at C++17 (clang's `FuzzedDataProvider.h` uses the C++17 `std::is_*_v` trait aliases and won't compile at C++14) and corrects two comments that wrongly claimed the library only ever builds at C++14. Switches the public header's includes to the C++ `<cstddef>`/`<cstdint>` forms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds two whole-program static-analysis gates and aligns the lint scripts, CI, and config with the microCodec library conventions. No decoder behavior changes; the source edits here are only fixes the new tools surfaced.
script/cppcheck.sh, CI): sees every first-party source in one pass, so it flags functions with no caller anywhere. It found thatget_equalizer()/get_bytes_per_sample()were uncalled and the whole EQ feature had no test coverage. Adds anequalizer_presetstest (default, set/get round-trip on every preset, a full decode under a non-flat preset, format constants). Also dropped a dead accumulator in the fuzz harness and cleaned up a few C-style casts / init sites.script/check-includes.sh,check_includes.py,esp_stubs.py, CI): runs clang-include-cleaner over every source/header in a host pass and an ESP pass (-DESP_PLATFORM+ stub ESP-IDF headers, so guarded ESP32 branches are analyzed without a cross toolchain). Vendoredsrc/opencore-mp3decexcluded. Fixed the missing/transitive includes it surfaced (opencore error-code headers,esp_system.h,<cstdint>).clang-tidy.shhonors a pre-set$CLANG_TIDY(CI pins clang-tidy-18); cppcheck pinned to 2.21.0 built from source so CI matches local;cppcheck.shscanssrc/with-iexclusion instead of a hand-listed set; timeout-minutes on all publish jobs; release-drafter permissions dropped to empty;.clang-format/.clang-tidycorrected to C++14 (the real floor).