Skip to content

Commit be5d7eb

Browse files
RemcoSmitsDevAJenbo
authored andcommitted
Support hover information for parameter variable
definitions. This is mainly usefull when you are working in a project with large docblocks that define some types that are hard to discover, so making the hover work for them allow you to easier discover the actual type of the variable.
1 parent aa9c6b1 commit be5d7eb

6 files changed

Lines changed: 101 additions & 26 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4444
- **Add iterable return type code action.** When PHPStan reports `missingType.iterableValue` for a return type (e.g. "return type has no value type specified in iterable type array"), a quickfix adds a `@return` docblock tag with the element type inferred from the function body. For example, a function returning `$array = ['hello']` produces `@return array<string>` rather than `array<mixed>`. Falls back to `<mixed>` only when the element type cannot be determined. Existing docblocks are updated in place; single-line docblocks are expanded to multi-line. The diagnostic is eagerly cleared once the `@return` tag contains a generic type.
4545
- **Remove unreachable statement code action.** When PHPStan reports `deadCode.unreachable`, a quickfix deletes the dead statement. The statement-removal helper is shared infrastructure that a future native dead-code diagnostic (D6) can reuse.
4646
- **Eloquent `$appends` array.** Entries in a model's `$appends` property now produce virtual properties, matching the existing treatment of `$fillable`, `$guarded`, `$hidden`, and `$visible`.
47+
- **Hover on parameter variables at their definition site.** Hovering on a function or method parameter now shows its resolved type instead of being suppressed. When a `@param` tag provides a richer type than the native hint (e.g. `list<Pen>` vs `array`), the docblock type is shown. Contributed by @RemcoSmitsDev in https://github.com/AJenbo/phpantom_lsp/pull/68.
4748

4849
### Changed
4950

