Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coverage: Library coverage #2567

Closed
onbjerg opened this issue Aug 3, 2022 · 6 comments · Fixed by #7510
Closed

Coverage: Library coverage #2567

onbjerg opened this issue Aug 3, 2022 · 6 comments · Fixed by #7510
Assignees
Labels
C-forge Command: forge Cmd-forge-coverage Command: forge coverage T-feature Type: feature
Milestone

Comments

@onbjerg
Copy link
Member

onbjerg commented Aug 3, 2022

Libraries are currently not covered and they are somewhat non-trivial to map instructions for.

Generally, there are two types of library functions:

  • Internal functions are inlined in the contract that use them
  • External functions are called via delegatecall

The way coverage works currently is in a few passes:

  • First, we detect any sections of code that we want to include in our coverage report
  • Second, we also take the base contracts of a contract into account, since all behavior of base contracts are inherited. We need this to map instructions to the items we generated in the first pass using source maps
  • Third, we use the source maps of concrete contracts to find instructions that mark the items gathered in our first and second step as covered. One item might be covered by multiple instructions in different contracts because of inheritance

The issue with adding library coverage is that the Solidity compiler does not give us any easily digestable information about what libraries are used in contracts. This is in contrast to base contracts which are easily identified using a property of contract AST nodes called linearizedBaseContracts.

In order to reliably find what libraries are used in contracts so we can map instructions onto coverage items present in libraries, we need to walk each node in the AST to find call expression nodes.

These call expression nodes include some crude type information, and in the case of library calls, the type information will show up as e.g. type(library LibraryName).

After parsing this information, we need to map the library name back onto an AST node ID so we can find what coverage items are present in the library. The rest of the analysis step in forge coverage uses AST node IDs to refer to contracts.

Some unknowns:

  • Is the type information correct, i.e. does it take shadowing and import aliases into account?
  • Are library names from the type information easily mappable onto AST node IDs corresponding to the library's AST?
@onbjerg onbjerg added T-feature Type: feature C-forge Command: forge Cmd-forge-coverage Command: forge coverage labels Aug 3, 2022
@onbjerg onbjerg added this to the v1.0.0 milestone Aug 3, 2022
@onbjerg onbjerg self-assigned this Aug 3, 2022
@onbjerg onbjerg added this to Foundry Aug 3, 2022
@onbjerg onbjerg moved this to Todo in Foundry Aug 3, 2022
@onbjerg onbjerg modified the milestones: v1.0.0, v0.3.0 Aug 15, 2022
@KittyFu307
Copy link

Hi @onbjerg , sorry to bother you, I would like to know, if it is a developer-defined library and the functions in the library are all internal functions, the coverage rate running through forge coverage cannot be supported at present, right?

@onbjerg
Copy link
Member Author

onbjerg commented Aug 19, 2022

No libraries are supported at the moment

@KittyFu307
Copy link

Will it support library coverage later?

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Feb 7, 2023

Shouldn't this issue be closed?

It looks like #3128 added support for library coverage, and I can also see that it works in my project.

Update: it looks like no, this issue cannot be just yet. There are still issues with library coverage, e.g. #4305.

@0xdapper
Copy link
Contributor

0xdapper commented Apr 4, 2023

Also generally outside of dynamically linked libraries any and all delegatecall'd code is not being considered as covered I think. Is that right? Might be worth updating the issue with what is the current way of how coverage works.

@PaulRBerg
Copy link
Contributor

Might be worth updating the issue with what is the current way of how coverage works.

Cc @gakonst. Many coverage-related issues are outdated, including #1961.

@jenpaff jenpaff moved this from Shelved to Completed in Foundry Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-coverage Command: forge coverage T-feature Type: feature
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

4 participants