alexfertel/bulloak

bulloak check panics

Closed this issue Β· 4 comments

Ok, here is the weirdest bug report I’ve ever submitted πŸ˜„

Say I have the following tree:

Foo
β”œβ”€β”€ when a
β”‚   └── it X
β”œβ”€β”€ when b
β”‚   └── it Y
└── when the method is called a second time
    └── it Z

bulloak scaffold ./test/Foo.tree -w writes the following file at ./test/Foo.t.sol:

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

contract Foo {
    function test_WhenA() external {
        // it X
    }

    function test_WhenB() external {
        // it Y
    }

    function test_WhenTheMethodIsCalledASecondTime() external {
        // it Z
    }
}

So far so good.

Now say requirements change and now my tree looks like this:

Foo
β”œβ”€β”€ when b
β”‚   └── it Y
└── when a
    └── it X

Notice, two things change in the tree:

  1. when b moves above when a
  2. when the method is called a second time gets removed

I run bulloak check ./test/Foo.tree --fix and get the following error:

thread 'main' panicked at /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bulloak-0.6.3/src/check/violation.rs:418:33:
should parse solidity string: [Diagnostic { loc: File(0, 734, 735), level: Error, ty: ParserError, message: "unrecognised token '}', expected \"(\"", notes: [] }]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

If I don’t reorder when b and when a, bulloak does not panic. So bulloak check ./test/Foo.tree --fix with the following tree works:

Foo
β”œβ”€β”€ when a
β”‚   └── it X
└── when b
    └── it Y

Here comes the surprising part: when I do reorder when a and when b statements, bulloak check only panics if the removed when statement has TheMethod string in it.

Given a modified tree:

Foo
β”œβ”€β”€ when b
β”‚   └── it Y
└── when a
    └── it X

Solidity test files bulloak check works with correctly:

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

contract Foo {
    function test_WhenA() external {
        // it X
    }

    function test_WhenB() external {
        // it Y
    }

-    function test_WhenTheMethodIsCalledASecondTime() external {
+    function test_WhenC() external {
        // it Z
    }
}

Solidity test files bulloak check panics with:

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

contract Foo {
    function test_WhenA() external {
        // it X
    }

    function test_WhenB() external {
        // it Y
    }

-    function test_WhenTheMethodIsCalledASecondTime() external {
+    function test_WhenTheMethod() external {
        // it Z
    }
}

What seems to be causing trouble is TheMethod part of the last test case name.

bulloak version 0.6.3

Oooh, great find! I need to do some digging to understand why this happens. Thanks for bringing this up!

Here is another wired case where bulloak panics.

Given the following test file:

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

contract Foo {
    function test_WhenB() external {
        // it reverts
    }

    function test_GivenA() external {
        // it reverts
    }

    function test_WhenC() external {
        // it X
    }
}

and the following new tree:

Foo
β”œβ”€β”€ given a
β”‚   └── it reverts
└── when b
    └── it reverts

bulloak check ./test/Foo.tree --fix works. But it panics with the following changes to the test file:

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

contract Foo {
    function test_WhenB() external {
        // it reverts
    }

    function test_GivenA() external {
        // it reverts
    }

    function test_WhenC() external {
-        // it X
+        // it need allowance
    }
}
thread 'main' panicked at /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bulloak-0.6.3/src/check/violation.rs:418:33:
should parse solidity string: [Diagnostic { loc: File(0, 772, 773), level: Error, ty: ParserError, message: "unrecognised token '}', expected \"(\", \"++\", \"--\", \".\", \"[\", \"case\", \"constant\", \"default\", \"external\", \"immutable\", \"internal\", \"leave\", \"override\", \"private\", \"public\", \"revert\", \"switch\", \"{\", identifier", notes: [] }]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

And a different panic message with the following test file:

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

contract Foo {
    function test_WhenB() external {
        // it reverts
    }

    function test_GivenA() external {
        // it reverts
    }

    function test_WhenC() external {
-        // it X
+        // it doesn’t for token
    }
}
thread 'main' panicked at /home/john/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bulloak-0.6.3/src/check/violation.rs:418:33:
should parse solidity string: [Diagnostic { loc: File(0, 777, 778), level: Error, ty: ParserError, message: "unrecognised token '}', expected \"(\"", notes: [] }]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Oooh, great find! I need to do some digging to understand why this happens. Thanks for bringing this up!

These issues occurred in bigger test files about 2 weeks ago, finally had time to distill to minimal reproducible examples and report. Happy to help πŸ™‚

Okay, I fixed the three of them f753b7a. The issue was kind of tricky to debug cause check --fix's implementation is brittle. Hopefully, in the future I'll have time to refactor it a bit and improve the test suite.

Let me know if you find any other bug, you are being extremely helpful @matmilbury so thank you!

Will push a new version in a bit Published the fix in v0.6.4!