fix(solidity): link enum values with case_of instead of contains - #4090
rajatnagda45 wants to merge 1 commit into
Conversation
A Solidity enum value is a discriminant case, not a declared member, so it should hang off its enum with a case_of edge like every other language that has enums (Java Graphify-Labs#1719, C#, Swift, Rust, VB.NET). Solidity was the lone remaining outlier, routing enum values through the same contains path used for struct fields. The relation is not cosmetic: case_of targets are excluded from constructor binding, so an enum value named like a type could previously be mistaken for one. Struct fields keep contains, since a field is a real declaration. Add a regression test asserting enum values get case_of (and not contains) while a struct field keeps contains.
|
Thanks for the pull request, @rajatnagda45. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. PR-changed functions: 1/1 verified (0 proven, 1 may-equivalent, 0 distinguished) · 0 not verified.
Graphify review — findings
Switches Solidity enum values from contains to case_of edges, matching Java, C#, Swift, Rust and VB.NET, while struct fields keep contains. Because case_of targets are excluded from constructor binding, an enum value that shares a name with a type no longer resolves as that type.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 21 functions depend on the 21 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_solidity()— 2 callers, 8 callees
Verification — 21 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 21 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
329 of 329 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_blade_extractor.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_cluster_exclude_hubs.py— full-run-safetytests/test_cobol_extractor.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_method_declarations.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safety- … and 279 more
changed code file(s) with no mapped test (
graphify/extractors/solidity.py) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
No difference found (not proven): No behavior difference found in extract\_solidity (not a proof).
The verifier ran both versions of extract\_solidity on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 more finding(s) on lines outside this diff (see the check run).
|
Landed in v0.9.77 via an authorship-preserving cherry-pick, so your commit is on |
2 participants
Problem
Closes #4089.
A Solidity enum linked its values to the enum with a
containsedge — the relation used for real declared members like struct fields. Every other language with enums emitscase_ofper value (Java #1719, C#, Swift, Rust, VB.NET #4054). Solidity was the lone remaining outlier.Implementation
One-line relation change in
graphify/extractors/solidity.py: theenum_valueedge now usescase_ofinstead ofcontains. Struct fields, state variables, events and errors are untouched — they are real declarations and keepcontains.This is not cosmetic:
case_oftargets are excluded from constructor binding (_member_nidsinextract.py), so an enum value named like a type can no longer be mistaken for a constructable member.Verification
test_solidity_enum_values_emit_case_of_not_contains— asserts enum values getcase_of(notcontains) while a struct field keepscontains. Verified it FAILS on pre-fix v8 and passes with the change.uv run pytest tests/→ 6491 passed, 15 skipped (security/wheel env-only tests deselected).ruff checkclean,pyrightclean on the changed file.Limitations
No behavioural change for struct/event/error members, which correctly remain
contains.