Skip to content

feat(ci,tooling): add vulture dead code detection to just & ci#2621

Merged
fselmo merged 5 commits into
ethereum:forks/amsterdamfrom
knQzx:add-vulture-dead-code-check
Apr 9, 2026
Merged

feat(ci,tooling): add vulture dead code detection to just & ci#2621
fselmo merged 5 commits into
ethereum:forks/amsterdamfrom
knQzx:add-vulture-dead-code-check

Conversation

@knQzx

@knQzx knQzx commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • wired up vulture into the just static pipeline so it runs automatically in CI alongside other lint checks
  • added whitelist entries for false positives (dataclass fields, abstract methods, libcst visitor hooks, dynamically discovered classes)
  • vulture was already in the dependencies and had config in pyproject.toml, it just wasn't hooked into the static analysis target

What changed

  • Justfile: added deadcode recipe, included it in static target
  • vulture_whitelist.py: added entries for 7 groups of false positives with comments explaining each

Test plan

  • uv run vulture src/ vulture_whitelist.py exits cleanly with 0 findings
  • just deadcode runs successfully

fixes #2531

@danceratopz danceratopz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Comment thread vulture_whitelist.py
@danceratopz danceratopz added A-tooling Area: Improvements or changes to auxiliary tooling such as uv, ruff, mypy, ... C-feat Category: an improvement or new feature A-ci Area: Continuous Integration labels Apr 7, 2026
@danceratopz danceratopz changed the title add vulture dead code detection to ci feat(ci,tooling): add vulture dead code detection to just & ci Apr 7, 2026
knQzx and others added 4 commits April 7, 2026 15:17
The `is_create` field on `Message` was added in c40db83
("refactor(spec-specs): Refactor state changes and frame hierarchy
(ethereum#1841)") but was never read anywhere. It is assigned in four
locations (`message.py`, `fork.py`, `system.py` x2) yet no logic
branches on its value. Vulture correctly flagged it as unused.

Remove the field from the dataclass and all assignment sites
rather than whitelisting it.
`ForkLoad._forks` was assigned in `__init__` but never read. It
became dead code in 0d2c52c ("feat(spec-tools): teach
`Hardfork` to clone forks") which refactored `ForkLoad` to store
a `hardfork: Final[Hardfork]` and rewrote `proof_of_stake` and
`is_after_fork()` to use it directly, but did not remove the
orphaned `_forks` attribute or its `Hardfork.discover()` call.

Remove the annotation and the assignment rather than whitelisting
it.
@codecov

codecov Bot commented Apr 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.24%. Comparing base (4bf8bbe) to head (0b059fb).
⚠️ Report is 7 commits behind head on forks/amsterdam.

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #2621   +/-   ##
================================================
  Coverage            86.24%   86.24%           
================================================
  Files                  599      599           
  Lines                36984    36984           
  Branches              3795     3795           
================================================
  Hits                 31895    31895           
  Misses                4525     4525           
  Partials               564      564           
Flag Coverage Δ
unittests 86.24% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@danceratopz danceratopz requested a review from SamWilsn April 8, 2026 13:30

@danceratopz danceratopz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! I removed two instances of dead code, @SamWilsn are you happy with those removals? Please see the commit messages for more details.

@SamWilsn

SamWilsn commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

@fselmo added Message.is_create, so I'm just double checking on that.

@fselmo

fselmo commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

@fselmo added Message.is_create, so I'm just double checking on that.

Yeah is_create was added in an older refactor. However, its functionality was essentially removed in this refactor but the field was still kept. Looks to be dead code after that PR 👍🏼

@fselmo fselmo merged commit fa12e99 into ethereum:forks/amsterdam Apr 9, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ci Area: Continuous Integration A-tooling Area: Improvements or changes to auxiliary tooling such as uv, ruff, mypy, ... C-feat Category: an improvement or new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable vulture dead code checks in CI

4 participants