feature request: add support for different targeting behaviours

[giovannidisiena]

Currently, Bulloak works with .tree files that target a single specific function. This is great for a small number of contracts or a subset of functionality to be tested but doesn't scale when there are lots of contracts and functions.

Ideally, it should be possible to scaffold on a per-contract basis, targeting multiple functions in a single .tree file that is emitted to the same test contract. Consider the following ERC20 tree as an example:

ERC20.t.sol
├── when decimals is called
│   └── it should return the decimals
├── when name is called
│   └── it should return the name
├── when symbol is called
│   └── it should return the symbol
├── when totalSupply is called
│   └── it should return the total supply
├── when balanceOf is called
│   └── it should return the balance of the address
├── when allowance is called
│   └── it should return the allowance of the owner to the spender
├── when mint is called
│   ├── when the caller does not have the minter role
│   │   └── it should revert
│   └── when the caller has the minter role
│       ├── given the address is the zero address
│       │   └── it should revert
│       └── given the address is not the zero address
│           ├── given the amount causes the total supply to overflow
│           │   └── it should revert
│           └── given the amount does not cause the total supply to overflow
│               ├── it should mint the tokens
│               ├── it should update the total supply
│               └── it should emit a {Transfer} event
├── when burn is called
│   ├── when the caller does not have sufficient balance
│   │   └── it should revert
│   └── when the caller has sufficient balance
│       ├── it should burn the tokens
│       ├── it should update the total supply
│       └── it should emit a {Transfer} event
├── when transfer is called
│   ├── given the address is the zero address
│   │   └── it should revert
│   └── given the address is not the zero address
│       ├── when the sender does not have sufficient balance
│       │   └── it should revert
│       └── when the sender has sufficient balance
│           ├── it should transfer the tokens
│           ├── it should emit a {Transfer} event
│           └── it should return true
├── when approve is called
│   ├── given the address is the zero address
│   │   └── it should revert
│   └── given the address is not the zero address
│       ├── it should set the allowance
│       ├── it should emit an {Approval} event
│       └── it should return true
├── when transferFrom is called
│   ├── given the from address is the zero address
│   │   └── it should revert
│   └── given the from address is not the zero address
│       ├── given the to address is the zero address
│       │   └── it should revert
│       └── given the to address is not the zero address
│           ├── given the amount is zero
│           │   └── it should revert
│           └── given the amount is not zero
│               └── when the spender is not the from address
│                   ├── when the spender has maximum allowance
│                   │   ├── it should transfer the tokens
│                   │   ├── it should emit a {Transfer} event
│                   │   └── it should return true
│                   └── when the spender does not have maximum allowance
│                       ├── when the spender does not have sufficient allowance
│                       │   └── it should revert
│                       └── when the spender has sufficient allowance
│                           ├── it should decrement the allowance
│                           ├── it should transfer the tokens
│                           ├── it should emit a {Transfer} event
│                           └── it should return true

For maximum flexibility of the directory structure, it may also be useful to specify the trees to be emitted to the same test contract. For example, it may be desirable to have all .tree files within some parent trees directory, further organized into subdirectories (e.g., token/ERC20, libraries/Strings,vendor/AccessControl`), which can be arbitrarily combined to scaffold any number of test files.

[alexfertel]

Hmmm, I think there is a gap in my understanding, so help me out here.

Consider the following ERC20 tree as an example:

The tree above is currently failing because of the dots on the name of the contract ERC20.t.sol, if you change it to ERC20 it should output something like:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.0;

contract ERC20 {
    function test_WhenDecimalsIsCalled() external {
        // it should return the decimals
    }

    function test_WhenNameIsCalled() external {
        // it should return the name
    }

    function test_WhenSymbolIsCalled() external {
        // it should return the symbol
    }

    function test_WhenTotalSupplyIsCalled() external {
        // it should return the total supply
    }

    function test_WhenBalanceOfIsCalled() external {
        // it should return the balance of the address
    }

    function test_WhenAllowanceIsCalled() external {
        // it should return the allowance of the owner to the spender
    }

    modifier whenMintIsCalled() {
        _;
    }

    function test_RevertWhen_TheCallerDoesNotHaveTheMinterRole() external whenMintIsCalled {
        // it should revert
    }

    modifier whenTheCallerHasTheMinterRole() {
        _;
    }

    function test_RevertGiven_TheAddressIsTheZeroAddress() external whenMintIsCalled whenTheCallerHasTheMinterRole {
        // it should revert
    }
}

I'll patch it and publish v0.6.1 so that names with dots are sanitized.

In yesterday's tweet, I thought you were referring to the following .tree:

ERC20.decimals
└── when decimals is called
    └── it should return the decimals

ERC20.name
└── when name is called
    └── it should return the decimals

ERC20.symbols
└── when symbols is called
    └── it should return the decimals

...

which is not supported right now by bulloak. Is this what you were thinking? Or is the above structure of one (1) tree good enough?

For maximum flexibility of the directory structure, it may also be useful to specify the trees to be emitted to the same test contract. For example, it may be desirable to have all .tree files within some parent trees directory, further organized into subdirectories (e.g., token/ERC20, libraries/Strings, vendor/AccessControl`), which can be arbitrarily combined to scaffold any number of test files.

