Skip to content

Commit

Permalink
Improve completion for namespaced classes
Browse files Browse the repository at this point in the history 
cc @joehoyle - this mainly allows us to get a correct list when the user starts typing Foo (without the new before it) inside a namespace
  • Loading branch information
@muglug
muglug committed Feb 15, 2021
1 parent 6b53e79 commit bd6efd7
Show file tree
Hide file tree
Showing 8 changed files with 203 additions and 36 deletions.
7 changes: 7 additions & 0 deletions src/Psalm/Codebase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,12 @@ public function getCompletionItemsForPartialSymbol(
int $offset,
string $file_path
) : array {
$fq_suggestion = false;

if (($type_string[1] ?? '') === '\\') {

This comment has been minimized.

Sign in to view

Copy link
@joehoyle

joehoyle Feb 15, 2021

Contributor

I guess this is 1 due to partial symbols always be preceded by a *?

This comment has been minimized.

Sign in to view

Copy link
@muglug

muglug Feb 15, 2021

Author Collaborator

yup
$fq_suggestion = true;
}

$matching_classlike_names = $this->classlikes->getMatchingClassLikeNames($type_string);

$completion_items = [];
Expand Down Expand Up @@ -1568,6 +1574,7 @@ public function getCompletionItemsForPartialSymbol(
);

if ($aliases
&& !$fq_suggestion
&& $aliases->namespace
&& $insertion_text === '\\' . $fq_class_name
&& $aliases->namespace_first_stmt_start
Expand Down
12 changes: 10 additions & 2 deletions src/Psalm/Internal/Analyzer/ClassAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2007,7 +2007,11 @@ private function checkImplementedInterfaces(
$interface_name,
$codebase->classlikes->interfaceExists($fq_interface_name)
? $fq_interface_name
: '*' . implode('\\', $interface_name->parts)
: '*'
. ($interface_name instanceof PhpParser\Node\Name\FullyQualified
? '\\'
: $this->getNamespace() . '-')
. implode('\\', $interface_name->parts)
);

$interface_location = new CodeLocation($this, $interface_name);
Expand Down Expand Up @@ -2392,7 +2396,11 @@ private function checkParentClass(
$extended_class,
$codebase->classlikes->classExists($parent_fq_class_name)
? $parent_fq_class_name
: '*' . implode('\\', $extended_class->parts)
: '*'
. ($extended_class instanceof PhpParser\Node\Name\FullyQualified
? '\\'
: $this->getNamespace() . '-')
. implode('\\', $extended_class->parts)
);
}

Expand Down
6 changes: 5 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ public static function analyze(
$stmt->class,
$codebase->classlikes->classExists($fq_class_name)
? $fq_class_name
: '*' . implode('\\', $stmt->class->parts)
: '*'
. ($stmt->class instanceof PhpParser\Node\Name\FullyQualified
? '\\'
: $statements_analyzer->getNamespace() . '-')
. implode('\\', $stmt->class->parts)
);
}
} elseif ($stmt->class instanceof PhpParser\Node\Stmt\Class_) {
Expand Down
6 changes: 5 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/ConstFetchAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ public static function analyze(
$stmt,
$const_type
? $fq_const_name
: '*' . $fq_const_name
: '*'
. ($stmt->name instanceof PhpParser\Node\Name\FullyQualified
? '\\'
: $statements_analyzer->getNamespace() . '-')
. $const_name
);

if ($const_type) {
Expand Down
6 changes: 5 additions & 1 deletion src/Psalm/Internal/Analyzer/Statements/Expression/InstanceofAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ public static function analyze(
$stmt->class,
$codebase->classlikes->classOrInterfaceExists($fq_class_name)
? $fq_class_name
: '*' . implode('\\', $stmt->class->parts)
: '*'
. ($stmt->class instanceof PhpParser\Node\Name\FullyQualified
? '\\'
: $statements_analyzer->getNamespace() . '-')
. implode('\\', $stmt->class->parts)
);
}

Expand Down
20 changes: 18 additions & 2 deletions src/Psalm/Internal/Codebase/ClassLikes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,14 +239,30 @@ public function getMatchingClassLikeNames(string $stub) : array
$stub = substr($stub, 1);
}

