Skip to content

Commit

Permalink
Implement binding rules for control statements (#1076)
Browse files Browse the repository at this point in the history
This PR adds support in the binding rules for:

- `if`/`else` statements
- `for` loops
- `while` and `do-while` loops
- `emit` statements and basic event definitions
- `try-catch` statements
- `revert` and basic error definitions
- `unchecked` blocks

It also includes a reorganization of the rules file to improve
readability and maintainability. Previously we were attempting to
maintain compatibility with strict evaluation mode which meant that
nodes and edges needed to be defined in previous rules. But since to
build the stack graph the lazy evaluation mode is used anyway, this
restriction can be dropped.

The file is now organized more or less hierarchically, with all the
rules affecting a node and its descendants grouped together.
  • Loading branch information
ggiraldez authored Aug 16, 2024
1 parent 05dd100 commit 7870dc9
Show file tree
Hide file tree
Showing 47 changed files with 2,084 additions and 530 deletions.
727 changes: 469 additions & 258 deletions crates/solidity/inputs/language/bindings/rules.msgb

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ pub enum AssertionError {
#[error("Duplicate assertion definition {0}")]
DuplicateDefinition(String),

#[error("No assertions found in line {0}")]
NoAssertionsFound(usize),

#[error("Failed {failed} of {total} bindings assertions:\n{errors:#?}")]
FailedAssertions {
failed: usize,
Expand Down Expand Up @@ -122,7 +125,7 @@ impl<'a> fmt::Display for DisplayCursor<'a> {
}

/// Collects bindings assertions in comments in the parsed source code
/// accessible through the given cursor. The definitionns take the following form:
/// accessible through the given cursor. The definitions take the following form:
///
/// uint x;
/// // ^def:1
Expand All @@ -149,12 +152,16 @@ impl<'a> fmt::Display for DisplayCursor<'a> {
/// // ^ref:2
/// //<ref:1
///
/// All single line comments should contain an assertion. It is considered an
/// error if they don't.
///
pub fn collect_assertions_into<'a>(
assertions: &mut Assertions<'a>,
mut cursor: Cursor,
file: &'a str,
version: &Version,
) -> Result<(), AssertionError> {
) -> Result<usize, AssertionError> {
let mut skipped = 0;
loop {
if cursor
.node()
Expand All @@ -167,7 +174,13 @@ pub fn collect_assertions_into<'a>(
Some(Assertion::Reference(assertion)) => {
assertions.insert_reference_assertion(assertion);
}
None => (),
Some(Assertion::Skipped) => {
skipped += 1;
}
None => {
let line = cursor.text_offset().line + 1;
return Err(AssertionError::NoAssertionsFound(line));
}
}
}

Expand All @@ -176,13 +189,14 @@ pub fn collect_assertions_into<'a>(
}
}

Ok(())
Ok(skipped)
}

