Skip to content

Commit

Permalink
Bindings API improvements and resolution across imported files (#1053)
Browse files Browse the repository at this point in the history
This PR implements the following changes:

- Add Solidity binding rules to resolve bindings across imported files.
- Add a new trait to resolve paths for imported files in source units.
- Implement a no-op path resolver and one that can resolve only relative
imports as specified
[here](https://docs.soliditylang.org/en/v0.8.26/path-resolution.html#relative-imports).
- Change all bindings test infrastructure (both snapshots and
assertions) to support providing multiple Solidity source files from a
single file `.sol` file.
- When resolving a reference to a definition, always pick the longest
path available. This ensures that we get the actual definition of the
identifier reference and not another definition along the path (eg. an
import alias).
- Remove the `Handle` interface from the bindings and replace it with
separate `Definition` and `Reference`.
- Support definiens nodes in the rules file (and add them for the
existing Solidity rules) to get to the CST node actually being defined
by a given identifier.
- Add a getter to `Definition` to get the definiens CST node.
- Provide separate functions to lookup either a `Definition` or a
`Reference` given a cursor (it was previously not possible to retrieve
both of them from an identifier that connected to separate stack graph
nodes)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Omar Tawfik <15987992+OmarTawfik@users.noreply.github.com>
Co-authored-by: Igor Matuszewski <Xanewok@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Antony Blakey <antony@nomic.foundation>
  • Loading branch information
5 people committed Aug 8, 2024
1 parent cb36a4b commit 00b2aa3
Show file tree
Hide file tree
Showing 64 changed files with 1,434 additions and 656 deletions.
14 changes: 10 additions & 4 deletions crates/codegen/runtime/cargo/src/runtime/bindings/mod.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
#[path = "generated/binding_rules.rs"]
mod binding_rules;

use metaslang_bindings;
use std::sync::Arc;

use metaslang_bindings::{self, PathResolver};
use semver::Version;

use crate::cst::KindTypes;

pub type Bindings = metaslang_bindings::Bindings<KindTypes>;
pub type Handle<'a> = metaslang_bindings::Handle<'a, KindTypes>;
pub type Definition<'a> = metaslang_bindings::Definition<'a, KindTypes>;
pub type Reference<'a> = metaslang_bindings::Reference<'a, KindTypes>;

pub fn create(version: Version) -> Bindings {
Bindings::create(version, binding_rules::BINDING_RULES_SOURCE)
pub fn create_with_resolver(
version: Version,
resolver: Arc<dyn PathResolver + Sync + Send>,
) -> Bindings {
Bindings::create(version, binding_rules::BINDING_RULES_SOURCE, resolver)
}

#[cfg(feature = "__private_testing_utils")]
Expand Down
86 changes: 0 additions & 86 deletions crates/codegen/runtime/cargo/src/runtime/cli/commands/bindings.rs

This file was deleted.

2 changes: 0 additions & 2 deletions crates/codegen/runtime/cargo/src/runtime/cli/commands/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use thiserror::Error;

#[cfg(feature = "__experimental_bindings_api")]
pub mod bindings;
pub mod parse;

#[derive(Debug, Error)]
Expand Down
15 changes: 0 additions & 15 deletions crates/codegen/runtime/cargo/src/runtime/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,6 @@ pub enum Commands {
#[clap(long)]
json: bool,
},

/// This is only intended for internal development
#[cfg(feature = "__experimental_bindings_api")]
Bindings {
/// File path to the source file to parse
file_path: String,

/// The language version to use for parsing
#[arg(short, long)]
version: Version,
},
}

impl Commands {
Expand All @@ -41,10 +30,6 @@ impl Commands {
version,
json,
} => commands::parse::execute(&file_path, version, json),
#[cfg(feature = "__experimental_bindings_api")]
Commands::Bindings { file_path, version } => {
commands::bindings::execute(&file_path, version)
}
};
match command_result {
Ok(()) => ExitCode::SUCCESS,
Expand Down
56 changes: 54 additions & 2 deletions crates/metaslang/bindings/src/builder/functions.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
use std::sync::Arc;

use metaslang_cst::KindTypes;
use metaslang_graph_builder::functions::Functions;
use semver::Version;

pub fn default_functions<KT: KindTypes + 'static>(version: Version) -> Functions<KT> {
use crate::PathResolver;

pub fn default_functions<KT: KindTypes + 'static>(
version: Version,
path_resolver: Arc<dyn PathResolver + Sync + Send>,
) -> Functions<KT> {
let mut functions = Functions::stdlib();
version::add_version_functions(&mut functions, version);
resolver::add_functions(&mut functions, path_resolver);
functions
}

pub mod version {
mod version {
use metaslang_cst::KindTypes;
use metaslang_graph_builder::functions::{Function, Functions, Parameters};
use metaslang_graph_builder::graph::{Graph, Value};
Expand Down Expand Up @@ -46,3 +54,47 @@ pub mod version {
}
}
}

mod resolver {
use std::sync::Arc;

use metaslang_cst::KindTypes;
use metaslang_graph_builder::functions::{Function, Functions, Parameters};
use metaslang_graph_builder::graph::{Graph, Value};
use metaslang_graph_builder::ExecutionError;

use crate::PathResolver;

pub fn add_functions<KT: KindTypes + 'static>(
functions: &mut Functions<KT>,
path_resolver: Arc<dyn PathResolver + Sync + Send>,
) {
functions.add("resolve-path".into(), ResolvePath { path_resolver });
}

struct ResolvePath {
path_resolver: Arc<dyn PathResolver + Sync + Send>,
}

impl<KT: KindTypes> Function<KT> for ResolvePath {
fn call(
&self,
_graph: &mut Graph<KT>,
parameters: &mut dyn Parameters,
) -> Result<Value, ExecutionError> {
let context_path = parameters.param()?.into_string()?;
let path_to_resolve = parameters.param()?.into_string()?;
parameters.finish()?;

if let Some(resolved_path) = self
.path_resolver
.as_ref()
.resolve_path(&context_path, &path_to_resolve)
{
Ok(resolved_path.into())
} else {
Ok("__SLANG_UNRESOLVED_IMPORT_PATH__".into())
}
}
}
}
60 changes: 33 additions & 27 deletions crates/metaslang/bindings/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ pub const ROOT_NODE_VAR: &str = "ROOT_NODE";
/// Name of the variable used to pass the file path.
pub const FILE_PATH_VAR: &str = "FILE_PATH";

pub struct Builder<'a, KT: KindTypes> {
pub(crate) struct Builder<'a, KT: KindTypes> {
msgb: &'a GraphBuilderFile<KT>,
functions: &'a Functions<KT>,
stack_graph: &'a mut StackGraph,
Expand All @@ -329,6 +329,15 @@ pub struct Builder<'a, KT: KindTypes> {
graph: Graph<KT>,
remapped_nodes: HashMap<usize, NodeID>,
injected_node_count: usize,
cursors: HashMap<Handle<Node>, Cursor<KT>>,
definiens: HashMap<Handle<Node>, Cursor<KT>>,
}

