diff --git a/crates/hir/src/hir_def/declaration.rs b/crates/hir/src/hir_def/declaration.rs index 99b832f6..27a6b241 100644 --- a/crates/hir/src/hir_def/declaration.rs +++ b/crates/hir/src/hir_def/declaration.rs @@ -112,9 +112,30 @@ pub enum NetStrength { #[derive(Debug, PartialEq, Eq, Clone)] pub struct ParamDecl { pub ty: DataTy, + pub kind: ParamDeclKind, + pub is_port: bool, pub decls: DeclsRange, } +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum ParamDeclKind { + Parameter, + LocalParam, +} + +impl ParamDeclKind { + pub fn is_overridable(self) -> bool { + matches!(self, Self::Parameter) + } + + pub fn keyword(self) -> &'static str { + match self { + Self::Parameter => "parameter", + Self::LocalParam => "localparam", + } + } +} + #[derive(Debug, PartialEq, Eq, Clone)] pub struct GenvarDecl { pub ty: DataTy, @@ -246,12 +267,24 @@ impl LowerDeclarationCtx<'_> { pub(crate) fn lower_param_decl_base( &mut self, param_decl: ast::ParameterDeclarationBase, + ) -> DeclarationId { + self.lower_param_decl_base_with_context(param_decl, None, false, false) + } + + pub(crate) fn lower_param_decl_base_with_context( + &mut self, + param_decl: ast::ParameterDeclarationBase, + inherited_kind: Option, + force_local: bool, + is_port: bool, ) -> DeclarationId { use ast::ParameterDeclarationBase::*; match param_decl { - ParameterDeclaration(param_decl) => self.lower_param_decl(param_decl), + ParameterDeclaration(param_decl) => { + self.lower_param_decl(param_decl, inherited_kind, force_local, is_port) + } TypeParameterDeclaration(type_param_decl) => { - self.lower_type_param_decl(type_param_decl) + self.lower_type_param_decl(type_param_decl, inherited_kind, force_local, is_port) } } } @@ -259,7 +292,15 @@ impl LowerDeclarationCtx<'_> { fn lower_type_param_decl( &mut self, type_param_decl: ast::TypeParameterDeclaration, + inherited_kind: Option, + force_local: bool, + is_port: bool, ) -> DeclarationId { + let kind = lower_param_decl_kind( + type_param_decl.keyword().map(|keyword| keyword.kind()), + inherited_kind, + force_local, + ); let start = self.decls.nxt_idx(); let ty = DataTy::Builtin( self.db.intern_ty(crate::hir_def::expr::data_ty::BuiltinDataTy::default()), @@ -267,19 +308,30 @@ impl LowerDeclarationCtx<'_> { let decls = DeclsRange::new(start..self.decls.nxt_idx()); alloc_idx_and_src! { - ParamDecl { ty, decls } => self.declarations, + ParamDecl { ty, kind, is_port, decls } => self.declarations, type_param_decl => self.declaration_srcs, } } - fn lower_param_decl(&mut self, param_decl: ast::ParameterDeclaration) -> DeclarationId { + fn lower_param_decl( + &mut self, + param_decl: ast::ParameterDeclaration, + inherited_kind: Option, + force_local: bool, + is_port: bool, + ) -> DeclarationId { + let kind = lower_param_decl_kind( + param_decl.keyword().map(|keyword| keyword.kind()), + inherited_kind, + force_local, + ); let ty = self.expr_ctx().lower_data_ty(param_decl.type_()); let parent = self.declarations.nxt_idx().into(); let decls = self.decl_ctx().lower_declarators(param_decl.declarators(), parent); alloc_idx_and_src! { - ParamDecl { ty, decls } => self.declarations, + ParamDecl { ty, kind, is_port, decls } => self.declarations, param_decl => self.declaration_srcs, } } @@ -315,3 +367,19 @@ impl LowerDeclarationCtx<'_> { } } } + +fn lower_param_decl_kind( + keyword: Option, + inherited_kind: Option, + force_local: bool, +) -> ParamDeclKind { + if force_local { + return ParamDeclKind::LocalParam; + } + + match keyword { + Some(TokenKind::LOCAL_PARAM_KEYWORD) => ParamDeclKind::LocalParam, + Some(TokenKind::PARAMETER_KEYWORD) => ParamDeclKind::Parameter, + _ => inherited_kind.unwrap_or(ParamDeclKind::Parameter), + } +} diff --git a/crates/hir/src/hir_def/module.rs b/crates/hir/src/hir_def/module.rs index c2856f46..96ee362d 100644 --- a/crates/hir/src/hir_def/module.rs +++ b/crates/hir/src/hir_def/module.rs @@ -32,7 +32,8 @@ use super::{ alloc_idx_and_src, block::{BlockInfo, BlockSrc, LocalBlockId}, declaration::{ - Declaration, DeclarationId, DeclarationSrc, LowerDeclaration, impl_lower_declaration, + Declaration, DeclarationId, DeclarationSrc, LowerDeclaration, ParamDeclKind, + impl_lower_declaration, }, expr::{ Expr, ExprSrc, LowerExpr, @@ -151,6 +152,19 @@ impl Module { self.param_ports.clone()?.nth(idx) } + pub fn overridable_param_id_by_idx(&self, idx: usize) -> Option { + self.declarations + .values() + .filter_map(|declaration| match declaration { + Declaration::ParamDecl(param_decl) if param_decl.kind.is_overridable() => { + Some(param_decl.decls.clone()) + } + _ => None, + }) + .flatten() + .nth(idx) + } + pub fn non_ansi_port_id_by_idx(&self, idx: usize) -> Option { let Ports::NonAnsi { ports, .. } = &self.ports else { return None; @@ -331,9 +345,19 @@ impl LowerModuleCtx<'_> { pub(crate) fn lower_module_decl(&mut self, decl: ast::ModuleDeclaration) { let header = decl.header(); + let has_param_ports = header.parameters().is_some(); if let Some(param_ports) = header.parameters() { + let mut inherited_kind = ParamDeclKind::Parameter; for decls in param_ports.declarations().children() { - self.declaration_ctx().lower_param_decl_base(decls); + let decl_id = self.declaration_ctx().lower_param_decl_base_with_context( + decls, + Some(inherited_kind), + false, + true, + ); + if let Declaration::ParamDecl(param_decl) = self.module.get(decl_id) { + inherited_kind = param_decl.kind; + } self.region_tree.handle_node(decls.syntax()); } @@ -367,9 +391,15 @@ impl LowerModuleCtx<'_> { } NetDeclaration(net_decl) => self.declaration_ctx().lower_net_decl(net_decl).into(), LocalVariableDeclaration(_) => continue, - ParameterDeclarationStatement(param_decl) => { - self.declaration_ctx().lower_param_decl_base(param_decl.parameter()).into() - } + ParameterDeclarationStatement(param_decl) => self + .declaration_ctx() + .lower_param_decl_base_with_context( + param_decl.parameter(), + None, + has_param_ports, + false, + ) + .into(), TypedefDeclaration(typedef_decl) => self.lower_typedef(typedef_decl).into(), GenvarDeclaration(genvar_decl) => { self.declaration_ctx().lower_genvar_decl(genvar_decl).into() diff --git a/crates/ide/src/code_action.rs b/crates/ide/src/code_action.rs index 03de8863..4426f9fd 100644 --- a/crates/ide/src/code_action.rs +++ b/crates/ide/src/code_action.rs @@ -16,8 +16,8 @@ pub use diagnostics::{ pub(crate) use edits::{apply_missing_list_edit, line_indent}; pub(crate) use engine::code_action; pub(crate) use module_members::{ - all_parameter_names, leading_parameter_names, missing_member_entry_text, port_names, - remaining_ordered_port_names, + all_overridable_parameter_names, leading_overridable_parameter_names, + missing_member_entry_text, port_names, remaining_ordered_port_names, }; #[cfg(test)] diff --git a/crates/ide/src/code_action/fixtures/code_actions/missing_parameter_honors_parameter_port_keyword_inheritance.sv b/crates/ide/src/code_action/fixtures/code_actions/missing_parameter_honors_parameter_port_keyword_inheritance.sv new file mode 100644 index 00000000..8ddd293a --- /dev/null +++ b/crates/ide/src/code_action/fixtures/code_actions/missing_parameter_honors_parameter_port_keyword_inheritance.sv @@ -0,0 +1,3 @@ +//- repair: MissingParameter +module child #(localparam int L = 1, int H = 2, parameter int A = 3, int B = 4) (); endmodule +module top; child #(/*caret*/.A(1)) u(); endmodule diff --git a/crates/ide/src/code_action/fixtures/code_actions/missing_parameter_skips_localparams.sv b/crates/ide/src/code_action/fixtures/code_actions/missing_parameter_skips_localparams.sv new file mode 100644 index 00000000..ce9c8104 --- /dev/null +++ b/crates/ide/src/code_action/fixtures/code_actions/missing_parameter_skips_localparams.sv @@ -0,0 +1,3 @@ +//- repair: MissingParameter +module child #(parameter A = 1, parameter B = 2) (); localparam L = 3; endmodule +module top; localparam L = 4; child #(/*caret*/.A(1)) u(); endmodule diff --git a/crates/ide/src/code_action/handlers/add_missing_parameters.rs b/crates/ide/src/code_action/handlers/add_missing_parameters.rs index b9f466d7..414cf6a8 100644 --- a/crates/ide/src/code_action/handlers/add_missing_parameters.rs +++ b/crates/ide/src/code_action/handlers/add_missing_parameters.rs @@ -12,8 +12,8 @@ use utils::get::{Get, GetRef}; use crate::{ code_action::{ CodeActionCollector, CodeActionCtx, CodeActionId, CodeActionKind, RepairKind, - all_parameter_names, apply_missing_list_edit, leading_parameter_names, - missing_member_entry_text, + all_overridable_parameter_names, apply_missing_list_edit, + leading_overridable_parameter_names, missing_member_entry_text, }, module_resolution::resolve_hir_instantiation_target, }; @@ -65,7 +65,7 @@ pub(super) fn add_missing_parameters( .unwrap_or_default(); let names: Vec<_> = if is_ordered { - leading_parameter_names(&target_module) + leading_overridable_parameter_names(&target_module) .into_iter() .skip(instantiation.param_assigns.len()) .collect() @@ -81,7 +81,7 @@ pub(super) fn add_missing_parameters( } } - all_parameter_names(&target_module) + all_overridable_parameter_names(&target_module) .into_iter() .filter(|name| !assigned_names.contains(name)) .collect() diff --git a/crates/ide/src/code_action/handlers/convert_ordered_connections.rs b/crates/ide/src/code_action/handlers/convert_ordered_connections.rs index 52eb0ae3..c24b1e99 100644 --- a/crates/ide/src/code_action/handlers/convert_ordered_connections.rs +++ b/crates/ide/src/code_action/handlers/convert_ordered_connections.rs @@ -14,7 +14,7 @@ use utils::get::{Get, GetRef}; use crate::{ code_action::{ CodeActionCollector, CodeActionCtx, CodeActionId, CodeActionKind, RepairKind, - leading_parameter_names, port_names, + leading_overridable_parameter_names, port_names, }, module_resolution::resolve_hir_instantiation_target, }; @@ -115,7 +115,7 @@ pub(super) fn convert_ordered_params( let instantiation = module.get(instantiation_id); let target_module_id = resolve_hir_instantiation_target(db, ctx.file_id(), instantiation)?; let target_module = db.module(target_module_id); - let param_names = leading_parameter_names(&target_module); + let param_names = leading_overridable_parameter_names(&target_module); let replacements = instantiation .param_assigns diff --git a/crates/ide/src/code_action/handlers/sort_named_instantiation_items.rs b/crates/ide/src/code_action/handlers/sort_named_instantiation_items.rs index 9ee0c222..0e6064ac 100644 --- a/crates/ide/src/code_action/handlers/sort_named_instantiation_items.rs +++ b/crates/ide/src/code_action/handlers/sort_named_instantiation_items.rs @@ -20,8 +20,8 @@ use utils::{ use crate::{ code_action::{ - CodeActionCollector, CodeActionCtx, CodeActionId, CodeActionKind, all_parameter_names, - line_indent, port_names, + CodeActionCollector, CodeActionCtx, CodeActionId, CodeActionKind, + all_overridable_parameter_names, line_indent, port_names, }, module_resolution::resolve_hir_instantiation_target, }; @@ -61,7 +61,7 @@ pub(super) fn sort_named_parameter_assignments( let (module, module_src_map) = db.module_with_source_map(module_id); let instantiation = module.get(instantiation_id); let target_module_id = resolve_hir_instantiation_target(db, ctx.file_id(), instantiation)?; - let parameter_order = all_parameter_names(&db.module(target_module_id)); + let parameter_order = all_overridable_parameter_names(&db.module(target_module_id)); let parameter_order_map: FxHashMap<_, _> = parameter_order.iter().enumerate().map(|(index, name)| (name.as_ref(), index)).collect(); diff --git a/crates/ide/src/code_action/module_members.rs b/crates/ide/src/code_action/module_members.rs index 6e7df959..d1f37c7d 100644 --- a/crates/ide/src/code_action/module_members.rs +++ b/crates/ide/src/code_action/module_members.rs @@ -38,21 +38,26 @@ pub(crate) fn remaining_ordered_port_names(module: &Module, connected: usize) -> } } -pub(crate) fn leading_parameter_names(module: &Module) -> Vec { +pub(crate) fn leading_overridable_parameter_names(module: &Module) -> Vec { module .declarations .values() .take_while(|declaration| matches!(declaration, Declaration::ParamDecl(_))) + .filter(|declaration| { + matches!(declaration, Declaration::ParamDecl(param_decl) if param_decl.kind.is_overridable()) + }) .flat_map(|declaration| declaration.decls()) .filter_map(|decl| module.get(decl).name.clone()) .collect() } -pub(crate) fn all_parameter_names(module: &Module) -> Vec { +pub(crate) fn all_overridable_parameter_names(module: &Module) -> Vec { module .declarations .values() - .filter(|declaration| matches!(declaration, Declaration::ParamDecl(_))) + .filter(|declaration| { + matches!(declaration, Declaration::ParamDecl(param_decl) if param_decl.kind.is_overridable()) + }) .flat_map(|declaration| declaration.decls()) .filter_map(|decl| module.get(decl).name.clone()) .collect() diff --git a/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_availability_matrix.snap b/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_availability_matrix.snap index 09e80a50..72739f27 100644 --- a/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_availability_matrix.snap +++ b/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_availability_matrix.snap @@ -17,6 +17,8 @@ reformat_number_literal_requires_enough_digits: - Convert literal to hexadecimal missing_parameter_repair_is_not_offered_when_nothing_is_missing: +missing_parameter_repair_skips_body_params_when_header_has_parameter_ports: + named_port_shorthand_collapse_requires_same_name: named_port_shorthand_requires_all_connections_named: diff --git a/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_edit_fixtures@missing_parameter_honors_parameter_port_keyword_inheritance.sv.snap b/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_edit_fixtures@missing_parameter_honors_parameter_port_keyword_inheritance.sv.snap new file mode 100644 index 00000000..fd745ab0 --- /dev/null +++ b/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_edit_fixtures@missing_parameter_honors_parameter_port_keyword_inheritance.sv.snap @@ -0,0 +1,7 @@ +--- +source: crates/ide/src/code_action/tests.rs +expression: fixed +input_file: crates/ide/src/code_action/fixtures/code_actions/missing_parameter_honors_parameter_port_keyword_inheritance.sv +--- +module child #(localparam int L = 1, int H = 2, parameter int A = 3, int B = 4) (); endmodule +module top; child #(.A(1), .B()) u(); endmodule diff --git a/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_edit_fixtures@missing_parameter_skips_localparams.sv.snap b/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_edit_fixtures@missing_parameter_skips_localparams.sv.snap new file mode 100644 index 00000000..300c39cb --- /dev/null +++ b/crates/ide/src/code_action/snapshots/ide__code_action__tests__code_action_edit_fixtures@missing_parameter_skips_localparams.sv.snap @@ -0,0 +1,7 @@ +--- +source: crates/ide/src/code_action/tests.rs +expression: fixed +input_file: crates/ide/src/code_action/fixtures/code_actions/missing_parameter_skips_localparams.sv +--- +module child #(parameter A = 1, parameter B = 2) (); localparam L = 3; endmodule +module top; localparam L = 4; child #(.A(1), .B()) u(); endmodule diff --git a/crates/ide/src/code_action/tests.rs b/crates/ide/src/code_action/tests.rs index ae79149a..fa7b9bd8 100644 --- a/crates/ide/src/code_action/tests.rs +++ b/crates/ide/src/code_action/tests.rs @@ -416,6 +416,11 @@ fn code_action_availability_matrix() { kind: LabelCaseKind::Repair(RepairKind::MissingParameter), text: "module child #(parameter A = 1) (); endmodule\nmodule top; child #(/*caret*/.A(1)) u(); endmodule\n", }, + LabelCase { + name: "missing_parameter_repair_skips_body_params_when_header_has_parameter_ports", + kind: LabelCaseKind::Repair(RepairKind::MissingParameter), + text: "module child #(parameter A = 1) (); parameter B = 2; endmodule\nmodule top; child #(/*caret*/.A(1)) u(); endmodule\n", + }, LabelCase { name: "named_port_shorthand_collapse_requires_same_name", kind: LabelCaseKind::NoDiagnostics, diff --git a/crates/ide/src/completion/engine/fixtures/filters_body_named_param_names_when_header_has_parameter_ports.v b/crates/ide/src/completion/engine/fixtures/filters_body_named_param_names_when_header_has_parameter_ports.v new file mode 100644 index 00000000..2b01c777 --- /dev/null +++ b/crates/ide/src/completion/engine/fixtures/filters_body_named_param_names_when_header_has_parameter_ports.v @@ -0,0 +1,2 @@ +module child #(parameter A = 1) (); parameter B = 2; endmodule +module top; child #(./*caret*/) u(); endmodule diff --git a/crates/ide/src/completion/engine/instantiation.rs b/crates/ide/src/completion/engine/instantiation.rs index f184ff05..7277151b 100644 --- a/crates/ide/src/completion/engine/instantiation.rs +++ b/crates/ide/src/completion/engine/instantiation.rs @@ -2,7 +2,7 @@ use hir::{ db::HirDb, hir_def::{ Ident, - declaration::{Declaration, DeclarationSrc}, + declaration::Declaration, expr::declarator::DeclaratorParent, module::{ModuleId, port::Ports}, }, @@ -11,7 +11,7 @@ use syntax::{ SyntaxAncestors, ast::{self, AstNode}, }; -use utils::get::{Get, GetRef}; +use utils::get::GetRef; use crate::db::root_db::RootDb; @@ -59,8 +59,7 @@ pub(super) fn overridable_params_of_module_in_order( db: &RootDb, module_id: ModuleId, ) -> Vec { - let (module, module_src_map) = db.module_with_source_map(module_id); - let tree = db.parse(module_id.file_id); + let module = db.module(module_id); let mut names = Vec::new(); @@ -71,22 +70,10 @@ pub(super) fn overridable_params_of_module_in_order( let DeclaratorParent::DeclarationId(declaration_id) = decl.parent else { continue; }; - let Declaration::ParamDecl(_) = module.get(declaration_id) else { - continue; - }; - - let Some(DeclarationSrc::ParameterDeclaration(ptr)) = module_src_map.get(declaration_id) - else { - continue; - }; - let Some(ast_decl) = ptr.to_node(&tree).and_then(ast::ParameterDeclaration::cast) else { - continue; - }; - - let Some(keyword) = ast_decl.keyword() else { + let Declaration::ParamDecl(param_decl) = module.get(declaration_id) else { continue; }; - if keyword.kind() != syntax::Token![parameter] { + if !param_decl.kind.is_overridable() { continue; } diff --git a/crates/ide/src/completion/engine/snapshots/ide__completion__engine__tests__completion_fixtures@filters_body_named_param_names_when_header_has_parameter_ports.v.snap b/crates/ide/src/completion/engine/snapshots/ide__completion__engine__tests__completion_fixtures@filters_body_named_param_names_when_header_has_parameter_ports.v.snap new file mode 100644 index 00000000..d5df3404 --- /dev/null +++ b/crates/ide/src/completion/engine/snapshots/ide__completion__engine__tests__completion_fixtures@filters_body_named_param_names_when_header_has_parameter_ports.v.snap @@ -0,0 +1,23 @@ +--- +source: crates/ide/src/completion/engine/tests.rs +expression: items +input_file: crates/ide/src/completion/engine/fixtures/filters_body_named_param_names_when_header_has_parameter_ports.v +--- +[ + CompletionItem { + label: "A", + kind: Text, + edit: Some( + TextEditItem { + ins: "A()", + del: 84..84, + }, + ), + snippet_edit: Some( + TextEditItem { + ins: "A(${1:expr})", + del: 84..84, + }, + ), + }, +] diff --git a/crates/ide/src/completion/engine/typed_filter.rs b/crates/ide/src/completion/engine/typed_filter.rs index 7b32e85b..51baee45 100644 --- a/crates/ide/src/completion/engine/typed_filter.rs +++ b/crates/ide/src/completion/engine/typed_filter.rs @@ -3,7 +3,7 @@ use hir::{ db::HirDb, hir_def::{ Ident, - declaration::{Declaration, DeclarationId, DeclarationSrc}, + declaration::Declaration, expr::declarator::DeclaratorParent, module::{ModuleId, port::Ports}, }, @@ -13,7 +13,7 @@ use hir::{ Ty, TyClass, packed_bit_width, type_class, type_of_decl, type_of_path_resolution, }, }; -use utils::get::{Get, GetRef}; +use utils::get::GetRef; use crate::db::root_db::RootDb; @@ -51,11 +51,13 @@ pub(super) fn expected_param_ty( let DeclaratorParent::DeclarationId(declaration_id) = target_module.get(decl_id).parent else { return None; }; - let Declaration::ParamDecl(_) = target_module.get(declaration_id) else { + let Declaration::ParamDecl(param_decl) = target_module.get(declaration_id) else { return None; }; - is_overridable_parameter_decl(db, target_module_id, declaration_id) + param_decl + .kind + .is_overridable() .then(|| type_of_decl(db, InContainer::new(target_module_id.into(), decl_id)).ty) } @@ -148,19 +150,3 @@ pub(super) fn is_compatible_typed_value(db: &RootDb, expected: &Ty, candidate: & _ => false, } } - -fn is_overridable_parameter_decl( - db: &RootDb, - module_id: ModuleId, - declaration_id: DeclarationId, -) -> bool { - let (_, module_src_map) = db.module_with_source_map(module_id); - let tree = db.parse(module_id.file_id); - let Some(DeclarationSrc::ParameterDeclaration(ptr)) = module_src_map.get(declaration_id) else { - return false; - }; - let Some(node) = ptr.to_node(&tree) else { - return false; - }; - node.first_token().is_some_and(|kw| kw.kind() == syntax::Token![parameter]) -} diff --git a/crates/ide/src/inlay_hint.rs b/crates/ide/src/inlay_hint.rs index f409f394..addb4405 100644 --- a/crates/ide/src/inlay_hint.rs +++ b/crates/ide/src/inlay_hint.rs @@ -349,7 +349,7 @@ fn process_instantiation( let assign_src = src_map.get(assign_id)?; check_or_throw!(collector.intersect(assign_src.range())); - let param_id = target_module.param_port_id_by_idx(id)?; + let param_id = target_module.overridable_param_id_by_idx(id)?; let param_name = target_module.get(param_id).name.as_ref()?; check_or_throw!(!should_skip(module.get(*assign_expr), param_name)); let target_src = InFile::new(target_file, target_src_map.get(param_id)?); diff --git a/crates/ide/src/inlay_hint/fixtures/ordered_parameter_hints_skip_localparams.sv b/crates/ide/src/inlay_hint/fixtures/ordered_parameter_hints_skip_localparams.sv new file mode 100644 index 00000000..09d6a90d --- /dev/null +++ b/crates/ide/src/inlay_hint/fixtures/ordered_parameter_hints_skip_localparams.sv @@ -0,0 +1,3 @@ +//- config: parameter +module child #(localparam L = 1, parameter P = 2) (); endmodule +module top; child #(3) u(); endmodule diff --git a/crates/ide/src/module_resolution.rs b/crates/ide/src/module_resolution.rs index 9428c64c..5a3d7324 100644 --- a/crates/ide/src/module_resolution.rs +++ b/crates/ide/src/module_resolution.rs @@ -140,7 +140,8 @@ fn resolve_named_param_in_module( }; let module = db.module(module_id); if let DeclaratorParent::DeclarationId(declaration_id) = module.get(decl_id).parent - && let Declaration::ParamDecl(_) = module.get(declaration_id) + && let Declaration::ParamDecl(param_decl) = module.get(declaration_id) + && param_decl.kind.is_overridable() { Some(PathResolution::ParamDecl(InModule::new(module_id, decl_id))) } else { diff --git a/crates/ide/src/module_resolution/fixtures/named_param_rejects_body_param_when_header_has_parameter_ports.sv b/crates/ide/src/module_resolution/fixtures/named_param_rejects_body_param_when_header_has_parameter_ports.sv new file mode 100644 index 00000000..707a1281 --- /dev/null +++ b/crates/ide/src/module_resolution/fixtures/named_param_rejects_body_param_when_header_has_parameter_ports.sv @@ -0,0 +1,7 @@ +//- root: best_effort +//- query: named_param +//- focus: /project/top.sv +//- file: /project/child.sv +module child #(parameter PORT = 1) (); parameter BODY = 2; endmodule +//- file: /project/top.sv +module top; child #(./*caret*/BODY(1)) u(); endmodule diff --git a/crates/ide/src/render.rs b/crates/ide/src/render.rs index 2580e5bf..94dd758e 100644 --- a/crates/ide/src/render.rs +++ b/crates/ide/src/render.rs @@ -467,7 +467,7 @@ fn render_declaration_prefix( prefix.push_str(&ty); prefix } - Declaration::ParamDecl(_) => format!("parameter {ty}"), + Declaration::ParamDecl(param_decl) => format!("{} {ty}", param_decl.kind.keyword()), Declaration::GenvarDecl(_) => format!("genvar {ty}"), Declaration::SpecparamDecl(_) => { if ty.is_empty() { diff --git a/crates/ide/src/signature_help.rs b/crates/ide/src/signature_help.rs index ce801814..2303ef14 100644 --- a/crates/ide/src/signature_help.rs +++ b/crates/ide/src/signature_help.rs @@ -276,8 +276,16 @@ fn sig_help_for_instantiation( } } - for port_decl in - target_module.declarations.values().take_while(|d| matches!(d, Declaration::ParamDecl(_))) + for port_decl in target_module + .declarations + .values() + .take_while(|declaration| matches!(declaration, Declaration::ParamDecl(_))) + .filter(|declaration| { + matches!( + declaration, + Declaration::ParamDecl(param_decl) if param_decl.kind.is_overridable() + ) + }) { let mut buf = String::new(); if !res.config.params_only { diff --git a/crates/ide/src/snapshots/ide__inlay_hint__tests__inlay_hint_fixtures@ordered_parameter_hints_skip_localparams.sv.snap b/crates/ide/src/snapshots/ide__inlay_hint__tests__inlay_hint_fixtures@ordered_parameter_hints_skip_localparams.sv.snap new file mode 100644 index 00000000..2f3a9862 --- /dev/null +++ b/crates/ide/src/snapshots/ide__inlay_hint__tests__inlay_hint_fixtures@ordered_parameter_hints_skip_localparams.sv.snap @@ -0,0 +1,6 @@ +--- +source: crates/ide/src/inlay_hint.rs +expression: hint_snapshot(hints) +input_file: crates/ide/src/inlay_hint/fixtures/ordered_parameter_hints_skip_localparams.sv +--- +ParamAssign @ 84 "P:" padding=(false, true) target=Some((43, 48)) edit=Some("TextEdit { changes: [TextEditItem { ins: \".P(\", del: 84..84 }, TextEditItem { ins: \")\", del: 85..85 }] }") diff --git a/crates/ide/src/snapshots/ide__module_resolution__tests__module_resolution_fixtures@named_param_rejects_body_param_when_header_has_parameter_ports.sv.snap b/crates/ide/src/snapshots/ide__module_resolution__tests__module_resolution_fixtures@named_param_rejects_body_param_when_header_has_parameter_ports.sv.snap new file mode 100644 index 00000000..37cff7e8 --- /dev/null +++ b/crates/ide/src/snapshots/ide__module_resolution__tests__module_resolution_fixtures@named_param_rejects_body_param_when_header_has_parameter_ports.sv.snap @@ -0,0 +1,6 @@ +--- +source: crates/ide/src/module_resolution.rs +expression: fixture_snapshot(fixture) +input_file: crates/ide/src/module_resolution/fixtures/named_param_rejects_body_param_when_header_has_parameter_ports.sv +--- +None diff --git a/crates/ide/src/verilog_2005.rs b/crates/ide/src/verilog_2005.rs index 38f2299d..3cc58d29 100644 --- a/crates/ide/src/verilog_2005.rs +++ b/crates/ide/src/verilog_2005.rs @@ -2681,7 +2681,7 @@ endmodule .unwrap() .expect("parameter hover expected"); assert!( - param_hover.info.as_str().contains("parameter logic DEPTH = WIDTH + 1"), + param_hover.info.as_str().contains("localparam logic DEPTH = WIDTH + 1"), "parameter hover should use parameter-specific renderer: {}", param_hover.info.as_str() ); @@ -2966,6 +2966,39 @@ endmodule ); } +#[test] +fn parameter_signature_help_skips_body_params_when_header_has_parameter_ports() { + let text = r#" +module child #(parameter PORT = 1) (); + parameter BODY = 2; +endmodule + +module top; + child #(/*marker:param*/1) u(); +endmodule +"#; + let (host, file_id, _clean_text, markers) = setup_marked(text); + let signature = host + .make_analysis() + .signature_help( + position(file_id, &markers, "param"), + crate::signature_help::SignatureHelpConfig { params_only: false }, + ) + .unwrap() + .expect("signature help expected for ordered parameter assignment"); + + assert!( + signature.label.contains("PORT"), + "port parameter expected in signature help: {}", + signature.label + ); + assert!( + !signature.label.contains("BODY"), + "body parameter should be local when the target has parameter ports: {}", + signature.label + ); +} + #[test] fn verilog_2005_direct_generate_subroutine_resolves_locally() { let text = r#"