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

fix: only skip modifiers & not regular fns with -m flag #82

Merged
merged 1 commit into from
Jul 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 30 additions & 16 deletions crates/bulloak/tests/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@ fn checks_modifiers_skipped() {
);
}

#[test]
fn checks_modifiers_skipped_issue_81() {
let cwd = env::current_dir().unwrap();
let binary_path = get_binary_path();
let tree_path = cwd.join("tests").join("check").join("issue_81.tree");

let output = cmd(&binary_path, "check", &tree_path, &["-m"]);
let stderr = String::from_utf8(output.stderr).unwrap();

assert!(stderr.contains(
"function \"test_WhenLastUpdatedTimeInPast\" is missing in .sol"
));
}

#[test]
fn checks_missing_sol_file() {
let cwd = env::current_dir().unwrap();
Expand All @@ -71,10 +85,10 @@ fn checks_missing_sol_file() {
cwd.join("tests").join("check").join("no_matching_sol.tree");

let output = cmd(&binary_path, "check", &tree_path, &[]);
let actual = String::from_utf8(output.stderr).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();

assert!(actual.contains("the tree is missing its matching Solidity file"));
assert!(actual.contains("no_matching_sol.tree"));
assert!(stderr.contains("the tree is missing its matching Solidity file"));
assert!(stderr.contains("no_matching_sol.tree"));
}

#[test]
Expand All @@ -84,11 +98,11 @@ fn checks_empty_contract() {
let tree_path = cwd.join("tests").join("check").join("empty_contract.tree");

let output = cmd(&binary_path, "check", &tree_path, &[]);
let actual = String::from_utf8(output.stderr).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();

assert!(actual
assert!(stderr
.contains(r#"function "test_ShouldNeverRevert" is missing in .sol"#));
assert!(actual.contains(
assert!(stderr.contains(
r#"function "test_ShouldNotFindTheSolidityFile" is missing in .sol"#
));
}
Expand All @@ -101,9 +115,9 @@ fn checks_missing_contract() {
cwd.join("tests").join("check").join("missing_contract.tree");

let output = cmd(&binary_path, "check", &tree_path, &[]);
let actual = String::from_utf8(output.stderr).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();

assert!(actual.contains(r#"contract "MissingContract" is missing in .sol"#));
assert!(stderr.contains(r#"contract "MissingContract" is missing in .sol"#));
}

#[test]
Expand All @@ -116,7 +130,7 @@ fn checks_missing_contract_identifier() {
.join("missing_contract_identifier.tree");

let output = cmd(&binary_path, "check", &tree_path, &[]);
let actual = String::from_utf8(output.stderr).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();

let formatted_message = format!(
"{}: an error occurred while parsing the tree: contract name missing at tree root #2\n {} {}",
Expand All @@ -125,7 +139,7 @@ fn checks_missing_contract_identifier() {
tree_path.display()
);

assert!(actual.contains(&formatted_message));
assert!(stderr.contains(&formatted_message));
}

#[test]
Expand All @@ -136,9 +150,9 @@ fn checks_contract_name_mismatch() {
cwd.join("tests").join("check").join("contract_names_mismatch.tree");

let output = cmd(&binary_path, "check", &tree_path, &[]);
let actual = String::from_utf8(output.stderr).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();

assert!(actual.contains(
assert!(stderr.contains(
r#"contract "ContractName" is missing in .sol -- found "ADifferentName" instead"#
));
}
Expand All @@ -153,7 +167,7 @@ fn checks_contract_name_mismatch_multiple_roots() {
.join("contract_names_mismatch_multiple_roots.tree");

let output = cmd(&binary_path, "check", &tree_path, &[]);
let actual = String::from_utf8(output.stderr).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();

let formatted_message = format!(
"{}: an error occurred while parsing the tree: contract name mismatch: expected 'ContractName', found 'MismatchedContractName'\n {} {}",
Expand All @@ -162,7 +176,7 @@ fn checks_contract_name_mismatch_multiple_roots() {
tree_path.display()
);

assert!(actual.contains(&formatted_message));
assert!(stderr.contains(&formatted_message));
}

#[test]
Expand All @@ -172,9 +186,9 @@ fn checks_invalid_tree() {
let tree_path = cwd.join("tests").join("check").join("invalid.tree");

let output = cmd(&binary_path, "check", &tree_path, &[]);
let actual = String::from_utf8(output.stderr).unwrap();
let stderr = String::from_utf8(output.stderr).unwrap();

assert!(actual.contains(
assert!(stderr.contains(
r#"an error occurred while parsing the tree: unexpected token '├'"#
));
}
Expand Down
41 changes: 41 additions & 0 deletions crates/bulloak/tests/check/issue_81.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.22;

import {Integration_Test} from "../../Integration.t.sol";

contract RecentAmountOf_Integration_Concrete_Test is Integration_Test {
function test_RevertGiven_Null() external {
bytes memory callData = abi.encodeCall(
flow.recentAmountOf,
nullStreamId
);
expectRevert_Null(callData);
}

function test_GivenPaused() external givenNotNull {
flow.pause(defaultStreamId);

// It should return zero.
uint128 recentAmount = flow.recentAmountOf(defaultStreamId);
assertEq(recentAmount, 0, "recent amount");
}

function test_WhenLastUpdatedTimeInPresent()
external
givenNotNull
givenNotPaused
{
// Update the last time to the current block timestamp.
updateLastTimeToBlockTimestamp(defaultStreamId);

// It should return zero.
uint128 recentAmount = flow.recentAmountOf(defaultStreamId);
assertEq(recentAmount, 0, "recent amount");
}

function test_BlahBlahBlah() external view givenNotNull givenNotPaused {
// It should return the correct recent amount.
uint128 recentAmount = flow.recentAmountOf(defaultStreamId);
assertEq(recentAmount, ONE_MONTH_STREAMED_AMOUNT, "recent amount");
}
}
11 changes: 11 additions & 0 deletions crates/bulloak/tests/check/issue_81.tree
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
RecentAmountOf_Integration_Concrete_Test
├── given null
│ └── it should revert
└── given not null
├── given paused
│ └── it should return zero
└── given not paused
├── when last updated time in present
│ └── it should return zero
└── when last updated time in past
└── it should return the correct recent amount
24 changes: 13 additions & 11 deletions crates/foundry/src/check/rules/structural_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,18 +144,20 @@ fn check_fns_structure(
// We didn't find a matching function, so this is a
// violation.
None => {
if !ctx.cfg.skip_modifiers {
violations.push(Violation::new(
ViolationKind::MatchingFunctionMissing(
fn_hir.clone(),
hir_idx,
),
Location::Code(
ctx.tree.to_string_lossy().into_owned(),
fn_hir.span.start.line,
),
))
if ctx.cfg.skip_modifiers && fn_hir.is_modifier() {
continue;
}

violations.push(Violation::new(
ViolationKind::MatchingFunctionMissing(
fn_hir.clone(),
hir_idx,
),
Location::Code(
ctx.tree.to_string_lossy().into_owned(),
fn_hir.span.start.line,
),
))
}
}
};
Expand Down
Loading