diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index efb97a441..deee84795 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Parent member override completion.** Typing a method name after `function` in a class body (for example `protected function get`) now suggests public and protected methods from parent classes and interfaces that can still be overridden or implemented, inserting a full signature snippet. The same flow suggests parent properties after `$` (for example `protected $tit`) and parent constants after `const`. Snippets insert `#[\Override]` above methods on PHP 8.3+, properties on PHP 8.5+, and constants on PHP 8.6+ (from `composer.json` / `config.platform.php`). Private members and ones already defined on the class are omitted. Contributed by @calebdw. - **Laravel schema dumps power Eloquent model properties.** PHPantom now scans Laravel database schema dumps from `database/schema` by default, reads `config/database.php` for connection drivers/defaults, and uses the parsed columns to synthesize Eloquent model properties with database types, nullability, and defaults in hover. Schema lookup respects model `$connection`, `$table`, Laravel `Connection`/`Table` attributes, dynamic table/connection overrides, and reloads when schema files or related config change. Contributed by @calebdw. - **Laravel migration scanning for Eloquent model properties.** PHPantom now parses Laravel migration files to infer database columns when schema dumps are not available or to overlay changes on top of dumps. Migrations are discovered from any non-vendor `database/migrations` directory (including nested modules like `modules/billing/database/migrations`), applied in global filename order, and support named and anonymous migration classes, `$connection` properties, `Schema::connection()` calls, `Blueprint::after()` nested closures, `virtualAs`/`storedAs` generated columns, and custom Blueprint macros registered via the existing macro scanner. Migration scanning is incremental: editing a single migration re-reads only that file and replays the cached plan over the base schema without re-reading other files. Configure with `[laravel.migrations] enabled` and `paths` in `.phpantom.toml`. Contributed by @calebdw. - **By-reference closure captures update outer variable types for immediately-invoked callables.** A closure passed to a callable parameter can now update the inferred type of variables captured with `use (&$var)` when the callable is considered immediately invoked. This follows PHPStan's defaults: function callable parameters are immediate unless marked with `@param-later-invoked-callable`, while method callable parameters are later-invoked unless marked with `@param-immediately-invoked-callable`. Contributed by @calebdw. diff --git a/src/code_actions/implement_methods.rs b/src/code_actions/implement_methods.rs index b05ad366e..97278e845 100644 --- a/src/code_actions/implement_methods.rs +++ b/src/code_actions/implement_methods.rs @@ -451,7 +451,7 @@ fn build_method_stubs( } /// Format the parameter list for a method stub. -fn format_params( +pub(crate) fn format_params( method: &MethodInfo, use_map: &HashMap, file_namespace: &Option, @@ -461,12 +461,16 @@ fn format_params( for param in &method.parameters { let mut s = String::new(); - // Type hint — prefer native type hint (what appears in PHP source) - // over the docblock-enriched one. + // Type hint — only the native PHP signature. Do not fall back to + // docblock `@param` types: parameter types are contravariant, so + // promoting a docblock type (e.g. `string`) onto an untyped parent + // parameter would illegally narrow the override. if let Some(ref hint) = param.native_type_hint { let shortened = shorten_php_type_direct(hint, use_map, file_namespace); - s.push_str(&shortened); - s.push(' '); + if !shortened.is_empty() { + s.push_str(&shortened); + s.push(' '); + } } // Variadic and reference markers. @@ -477,7 +481,13 @@ fn format_params( s.push_str("..."); } - s.push_str(¶m.name); + // Parser stores names without `$`; virtual/docblock params may + // already include it. Always emit a single leading `$`. + let pname = param.name.as_str(); + if !pname.starts_with('$') { + s.push('$'); + } + s.push_str(pname); // Default value. if let Some(ref default) = param.default_value { @@ -513,7 +523,7 @@ fn is_valid_native_hint(ty: &PhpType) -> bool { } /// Format the return type hint for a method stub. -fn format_return_type( +pub(crate) fn format_return_type( method: &MethodInfo, use_map: &HashMap, file_namespace: &Option, @@ -714,6 +724,28 @@ mod tests { assert_eq!(result, "string $name, int $age = 0"); } + #[test] + fn format_params_adds_dollar_when_name_has_none() { + // Real parsed methods store names without `$`. + let method = MethodInfo { + parameters: vec![ParameterInfo { + name: crate::atom::atom("key"), + is_required: true, + type_hint: Some(PhpType::parse("string")), + native_type_hint: None, + description: None, + default_value: None, + is_variadic: false, + is_reference: false, + closure_this_type: None, + }], + ..MethodInfo::virtual_method("getAttribute", None) + }; + let result = format_params(&method, &HashMap::new(), &None); + // Docblock-only type must not be promoted (contravariance). + assert_eq!(result, "$key"); + } + #[test] fn format_params_variadic_and_reference() { let method = MethodInfo { diff --git a/src/completion/context/mod.rs b/src/completion/context/mod.rs index d30a4c18a..080050031 100644 --- a/src/completion/context/mod.rs +++ b/src/completion/context/mod.rs @@ -15,5 +15,6 @@ pub(crate) mod constant_completion; pub(crate) mod function_completion; pub(crate) mod keyword_completion; pub(crate) mod namespace_completion; +pub(crate) mod override_completion; pub(crate) mod symbol_ranking; pub(crate) mod type_hint_completion; diff --git a/src/completion/context/override_completion.rs b/src/completion/context/override_completion.rs new file mode 100644 index 000000000..f9ddc3809 --- /dev/null +++ b/src/completion/context/override_completion.rs @@ -0,0 +1,946 @@ +//! Member override completion in a class body. +//! +//! - Methods: after `function get|` — parent/interface methods with signatures +//! - Properties: after `protected $tit|` — parent public/protected properties +//! - Constants: after `public const FO|` — parent public/protected constants +//! +//! Override snippets include `#[\Override]` according to the PHP versions that +//! support it for each member kind: methods on PHP 8.3+, properties on PHP +//! 8.5+, and constants on PHP 8.6+. + +use std::collections::{HashMap, HashSet}; +use std::sync::Arc; + +use tower_lsp::lsp_types::{ + CompletionItem, CompletionItemKind, CompletionItemLabelDetails, CompletionTextEdit, + InsertTextFormat, Position, Range, TextEdit, +}; + +use crate::code_actions::implement_methods::{ + detect_class_indent, format_params, format_return_type, +}; +use crate::php_type::PhpType; +use crate::types::{ + ClassInfo, ClassLikeKind, ConstantInfo, MethodInfo, PhpVersion, PropertyInfo, PropertySource, + Visibility, +}; +use crate::util::{find_class_at_offset, position_to_offset, short_name}; + +const METHOD_OVERRIDE_ATTR_MIN: PhpVersion = PhpVersion::new(8, 3); +const PROPERTY_OVERRIDE_ATTR_MIN: PhpVersion = PhpVersion::new(8, 5); +const CONSTANT_OVERRIDE_ATTR_MIN: PhpVersion = PhpVersion::new(8, 6); + +/// Collect public/protected methods from parents and interfaces that the +/// current class can still override or implement. +pub(crate) fn collect_overridable_methods( + class: &ClassInfo, + partial: &str, + class_loader: &dyn Fn(&str) -> Option>, +) -> Vec<(MethodInfo, String)> { + let mut own_names: HashSet = class + .methods + .iter() + .map(|m| m.name.to_lowercase()) + .collect(); + + // Trait methods already on this class count as implemented. + collect_trait_method_names(&class.used_traits, class_loader, &mut own_names, 0); + + let mut results: Vec<(MethodInfo, String)> = Vec::new(); + let mut seen: HashSet = HashSet::new(); + let mut visited: HashSet = HashSet::new(); + + let mut collector = MethodCollector { + partial, + class_loader, + own_names: &own_names, + seen: &mut seen, + visited: &mut visited, + results: &mut results, + }; + + collector.collect_from_parent_chain(&class.parent_class, 0); + + for iface in &class.interfaces { + if class.kind == ClassLikeKind::Enum { + let s: &str = iface; + let stripped = s.strip_prefix('\\').unwrap_or(s); + if stripped == "BackedEnum" || stripped == "UnitEnum" { + continue; + } + } + collector.collect_from_interface(iface, 0); + } + + results +} + +fn collect_trait_method_names( + traits: &[crate::atom::Atom], + class_loader: &dyn Fn(&str) -> Option>, + names: &mut HashSet, + depth: usize, +) { + if depth > crate::types::MAX_INHERITANCE_DEPTH as usize { + return; + } + for tname in traits { + let Some(tr) = class_loader(tname) else { + continue; + }; + for m in &tr.methods { + names.insert(m.name.to_lowercase()); + } + collect_trait_method_names(&tr.used_traits, class_loader, names, depth + 1); + } +} + +struct MethodCollector<'a> { + partial: &'a str, + class_loader: &'a dyn Fn(&str) -> Option>, + own_names: &'a HashSet, + seen: &'a mut HashSet, + visited: &'a mut HashSet, + results: &'a mut Vec<(MethodInfo, String)>, +} + +impl MethodCollector<'_> { + fn collect_from_parent_chain(&mut self, parent_name: &Option, depth: usize) { + if depth > crate::types::MAX_INHERITANCE_DEPTH as usize { + return; + } + let Some(pname) = parent_name else { + return; + }; + if !self.visited.insert(pname.to_string()) { + return; + } + let Some(parent) = (self.class_loader)(pname) else { + return; + }; + + self.push_from_class(&parent); + self.collect_from_traits(&parent.used_traits, depth + 1); + + for iface in &parent.interfaces { + self.collect_from_interface(iface, depth + 1); + } + + self.collect_from_parent_chain(&parent.parent_class, depth + 1); + } + + fn collect_from_traits(&mut self, traits: &[crate::atom::Atom], depth: usize) { + if depth > crate::types::MAX_INHERITANCE_DEPTH as usize { + return; + } + for tname in traits { + if !self.visited.insert(tname.to_string()) { + continue; + } + let Some(tr) = (self.class_loader)(tname) else { + continue; + }; + self.push_from_class(&tr); + self.collect_from_traits(&tr.used_traits, depth + 1); + } + } + + fn collect_from_interface(&mut self, iface_name: &str, depth: usize) { + if depth > crate::types::MAX_INHERITANCE_DEPTH as usize { + return; + } + if !self.visited.insert(iface_name.to_string()) { + return; + } + let Some(iface) = (self.class_loader)(iface_name) else { + return; + }; + self.push_from_class(&iface); + for parent_iface in &iface.interfaces { + self.collect_from_interface(parent_iface, depth + 1); + } + } + + fn push_from_class(&mut self, class: &ClassInfo) { + let declaring = class.fqn().to_string(); + for method in &class.methods { + if method.visibility == Visibility::Private { + continue; + } + if method.name.starts_with("__") { + continue; + } + if method.is_virtual { + continue; + } + if !self.partial.is_empty() + && !starts_with_ignore_ascii_case(&method.name, self.partial) + { + continue; + } + let lower = method.name.to_lowercase(); + if self.own_names.contains(&lower) || !self.seen.insert(lower) { + continue; + } + self.results.push(((**method).clone(), declaring.clone())); + } + } +} + +/// Options for building method-override completion items. +pub(crate) struct OverrideCompletionOpts<'a> { + pub use_map: &'a HashMap, + pub file_namespace: &'a Option, + pub indent: &'a str, + pub replace_range: Range, + pub php_version: PhpVersion, + pub line_start: Position, +} + +/// Build completion items for overridable methods matching `partial`. +/// +/// When `php_version >= 8.3`, each item also inserts `#[\Override]` on the +/// line above the declaration via `additional_text_edits`. +pub(crate) fn build_override_completions( + methods: &[(MethodInfo, String)], + opts: &OverrideCompletionOpts<'_>, +) -> Vec { + let add_override = opts.php_version >= METHOD_OVERRIDE_ATTR_MIN; + // Insert the attribute at the start of the declaration line. The + // indent is included here because this edit is at column 0 of the + // line (absolute), not mid-line like the name snippet. + let override_edit = if add_override { + Some(TextEdit { + range: Range { + start: opts.line_start, + end: opts.line_start, + }, + new_text: format!("{}#[\\Override]\n", opts.indent), + }) + } else { + None + }; + + let mut items = Vec::new(); + for (method, declaring) in methods { + let params = format_params(method, opts.use_map, opts.file_namespace); + let return_type = format_return_type(method, opts.use_map, opts.file_namespace); + let label = if return_type.is_empty() { + format!("{}({})", method.name, params) + } else { + format!("{}({}){}", method.name, params, return_type) + }; + + // Escape `$` in the signature so LSP snippet parsing does not + // treat `$attributes` as a tabstop/variable (which drops the `$` + // and can eat the name). Keep a real `$0` for the final cursor. + let params_escaped = params.replace('$', "\\$"); + let return_escaped = return_type.replace('$', "\\$"); + + // Brace lines intentionally have no leading indent. Clients + // re-indent multi-line snippet continuations relative to the + // insertion line (` public function …`), so baking in the + // member indent here would double it (` {`). + let insert_text = format!( + "{}({}){}\n{{\n $0\n}}", + method.name, params_escaped, return_escaped + ); + + let sort_prefix = if method.is_abstract { "0" } else { "1" }; + let sort_text = format!("{sort_prefix}_{}", method.name.to_ascii_lowercase()); + + items.push(CompletionItem { + label: label.clone(), + kind: Some(CompletionItemKind::METHOD), + detail: Some(format!("override · {}", short_name(declaring))), + filter_text: Some(method.name.to_string()), + sort_text: Some(sort_text), + insert_text: Some(insert_text.clone()), + insert_text_format: Some(InsertTextFormat::SNIPPET), + text_edit: Some(CompletionTextEdit::Edit(TextEdit { + range: opts.replace_range, + new_text: insert_text, + })), + additional_text_edits: override_edit.clone().map(|e| vec![e]), + label_details: Some(CompletionItemLabelDetails { + detail: None, + description: Some(short_name(declaring).to_string()), + }), + ..CompletionItem::default() + }); + } + + items.sort_by(|a, b| a.sort_text.cmp(&b.sort_text)); + items +} + +/// Collect public/protected properties from parents that the class can still +/// redeclare. +pub(crate) fn collect_overridable_properties( + class: &ClassInfo, + partial: &str, + class_loader: &dyn Fn(&str) -> Option>, +) -> Vec<(PropertyInfo, String)> { + let mut own: HashSet = class + .properties + .iter() + .map(|p| p.name.to_lowercase()) + .collect(); + collect_trait_property_names(&class.used_traits, class_loader, &mut own, 0); + + let mut results = Vec::new(); + let mut seen = HashSet::new(); + let mut visited = HashSet::new(); + let mut parent_name = class.parent_class; + let mut depth = 0usize; + while let Some(ref pname) = parent_name { + if depth > crate::types::MAX_INHERITANCE_DEPTH as usize { + break; + } + if !visited.insert(pname.to_string()) { + break; + } + let Some(parent) = class_loader(pname) else { + break; + }; + let declaring = parent.fqn().to_string(); + for prop in &parent.properties { + if prop.visibility == Visibility::Private || prop.is_virtual { + continue; + } + if !partial.is_empty() && !starts_with_ignore_ascii_case(&prop.name, partial) { + continue; + } + let lower = prop.name.to_lowercase(); + if own.contains(&lower) || !seen.insert(lower) { + continue; + } + results.push((prop.clone(), declaring.clone())); + } + let mut collector = PropertyCollector { + partial, + class_loader, + own: &own, + seen: &mut seen, + visited: &mut visited, + results: &mut results, + }; + collector.collect_from_traits(&parent.used_traits, depth + 1); + parent_name = parent.parent_class; + depth += 1; + } + results +} + +fn collect_trait_property_names( + traits: &[crate::atom::Atom], + class_loader: &dyn Fn(&str) -> Option>, + names: &mut HashSet, + depth: usize, +) { + if depth > crate::types::MAX_INHERITANCE_DEPTH as usize { + return; + } + for tname in traits { + let Some(tr) = class_loader(tname) else { + continue; + }; + for p in &tr.properties { + names.insert(p.name.to_lowercase()); + } + collect_trait_property_names(&tr.used_traits, class_loader, names, depth + 1); + } +} + +struct PropertyCollector<'a> { + partial: &'a str, + class_loader: &'a dyn Fn(&str) -> Option>, + own: &'a HashSet, + seen: &'a mut HashSet, + visited: &'a mut HashSet, + results: &'a mut Vec<(PropertyInfo, String)>, +} + +impl PropertyCollector<'_> { + fn collect_from_traits(&mut self, traits: &[crate::atom::Atom], depth: usize) { + if depth > crate::types::MAX_INHERITANCE_DEPTH as usize { + return; + } + for tname in traits { + if !self.visited.insert(tname.to_string()) { + continue; + } + let Some(tr) = (self.class_loader)(tname) else { + continue; + }; + self.push_from_trait(&tr); + self.collect_from_traits(&tr.used_traits, depth + 1); + } + } + + fn push_from_trait(&mut self, tr: &ClassInfo) { + let declaring = tr.fqn().to_string(); + for prop in &tr.properties { + if prop.visibility == Visibility::Private || prop.is_virtual { + continue; + } + if !self.partial.is_empty() && !starts_with_ignore_ascii_case(&prop.name, self.partial) + { + continue; + } + let lower = prop.name.to_lowercase(); + if self.own.contains(&lower) || !self.seen.insert(lower) { + continue; + } + self.results.push((prop.clone(), declaring.clone())); + } + } +} + +/// Collect public/protected constants from parents. +pub(crate) fn collect_overridable_constants( + class: &ClassInfo, + partial: &str, + class_loader: &dyn Fn(&str) -> Option>, +) -> Vec<(ConstantInfo, String)> { + let own: HashSet = class + .constants + .iter() + .map(|c| c.name.to_lowercase()) + .collect(); + + let mut results = Vec::new(); + let mut seen = HashSet::new(); + let mut visited = HashSet::new(); + let mut parent_name = class.parent_class; + let mut depth = 0usize; + while let Some(ref pname) = parent_name { + if depth > crate::types::MAX_INHERITANCE_DEPTH as usize { + break; + } + if !visited.insert(pname.to_string()) { + break; + } + let Some(parent) = class_loader(pname) else { + break; + }; + let declaring = parent.fqn().to_string(); + for c in &parent.constants { + if c.visibility == Visibility::Private || c.is_enum_case { + continue; + } + if !partial.is_empty() && !starts_with_ignore_ascii_case(&c.name, partial) { + continue; + } + let lower = c.name.to_lowercase(); + if own.contains(&lower) || !seen.insert(lower) { + continue; + } + results.push((c.clone(), declaring.clone())); + } + parent_name = parent.parent_class; + depth += 1; + } + results +} + +/// Build property-name override completions (`$title` already typed `$`). +/// +/// Inserts `name = default` when the parent has an initializer so the +/// user can override `protected $attributes = []` style members in one go. +pub(crate) fn build_property_override_completions( + props: &[(PropertyInfo, String)], + opts: &NameOverrideCompletionOpts<'_>, +) -> Vec { + let override_edit = if opts.php_version >= PROPERTY_OVERRIDE_ATTR_MIN { + Some(TextEdit { + range: Range { + start: opts.line_start, + end: opts.line_start, + }, + new_text: format!("{}#[\\Override]\n", opts.indent), + }) + } else { + None + }; + let mut items = Vec::new(); + for (prop, declaring) in props { + let type_str = prop + .native_type_hint + .as_ref() + .or(prop.type_hint.as_ref()) + .map(|t| shorten_type_display(t, opts.use_map, opts.file_namespace)) + .filter(|s| !s.is_empty()); + let default = property_default_value(prop); + let insert = match default { + Some(d) => format!("{} = {}", prop.name, d), + None => prop.name.to_string(), + }; + let label = match (&type_str, default) { + (Some(t), Some(d)) => format!("${}: {} = {}", prop.name, t, d), + (Some(t), None) => format!("${}: {}", prop.name, t), + (None, Some(d)) => format!("${} = {}", prop.name, d), + (None, None) => format!("${}", prop.name), + }; + items.push(CompletionItem { + label, + kind: Some(CompletionItemKind::PROPERTY), + detail: Some(format!("override · {}", short_name(declaring))), + filter_text: Some(prop.name.to_string()), + sort_text: Some(format!("0_{}", prop.name.to_ascii_lowercase())), + insert_text: Some(insert.clone()), + text_edit: Some(CompletionTextEdit::Edit(TextEdit { + range: opts.replace_range, + new_text: insert, + })), + additional_text_edits: override_edit.clone().map(|e| vec![e]), + label_details: Some(CompletionItemLabelDetails { + detail: None, + description: Some(short_name(declaring).to_string()), + }), + ..CompletionItem::default() + }); + } + items.sort_by(|a, b| a.sort_text.cmp(&b.sort_text)); + items +} + +fn property_default_value(prop: &PropertyInfo) -> Option<&str> { + let Some(PropertySource::DeclaredDefault { value }) = prop.source.as_ref() else { + return None; + }; + let value = value.trim(); + if value.is_empty() { None } else { Some(value) } +} + +pub(crate) struct NameOverrideCompletionOpts<'a> { + pub use_map: &'a HashMap, + pub file_namespace: &'a Option, + pub indent: &'a str, + pub replace_range: Range, + pub php_version: PhpVersion, + pub line_start: Position, +} + +/// Build constant-name override completions. +/// +/// Inserts `NAME = value` when the parent constant has an initializer. +pub(crate) fn build_constant_override_completions( + constants: &[(ConstantInfo, String)], + opts: &NameOverrideCompletionOpts<'_>, +) -> Vec { + let override_edit = if opts.php_version >= CONSTANT_OVERRIDE_ATTR_MIN { + Some(TextEdit { + range: Range { + start: opts.line_start, + end: opts.line_start, + }, + new_text: format!("{}#[\\Override]\n", opts.indent), + }) + } else { + None + }; + let mut items = Vec::new(); + for (c, declaring) in constants { + let type_str = c + .type_hint + .as_ref() + .map(|t| shorten_type_display(t, opts.use_map, opts.file_namespace)) + .filter(|s| !s.is_empty()); + let default = c.value.as_deref().map(str::trim).filter(|s| !s.is_empty()); + let insert = match default { + Some(d) => format!("{} = {}", c.name, d), + None => c.name.to_string(), + }; + let label = match (&type_str, default) { + (Some(t), Some(d)) => format!("{}: {} = {}", c.name, t, d), + (Some(t), None) => format!("{}: {}", c.name, t), + (None, Some(d)) => format!("{} = {}", c.name, d), + (None, None) => c.name.to_string(), + }; + items.push(CompletionItem { + label, + kind: Some(CompletionItemKind::CONSTANT), + detail: Some(format!("override · {}", short_name(declaring))), + filter_text: Some(c.name.to_string()), + sort_text: Some(format!("0_{}", c.name.to_ascii_lowercase())), + insert_text: Some(insert.clone()), + text_edit: Some(CompletionTextEdit::Edit(TextEdit { + range: opts.replace_range, + new_text: insert, + })), + additional_text_edits: override_edit.clone().map(|e| vec![e]), + label_details: Some(CompletionItemLabelDetails { + detail: None, + description: Some(short_name(declaring).to_string()), + }), + ..CompletionItem::default() + }); + } + items.sort_by(|a, b| a.sort_text.cmp(&b.sort_text)); + items +} + +fn shorten_type_display( + ty: &PhpType, + use_map: &HashMap, + file_namespace: &Option, +) -> String { + ty.resolve_names(&|name| { + for (short, fqn) in use_map { + if fqn.trim_start_matches('\\') == name { + return short.clone(); + } + } + if let Some(ns) = file_namespace { + let prefix = format!("{ns}\\"); + if let Some(rest) = name.strip_prefix(&prefix) + && !rest.contains('\\') + { + return rest.to_string(); + } + } + name.to_string() + }) + .to_string() +} + +/// Extract the partial method name and its LSP range at the cursor. +pub(crate) fn extract_method_name_partial( + content: &str, + position: Position, +) -> Option<(String, Range)> { + let offset = position_to_offset(content, position) as usize; + if offset > content.len() { + return None; + } + let bytes = content.as_bytes(); + let mut start = offset; + while start > 0 { + let b = bytes[start - 1]; + if b.is_ascii_alphanumeric() || b == b'_' { + start -= 1; + } else { + break; + } + } + let partial = content[start..offset].to_string(); + + let start_pos = offset_to_position(content, start); + let end_pos = position; + Some(( + partial, + Range { + start: start_pos, + end: end_pos, + }, + )) +} + +fn offset_to_position(content: &str, byte_offset: usize) -> Position { + let mut line = 0u32; + let mut col = 0u32; + for (i, ch) in content.char_indices() { + if i >= byte_offset { + break; + } + if ch == '\n' { + line += 1; + col = 0; + } else { + col += 1; + } + } + Position { + line, + character: col, + } +} + +/// Whether the cursor is after the `function` keyword (not `const`/`case`). +pub(crate) fn is_after_function_keyword(content: &str, position: Position) -> bool { + after_keyword(content, position, "function") +} + +/// Whether the cursor is after the `const` keyword (class constant name). +pub(crate) fn is_after_const_keyword(content: &str, position: Position) -> bool { + let bytes = content.as_bytes(); + let cursor = (position_to_offset(content, position) as usize).min(bytes.len()); + let mut i = cursor; + while i > 0 && is_ident_byte(bytes[i - 1]) { + i -= 1; + } + while i > 0 && bytes[i - 1].is_ascii_whitespace() { + i -= 1; + } + if check_keyword_ending_at_bytes(bytes, i, b"const") { + return !preceded_by_use_keyword_bytes(bytes, i - "const".len()); + } + has_const_keyword_before_name(bytes, i) +} + +fn after_keyword(content: &str, position: Position, keyword: &str) -> bool { + let bytes = content.as_bytes(); + let cursor = (position_to_offset(content, position) as usize).min(bytes.len()); + let mut i = cursor; + while i > 0 && is_ident_byte(bytes[i - 1]) { + i -= 1; + } + while i > 0 && bytes[i - 1].is_ascii_whitespace() { + i -= 1; + } + check_keyword_ending_at_bytes(bytes, i, keyword.as_bytes()) +} + +fn is_ident_byte(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' +} + +fn check_keyword_ending_at_bytes(bytes: &[u8], pos: usize, keyword: &[u8]) -> bool { + if pos < keyword.len() { + return false; + } + let start = pos - keyword.len(); + if &bytes[start..pos] != keyword { + return false; + } + if start > 0 && is_ident_byte(bytes[start - 1]) { + return false; + } + if pos < bytes.len() && is_ident_byte(bytes[pos]) { + return false; + } + true +} + +fn preceded_by_use_keyword_bytes(bytes: &[u8], keyword_start: usize) -> bool { + let mut before = keyword_start; + while before > 0 && bytes[before - 1].is_ascii_whitespace() { + before -= 1; + } + check_keyword_ending_at_bytes(bytes, before, b"use") +} + +fn has_const_keyword_before_name(bytes: &[u8], pos: usize) -> bool { + let mut line_start = pos; + while line_start > 0 && bytes[line_start - 1] != b'\n' { + line_start -= 1; + } + bytes[line_start..pos] + .windows(b"const".len()) + .enumerate() + .any(|(idx, window)| { + if window != b"const" { + return false; + } + let start = line_start + idx; + let end = start + b"const".len(); + (start == 0 || !is_ident_byte(bytes[start - 1])) + && (end >= bytes.len() || !is_ident_byte(bytes[end])) + && !preceded_by_use_keyword_bytes(bytes, start) + }) +} + +/// Property name after `$` on a property declaration line (not a parameter). +pub(crate) fn is_property_declaration_name_position(content: &str, position: Position) -> bool { + let bytes = content.as_bytes(); + let cursor = (position_to_offset(content, position) as usize).min(bytes.len()); + is_property_declaration_name_position_at_offset(bytes, cursor) +} + +pub(crate) fn is_member_declaration_name_position_at_offset(content: &str, cursor: usize) -> bool { + let bytes = content.as_bytes(); + let cursor = cursor.min(bytes.len()); + is_function_or_const_name_position_at_offset(bytes, cursor) + || is_property_declaration_name_position_at_offset(bytes, cursor) +} + +fn is_function_or_const_name_position_at_offset(bytes: &[u8], cursor: usize) -> bool { + let mut i = cursor; + while i > 0 && is_ident_byte(bytes[i - 1]) { + i -= 1; + } + + let after_ident = i; + while i > 0 && bytes[i - 1].is_ascii_whitespace() { + i -= 1; + } + if i == after_ident && after_ident != cursor { + return false; + } + if i == after_ident { + return false; + } + + if check_keyword_ending_at_bytes(bytes, i, b"fn") + || check_keyword_ending_at_bytes(bytes, i, b"case") + { + return true; + } + if check_keyword_ending_at_bytes(bytes, i, b"function") { + return !preceded_by_use_keyword_bytes(bytes, i - "function".len()); + } + if check_keyword_ending_at_bytes(bytes, i, b"const") { + return !preceded_by_use_keyword_bytes(bytes, i - "const".len()); + } + has_const_keyword_before_name(bytes, i) +} + +fn is_property_declaration_name_position_at_offset(bytes: &[u8], cursor: usize) -> bool { + // Skip partial name. + let mut i = cursor; + while i > 0 && is_ident_byte(bytes[i - 1]) { + i -= 1; + } + // Must be immediately after `$`. + if i == 0 || bytes[i - 1] != b'$' { + return false; + } + let dollar = i - 1; + // Walk back on the same line looking for declaration context. + let mut j = dollar; + while j > 0 && bytes[j - 1] != b'\n' { + j -= 1; + } + // Parameters live after `function` on the same line / signature. + let line = &bytes[j..dollar]; + if contains_ascii_word(line, b"function") { + return false; + } + // Property declarations have a visibility/static/readonly/var keyword. + const MARKERS: &[&str] = &[ + "public", + "protected", + "private", + "static", + "readonly", + "var", + ]; + MARKERS + .iter() + .any(|m| contains_ascii_word(line, m.as_bytes())) +} + +fn contains_ascii_word(bytes: &[u8], word: &[u8]) -> bool { + if word.is_empty() || bytes.len() < word.len() { + return false; + } + bytes.windows(word.len()).enumerate().any(|(idx, window)| { + window.eq_ignore_ascii_case(word) + && (idx == 0 || !is_ident_byte(bytes[idx - 1])) + && (idx + word.len() == bytes.len() || !is_ident_byte(bytes[idx + word.len()])) + }) +} + +fn starts_with_ignore_ascii_case(value: &str, prefix: &str) -> bool { + value + .as_bytes() + .get(..prefix.len()) + .is_some_and(|head| head.eq_ignore_ascii_case(prefix.as_bytes())) +} + +/// Byte offset of the start of the line containing `position`. +pub(crate) fn line_start_position(content: &str, position: Position) -> Position { + let offset = position_to_offset(content, position) as usize; + let line_start = content[..offset.min(content.len())] + .rfind('\n') + .map(|i| i + 1) + .unwrap_or(0); + offset_to_position(content, line_start) +} + +/// Resolve the enclosing class at the cursor, if any. +pub(crate) fn enclosing_class_at_position<'a>( + classes: &'a [Arc], + content: &str, + position: Position, +) -> Option<&'a ClassInfo> { + let offset = position_to_offset(content, position); + find_class_at_offset(classes, offset) +} + +/// Indent string for the current declaration line (member indent). +pub(crate) fn indent_for_position(content: &str, position: Position, class: &ClassInfo) -> String { + let offset = position_to_offset(content, position) as usize; + let line_start = content[..offset.min(content.len())] + .rfind('\n') + .map(|i| i + 1) + .unwrap_or(0); + let line = &content[line_start..offset.min(content.len())]; + let line_indent: String = line.chars().take_while(|c| c.is_whitespace()).collect(); + if !line_indent.is_empty() { + return line_indent; + } + detect_class_indent(content, class) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::atom::atom; + use crate::test_fixtures::make_class; + use crate::types::{ConstantInfo, Visibility}; + + #[test] + fn collects_parent_constants() { + let mut base = make_class("Base"); + base.constants = vec![ + ConstantInfo { + name: atom("STATUS_OK"), + name_offset: 0, + type_hint: None, + visibility: Visibility::Public, + deprecation_message: None, + deprecated_replacement: None, + see_refs: Vec::new(), + description: None, + is_enum_case: false, + enum_value: None, + value: Some("1".into()), + is_virtual: false, + }, + ConstantInfo { + name: atom("SECRET"), + name_offset: 0, + type_hint: None, + visibility: Visibility::Private, + deprecation_message: None, + deprecated_replacement: None, + see_refs: Vec::new(), + description: None, + is_enum_case: false, + enum_value: None, + value: None, + is_virtual: false, + }, + ] + .into(); + + let mut child = make_class("Child"); + child.parent_class = Some(atom("Base")); + + let loader = |name: &str| -> Option> { + if name == "Base" { + Some(Arc::new(base.clone())) + } else { + None + } + }; + let consts = collect_overridable_constants(&child, "", &loader); + let names: Vec<_> = consts.iter().map(|(c, _)| c.name.as_str()).collect(); + assert!(names.contains(&"STATUS_OK"), "got {names:?}"); + assert!(!names.contains(&"SECRET"), "got {names:?}"); + } + + #[test] + fn after_const_keyword_detects_class_const() { + let src = " = content.chars().collect(); let cursor = position_to_char_offset(&chars, position)?; + // Member *names* after `function` / `const` / `case` are not types. + // Without this, `public const ST` looks like a property type position + // (modifier keyword earlier on the line) and steals completion. + if is_function_or_const_name_position_chars(&chars, cursor) { + return None; + } + // ── Extract partial identifier ────────────────────────────────── let mut partial_start = cursor; while partial_start > 0 @@ -209,21 +217,19 @@ pub(crate) fn detect_type_hint_context( /// by `$`. Type positions like `protected User|` intentionally still /// offer class names. pub(crate) fn is_function_or_const_name_position(content: &str, position: Position) -> bool { - let chars: Vec = content.chars().collect(); - let Some(cursor) = position_to_char_offset(&chars, position) else { - return false; - }; + let bytes = content.as_bytes(); + let cursor = (position_to_offset(content, position) as usize).min(bytes.len()); // Skip the partial identifier being typed. let mut i = cursor; - while i > 0 && (chars[i - 1].is_alphanumeric() || chars[i - 1] == '_') { + while i > 0 && is_ident_byte(bytes[i - 1]) { i -= 1; } // Require whitespace between the keyword and the name (or empty name // still after the keyword: `function |`). let after_ident = i; - while i > 0 && chars[i - 1].is_ascii_whitespace() { + while i > 0 && bytes[i - 1].is_ascii_whitespace() { i -= 1; } if i == after_ident && after_ident != cursor { @@ -237,27 +243,124 @@ pub(crate) fn is_function_or_const_name_position(content: &str, position: Positi return false; } - if check_keyword_ending_at(&chars, i, "fn") || check_keyword_ending_at(&chars, i, "case") { + if check_keyword_ending_at_bytes(bytes, i, b"fn") + || check_keyword_ending_at_bytes(bytes, i, b"case") + { return true; } // `function` / `const` declare member names, but not after `use` // (`use function foo`, `use const BAR` still need symbol completion). - if check_keyword_ending_at(&chars, i, "function") { - return !preceded_by_use_keyword(&chars, i - "function".len()); + if check_keyword_ending_at_bytes(bytes, i, b"function") { + return !preceded_by_use_keyword_bytes(bytes, i - "function".len()); } - if check_keyword_ending_at(&chars, i, "const") { - return !preceded_by_use_keyword(&chars, i - "const".len()); + if check_keyword_ending_at_bytes(bytes, i, b"const") { + return !preceded_by_use_keyword_bytes(bytes, i - "const".len()); } - false + has_const_keyword_before_name(bytes, i) +} + +fn is_function_or_const_name_position_chars(chars: &[char], cursor: usize) -> bool { + let mut i = cursor; + while i > 0 && (chars[i - 1].is_alphanumeric() || chars[i - 1] == '_') { + i -= 1; + } + + let after_ident = i; + while i > 0 && chars[i - 1].is_ascii_whitespace() { + i -= 1; + } + if i == after_ident && after_ident != cursor { + return false; + } + if i == after_ident { + return false; + } + + if check_keyword_ending_at(chars, i, "fn") || check_keyword_ending_at(chars, i, "case") { + return true; + } + if check_keyword_ending_at(chars, i, "function") { + return !preceded_by_use_keyword_chars(chars, i - "function".len()); + } + if check_keyword_ending_at(chars, i, "const") { + return !preceded_by_use_keyword_chars(chars, i - "const".len()); + } + has_const_keyword_before_name_chars(chars, i) } -/// Whether `pos` (end of a keyword span) is preceded by the `use` keyword -/// with only whitespace between (`use function` / `use const`). -fn preceded_by_use_keyword(chars: &[char], keyword_start: usize) -> bool { +fn preceded_by_use_keyword_chars(chars: &[char], keyword_start: usize) -> bool { let before = skip_whitespace_backward(chars, keyword_start); check_keyword_ending_at(chars, before, "use") } +fn has_const_keyword_before_name_chars(chars: &[char], pos: usize) -> bool { + let mut line_start = pos; + while line_start > 0 && chars[line_start - 1] != '\n' { + line_start -= 1; + } + let mut i = line_start; + while i + "const".len() <= pos { + let end = i + "const".len(); + if check_keyword_ending_at(chars, end, "const") + && (end >= chars.len() || !(chars[end].is_alphanumeric() || chars[end] == '_')) + && !preceded_by_use_keyword_chars(chars, i) + { + return true; + } + i += 1; + } + false +} + +fn is_ident_byte(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' +} + +fn check_keyword_ending_at_bytes(bytes: &[u8], pos: usize, keyword: &[u8]) -> bool { + if pos < keyword.len() { + return false; + } + let start = pos - keyword.len(); + if &bytes[start..pos] != keyword { + return false; + } + if start > 0 && is_ident_byte(bytes[start - 1]) { + return false; + } + if pos < bytes.len() && is_ident_byte(bytes[pos]) { + return false; + } + true +} + +fn preceded_by_use_keyword_bytes(bytes: &[u8], keyword_start: usize) -> bool { + let mut before = keyword_start; + while before > 0 && bytes[before - 1].is_ascii_whitespace() { + before -= 1; + } + check_keyword_ending_at_bytes(bytes, before, b"use") +} + +fn has_const_keyword_before_name(bytes: &[u8], pos: usize) -> bool { + let mut line_start = pos; + while line_start > 0 && bytes[line_start - 1] != b'\n' { + line_start -= 1; + } + bytes[line_start..pos] + .windows(b"const".len()) + .enumerate() + .any(|(idx, window)| { + if window != b"const" { + return false; + } + let start = line_start + idx; + let end = start + b"const".len(); + (start == 0 || !is_ident_byte(bytes[start - 1])) + && (end >= bytes.len() || !is_ident_byte(bytes[end])) + && !preceded_by_use_keyword_bytes(bytes, start) + }) +} + // ─── Private helpers ──────────────────────────────────────────────────────── /// Skip whitespace (spaces, tabs, newlines) backward from `pos` diff --git a/src/completion/context/type_hint_completion_tests.rs b/src/completion/context/type_hint_completion_tests.rs index b9be5b40e..d6970d2c5 100644 --- a/src/completion/context/type_hint_completion_tests.rs +++ b/src/completion/context/type_hint_completion_tests.rs @@ -315,6 +315,19 @@ fn const_name_position_after_const_keyword() { )); } +#[test] +fn typed_const_name_position_after_const_type() { + use super::is_function_or_const_name_position; + let src = " Option> { + use crate::completion::context::override_completion::{ + NameOverrideCompletionOpts, build_constant_override_completions, + build_override_completions, build_property_override_completions, + collect_overridable_constants, collect_overridable_methods, + collect_overridable_properties, enclosing_class_at_position, + extract_method_name_partial, indent_for_position, is_after_const_keyword, + is_after_function_keyword, is_property_declaration_name_position, line_start_position, + }; + + let class = enclosing_class_at_position(&ctx.classes, content, position)?; + if !matches!( + class.kind, + crate::types::ClassLikeKind::Class | crate::types::ClassLikeKind::Trait + ) { + return None; + } + if class.parent_class.is_none() && class.interfaces.is_empty() { + return None; + } + + let class_loader = self.class_loader(ctx); + let (partial, range) = extract_method_name_partial(content, position)?; + + if is_after_function_keyword(content, position) { + let methods = collect_overridable_methods(class, &partial, &class_loader); + if methods.is_empty() { + return None; + } + let indent = indent_for_position(content, position, class); + let items = build_override_completions( + &methods, + &crate::completion::context::override_completion::OverrideCompletionOpts { + use_map: &ctx.use_map, + file_namespace: &ctx.namespace, + indent: &indent, + replace_range: range, + php_version: self.php_version(), + line_start: line_start_position(content, position), + }, + ); + return if items.is_empty() { None } else { Some(items) }; + } + + if is_after_const_keyword(content, position) { + let constants = collect_overridable_constants(class, &partial, &class_loader); + if constants.is_empty() { + return None; + } + let indent = indent_for_position(content, position, class); + let items = build_constant_override_completions( + &constants, + &NameOverrideCompletionOpts { + use_map: &ctx.use_map, + file_namespace: &ctx.namespace, + indent: &indent, + replace_range: range, + php_version: self.php_version(), + line_start: line_start_position(content, position), + }, + ); + return if items.is_empty() { None } else { Some(items) }; + } + + if is_property_declaration_name_position(content, position) { + let props = collect_overridable_properties(class, &partial, &class_loader); + if props.is_empty() { + return None; + } + let indent = indent_for_position(content, position, class); + let items = build_property_override_completions( + &props, + &NameOverrideCompletionOpts { + use_map: &ctx.use_map, + file_namespace: &ctx.namespace, + indent: &indent, + replace_range: range, + php_version: self.php_version(), + line_start: line_start_position(content, position), + }, + ); + return if items.is_empty() { None } else { Some(items) }; + } + + None + } + fn try_class_constant_function_completion( &self, content: &str, diff --git a/src/hover/member.rs b/src/hover/member.rs index fcf81ea02..65531cc4d 100644 --- a/src/hover/member.rs +++ b/src/hover/member.rs @@ -65,6 +65,7 @@ fn format_attribute_default_details(source: &AttributeDefaultSource) -> Vec Vec { match source { + PropertySource::DeclaredDefault { .. } => Vec::new(), PropertySource::DatabaseColumn { column, attribute_default, @@ -642,7 +643,10 @@ impl Backend { } if let Some(ref source) = property.source { - lines.push(format_property_source(source).join("\n")); + let source_lines = format_property_source(source); + if !source_lines.is_empty() { + lines.push(source_lines.join("\n")); + } } if let Some(ref msg) = property.deprecation_message { diff --git a/src/parser/classes.rs b/src/parser/classes.rs index 0fa5b64e3..56d4cd8a3 100644 --- a/src/parser/classes.rs +++ b/src/parser/classes.rs @@ -2723,7 +2723,8 @@ impl Backend { }); } ClassLikeMember::Property(property) => { - let mut prop_infos = extract_property_info(property); + let mut prop_infos = + extract_property_info(property, doc_ctx.map(|c| c.content)); // Extract the attribute lists from the property variant // so we can check for #[Deprecated] below. diff --git a/src/parser/mod.rs b/src/parser/mod.rs index c8e509352..aceea063c 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -1169,41 +1169,62 @@ pub(crate) fn extract_visibility<'a>( } /// Extract property information from a class member Property node. -pub(crate) fn extract_property_info(property: &Property) -> Vec { +pub(crate) fn extract_property_info( + property: &Property, + content: Option<&str>, +) -> Vec { + use mago_syntax::cst::class_like::property::PropertyItem; + let is_static = property.modifiers().iter().any(|m| m.is_static()); let visibility = extract_visibility(property.modifiers().iter()); let native_hint = property.hint().map(|h| extract_hint_type(h)); - property - .variables() - .iter() - .map(|var| { - let raw_name = bytes_to_str(var.name).to_string(); - // Strip the leading `$` for property names since PHP access - // syntax is `$this->name` not `$this->$name`. - let name = if let Some(stripped) = raw_name.strip_prefix('$') { - atom(stripped) - } else { - atom(&raw_name) - }; + let mut props = Vec::with_capacity(property.variables().len()); + let mut push_item = |item: &PropertyItem| { + let default_value = match item { + PropertyItem::Concrete(c) => content.and_then(|src| { + let start = c.value.span().start.offset as usize; + let end = c.value.span().end.offset as usize; + src.get(start..end).map(Box::from) + }), + PropertyItem::Abstract(_) => None, + }; - PropertyInfo { - name, - name_offset: var.span.start.offset, - type_hint: native_hint.clone(), - native_type_hint: native_hint.clone(), - description: None, - is_static, - visibility, - deprecation_message: None, - deprecated_replacement: None, - see_refs: Vec::new(), - is_virtual: false, - source: None, + let var = item.variable(); + let raw_name = bytes_to_str(var.name).to_string(); + let name = if let Some(stripped) = raw_name.strip_prefix('$') { + atom(stripped) + } else { + atom(&raw_name) + }; + + props.push(PropertyInfo { + name, + name_offset: var.span.start.offset, + type_hint: native_hint.clone(), + native_type_hint: native_hint.clone(), + description: None, + is_static, + visibility, + deprecation_message: None, + deprecated_replacement: None, + see_refs: Vec::new(), + is_virtual: false, + source: default_value.map(|value| PropertySource::DeclaredDefault { value }), + }); + }; + + match property { + Property::Plain(plain) => { + for item in &plain.items { + push_item(item); } - }) - .collect() + } + Property::Hooked(hooked) => push_item(&hooked.item), + } + + props } use crate::Backend; diff --git a/src/types/mod.rs b/src/types/mod.rs index 488972e46..c71737aa3 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -742,6 +742,9 @@ pub struct AttributeDefaultSource { #[derive(Debug, Clone, PartialEq, Eq)] pub enum PropertySource { + DeclaredDefault { + value: Box, + }, DatabaseColumn { column: DatabaseColumnSource, attribute_default: Option, diff --git a/tests/integration/completion_basic.rs b/tests/integration/completion_basic.rs index 382362109..c910478e9 100644 --- a/tests/integration/completion_basic.rs +++ b/tests/integration/completion_basic.rs @@ -1,4 +1,5 @@ use crate::common::create_test_backend; +use phpantom_lsp::types::PhpVersion; use tower_lsp::LanguageServer; use tower_lsp::lsp_types::*; @@ -1079,6 +1080,676 @@ async fn test_completion_after_function_keyword_does_not_suggest_classes() { ); } +#[tokio::test] +async fn test_completion_suggests_parent_method_overrides() { + let backend = create_test_backend(); + + let uri = Url::parse("file:///override_methods.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + + let filter_names: Vec<&str> = items + .iter() + .filter_map(|i| i.filter_text.as_deref()) + .collect(); + + assert!( + filter_names.contains(&"getContent"), + "should suggest public parent method getContent, got: {:?}", + items.iter().map(|i| i.label.clone()).collect::>() + ); + assert!( + filter_names.contains(&"getTitle"), + "should suggest protected parent method getTitle, got: {:?}", + items.iter().map(|i| i.label.clone()).collect::>() + ); + assert!( + filter_names.contains(&"getFormattedDate"), + "should suggest getFormattedDate with params, got: {:?}", + items.iter().map(|i| i.label.clone()).collect::>() + ); + assert!( + !filter_names.contains(&"secret"), + "must not suggest private parent method, got: {:?}", + filter_names + ); + assert!( + !items + .iter() + .any(|i| i.kind == Some(CompletionItemKind::CLASS)), + "must not suggest classes alongside overrides, got: {:?}", + items.iter().map(|i| i.label.clone()).collect::>() + ); + + let dated = items + .iter() + .find(|i| i.filter_text.as_deref() == Some("getFormattedDate")) + .expect("getFormattedDate item"); + let insert = dated + .insert_text + .as_deref() + .or_else(|| { + dated.text_edit.as_ref().map(|te| match te { + CompletionTextEdit::Edit(e) => e.new_text.as_str(), + CompletionTextEdit::InsertAndReplace(e) => e.new_text.as_str(), + }) + }) + .unwrap_or(""); + assert!( + insert.contains("getFormattedDate(") + && (insert.contains("$format") || insert.contains("\\$format")), + "insert should include full signature with $param names, got: {insert}" + ); + // Snippet insert must escape `$` so clients don't treat `$format` as a tabstop. + assert!( + insert.contains("\\$format"), + "param $ must be snippet-escaped as \\$, got: {insert}" + ); + assert!( + !insert.contains("(format") && !insert.contains(", format"), + "param names must not omit $, got: {insert}" + ); + assert!( + !insert.contains("function getFormattedDate"), + "snippet must not re-insert the function keyword, got: {insert}" + ); + // Brace lines must not carry member indent — clients re-indent them. + assert!( + insert.contains("\n{\n") && insert.contains("\n}\n") || insert.ends_with("\n}"), + "braces should start at column 0 of the snippet line, got: {insert:?}" + ); + assert!( + !insert.contains("\n {") && !insert.contains("\n {"), + "braces must not be pre-indented (avoids double indent), got: {insert:?}" + ); +} + +#[tokio::test] +async fn test_completion_override_skips_already_implemented() { + let backend = create_test_backend(); + + let uri = Url::parse("file:///override_skip.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + let filter_names: Vec<&str> = items + .iter() + .filter_map(|i| i.filter_text.as_deref()) + .collect(); + + assert!( + filter_names.contains(&"getTitle"), + "should still suggest unimplemented getTitle, got: {filter_names:?}" + ); + assert!( + !filter_names.contains(&"getContent"), + "should not re-suggest already implemented getContent, got: {filter_names:?}" + ); +} + +#[tokio::test] +async fn test_completion_override_includes_override_attribute_on_php83() { + // Default test backend uses PhpVersion::default() = 8.5, so #[Override] applies. + let backend = create_test_backend(); + + let uri = Url::parse("file:///override_attr.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + let item = items + .iter() + .find(|i| i.filter_text.as_deref() == Some("getContent")) + .expect("getContent override"); + let additional = item + .additional_text_edits + .as_ref() + .expect("additional edits"); + assert!( + additional + .iter() + .any(|e| e.new_text.contains("#[\\Override]") || e.new_text.contains("#[Override]")), + "PHP 8.3+ should insert #[Override], got: {additional:?}" + ); +} + +#[tokio::test] +async fn test_completion_suggests_parent_property_overrides() { + let backend = create_test_backend(); + + let uri = Url::parse("file:///override_props.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + let names: Vec<&str> = items + .iter() + .filter_map(|i| i.filter_text.as_deref()) + .collect(); + + assert!( + names.contains(&"title"), + "should suggest parent property title, got: {names:?}" + ); + assert!( + names.contains(&"count"), + "should suggest protected parent property count, got: {names:?}" + ); + assert!( + !names.contains(&"secret"), + "must not suggest private parent property, got: {names:?}" + ); + + let title = items + .iter() + .find(|i| i.filter_text.as_deref() == Some("title")) + .expect("title item"); + let insert = title.insert_text.as_deref().unwrap_or(""); + assert!( + insert.contains("title = ''") || insert.contains("title = \"\""), + "property override should include default value, got: {insert:?}" + ); + assert!( + title + .additional_text_edits + .as_ref() + .is_some_and(|edits| edits.iter().any(|e| e.new_text.contains("#[\\Override]"))), + "PHP 8.5+ should insert #[Override] for property overrides" + ); +} + +#[tokio::test] +async fn test_completion_property_override_skips_override_attribute_before_php85() { + let backend = create_test_backend(); + backend.set_php_version(PhpVersion::new(8, 4)); + + let uri = Url::parse("file:///override_props_php84.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + let title = items + .iter() + .find(|i| i.filter_text.as_deref() == Some("title")) + .expect("title item"); + assert!( + title.additional_text_edits.is_none(), + "PHP 8.4 should not insert #[Override] for property overrides" + ); +} + +#[tokio::test] +async fn test_completion_suggests_parent_property_overrides_after_type() { + let backend = create_test_backend(); + + let uri = Url::parse("file:///override_typed_props.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + let attributes = items + .iter() + .find(|i| i.filter_text.as_deref() == Some("attributes")) + .expect("attributes item"); + let insert = attributes.insert_text.as_deref().unwrap_or(""); + assert!( + insert.contains("attributes = []"), + "typed property override should include default value, got: {insert:?}" + ); +} + +#[tokio::test] +async fn test_completion_suggests_parent_constant_overrides() { + let backend = create_test_backend(); + + let uri = Url::parse("file:///override_consts.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + let names: Vec<&str> = items + .iter() + .filter_map(|i| i.filter_text.as_deref()) + .collect(); + + assert!( + names.contains(&"STATUS_OK"), + "should suggest STATUS_OK, got: {names:?}" + ); + assert!( + names.contains(&"STATUS_PENDING"), + "should suggest STATUS_PENDING, got: {names:?}" + ); + assert!( + !names.contains(&"SECRET"), + "must not suggest private constant, got: {names:?}" + ); + + let ok = items + .iter() + .find(|i| i.filter_text.as_deref() == Some("STATUS_OK")) + .expect("STATUS_OK item"); + let insert = ok.insert_text.as_deref().unwrap_or(""); + assert!( + insert.contains("STATUS_OK = 1"), + "constant override should include value, got: {insert:?}" + ); + assert!( + ok.additional_text_edits.is_none(), + "PHP 8.5 should not insert #[Override] for constant overrides" + ); +} + +#[tokio::test] +async fn test_completion_constant_override_includes_override_attribute_on_php86() { + let backend = create_test_backend(); + backend.set_php_version(PhpVersion::new(8, 6)); + + let uri = Url::parse("file:///override_consts_php86.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + let ok = items + .iter() + .find(|i| i.filter_text.as_deref() == Some("STATUS_OK")) + .expect("STATUS_OK item"); + assert!( + ok.additional_text_edits + .as_ref() + .is_some_and(|edits| edits.iter().any(|e| e.new_text.contains("#[\\Override]"))), + "PHP 8.6+ should insert #[Override] for constant overrides" + ); +} + +#[tokio::test] +async fn test_completion_suggests_parent_constant_overrides_after_type() { + let backend = create_test_backend(); + + let uri = Url::parse("file:///override_typed_consts.php").unwrap(); + let text = concat!( + " items, + Some(CompletionResponse::List(list)) => list.items, + None => Vec::new(), + }; + let ok = items + .iter() + .find(|i| i.filter_text.as_deref() == Some("STATUS_OK")) + .expect("STATUS_OK item"); + let insert = ok.insert_text.as_deref().unwrap_or(""); + assert!( + insert.contains("STATUS_OK = 'ok'"), + "typed constant override should include value, got: {insert:?}" + ); +} + #[tokio::test] async fn test_completion_suggests_backed_enum_types_after_enum_colon() { let backend = create_test_backend();