I see, so something like bulloak scaffold tests/**/*.tree --contract TestContract.t.sol which would override all the contract names defined in the trees and output the functions to a TestContract.t.sol.

---

Next steps:

• Patch contract name bug in v0.6.1.
• Maybe implement multiple roots in a single .tree.
• Implement emitting multiple trees to a single contract.

Wdyt @giovannidisiena ?

[alexfertel]

Ok, published v0.6.1 which should fix name sanitizing.

[giovannidisiena]

The tree above is currently failing because of the dots on the name of the contract ERC20.t.sol

Actually it appears it is currently failing due to a condition being found more than once (e.g. "when the caller does not have the admin role" on lines 112 and 122).

In yesterday's tweet, I thought you were referring to the following .tree:

This would work nicely! The structure I shared is good enough, assuming the above issue can be fixed, but I much prefer the layout you propose.

I see, so something like bulloak scaffold tests/*/.tree --contract TestContract.t.sol which would override all the contract names defined in the trees and output the functions to a TestContract.t.sol.

Yes, exactly.

Agree with the next steps. Appreciate the help @alexfertel 🙏

[alexfertel]

Actually it appears it is currently failing due to a condition being found more than once (e.g. "when the caller does not have the admin role" on lines 112 and 122).

Yes, conditions can't be duplicated because that would not be valid Solidity syntax.

This would work nicely! The structure I shared is good enough, assuming the above issue can be fixed, but I much prefer the layout you propose.

Yeah, I'll work on adding support for this, but it might take a while.

Thanks for bringing these up!

[giovannidisiena]

Yes, conditions can't be duplicated because that would not be valid Solidity syntax.

Ahh makes sense.

Yeah, I'll work on adding support for this, but it might take a while.

No worries. Let me know if I can contribute at all - I’m new to Rust, so may need some guidance, but might be good impetus to get stuck in.

[alexfertel]

Let me know if I can contribute at all - I’m new to Rust, so may need some guidance, but might be good impetus to get stuck in.

Of course! To be quite honest, this one might not be a great first issue to tackle complexity-wise, but if you're down to try it, please do!

There are a few decisions to make, like how do we parse the separate trees, and then how do we combine the different HIRs.

High-level pointers:

• bulloak scaffold:
  1. Find all roots in a .tree.
  2. Parse & translate them.
  3. Check that all roots belong to the same contract, i.e. Contract for each root Contract.function is the same.
  4. Merge all the HIRs by collecting the function nodes and appending them to the root contract node.
  5. Combine and report any errors.
  6. Add tests for all of the above.
• bulloak check. Most of the work for this one should be done with the above since check mostly works with parse trees derived from HIRs.
  1. Add iii from scaffold as a check.

If you are gonna give it a go lmk! Otherwise, I'll work on it when I have some time. Also, if you have any questions don't hesitate to ask.

[giovannidisiena]

Thanks for the pointers. I'm familiarising myself with the code at the moment, so will plan to give it a go and will keep you updated!

[giovannidisiena]

@alexfertel I opened a draft PR #51 with some initial thoughts and questions - feedback would be greatly appreciated 🙏