Skip to content

fix(completion): magic method names displayed inside argument list #2815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dantleech merged 1 commit into from
Jan 4, 2025
Merged

fix(completion): magic method names displayed inside argument list #2815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions lib/Completion/Bridge/TolerantParser/CompletionContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,16 @@ public static function classMembersBody(?Node $node): bool
return false;
}

$nodeBeforeOffset = NodeUtil::firstDescendantNodeBeforeOffset($node->getRoot(), $node->parent->getStartPosition());

if ($node instanceof Variable) {
return false;
}

if ($node->parent instanceof MethodDeclaration && $node->openBrace instanceof MissingToken) {
Copy link
Collaborator

@dantleech dantleech Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this causes lots of warnings:

[ERROR][2025-01-27 09:45:46] .../vim/lsp/rpc.lua:770	"rpc"	"/home/daniel/.local/bin/phpactor"	"stderr"	"Warning: Undefined property: Microsoft\\PhpParser\\Node\\DelimitedList\\ParameterDeclarationList::$openBrace in /home/daniel/www/phpactor/phpactor/lib/Completion/Bridge/TolerantParser/CompletionContext.php on line 184\n"
[ERROR][2025-01-27 09:45:56] .../vim/lsp/rpc.lua:770	"rpc"	"/home/daniel/.local/bin/phpactor"	"stderr"	"Warning: Undefined property: Microsoft\\PhpParser\\Node\\DelimitedList\\ParameterDeclarationList::$openBrace in /home/daniel/www/phpactor/phpactor/lib/Completion/Bridge/TolerantParser/CompletionContext.php on line 184\n"
[ERROR][2025-01-27 09:45:56] .../vim/lsp/rpc.lua:770	"rpc"	"/home/daniel/.local/bin/phpactor"	"stderr"	"Warning: Undefined property: Microsoft\\PhpParser\\Node\\DelimitedList\\ParameterDeclarationList::$openBrace in /home/daniel/www/phpactor/phpactor/lib/Completion/Bridge/TolerantParser/CompletionContext.php on line 184\n"

Copy link
Contributor Author

@przepompownia przepompownia Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2832 should precise the type of $node in that case.

return false;
}

$nodeBeforeOffset = NodeUtil::firstDescendantNodeBeforeOffset($node->getRoot(), $node->parent->getStartPosition());

if ($nodeBeforeOffset instanceof ClassMembersNode) {
return true;
}
Expand Down Expand Up @@ -273,7 +277,16 @@ public static function anonymousUse(Node $node): bool

public static function methodName(Node $node): bool
{
return $node->parent instanceof MethodDeclaration;
// If the body (as the current node) is empty, the parent is MethodDeclaration
if ($node instanceof CompoundStatementNode && !$node->openBrace instanceof MissingToken) {
return false;
}

if (!$node->parent instanceof MethodDeclaration) {
return false;
}

return $node->parent->openParen instanceof MissingToken;
}

public static function declaration(Node $node, ByteOffset $offset): bool
Expand Down
4 changes: 4 additions & 0 deletions ...mpletion/Tests/Integration/Bridge/TolerantParser/WorseReflection/KeywordCompletorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ public function provideComplete(): Generator
[...$this->expectMagicMethods()],
];

yield 'no magic methods here' => [
'<?php class Foobar { public function x(<>)',
[],
];

yield 'class implements 1' => [
'<?php class Foobar <>',
Expand Down
25 changes: 25 additions & 0 deletions lib/Completion/Tests/Unit/Bridge/TolerantParser/CompletionContextTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,4 +301,29 @@ public function providePromotedProperty(): Generator
false,
];
}

/**
* @dataProvider provideMethodName
*/
public function testMethodName(string $source, bool $expected): void
{
[$source, $offset] = ExtractOffset::fromSource($source);
$node = (new Parser())->parseSourceFile($source)->getDescendantNodeAtPosition((int)$offset);
self::assertEquals($expected, CompletionContext::methodName($node));
}

/**
* @return Generator<array<int,mixed>>
*/
public function provideMethodName(): Generator
{
yield [
'<?php class A { public function __construct(<>',
false,
];
yield [
'<?php class A { public function __con<>',
true,
];
}
}
55 changes: 5 additions & 50 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,11 +525,6 @@ parameters:
count: 1
path: lib/CodeBuilder/Util/TextUtil.php

-
message: "#^Offset 0 does not exist on array\\{0\\?\\: string, 1\\?\\: non\\-empty\\-string\\}\\.$#"
count: 1
path: lib/CodeTransform/Adapter/TolerantParser/Refactor/TolerantExtractExpression.php

-
message: "#^Cannot use array destructuring on array\\|null\\.$#"
count: 1
Expand Down Expand Up @@ -680,6 +675,11 @@ parameters:
count: 1
path: lib/Completion/Bridge/TolerantParser/ChainTolerantCompletor.php