$fully_qualified = false;

if ($stub[0] === '\\') {
$fully_qualified = true;
$stub = substr($stub, 1);
} else {
// for any not-fully-qualified class name the bit we care about comes after a dash
[, $stub] = explode('-', $stub);
}

$stub = preg_quote(strtolower($stub));

if ($fully_qualified) {
$stub = '^' . $stub;
} else {
$stub = '(^|\\\)' . $stub;
}

foreach ($this->existing_classes as $fq_classlike_name => $found) {
if (!$found) {
continue;
}

if (preg_match('@(^|\\\)' . $stub . '.*@i', $fq_classlike_name)) {
if (preg_match('@' . $stub . '.*@i', $fq_classlike_name)) {
$matching_classes[] = $fq_classlike_name;
}
}
Expand All @@ -256,7 +272,7 @@ public function getMatchingClassLikeNames(string $stub) : array
continue;
}

if (preg_match('@(^|\\\)' . $stub . '.*@i', $fq_classlike_name)) {
if (preg_match('@' . $stub . '.*@i', $fq_classlike_name)) {
$matching_classes[] = $fq_classlike_name;
}
}
Expand Down
35 changes: 21 additions & 14 deletions src/Psalm/Internal/Codebase/Functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,21 @@ public function getMatchingFunctionNames(
$stub = substr($stub, 1);
}

$fully_qualified = false;

if ($stub[0] === '\\') {
$fully_qualified = true;
$stub = substr($stub, 1);
$stub_namespace = '';
} else {
// functions can reference either the current namespace or root-namespaced
// equivalents. We therefore want to make both candidates.
[$stub_namespace, $stub] = explode('-', $stub);
}

/** @var array<lowercase-string, FunctionStorage> */
$matching_functions = [];

$stub_lc = strtolower($stub);
$file_storage = $this->file_storage_provider->get($file_path);

$current_namespace_aliases = null;
Expand All @@ -290,18 +301,11 @@ public function getMatchingFunctionNames(
$stub . '*',
];

if ($current_namespace_aliases) {
// As the stub still include the current namespace in the symbol,
// remove the current namespace to provide the function-only token
// and match against global functions and all imported namespaces.
// "Bar/foo" will become "foo".
if ($current_namespace_aliases->namespace
&& strpos($stub_lc, strtolower($current_namespace_aliases->namespace) . '\\') === 0
) {
$stub = substr($stub, strlen($current_namespace_aliases->namespace) + 1);
$match_function_patterns[] = $stub . '*';
}
if ($stub_namespace) {
$match_function_patterns[] = $stub_namespace . '\\' . $stub . '*';
}

if ($current_namespace_aliases) {
foreach ($current_namespace_aliases->functions as $alias_name => $function_name) {
if (strpos($alias_name, $stub) === 0) {
try {
Expand All @@ -310,8 +314,11 @@ public function getMatchingFunctionNames(
}
}
}
foreach ($current_namespace_aliases->uses as $namespace_name) {
$match_function_patterns[] = $namespace_name . '\\' . $stub . '*';

if (!$fully_qualified) {
foreach ($current_namespace_aliases->uses as $namespace_name) {
$match_function_patterns[] = $namespace_name . '\\' . $stub . '*';
}
}
}

Expand Down
Loading

2 comments on commit bd6efd7

@joehoyle
Copy link
Contributor

@joehoyle joehoyle commented on bd6efd7 Feb 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

👍, it seems more and more information is getting packed into the reference map symbols, like the *, perhaps there's a canonical place that the - can be documented as to exactly what is signifies. Or-- perhaps the file maps need to have a bit more structured data than they do right now?

@muglug
Copy link
Collaborator Author

@muglug muglug commented on bd6efd7 Feb 15, 2021

Choose a reason for hiding this comment

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

Yeah, I'd be happy to accept a PR!

Please sign in to comment.