Repository navigation
Lint Compiler Warnings in Lambdas on the Generated Code - #262
Merged
Merged
Conversation
Rechecks each compiled example device's generated main.cpp syntax-only with the build's own flags plus -Wdouble-promotion, and fails the compile leg on a warning that lands in this repository's code: a YAML lambda line, a lambda ESPHome inlined as an expression, or a component file copied into the build. Generated and third-party code is set aside. A VS Code task runs the same lint after a local compile and puts findings in the Problems pane. Fixes the two double-promotion lambdas it finds on the builds checked so far. Closes #256
Inline search matches whole tokens, skips an expression that is only an id(), searches only the YAML the build includes, and reports a block lambda's code line. Code after a remote package's #line is set aside. Comments are stripped with ESPHome's own remover, and a global's member access matches its real rewrite. The YAML is read before a --compile run starts, and an unreadable file fails the run. Adds f suffixes to the remaining double literals in the templates and the Dial example, and casts the EasyStart component's float log arguments.
ptr727
added
the
comments
ptr727
added
the
comments
Owner
Author
|
A recorded local strict-review pass covers head |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #262 +/- ##
============================================
+ Coverage 43.07% 84.17% +41.09%
============================================
Files 1 3 +2
Lines 130 613 +483
============================================
+ Hits 56 516 +460
- Misses 74 97 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The review includes one critical compiler-exit handling issue and multiple unresolved attribution, ownership, and documentation findings.
8 open issues
What changed in this PR
This pull request adds a compile-time C++ lint for ESPHome-generated code, integrates it into CI and VS Code, fixes detected floating-point promotion warnings, and documents the workflow.
Changes:
- Added the
compile-lintworkspace package, compiler diagnostics, YAML/source attribution, tests, and lockfile updates. - Integrated linting into pull-request compile jobs and the VS Code compile-and-lint task.
- Updated templates, examples, EasyStart logging, repository documentation, and release notes.
| File | Summary | Review findings |
|---|---|---|
uv.lock |
Locks the new compile-lint package and dependencies. | — |
tools/compile-lint/src/compile_lint/py.typed |
Marks the package as typed. | — |
tools/compile-lint/src/compile_lint/lint.py |
Implements compilation, diagnostic parsing, attribution, annotations, and summaries. | P1 / critical (2 votes): Any nonzero compiler exit must fail the lint, even when diagnostics are unclassified. P2 / moderate (4 votes): Continue inline matching after remote directives. P2 / moderate (3 votes): Include YAML files without local directives in the inline search set. P2 / moderate (4 votes): Preserve raw scalar line mappings for folded multi-line lambdas. P1 / moderate (1 vote): Restrict the YAML index to lambda values. P1 / moderate (2 votes): Preserve block-comment state across source lines. P1 / moderate (1 vote): Exclude generated build roots from direct source ownership. |
tools/compile-lint/src/compile_lint/__main__.py |
Provides module execution support. | — |
tools/compile-lint/src/compile_lint/__init__.py |
Defines the package metadata. | — |
tools/compile-lint/pyproject.toml |
Defines the compile-lint project and console script. | — |
tests/compile_lint/test_lint.py |
Tests compiler invocation, attribution, reporting, and CLI behavior. | — |
test/m5stack-dial.yaml |
Fixes a floating-point promotion warning. | — |
test/m5stack-cores3.yaml |
Fixes a floating-point promotion warning. | — |
templates/smarthome-ceilsense.yaml |
Fixes uptime conversion promotion. | — |
templates/konnected-blaq.yaml |
Fixes uptime conversion promotion. | — |
templates/efun-sh331.yaml |
Fixes calibration-lambda promotion. | — |
templates/basic.yaml |
Fixes a floating-point promotion warning. | — |
templates/apollo-plt-1b.yaml |
Fixes a floating-point promotion warning. | — |
README.md |
Documents the compile lint and related release changes. | P2 / nit (4 votes): Describe findings as landing on YAML or source lines, since owned component diagnostics use source files. |
pyproject.toml |
Adds the workspace member and shared tooling configuration. | — |
OPERATIONS.md |
Documents compile-lint usage and behavior. | P2 / nit (4 votes): Document YAML or source-line locations for component findings. |
HISTORY.md |
Records the compile-lint release changes. | P2 / nit (4 votes): Describe findings as landing on YAML or source lines, since owned component diagnostics use source files. |
GOVERNANCE.md |
Adds the tools directory to the repository layout. | — |
easystart/components/easystart/easystart.h |
Casts floating-point log arguments to double. |
— |
CODESTYLE.md |
Updates Python workspace and generated-code analysis guidance. | — |
.vscode/tasks.json |
Adds the compile-and-lint task and problem matcher. | — |
.github/workflows/test-pull-request.yml |
Runs compile-lint after each applicable compile leg and reports failures. | — |
Review effort: Lite
…he Log Comments are removed across a whole lambda, with each emitted line mapped back to its YAML line, and a plain, quoted or folded lambda is one logical line. Code after a remote package's #line is set aside only where it copies that package's lambda. The inline search covers the build's config, found from ESPHome's storage file, and every repository YAML it includes. A recheck that fails outside this repository's code fails the run. CI reads the build path from the compile log, since a package can set its own build_path. Casts the float printf arguments in the Heltec and NORVI examples.
Owner
Author
|
A recorded local strict-review pass covers head |
Owner
Author
|
A recorded local strict-review pass covers head |
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 a compile-time C++ lint over each ESPHome build, so a compiler warning in this repository's own code surfaces while developing and at pull request time rather than in a log nobody reads.
What It Does
tools/compile-lint(a new uv workspace member) reruns the compile of the generatedmain.cppsyntax-only, with the build's own flags plus-Wdouble-promotion, from the build'scompile_commands.json. It works on the ESP-IDF and the ESP8266 builds alike, and never builds or changes firmware.#linedirectives in a copy, so every diagnostic keeps itsmain.cppline. It then attributes a line to YAML only where it matches the lambda line after ESPHome's own rewrites (substitutions,id(), globals, its comment remover). Generated widget and action code, which ESPHome attributes to the same YAML lines, is set aside.fsuffixes), and casts the EasyStart component's float log arguments.Verification
Local Review
Every
introducedfinding the local review raised is fixed in this pull request, the last being anargs:entry beside a substitutedformat:.Found by CI on the First Run
The first CI run is the lint's real end-to-end test, and it found what the local caches had hidden:
it.printfin the Heltec and NORVI example display lambdas, now cast withstatic_cast<double>(), the maintainer's choice over exempting printf-style calls. Thetext: format:args:entries are linted the same way, and the CoreS3 and Dial float arguments are cast.build_path, so CI now reads the build path from the compile log rather than globbing/cache/build.Pre-existing Findings, Not Filed
pre-existing,style: CODESTYLE.md "This Repository's Python", the sentence opening "The rootpyproject.toml" runs past the 25-word sentence cap.Closes #256 at the promotion to main.
🤖 Generated with Claude Code