-
message: "#^Access to an undefined property Microsoft\\\\PhpParser\\\\Node\\:\\:\\$openBrace\\.$#"
count: 1
path: lib/Completion/Bridge/TolerantParser/CompletionContext.php

-
message: "#^Cannot call method getEndPosition\\(\\) on Microsoft\\\\PhpParser\\\\Node\\|null\\.$#"
count: 1
Expand Down Expand Up @@ -3590,11 +3590,6 @@ parameters:
count: 2
path: lib/Extension/LanguageServerCompletion/Tests/Integration/MarkdownObjectRendererTest.php

-
message: "#^Call to an undefined method Phpactor\\\\WorseReflection\\\\Core\\\\Reflection\\\\ReflectionClassLike\\:\\:constants\\(\\)\\.$#"
count: 2
path: lib/Extension/LanguageServerCompletion/Tests/Integration/MarkdownObjectRendererTest.php

-
message: "#^Call to an undefined method Phpactor\\\\WorseReflection\\\\Core\\\\Reflection\\\\ReflectionClassLike\\:\\:properties\\(\\)\\.$#"
count: 5
Expand Down Expand Up @@ -4270,31 +4265,6 @@ parameters:
count: 1
path: lib/Extension/Logger/Tests/Unit/LoggingExtensionTest.php

-
message: "#^Method Phpactor\\\\Extension\\\\Navigation\\\\Application\\\\Navigator\\:\\:__construct\\(\\) has parameter \\$autoCreateConfig with no value type specified in iterable type array\\.$#"
count: 1
path: lib/Extension/Navigation/Application/Navigator.php

-
message: "#^Method Phpactor\\\\Extension\\\\Navigation\\\\Application\\\\Navigator\\:\\:canCreateNew\\(\\) has no return type specified\\.$#"
count: 1
path: lib/Extension/Navigation/Application/Navigator.php

-
message: "#^Method Phpactor\\\\Extension\\\\Navigation\\\\Application\\\\Navigator\\:\\:destination\\(\\) has no return type specified\\.$#"
count: 1
path: lib/Extension/Navigation/Application/Navigator.php

-
message: "#^Method Phpactor\\\\Extension\\\\Navigation\\\\Application\\\\Navigator\\:\\:destinationsFor\\(\\) has no return type specified\\.$#"
count: 1
path: lib/Extension/Navigation/Application/Navigator.php

-
message: "#^Method Phpactor\\\\Extension\\\\Navigation\\\\Application\\\\Navigator\\:\\:variant\\(\\) has no return type specified\\.$#"
count: 1
path: lib/Extension/Navigation/Application/Navigator.php

-
message: "#^Method Phpactor\\\\Extension\\\\Navigation\\\\Handler\\\\NavigateHandler\\:\\:handle\\(\\) has no return type specified\\.$#"
count: 1
Expand All @@ -4305,11 +4275,6 @@ parameters:
count: 1
path: lib/Extension/Navigation/Handler/NavigateHandler.php

-
message: "#^Parameter \\#1 \\$destinations of static method Phpactor\\\\PathFinder\\\\PathFinder\\:\\:fromDestinations\\(\\) expects array\\<string, string\\>, mixed given\\.$#"
count: 1
path: lib/Extension/Navigation/NavigationExtension.php

-
message: "#^Parameter \\#1 \\$navigator of class Phpactor\\\\Extension\\\\Navigation\\\\Application\\\\Navigator constructor expects Phpactor\\\\Extension\\\\Navigation\\\\Navigator\\\\Navigator, mixed given\\.$#"
count: 1
Expand Down Expand Up @@ -4340,11 +4305,6 @@ parameters:
count: 1
path: lib/Extension/Navigation/NavigationExtension.php

-
message: "#^Parameter \\#3 \\$autoCreateConfig of class Phpactor\\\\Extension\\\\Navigation\\\\Application\\\\Navigator constructor expects array, mixed given\\.$#"
count: 1
path: lib/Extension/Navigation/NavigationExtension.php

-
message: "#^Method Phpactor\\\\Extension\\\\Navigation\\\\Navigator\\\\ChainNavigator\\:\\:destinationsFor\\(\\) return type has no value type specified in iterable type array\\.$#"
count: 1
Expand Down Expand Up @@ -5245,11 +5205,6 @@ parameters:
count: 1
path: lib/Extension/WorseReflectionAnalyse/WorseReflectionAnalyseExtension.php

-
message: "#^Call to an undefined method Phpactor\\\\WorseReflection\\\\Core\\\\Reflection\\\\ReflectionClassLike\\:\\:constants\\(\\)\\.$#"
count: 1
path: lib/Extension/WorseReflectionExtra/Application/ClassReflector.php

-
message: "#^Method Phpactor\\\\Extension\\\\WorseReflectionExtra\\\\Application\\\\ClassReflector\\:\\:reflect\\(\\) return type has no value type specified in iterable type array\\.$#"
count: 1
Expand Down
Loading