src/completion/resolver.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,8 @@ pub(crate) fn resolve_target_classes(
524524
// vec would poison the cache: a later top-level lookup (at
525525
// depth 0) would hit the cached empty entry and produce a
526526
// false "type could not be resolved" diagnostic.
527-
let skip_cache = result.is_empty()
528-
&& super::variable::resolution::is_var_resolution_depth_limited();
527+
let skip_cache =
528+
result.is_empty() && super::variable::resolution::is_var_resolution_depth_limited();
529529
if !skip_cache {
530530
DIAG_SUBJECT_CACHE.with(|cell| {
531531
let mut borrow = cell.borrow_mut();

src/completion/variable/rhs_resolution.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -479,15 +479,20 @@ fn resolve_rhs_instantiation(
479479
// is not yet known. Now that generic args are concrete,
480480
// resolve those mixins and merge their members.
481481
if cls.mixins.iter().any(|m| cls.template_params.contains(m)) {
482-
let generic_subs = crate::inheritance::build_generic_subs(cls, &type_args);
482+
let generic_subs =
483+
crate::inheritance::build_generic_subs(cls, &type_args);
483484
if !generic_subs.is_empty() {
484-
let mixin_members = crate::virtual_members::phpdoc::resolve_template_param_mixins(
485-
cls,
486-
&generic_subs,
487-
ctx.class_loader,
488-
);
485+
let mixin_members =
486+
crate::virtual_members::phpdoc::resolve_template_param_mixins(
487+
cls,
488+
&generic_subs,
489+
ctx.class_loader,
490+
);
489491
if !mixin_members.is_empty() {
490-
crate::virtual_members::merge_virtual_members(&mut substituted, mixin_members);
492+
crate::virtual_members::merge_virtual_members(
493+
&mut substituted,
494+
mixin_members,
495+
);
491496
}
492497
}
493498
}

src/hover/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -502,14 +502,15 @@ impl Backend {
502502
SymbolKind::Variable { name } => {
503503
// Suppress hover when the cursor is on a variable at its
504504
// definition site where the type is already visible in
505-
// the signature (parameters, properties, static/global
506-
// declarations). For assignments, foreach bindings, and
507-
// catch bindings the resolved type is not obvious from the
508-
// source text, so hover is useful there.
505+
// the signature (properties, static/global declarations).
506+
// For parameters, assignments, foreach bindings, and catch
507+
// bindings, hover is useful to show the resolved type and
508+
// any docblock descriptions.
509509
if let Some(def_kind) = self.lookup_var_def_kind_at(uri, name, cursor_offset)
510510
&& !matches!(
511511
def_kind,
512512
VarDefKind::Assignment
513+
| VarDefKind::Parameter
513514
| VarDefKind::Foreach
514515
| VarDefKind::Catch
515516
| VarDefKind::ArrayDestructuring

src/inheritance.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,10 @@ pub(crate) fn apply_substitution<'a>(
10071007
///
10081008
/// Returns an empty map when no substitutions can be made (e.g. when
10091009
/// `template_params` or `type_args` is empty).
1010-
pub(crate) fn build_generic_subs(class: &ClassInfo, type_args: &[&str]) -> HashMap<String, PhpType> {
1010+
pub(crate) fn build_generic_subs(
1011+
class: &ClassInfo,
1012+
type_args: &[&str],
1013+
) -> HashMap<String, PhpType> {
10111014
if class.template_params.is_empty() || type_args.is_empty() {
10121015
return HashMap::new();
10131016
}

tests/integration/hover.rs

Lines changed: 77 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -398,29 +398,94 @@ class Consumer {
398398
}
399399

400400
#[test]
401-
fn hover_suppressed_on_parameter_definition_site() {
401+
fn hover_active_on_parameter_definition_site() {
402402
let backend = create_test_backend();
403403
let uri = "file:///test.php";
404404
let content = r#"<?php
405-
class Builder {
406-
public function scopeOfGenre(\Illuminate\Database\Eloquent\Builder $query, string $genre): void {
407-
$query->where('genre', $genre);
405+
class Order { public string $id; }
406+
class Service {
407+
public function process(Order $query, string $genre): void {
408+
$query;
408409
}
409410
}
410411
"#;
411412

412-
// Hover on `$query` at the parameter definition site (line 2, col ~72)
413-
let hover = hover_at(&backend, uri, content, 2, 73);
413+
// Hover on `$query` at the parameter definition site (line 3, col on $query)
414+
let hover = hover_at(&backend, uri, content, 3, 35)
415+
.expect("hover should be active on parameter $query");
416+
let text = hover_text(&hover);
414417
assert!(
415-
hover.is_none(),
416-
"hover should be suppressed on parameter $query"
418+
text.contains("$query"),
419+
"hover should show the parameter name: {}",
420+
text
421+
);
422+
assert!(
423+
text.contains("Order"),
424+
"hover should show the resolved type Order: {}",
425+
text
417426
);
418427

419-
// Hover on `$genre` at the parameter definition site (line 2, col ~87)
420-
let hover = hover_at(&backend, uri, content, 2, 88);
428+
// Hover on `$genre` at the parameter definition site (line 3, col on $genre)
429+
let hover = hover_at(&backend, uri, content, 3, 50)
430+
.expect("hover should be active on parameter $genre");
431+
let text = hover_text(&hover);
421432
assert!(
422-
hover.is_none(),
423-
"hover should be suppressed on parameter $genre"
433+
text.contains("$genre") && text.contains("string"),
434+
"hover should show the parameter name and type: {}",
435+
text
436+
);
437+
}
438+
439+
#[test]
440+
fn hover_parameter_definition_shows_docblock_type() {
441+
let backend = create_test_backend();
442+
let uri = "file:///test.php";
443+
let content = r#"<?php
444+
class Pen { public function write(): string { return ''; } }
445+
class Drawer {
446+
/** @param list<Pen> $pens The pens to use. */
447+
public function fill(array $pens): void {
448+
$pens;
449+
}
450+
}
451+
"#;
452+
453+
// Hover on `$pens` at the parameter definition site (line 4)
454+
let hover = hover_at(&backend, uri, content, 4, 33)
455+
.expect("hover should be active on parameter $pens with docblock type");
456+
let text = hover_text(&hover);
457+
assert!(
458+
text.contains("$pens"),
459+
"hover should show the parameter name: {}",
460+
text
461+
);
462+
}
463+
464+
#[test]
465+
fn hover_parameter_definition_standalone_function() {
466+
let backend = create_test_backend();
467+
let uri = "file:///test.php";
468+
let content = r#"<?php
469+
class Pen { public function write(): string { return ''; } }
470+
/** @param Pen $tool The writing instrument. */
471+
function draw(Pen $tool): void {
472+
$tool;
473+
}
474+
"#;
475+
476+
// Hover on `$tool` at the parameter definition site (line 3)
477+
let hover = hover_at(&backend, uri, content, 3, 19)
478+
.expect("hover should be active on standalone function parameter $tool");
479+
let text = hover_text(&hover);
480+
assert!(
481+
text.contains("$tool"),
482+
"hover should show the parameter name: {}",
483+
text
484+
);
485+
assert!(
486+
text.contains("Pen"),
487+
"hover should show the type Pen: {}",
488+
text
424489
);
425490
}
426491

0 commit comments

Comments
 (0)