Forge coverage for modifiers is missing/incomplete

[ChiTimesChi]

Component

Forge

Have you ensured that all of these are up to date?

• Foundry
• Foundryup

What version of Foundry are you on?

forge 0.2.0 (28312e7 2022-10-13T00:10:05.477412592Z)

What command(s) is the bug in?

forge coverage

Operating System

Linux

Describe the bug

Contract example

Contracts Bar and Foo are identical in terms of what they are doing, the difference is whether the security check is done in the function modifier or not.

Full example is available here: https://github.com/ChiTimesChi/coverage-modifier

abstract contract Ownable {
    address public owner;

    constructor() {
        owner = msg.sender;
    }

    modifier onlyOwner() {
        require(owner == msg.sender, "!owner");
        _;
    }
}

contract Bar is Ownable {
    event BarEvent();

    function bar() external {
        require(owner == msg.sender, "!owner");
        emit BarEvent();
    }
}

contract Foo is Ownable {
    event FooEvent();

    function foo() external onlyOwner {
        emit FooEvent();
    }
}

Tests example

This set of tests doesn't include "try calling as not owner" tests.

contract FoobarTest is Test {
    event BarEvent();
    event FooEvent();

    Foo internal foo;
    Bar internal bar;

    function setUp() public {
        foo = new Foo();
        bar = new Bar();
    }

    function testFooAsOwner() public {
        vm.expectEmit(true, true, true, true);
        emit FooEvent();
        foo.foo();
    }

    function testBarAsOwner() public {
        vm.expectEmit(true, true, true, true);
        emit BarEvent();
        bar.bar();
    }
}

Coverage

As you can see, the coverage report hints that the Bar tests are incomplete, but for Foo there's no way to tell that. The coverage for Ownable is missing entirely (contract being non abstract doesn't change anything).

Expected coverage

What I was hoping to see? Ideally, if a function has a few modifiers, the coverage report should not differ too much from the very same function rewritten without any modifiers. Meaning that coverage should give me a clue as to whether the tests are covering not only all branches within the function, but within its modifiers as well.

sc-forks/solidity-coverage#286 is a good read as well.

[ChiTimesChi]

@rkrasiuk any update for this? I wasn't able to find any good solution to this, and ensuring that the tests cover all functions with "security" modifiers like onlyOwner is kind of crucial.

[rkrasiuk]

@onbjerg could you take a look pls?

[yorhodes]

forge coverage basically unusable without this and other bug fixes