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

Docs : Missing Imports ( chapter 8 ) #1150

Open
0xumarkhatab opened this issue Mar 21, 2024 · 3 comments
Open

Docs : Missing Imports ( chapter 8 ) #1150

0xumarkhatab opened this issue Mar 21, 2024 · 3 comments
Assignees

Comments

@0xumarkhatab
Copy link

Chapter 8 -> section 8.1 lacks import of stdError & Vm

Issue

There are multiple places where forge-std's components are used without importing them .

  1. Lack of stdError and Vm import
A good practice is to use the pattern test_Revert[If|When]_Condition in combination with the [expectRevert](https://book.getfoundry.sh/cheatcodes/expect-revert.html) cheatcode (cheatcodes are explained in greater detail in the following [section](https://book.getfoundry.sh/forge/cheatcodes.html)). Also, other testing practices can be found in the [Tutorials section](https://book.getfoundry.sh/tutorials/best-practices.html). Now, instead of using testFail, you know exactly what reverted and with which error:

    function test_CannotSubtract43() public {
        vm.expectRevert(stdError.arithmeticError);
        testNumber -= 43;
    }

Impact

Beginners get confused when stdError is not found upon compilation of smart contracts because foundry demands explicit imports

Fix

Add following lines in the docs to import used components

import {Vm} from "forge-std/Vm.sol";
import {stdError} from "forge-std/StdError.sol";
@zerosnacks
Copy link
Member

If you import as follows: import "forge-std/Test.sol";, as mentioned at the start, it will work

That said, I think we should modify the example code to reflect the best practices (e.g. named imports), especially as this is the entrypoint for beginners. We should also update the Solidity pragma to use pragma solidity ^0.8.13; to reflect the one generated by forge init.

@0xumarkhatab would you like to open a PR for this?

@zsskar
Copy link

zsskar commented Apr 2, 2024

Can I fix this little issue ?

@onbjerg
Copy link
Member

onbjerg commented Apr 2, 2024

Yes, feel free!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

4 participants