Skip to content

Commit

Permalink
Add bindings test to Sanctuary (#1159)
Browse files Browse the repository at this point in the history
This PR adds a step to Sanctuary testing to exercise bindings. We now
also consider a Sanctuary test to fail if any of the references found in
it cannot be resolved to one (or possibly more) definitions.

This also includes a small binding rules fix to avoid crashing if we
find a Solidity source file with an assembly block that has a `YulPath`
with more than one identifier (eg. a member access like `x.slot`). This
is properly tested in #1149, but without it running Sanctuary tests will
crash since there are contracts with the aforementioned contents.
  • Loading branch information
ggiraldez authored Nov 28, 2024
1 parent 3a82f06 commit 6cb6190
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 24 deletions.
7 changes: 6 additions & 1 deletion .github/workflows/sanctuary.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ on:
type: "string"
required: true
default: "mainnet"
check_bindings:
description: "Check name bindings on contracts, failing if there's any unresolved symbol."
type: "boolean"
required: false
default: false

jobs:
sanctuary:
Expand Down Expand Up @@ -54,4 +59,4 @@ jobs:
- name: "infra run solidity_testing_sanctuary"
uses: "./.github/actions/devcontainer/run"
with:
runCmd: "./scripts/bin/infra run --release --bin solidity_testing_sanctuary -- --shards-count ${{ env.SHARDS_COUNT }} --shard-index ${{ matrix.shard_index }} ${{ inputs.chain }} ${{ inputs.network }}"
runCmd: "./scripts/bin/infra run --release --bin solidity_testing_sanctuary -- --shards-count ${{ env.SHARDS_COUNT }} --shard-index ${{ matrix.shard_index }} ${{ inputs.check_bindings == true && '--check-bindings' || '' }} ${{ inputs.chain }} ${{ inputs.network }}"
1 change: 1 addition & 0 deletions Cargo.lock

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

4 changes: 3 additions & 1 deletion crates/solidity/inputs/language/bindings/rules.msgb
Original file line number Diff line number Diff line change
Expand Up @@ -2265,9 +2265,11 @@ inherit .lexical_scope
edge @path.lexical_scope -> @expr.lexical_scope
}

