diff --git a/docs/pages/reference/completions.md b/docs/pages/reference/completions.md index 3930c20..2948fa6 100644 --- a/docs/pages/reference/completions.md +++ b/docs/pages/reference/completions.md @@ -82,6 +82,14 @@ Final member set is composed from: - `using_for` matches, - `using_for_wildcard`. +The last two are included only when the receiver is a **value**, not when it +names a type: `paymentToken.` (declared as `IERC20`) gets `using SafeERC20 for +IERC20` extensions, while `IERC20.` gets only the interface's own members. Both +carry the same `typeIdentifier`, so the distinction is carried down from name +resolution — a `name_to_type` hit is a value, a `name_to_node_id` hit is a type +name. Casts (`IERC20(addr).`), mapping indexes and chained members are values +regardless of the name in front of them. + ## Scope-aware name resolution When resolving a symbol in context, completion walks: diff --git a/src/completion.rs b/src/completion.rs index 961a7c5..310a5d9 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -1261,12 +1261,29 @@ fn strip_type_suffix(type_id: &str) -> &str { /// Look up using-for completions for a type, trying suffix variants. /// The AST stores types with different suffixes (_storage_ptr, _storage, _memory_ptr, etc.) /// across different contexts, so we try multiple forms. -fn lookup_using_for(cache: &CompletionCache, type_id: &str) -> Vec { +fn lookup_using_for( + cache: &CompletionCache, + type_id: &str, + node_id: Option, +) -> Vec { // Exact match first if let Some(items) = cache.using_for.get(type_id) { return items.clone(); } + // A contract name that has no entry in `type_to_node` resolves to the + // synthetic `__node_id_N` marker, which can never match a real key like + // `t_contract$_IHooks_$1840`. Recover by matching on the node id instead. + if let Some(nid) = node_id + && type_id.starts_with("__node_id_") + && let Some((_, items)) = cache + .using_for + .iter() + .find(|(tid, _)| extract_node_id_from_type(tid.as_str()) == Some(nid)) + { + return items.clone(); + } + // Strip to base form, then try all common suffix variants let base = strip_type_suffix(type_id); let variants = [ @@ -1290,7 +1307,11 @@ fn lookup_using_for(cache: &CompletionCache, type_id: &str) -> Vec Vec { +fn completions_for_type( + cache: &CompletionCache, + type_id: &str, + receiver: ReceiverKind, +) -> Vec { // Address type if type_id == "t_address" || type_id == "t_address_payable" { let mut items = address_members(); @@ -1335,16 +1356,17 @@ fn completions_for_type(cache: &CompletionCache, type_id: &str) -> Vec Vec Option { - // Direct type lookup +/// Whether the expression before the dot *names a type* or *is a value* of one. +/// +/// Both produce the same `typeIdentifier` — `IERC20.` and a `paymentToken` +/// declared as `IERC20` both resolve to `t_contract$_IERC20_$N` — so the +/// distinction cannot be recovered from the type alone and has to be carried +/// down from wherever the receiver was resolved. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReceiverKind { + /// `IERC20.` — the contract/library/interface itself. + TypeName, + /// `paymentToken.` — a value whose type is that contract. + Value, +} + +/// Resolve a type identifier for a name, considering name_to_type and +/// name_to_node_id, reporting whether the name was a variable (a value) or a +/// contract/library/interface name (a type). +fn resolve_name_to_type_id_kind( + cache: &CompletionCache, + name: &str, +) -> Option<(String, ReceiverKind)> { + // Direct type lookup — the name is a variable, so this is a value. if let Some(tid) = cache.name_to_type.get(name) { - return Some(tid.to_string()); + return Some((tid.to_string(), ReceiverKind::Value)); } // Contract/library/interface name → synthesize a type id from node id if let Some(node_id) = cache.name_to_node_id.get(name) { // Find a matching typeIdentifier in type_to_node (reverse lookup) for (tid, nid) in &cache.type_to_node { if nid == node_id { - return Some(tid.to_string()); + return Some((tid.to_string(), ReceiverKind::TypeName)); } } // Fallback: use a synthetic marker so completions_for_type can resolve via node_id - return Some(format!("__node_id_{}", node_id)); + return Some((format!("__node_id_{}", node_id), ReceiverKind::TypeName)); } None } @@ -1406,7 +1447,7 @@ pub fn find_innermost_scope( /// declarations for a matching name. If not found, follow `scope_parent` to the /// next enclosing scope and check again. Stop at the first match. /// -/// Falls back to `resolve_name_to_type_id` (flat lookup) if scope resolution +/// Falls back to `resolve_name_to_type_id_kind` (flat lookup) if scope resolution /// finds nothing, or if the scope data is unavailable. pub fn resolve_name_in_scope( cache: &CompletionCache, @@ -1414,7 +1455,20 @@ pub fn resolve_name_in_scope( byte_pos: usize, file_id: FileId, ) -> Option { - let mut current_scope = find_innermost_scope(cache, byte_pos, file_id)?; + resolve_name_in_scope_kind(cache, name, byte_pos, file_id).map(|(tid, _)| tid) +} + +/// Like [`resolve_name_in_scope`], but also reports whether the resolved name +/// was a declaration in scope (a value) or a contract name (a type). +fn resolve_name_in_scope_kind( + cache: &CompletionCache, + name: &str, + byte_pos: usize, + file_id: FileId, +) -> Option<(String, ReceiverKind)> { + let Some(mut current_scope) = find_innermost_scope(cache, byte_pos, file_id) else { + return resolve_name_to_type_id_kind(cache, name); + }; // Walk up the scope chain loop { @@ -1422,7 +1476,7 @@ pub fn resolve_name_in_scope( if let Some(decls) = cache.scope_declarations.get(¤t_scope) { for decl in decls { if decl.name == name { - return Some(decl.type_id.clone()); + return Some((decl.type_id.clone(), ReceiverKind::Value)); } } } @@ -1435,7 +1489,7 @@ pub fn resolve_name_in_scope( if let Some(decls) = cache.scope_declarations.get(&base_id) { for decl in decls { if decl.name == name { - return Some(decl.type_id.clone()); + return Some((decl.type_id.clone(), ReceiverKind::Value)); } } } @@ -1451,7 +1505,7 @@ pub fn resolve_name_in_scope( // Scope walk found nothing — fall back to flat lookup // (handles contract/library names which aren't in scope_declarations) - resolve_name_to_type_id(cache, name) + resolve_name_to_type_id_kind(cache, name) } /// Resolve a name within a type context to get the member's type. @@ -1535,10 +1589,19 @@ fn resolve_name( name: &str, scope_ctx: Option<&ScopeContext>, ) -> Option { + resolve_name_kind(cache, name, scope_ctx).map(|(tid, _)| tid) +} + +/// Like [`resolve_name`], but also reports whether the name was a value or a type. +fn resolve_name_kind( + cache: &CompletionCache, + name: &str, + scope_ctx: Option<&ScopeContext>, +) -> Option<(String, ReceiverKind)> { if let Some(ctx) = scope_ctx { - resolve_name_in_scope(cache, name, ctx.byte_pos, ctx.file_id) + resolve_name_in_scope_kind(cache, name, ctx.byte_pos, ctx.file_id) } else { - resolve_name_to_type_id(cache, name) + resolve_name_to_type_id_kind(cache, name) } } @@ -1553,11 +1616,10 @@ pub fn get_dot_completions( return items; } - // Try to resolve the identifier's type - let type_id = resolve_name(cache, identifier, scope_ctx); - - if let Some(tid) = type_id { - return completions_for_type(cache, &tid); + // Plain access, so a contract name stays a type name and a variable stays + // a value — exactly the distinction using-for extensions hinge on. + if let Some((tid, receiver)) = resolve_name_kind(cache, identifier, scope_ctx) { + return completions_for_type(cache, &tid, receiver); } vec![] @@ -1590,13 +1652,15 @@ pub fn get_chain_completions( } // foo(). — could be a function call or a type cast like IFoo(addr). // First check if it's a type cast: name matches a contract/interface/library + // A cast yields a *value* of that type, so extensions apply even + // though the name itself is a contract. if let Some(type_id) = resolve_name(cache, &seg.name, scope_ctx) { - return completions_for_type(cache, &type_id); + return completions_for_type(cache, &type_id, ReceiverKind::Value); } // Otherwise look up as a function call — check all function_return_types for ((_, fn_name), ret_type) in &cache.function_return_types { if fn_name == &seg.name { - return completions_for_type(cache, ret_type); + return completions_for_type(cache, ret_type, ReceiverKind::Value); } } return vec![]; @@ -1607,7 +1671,7 @@ pub fn get_chain_completions( && tid.starts_with("t_mapping") && let Some(val_type) = extract_mapping_value_type(&tid) { - return completions_for_type(cache, &val_type); + return completions_for_type(cache, &val_type, ReceiverKind::Value); } return vec![]; } @@ -1651,9 +1715,10 @@ pub fn get_chain_completions( current_type = resolve_member_type(cache, &ctx_type, &seg.name, &seg.kind); } - // Return completions for the final resolved type + // Return completions for the final resolved type. Anything reached by + // walking members is a value, never a type name. match current_type { - Some(tid) => completions_for_type(cache, &tid), + Some(tid) => completions_for_type(cache, &tid, ReceiverKind::Value), None => vec![], } } diff --git a/tests/completion.rs b/tests/completion.rs index d2e9080..8f03ddc 100644 --- a/tests/completion.rs +++ b/tests/completion.rs @@ -3690,3 +3690,78 @@ fn test_type_meta_items_have_detail() { let iid_item = items.iter().find(|i| i.label == "interfaceId").unwrap(); assert_eq!(iid_item.detail.as_deref(), Some("bytes4")); } + +// ── using-for extensions on values (issue #225) ──────────────────────────── + +/// `using Hooks for IHooks` must offer the library's functions on a *value* +/// typed `IHooks`. Previously the receiver's kind was inferred from its +/// typeIdentifier, and because `IHooks` is a contract the extensions were +/// suppressed on values as well as on the type name. +#[test] +fn test_using_for_extensions_appear_on_a_contract_typed_value() { + let cache = load_cache(); + + let labels: Vec = get_dot_completions(&cache, "hooks", None) + .into_iter() + .map(|i| i.label) + .collect(); + + assert!( + !labels.is_empty(), + "`hooks` (declared as IHooks) resolved to nothing" + ); + // `hasPermission` and `isValidHookAddress` exist only on the Hooks library, + // not on the IHooks interface, so they can only have arrived via using-for. + for expected in ["hasPermission", "isValidHookAddress"] { + assert!( + labels.iter().any(|l| l == expected), + "`hooks.` is missing the `using Hooks for IHooks` extension {expected:?}; got {labels:?}" + ); + } +} + +/// The flip side: typing the *type name* must not pull in library extensions, +/// which is the behavior the original suppression was written to protect. +#[test] +fn test_using_for_extensions_absent_on_the_type_name() { + let cache = load_cache(); + + let labels: Vec = get_dot_completions(&cache, "IHooks", None) + .into_iter() + .map(|i| i.label) + .collect(); + + // Library-only names must not leak onto the type. Names such as `beforeSwap` + // are deliberately not asserted here: IHooks declares them itself, so they + // legitimately appear either way. + for unexpected in ["hasPermission", "isValidHookAddress"] { + assert!( + !labels.iter().any(|l| l == unexpected), + "`IHooks.` should not offer the library extension {unexpected:?}; got {labels:?}" + ); + } +} + +/// A cast produces a value, so `IHooks(addr).` gets the extensions even though +/// the name before the parenthesis is a contract. +#[test] +fn test_using_for_extensions_appear_after_a_cast() { + use solidity_language_server::completion::get_chain_completions; + + let cache = load_cache(); + let chain = vec![DotSegment { + name: "IHooks".to_string(), + kind: AccessKind::Call, + call_args: Some("addr".to_string()), + }]; + + let labels: Vec = get_chain_completions(&cache, &chain, None) + .into_iter() + .map(|i| i.label) + .collect(); + + assert!( + labels.iter().any(|l| l == "hasPermission"), + "`IHooks(addr).` should offer library extensions; got {labels:?}" + ); +}