Retain references to ImportDeconstructionSymbols instead of following to the aliased definition

crates/solidity/outputs/cargo/crate/src/backend/binder/mod.rs

@@ -481,11 +481,14 @@ impl Binder {
         }
     }
 
-    fn resolve_in_scope_internal(&self, scope_id: ScopeId, symbol: &str) -> Resolution {
+    // This function attempts to lexically resolve `symbol` starting from the
+    // given scope. Certain scopes can delegate to their "parent" scopes if
+    // the symbol is not found there.
+    pub(crate) fn resolve_in_scope(&self, scope_id: ScopeId, symbol: &str) -> Resolution {
         let scope = self.get_scope_by_id(scope_id);
         match scope {
             Scope::Block(block_scope) => block_scope.definitions.get(symbol).copied().map_or_else(
-                || self.resolve_in_scope_internal(block_scope.parent_scope_id, symbol),
+                || self.resolve_in_scope(block_scope.parent_scope_id, symbol),
                 Resolution::Definition,
             ),
             Scope::Contract(contract_scope) => {
@@ -496,7 +499,7 @@ impl Binder {
                 )
                 .or_else(|| {
                     // Otherwise, delegate to the containing file scope.
-                    self.resolve_in_scope_internal(contract_scope.file_scope_id, symbol)
+                    self.resolve_in_scope(contract_scope.file_scope_id, symbol)
                 })
             }
             Scope::Enum(enum_scope) => enum_scope.definitions.get(symbol).into(),
@@ -506,16 +509,16 @@ impl Binder {
                 .get(symbol)
                 .copied()
                 .map_or_else(
-                    || self.resolve_in_scope_internal(function_scope.parameters_scope_id, symbol),
+                    || self.resolve_in_scope(function_scope.parameters_scope_id, symbol),
                     Resolution::Definition,
                 )
-                .or_else(|| self.resolve_in_scope_internal(function_scope.parent_scope_id, symbol)),
+                .or_else(|| self.resolve_in_scope(function_scope.parent_scope_id, symbol)),
             Scope::Modifier(modifier_scope) => {
                 if symbol == "_" {
                     Resolution::BuiltIn(BuiltIn::ModifierUnderscore)
                 } else {
                     modifier_scope.definitions.get(symbol).copied().map_or_else(
-                        || self.resolve_in_scope_internal(modifier_scope.parent_scope_id, symbol),
+                        || self.resolve_in_scope(modifier_scope.parent_scope_id, symbol),
                         Resolution::Definition,
                     )
                 }
@@ -534,7 +537,7 @@ impl Binder {
                 .get(symbol)
                 .copied()
                 .map_or_else(
-                    || self.resolve_in_scope_internal(yul_block_scope.parent_scope_id, symbol),
+                    || self.resolve_in_scope(yul_block_scope.parent_scope_id, symbol),
                     Resolution::Definition,
                 ),
             Scope::YulFunction(yul_function_scope) => {
@@ -545,40 +548,33 @@ impl Binder {
                     .get(symbol)
                     .copied()
                     .map_or_else(
-                        || {
-                            self.resolve_in_scope_internal(
-                                yul_function_scope.parent_scope_id,
-                                symbol,
-                            )
-                        },
+                        || self.resolve_in_scope(yul_function_scope.parent_scope_id, symbol),
                         Resolution::Definition,
                     )
             }
         }
     }
 
-    // This will attempt to lexically resolve `symbol` starting from the given
-    // scope. This means that scopes can delegate to their "parent" scopes if
-    // the symbol is not found there, and also that imported symbols are
-    // followed recursively. This function handles the latter (following
-    // imported symbols recursively), while `resolve_in_scope_internal`
-    // delegates to parent or otherwise linked scopes.
-    pub(crate) fn resolve_in_scope(&self, scope_id: ScopeId, symbol: &str) -> Resolution {
+    // If the resolution points to definitions that are symbols aliases (eg.
+    // import deconstruction symbols) this function will recursively follow them
+    // and return an appropriate resulting resolution. Ambiguities and missing
+    // definitions can occur along the followed aliases, so there is no
+    // guarantee that a `Resolved` resolution will yield another `Resolved`
+    // resolution.
+    pub(crate) fn follow_symbol_aliases(&self, resolution: &Resolution) -> Resolution {
+        match resolution {
+            Resolution::Unresolved | Resolution::BuiltIn(_) => return resolution.clone(),
+            _ => {}
+        }
+
         // TODO: since this function uses the results from other resolution
         // functions, we making more allocations than necessary; it may be worth
         // it to try and avoid them by returning iterators from the delegated
         // resolution functions
         let mut found_ids = Vec::new();
-        let mut working_set = Vec::new();
         let mut seen_ids = HashSet::new();
+        let mut working_set = resolution.get_definition_ids();
 
-        let initial_resolution = self.resolve_in_scope_internal(scope_id, symbol);
-        match initial_resolution {
-            Resolution::Unresolved | Resolution::BuiltIn(_) => return initial_resolution,
-            _ => {}
-        }
-
-        working_set.extend(initial_resolution.get_definition_ids().iter().rev());
         while let Some(definition_id) = working_set.pop() {
             if !seen_ids.insert(definition_id) {
                 // we already processed this definition
@@ -597,16 +593,17 @@ impl Binder {
                     continue;
                 };
                 working_set.extend(
-                    self.resolve_in_scope_internal(scope_id, &imported_symbol.symbol)
-                        .get_definition_ids()
-                        .iter()
-                        .rev(),
+                    self.resolve_in_scope(scope_id, &imported_symbol.symbol)
+                        .get_definition_ids(),
                 );
             } else {
                 found_ids.push(definition_id);
             }
         }
 
+        // reverse the result to maintain the original ordering, since we
+        // resolved from last to first in the loop above
+        found_ids.reverse();
         Resolution::from(found_ids)
     }
 

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/node_extensions/identifiers.rs

@@ -39,13 +39,26 @@ impl IdentifierStruct {
             .is_some()
     }
 
-    // only makes sense if `is_reference()` is true
+    /// Attempts to resolve the identifier to a definition, following symbol
+    /// aliases (import deconstructions). Returns `None` if the identifier is
+    /// not acting as a reference, or the reference cannot be resolved.
     pub fn resolve_to_definition(&self) -> Option<Definition> {
-        let reference = self
+        let definition_id = self
             .semantic
-            .binder()
-            .find_reference_by_identifier_node_id(self.ir_node.id())?;
-        let definition_id = reference.resolution.as_definition_id()?;
+            .resolve_reference_identifier_to_definition_id(self.ir_node.id())?;
+        Definition::try_create(definition_id, &self.semantic)
+    }
+
+    /// Attempts to resolve the identifier to an immediate definition, without
+    /// following symbol aliases, possibly returning import deconstruction
+    /// symbols. If the identifier refers to a direct definition, this is
+    /// equivalent to `resolve_to_definition()`. Returns `None` if the
+    /// identifier is not acting as a reference, or the reference cannot be
+    /// resolved.
+    pub fn resolve_to_immediate_definition(&self) -> Option<Definition> {
+        let definition_id = self
+            .semantic
+            .resolve_reference_identifier_to_immediate_definition_id(self.ir_node.id())?;
         Definition::try_create(definition_id, &self.semantic)
     }
 
@@ -137,6 +150,14 @@ impl Reference {
             }
         }
     }
+
+    pub fn resolve_to_immediate_definition(&self) -> Option<Definition> {
+        match self {
+            Reference::Identifier(identifier) | Reference::YulIdentifier(identifier) => {
+                identifier.resolve_to_immediate_definition()
+            }
+        }
+    }
 }
 
 impl SemanticAnalysis {
@@ -152,6 +173,25 @@ impl SemanticAnalysis {
             })
             .collect()
     }
+
+    fn resolve_reference_identifier_to_definition_id(&self, node_id: NodeId) -> Option<NodeId> {
+        let reference = self
+            .binder()
+            .find_reference_by_identifier_node_id(node_id)?;
+        self.binder()
+            .follow_symbol_aliases(&reference.resolution)
+            .as_definition_id()
+    }
+
+    fn resolve_reference_identifier_to_immediate_definition_id(
+        &self,
+        node_id: NodeId,
+    ) -> Option<NodeId> {
+        let reference = self
+            .binder()
+            .find_reference_by_identifier_node_id(node_id)?;
+        reference.resolution.as_definition_id()
+    }
 }
 
 impl IdentifierPathStruct {
@@ -163,13 +203,25 @@ impl IdentifierPathStruct {
             .join(".")
     }
 
+    /// Attempts to resolve the identifier path to a definition, following
+    /// symbol aliases (import deconstructions).
     pub fn resolve_to_definition(&self) -> Option<Definition> {
         let ir_node = self.ir_nodes.last()?;
-        let reference = self
+        let definition_id = self
             .semantic
-            .binder()
-            .find_reference_by_identifier_node_id(ir_node.id())?;
-        let definition_id = reference.resolution.as_definition_id()?;
+            .resolve_reference_identifier_to_definition_id(ir_node.id())?;
+        Definition::try_create(definition_id, &self.semantic)
+    }
+
+    /// Attempts to resolve the identifier path to an immediate definition,
+    /// without following symbol aliases, possibly returning import
+    /// deconstruction symbols. If the path refers to a direct definition, this
+    /// is equivalent to `resolve_to_definition`.
+    pub fn resolve_to_immediate_definition(&self) -> Option<Definition> {
+        let ir_node = self.ir_nodes.last()?;
+        let definition_id = self
+            .semantic
+            .resolve_reference_identifier_to_immediate_definition_id(ir_node.id())?;
         Definition::try_create(definition_id, &self.semantic)
     }
 }

crates/solidity/outputs/cargo/crate/src/backend/passes/p3_linearise_contracts/mod.rs

@@ -122,14 +122,26 @@ impl<'a> Pass<'a> {
             } else {
                 Resolution::Unresolved
             };
-            let definition_id = resolution.as_definition_id();
 
             let reference = Reference::new(Rc::clone(identifier), resolution.clone());
             self.binder.insert_reference(reference);
 
+            // Unless we used namespace resolution and in order to continue
+            // resolving the identifier path, we should ensure we've followed
+            // through any symbol alias (ie. import deconstruction symbol). This
+            // is not needed for namespaced resolution because there cannot be
+            // import directives inside contracts, interfaces or libraries which
+            // changes the lookup mode (see below).
+            let resolution = if use_lexical_resolution {
+                self.binder.follow_symbol_aliases(&resolution)
+            } else {
+                resolution
+            };
+
             // recurse into file scopes pointed by the resolved definition
             // to resolve the next identifier in the path
-            scope_id = definition_id
+            scope_id = resolution
+                .as_definition_id()
                 .and_then(|node_id| self.binder.find_definition_by_id(node_id))
                 .and_then(|definition| match definition {
                     Definition::Import(ImportDefinition {

crates/solidity/outputs/cargo/crate/src/backend/passes/p4_type_definitions/resolution.rs

@@ -32,14 +32,26 @@ impl Pass<'_> {
             } else {
                 Resolution::Unresolved
             };
-            let definition_id = resolution.as_definition_id();
 
             let reference = Reference::new(Rc::clone(identifier), resolution.clone());
             self.binder.insert_reference(reference);
 
+            // Unless we used namespace resolution and in order to continue
+            // resolving the identifier path, we should ensure we've followed
+            // through any symbol alias (ie. import deconstruction symbol). This
+            // is not needed for namespaced resolution because there cannot be
+            // import directives inside contracts, interfaces or libraries which
+            // changes the lookup mode (see below).
+            let resolution = if use_lexical_resolution {
+                self.binder.follow_symbol_aliases(&resolution)
+            } else {
+                resolution
+            };
+
             // recurse into file scopes pointed by the resolved definition
             // to resolve the next identifier in the path
-            scope_id = definition_id
+            scope_id = resolution
+                .as_definition_id()
                 .and_then(|node_id| self.binder.find_definition_by_id(node_id))
                 .and_then(|definition| match definition {
                     Definition::Import(ImportDefinition {
@@ -147,7 +159,13 @@ impl Pass<'_> {
                 self.binder
                     .find_reference_by_identifier_node_id(identifier.id())
             })
-            .and_then(|reference| reference.resolution.as_definition_id())?;
+            .and_then(|reference| {
+                // reference may resolve to an imported library, so we need to
+                // follow aliases
+                self.binder
+                    .follow_symbol_aliases(&reference.resolution)
+                    .as_definition_id()
+            })?;
 
         let Some(Definition::Library(_)) = self.binder.find_definition_by_id(definition_id) else {
             // the referenced definition is not a library

crates/solidity/outputs/cargo/crate/src/backend/passes/p4_type_definitions/typing.rs

@@ -16,7 +16,14 @@ impl Pass<'_> {
                 self.binder
                     .find_reference_by_identifier_node_id(identifier.id())
             })
-            .and_then(|reference| reference.resolution.as_definition_id())
+            .and_then(|reference| {
+                // We should follow symbol aliases here. This is only relevant
+                // if the path has a single element, as in other cases symbols
+                // cannot be aliased.
+                self.binder
+                    .follow_symbol_aliases(&reference.resolution)
+                    .as_definition_id()
+            })
             .and_then(|node_id| self.type_of_definition(node_id, data_location))
     }
 

crates/solidity/outputs/cargo/crate/src/backend/passes/p4_type_definitions/visitor.rs

@@ -87,15 +87,22 @@ impl Visitor for Pass<'_> {
 
     fn enter_import_deconstruction(&mut self, node: &input_ir::ImportDeconstruction) -> bool {
         for symbol in &node.symbols {
-            let target_symbol = if let Some(alias) = &symbol.alias {
-                alias
-            } else {
-                &symbol.name
+            // find the associated definition to get the imported file ID
+            let Some(Definition::ImportedSymbol(imported_symbol)) =
+                self.binder.find_definition_by_id(symbol.node_id)
+            else {
+                unreachable!("expected to find definition associated to imported symbol");
             };
-            let resolution = self.binder.resolve_in_scope(
-                self.current_contract_or_file_scope_id(),
-                &target_symbol.unparse(),
-            );
+            // now we can get the target scope ID
+            let scope_id = imported_symbol
+                .resolved_file_id
+                .as_ref()
+                .and_then(|file_id| self.binder.scope_id_for_file_id(file_id));
+
+            let resolution = scope_id.map_or(Resolution::Unresolved, |scope_id| {
+                self.binder
+                    .resolve_in_scope(scope_id, &symbol.name.unparse())
+            });
             let reference = Reference::new(Rc::clone(&symbol.name), resolution);
             self.binder.insert_reference(reference);
         }

crates/solidity/outputs/cargo/crate/src/backend/passes/p5_resolve_references/resolution.rs

@@ -411,7 +411,10 @@ impl Pass<'_> {
                 resolution
             };
 
-            scope_id = resolution
+            // NOTE: we need to follow symbol aliases to resolve the next scope to use
+            scope_id = self
+                .binder
+                .follow_symbol_aliases(&resolution)
                 .as_definition_id()
                 .and_then(|definition_id| self.binder.scope_id_for_node_id(definition_id));
 

crates/solidity/outputs/cargo/crate/src/backend/passes/p5_resolve_references/typing.rs

@@ -139,12 +139,14 @@ impl Pass<'_> {
     }
 
     fn typing_of_identifier(&self, identifier: &Rc<TerminalNode>) -> Typing {
-        let resolution = self
+        let resolution = &self
             .binder
             .find_reference_by_identifier_node_id(identifier.id())
             .unwrap()
-            .resolution
-            .clone();
+            .resolution;
+        // The resolution may point to an imported symbol, so we need to follow
+        // through in order to get to the actual typing
+        let resolution = self.binder.follow_symbol_aliases(resolution);
         self.typing_of_resolution(&resolution)
     }
 

crates/solidity/outputs/cargo/crate/src/backend/passes/p5_resolve_references/visitor.rs

@@ -635,7 +635,13 @@ impl Visitor for Pass<'_> {
             let definition_id = self
                 .binder
                 .find_reference_by_identifier_node_id(node.error.last().unwrap().id())
-                .and_then(|reference| reference.resolution.as_definition_id());
+                .and_then(|reference| {
+                    // Follow symbol aliases as the error type argument to
+                    // revert could be an imported symbol
+                    self.binder
+                        .follow_symbol_aliases(&reference.resolution)
+                        .as_definition_id()
+                });
             self.resolve_named_arguments(named_arguments, definition_id);
         }
     }