#[derive(Clone, Debug, PartialEq)]
enum Assertion<'a> {
Definition(DefinitionAssertion<'a>),
Reference(ReferenceAssertion<'a>),
Skipped,
}

static ASSERTION_REGEX: Lazy<Regex> = Lazy::new(|| {
Expand Down Expand Up @@ -253,7 +267,7 @@ fn find_assertion_in_comment<'a>(
// Assertion target may not be parseable with the current version
if let Some(version_req) = version_req {
if !version_req.matches(version) {
return Ok(None);
return Ok(Some(Assertion::Skipped));
}
}
Err(AssertionError::InvalidAssertion(
Expand Down Expand Up @@ -283,6 +297,11 @@ fn search_asserted_node_backwards(mut cursor: Cursor, anchor_column: usize) -> O
Ordering::Greater => continue,
Ordering::Less => (),
}
// Skip over empty nodes which the parser may insert to fulfill the
// grammar (eg. commonly an empty Statements node)
if cursor.text_range().is_empty() {
continue;
}

// Node is not found, and probably the anchor is invalid
break;
Expand Down Expand Up @@ -368,8 +387,6 @@ fn check_reference_assertion(
version: &Version,
assertion: &ReferenceAssertion<'_>,
) -> Result<(), String> {
let resolution = find_and_resolve_reference(bindings, assertion)?;

let ReferenceAssertion {
id, version_req, ..
} = assertion;
Expand All @@ -380,6 +397,18 @@ fn check_reference_assertion(
true
};

let resolution = match find_and_resolve_reference(bindings, assertion) {
Ok(resolution) => resolution,
Err(err) => {
if version_matches {
return Err(err);
}
// the reference was not found, but that's ok if the assertion
// should not match for this version
None
}
};

match (version_matches, id) {
(true, None) => {
if let Some(resolved_handle) = resolution {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ fn check_assertions_with_version(version: &Version, contents: &str) -> Result<()
let mut bindings =
bindings::create_with_resolver(version.clone(), Arc::new(TestsPathResolver {}));
let mut assertions = Assertions::new();
let mut skipped = 0;

let parts = split_multi_file(contents);

Expand All @@ -50,7 +51,7 @@ fn check_assertions_with_version(version: &Version, contents: &str) -> Result<()
}

bindings.add_file(file_path, parse_output.create_tree_cursor());
collect_assertions_into(
skipped += collect_assertions_into(
&mut assertions,
parse_output.create_tree_cursor(),
file_path,
Expand All @@ -63,7 +64,7 @@ fn check_assertions_with_version(version: &Version, contents: &str) -> Result<()
match result {
Ok(count) => {
assert!(count > 0, "No assertions found with version {version}");
println!("Version {version}, {count} assertions OK");
println!("Version {version}, {count} assertions OK, {skipped} skipped");
}
Err(err) => {
panic!("Failed bindings assertions in version {version}:\n{err}");
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ contract Second {
// ^ref:5
}
function get_first_choice() public returns (First.Choice) {
// Cannot access a member function through the contract type
return First.get_choice();
// ^ref:1
// ^ref:!
// ^ref:! -- cannot access a member function through the contract type
}
function other_choice() public returns (First.Choice) {
// Cannot access a state variable in another contract
return First.choice;
// ^ref:1
// ^ref:!
// ^ref:! -- cannot access a state variable in another contract
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
contract Test {
function test() public returns (int) {
int i = 1;
// ^def:1
do {
int odd = i % 2;
// ^def:2
// ^ref:1
i *= 3;
//<ref:1
} while (i < 100);
// ^ref:1
return odd;
// ^ref:! (>= 0.5.0)
// ^ref:2 (< 0.5.0)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
contract Test {
event TestEvent(int id);
// ^def:1

function test_emit() public {
int x = 1;
// ^def:2

emit TestEvent(x);
// ^ref:2 (>= 0.4.21)
// ^ref:1 (>= 0.4.21)
emit Utils.Debug(x);
// ^ref:2 (>= 0.4.21)
// ^ref:4 (>= 0.4.21)
// ^ref:3 (>= 0.4.21)
}
}

library Utils {
// ^def:3
event Debug(int subject);
// ^def:4
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
contract Test {
function empty_init() public {
int i = 1;
// ^def:1
int x = 0;
// ^def:2
for (; i < 10; i++) {
// ^ref:1
// ^ref:1
x += i;
//<ref:2
// ^ref:1
}
}

function empty_condition() public {
for (int i = 0;; i++) {
// ^def:3
// ^ref:3
if (i > 10) break;
// ^ref:3
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
contract Test {
function simple_loop() public returns (int) {
int x = 1;
// ^def:1
for (int i = 0; i < 10; i++) {
// ^def:2
// ^ref:2
// ^ref:2
x = x * 2;
//<ref:1
// ^ref:1
}
return x + i;
// ^ref:! (>= 0.5.0)
// ^ref:2 (< 0.5.0)
}

function loop_with_outside_var() public {
int z;
// ^def:3
int w = 0;
// ^def:4
for ( z = 10; z > 0; w += z) {
//^ref:3
// ^ref:3
// ^ref:4
// ^ref:3
z--;
//<ref:3
}
}
}
Loading

0 comments on commit 7870dc9

Please sign in to comment.