pub(crate) struct BuildResult<KT: KindTypes> {
#[cfg(feature = "__private_testing_utils")]
pub graph: Graph<KT>,
pub cursors: HashMap<Handle<Node>, Cursor<KT>>,
pub definiens: HashMap<Handle<Node>, Cursor<KT>>,
}

impl<'a, KT: KindTypes + 'static> Builder<'a, KT> {
Expand All @@ -348,11 +357,14 @@ impl<'a, KT: KindTypes + 'static> Builder<'a, KT> {
graph: Graph::new(),
remapped_nodes: HashMap::new(),
injected_node_count: 0,
cursors: HashMap::new(),
definiens: HashMap::new(),
}
}

fn build_global_variables(&mut self, file_path: &str) -> Variables<'a> {
fn build_global_variables(&mut self) -> Variables<'a> {
let mut variables = Variables::new();
let file_path = self.stack_graph[self.file].name();
variables
.add(FILE_PATH_VAR.into(), file_path.into())
.expect("failed to add FILE_PATH variable");
Expand Down Expand Up @@ -380,19 +392,12 @@ impl<'a, KT: KindTypes + 'static> Builder<'a, KT> {
variables
}

#[cfg(feature = "__private_testing_utils")]
pub(crate) fn graph(self) -> Graph<KT> {
self.graph
}

