Skip to content

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Sep 6, 2025

Closes #10964.

By using Solar to analyze contracts to find items we can avoid requesting AST output from solc, saving a considerable amount of serde time.

This also fixes a few bugs, while trying to remain 100% compatible with the previous solc AST implementation. See individual commits and their descriptions for more details.


Default project:
image

ithacaxyz/account:
image

@DaniPopes DaniPopes self-assigned this Sep 8, 2025
@DaniPopes DaniPopes moved this to In Progress in Foundry Sep 8, 2025
Comment on lines +221 to +222
},
stmt.span,
Copy link
Member Author

@DaniPopes DaniPopes Sep 12, 2025

Choose a reason for hiding this comment

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

this is not entirely equivalent, but the previous logic resulted in weird spans for else-if chains (left is before, right is after):
image

this was added in #3094

@DaniPopes DaniPopes marked this pull request as ready for review September 12, 2025 19:20
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

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

lgtm!

@DaniPopes DaniPopes moved this from In Progress to Ready For Review in Foundry Sep 15, 2025
@onbjerg onbjerg merged commit 632f36b into master Sep 15, 2025
25 checks passed
@onbjerg onbjerg deleted the dani/solar-coverage-1 branch September 15, 2025 12:56
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in Foundry Sep 15, 2025
@grandizzy grandizzy moved this from Done to Completed in Foundry Sep 15, 2025
MerkleBoy pushed a commit to MerkleBoy/foundry that referenced this pull request Sep 17, 2025
* feat(coverage): analyze with solar

* chore: walk stmts normally, sort items

* wip

* chore: display all items with relevant source in debug format

* wip

* upd

* chore: clippy

* test: update do_while_lcov

The `++i` line was reported first, so with the old check all previous
lines were ignored. Now we track all lines regardless, so this change is
more correct.

* fix: legacy: do not recurse into emit, revert

Fixes branch_with_calldata_reads test.

* correct span

* fix: inline config path

* fix: resolve function kinds, ignore type conversions / struct ctors

* fix: walk only functions

* fix: push stmt for yul stmt expr early

* test: add a test case for if (..) return

If statements like `if (x) return y;` were missed by the
previous `has_statements` function.

* test: hoist contract instantiations out of test fns

* test: add test case for single if with continue/break

Same as previous test with `return`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Completed

Development

Successfully merging this pull request may close these issues.

replace Solc AST usage in forge coverage

3 participants