@path [YulPath @name [YulIdentifier]] {
@path [YulPath] {
node @path.lexical_scope
}

@path [YulPath @name [YulIdentifier]] {
node ref
attr (ref) node_reference = @name

Expand Down

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

3 changes: 2 additions & 1 deletion crates/solidity/testing/sanctuary/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ rayon = { workspace = true }
semver = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
slang_solidity = { workspace = true, features = ["__private_ariadne_errors"] }
metaslang_bindings = { workspace = true }
slang_solidity = { workspace = true, features = ["__private_ariadne_errors", "__experimental_bindings_api"] }
strum_macros = { workspace = true }
url = { workspace = true }

Expand Down
10 changes: 9 additions & 1 deletion crates/solidity/testing/sanctuary/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl Events {
};
}

pub fn parse_error(&self, message: impl AsRef<str>) {
fn test_error(&self, message: impl AsRef<str>) {
match self.failed.position().cmp(&MAX_PRINTED_FAILURES) {
cmp::Ordering::Less => {
self.reporter.println(message);
Expand All @@ -122,6 +122,14 @@ impl Events {
};
}

pub fn parse_error(&self, message: impl AsRef<str>) {
self.test_error(message);
}

pub fn bindings_error(&self, message: impl AsRef<str>) {
self.test_error(message);
}

pub fn trace(&self, message: impl AsRef<str>) {
self.reporter.println(message);
}
Expand Down
17 changes: 11 additions & 6 deletions crates/solidity/testing/sanctuary/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ struct Cli {
/// Disables parallelism, and logs traces to help with debugging errors or panics.
#[arg(long, default_value_t = false)]
trace: bool,

/// Enables checking bindings for each contract, failing if any symbol cannot be resolved.
#[arg(long, default_value_t = false)]
check_bindings: bool,
}

#[derive(Debug, Parser)]
Expand All @@ -45,6 +49,7 @@ fn main() -> Result<()> {
chain,
sharding_options,
trace,
check_bindings,
} = Cli::parse();

Terminal::step(format!(
Expand Down Expand Up @@ -80,9 +85,9 @@ fn main() -> Result<()> {
events.start_directory(files.len());

if trace {
run_with_traces(files, &events)?;
run_with_traces(files, &events, check_bindings)?;
} else {
run_in_parallel(files, &events)?;
run_in_parallel(files, &events, check_bindings)?;
}

events.finish_directory();
Expand All @@ -104,26 +109,26 @@ fn main() -> Result<()> {
Ok(())
}

fn run_with_traces(files: &Vec<SourceFile>, events: &Events) -> Result<()> {
fn run_with_traces(files: &Vec<SourceFile>, events: &Events, check_bindings: bool) -> Result<()> {
for file in files {
let compiler = &file.compiler;
let path = file.path.strip_repo_root()?;

events.trace(format!("[{compiler}] Starting: {path:?}"));

run_test(file, events)?;
run_test(file, events, check_bindings)?;

events.trace(format!("[{compiler}] Finished: {path:?}"));
}

Ok(())
}

fn run_in_parallel(files: &Vec<SourceFile>, events: &Events) -> Result<()> {
fn run_in_parallel(files: &Vec<SourceFile>, events: &Events, check_bindings: bool) -> Result<()> {
files
.par_iter()
.panic_fuse(/* Halt as soon as possible if a child panics */)
.try_for_each(|file| run_test(file, events))
.try_for_each(|file| run_test(file, events, check_bindings))
}

#[test]
Expand Down
124 changes: 111 additions & 13 deletions crates/solidity/testing/sanctuary/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
use std::cmp::min;
use std::path::Path;
use std::sync::Arc;

use anyhow::Result;
use infra_utils::paths::PathExtensions;
use itertools::Itertools;
use metaslang_bindings::PathResolver;
use semver::Version;
use slang_solidity::cst::NonterminalKind;
use slang_solidity::parser::Parser;
use slang_solidity::bindings::Bindings;
use slang_solidity::cst::{Cursor, NonterminalKind, TextIndex, TextRange};
use slang_solidity::diagnostic::{Diagnostic, Severity};
use slang_solidity::parser::{ParseOutput, Parser};
use slang_solidity::{bindings, transform_built_ins_node};

use crate::datasets::{DataSet, SourceFile};
use crate::events::{Events, TestOutcome};
Expand Down Expand Up @@ -59,7 +64,7 @@ pub(crate) fn select_tests<'d>(
}
}

pub fn run_test(file: &SourceFile, events: &Events) -> Result<()> {
pub fn run_test(file: &SourceFile, events: &Events, check_bindings: bool) -> Result<()> {
if !file.path.exists() {
// Index can be out of date:
events.test(TestOutcome::NotFound);
Expand Down Expand Up @@ -100,23 +105,36 @@ pub fn run_test(file: &SourceFile, events: &Events) -> Result<()> {

let parser = Parser::create(version.clone())?;
let output = parser.parse(NonterminalKind::SourceUnit, &source);
let source_id = file.path.strip_repo_root()?.unwrap_str();

if output.is_valid() {
events.test(TestOutcome::Passed);
return Ok(());
}
let with_color = true;

events.test(TestOutcome::Failed);
if !output.is_valid() {
for error in output.errors() {
let report = slang_solidity::diagnostic::render(error, source_id, &source, with_color);

let with_color = true;
let source_id = file.path.strip_repo_root()?.unwrap_str();
events.parse_error(format!("[{version}] {report}"));
}

for error in output.errors() {
let report = slang_solidity::diagnostic::render(error, source_id, &source, with_color);
events.test(TestOutcome::Failed);
return Ok(());
}

events.parse_error(format!("[{version}] {report}"));
if check_bindings {
let unresolved_references = run_bindings_check(&version, source_id, &output)?;
if !unresolved_references.is_empty() {
for unresolved in &unresolved_references {
let report =
slang_solidity::diagnostic::render(unresolved, source_id, &source, with_color);
events.bindings_error(format!("[{version}] {report}"));
}

events.test(TestOutcome::Failed);
return Ok(());
}
}

events.test(TestOutcome::Passed);
Ok(())
}

Expand Down Expand Up @@ -161,3 +179,83 @@ fn uses_exotic_parser_bug(file: &Path) -> bool {
.iter()
.any(|path| file.ends_with(path))
}

fn run_bindings_check(
version: &Version,
source_id: &str,
output: &ParseOutput,
) -> Result<Vec<UnresolvedReference>> {
let mut unresolved = Vec::new();
let bindings = create_bindings(version, source_id, output)?;

for reference in bindings.all_references() {
if reference.get_file().is_system() {
// skip built-ins
continue;
}
// We're not interested in the exact definition a reference resolves
// to, so we lookup all of them and fail if we find none.
if reference.definitions().is_empty() {
let cursor = reference.get_cursor().unwrap();
unresolved.push(UnresolvedReference { cursor });
}
}

Ok(unresolved)
}

fn create_bindings(version: &Version, source_id: &str, output: &ParseOutput) -> Result<Bindings> {
let mut bindings = bindings::create_with_resolver(
version.clone(),
Arc::new(SingleFileResolver {
source_id: source_id.into(),
}),
);
let parser = Parser::create(version.clone())?;
let built_ins_tree = parser
.parse(
NonterminalKind::SourceUnit,
bindings::get_built_ins(version),
)
.tree();
let built_ins_cursor =
transform_built_ins_node(&built_ins_tree).cursor_with_offset(TextIndex::ZERO);

bindings.add_system_file("built_ins.sol", built_ins_cursor);
bindings.add_user_file(source_id, output.create_tree_cursor());
Ok(bindings)
}

/// Bindings `PathResolver` that always resolves to the given `source_id`.
/// This is useful for Sanctuary since all dependencies are concatenated in the
/// same file, but the import directives are retained.
struct SingleFileResolver {
source_id: String,
}

impl PathResolver for SingleFileResolver {
fn resolve_path(&self, _context_path: &str, _path_to_resolve: &str) -> Option<String> {
Some(self.source_id.clone())
}
}

struct UnresolvedReference {
pub cursor: Cursor,
}

impl Diagnostic for UnresolvedReference {
fn text_range(&self) -> TextRange {
self.cursor.text_range()
}

fn severity(&self) -> Severity {
Severity::Error
}

fn message(&self) -> String {
format!(
"Unresolved reference to `{symbol}`",
symbol = self.cursor.node().unparse()
)
}
}

0 comments on commit 6cb6190

Please sign in to comment.