/// Executes this builder.
pub fn build(
&mut self,
file_path: &str,
mut self,
cancellation_flag: &dyn CancellationFlag,
on_added_node: impl FnMut(Handle<Node>, &Cursor<KT>),
) -> Result<(), BuildError> {
let variables = self.build_global_variables(file_path);
) -> Result<BuildResult<KT>, BuildError> {
let variables = self.build_global_variables();

let config = ExecutionConfig::new(self.functions, &variables)
.lazy(true)
Expand All @@ -418,7 +423,14 @@ impl<'a, KT: KindTypes + 'static> Builder<'a, KT> {
&(cancellation_flag as &dyn CancellationFlag),
)?;

self.load(cancellation_flag, on_added_node)
self.load(cancellation_flag)?;

Ok(BuildResult {
#[cfg(feature = "__private_testing_utils")]
graph: self.graph,
cursors: self.cursors,
definiens: self.definiens,
})
}

/// Create a graph node to represent the stack graph node. It is the callers responsibility to
Expand Down Expand Up @@ -511,11 +523,7 @@ impl std::fmt::Display for DisplayBuildErrorPretty<'_> {
}

impl<'a, KT: KindTypes + 'static> Builder<'a, KT> {
fn load(
&mut self,
cancellation_flag: &dyn CancellationFlag,
mut on_added_node: impl FnMut(Handle<Node>, &Cursor<KT>),
) -> Result<(), BuildError> {
fn load(&mut self, cancellation_flag: &dyn CancellationFlag) -> Result<(), BuildError> {
let cancellation_flag: &dyn stack_graphs::CancellationFlag = &cancellation_flag;

// By default graph ids are used for stack graph local_ids. A remapping is computed
Expand Down Expand Up @@ -561,13 +569,14 @@ impl<'a, KT: KindTypes + 'static> Builder<'a, KT> {
self.load_node_debug_info(node_ref, handle);

// For every added graph node which links to a corresponding source
// node, invoke the callback so our caller can link the newly built
// node with the matching CST cursor.
// node, save the corresponding CST cursor so our caller can extract
// that info later.
let node = &self.graph[node_ref];
if let Some(source_node) = node.attributes.get(SOURCE_NODE_ATTR) {
let syntax_node_ref = source_node.as_syntax_node_ref()?;
let source_node = &self.graph[syntax_node_ref];
on_added_node(handle, source_node);

self.cursors.insert(handle, source_node.clone());
}
}

Expand Down Expand Up @@ -816,8 +825,6 @@ impl<'a, KT: KindTypes> Builder<'a, KT> {
Ok(())
}

// TODO: we can probably remove this and all references to definiens since
// we're not gonna need it?
fn load_definiens_info(
&mut self,
node_ref: GraphNodeRef,
Expand All @@ -829,10 +836,9 @@ impl<'a, KT: KindTypes> Builder<'a, KT> {
Some(definiens_node) => &self.graph[definiens_node.as_syntax_node_ref()?],
None => return Ok(()),
};
let _definiens_range = definiens_node.text_range();
let _source_info = self.stack_graph.source_info_mut(node_handle);
// TODO: map node's TextRange into a Span
// source_info.definiens_span = text_range_into_span(definiens_range);
// Save the definiens CST cursor so our caller can extract the mapping
// to the stack graph node later
self.definiens.insert(node_handle, definiens_node.clone());
Ok(())
}

Expand Down
Loading

0 comments on commit 00b2aa3

Please sign in to comment.