Skip to content
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.

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

Show custom tip for unfound Drupal symbols #2603

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/Rules/Classes/ClassConstantRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public function processNode(Node $node, Scope $scope): array
return [
RuleErrorBuilder::message(sprintf('Class %s not found.', $className))
->identifier('class.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip($className)
->build(),
];
}
Expand All @@ -106,7 +106,7 @@ public function processNode(Node $node, Scope $scope): array
sprintf('Access to constant %s on an unknown class %s.', $constantName, $className),
)
->identifier('class.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip($className)
->build(),
];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Classes/ExistingClassInClassExtendsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function processNode(Node $node, Scope $scope): array
))
->identifier('class.notFound')
->nonIgnorable()
->discoveringSymbolsTip()
->discoveringSymbolsTip($extendedClassName)
->build();
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Classes/ExistingClassInInstanceOfRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function processNode(Node $node, Scope $scope): array
RuleErrorBuilder::message(sprintf('Class %s not found.', $name))
->identifier('class.notFound')
->line($class->getLine())
->discoveringSymbolsTip()
->discoveringSymbolsTip($name)
->build(),
];
} elseif ($this->checkClassCaseSensitivity) {
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Classes/ExistingClassInTraitUseRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public function processNode(Node $node, Scope $scope): array
$messages[] = RuleErrorBuilder::message(sprintf('%s uses unknown trait %s.', $currentName, $traitName))
->identifier('trait.notFound')
->nonIgnorable()
->discoveringSymbolsTip()
->discoveringSymbolsTip($traitName)
->build();
} else {
$reflection = $this->reflectionProvider->getClass($traitName);
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Classes/ExistingClassesInClassImplementsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function processNode(Node $node, Scope $scope): array
))
->identifier('interface.notFound')
->nonIgnorable()
->discoveringSymbolsTip()
->discoveringSymbolsTip($implementedClassName)
->build();
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Classes/ExistingClassesInEnumImplementsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function processNode(Node $node, Scope $scope): array
))
->identifier('interface.notFound')
->nonIgnorable()
->discoveringSymbolsTip()
->discoveringSymbolsTip($implementedClassName)
->build();
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function processNode(Node $node, Scope $scope): array
))
->identifier('interface.notFound')
->nonIgnorable()
->discoveringSymbolsTip()
->discoveringSymbolsTip($extendedInterfaceName)
->build();
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Classes/InstantiationRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private function checkClassName(string $class, bool $isName, Node $node, Scope $
return [
RuleErrorBuilder::message(sprintf('Instantiated class %s not found.', $class))
->identifier('class.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip($class)
->build(),
];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Classes/MixinRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function processNode(Node $node, Scope $scope): array
if (!$this->reflectionProvider->hasClass($class)) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains unknown class %s.', $class))
->identifier('class.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip($class)
->build();
} elseif ($this->reflectionProvider->getClass($class)->isTrait()) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains invalid type %s.', $class))
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Constants/ConstantRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public function processNode(Node $node, Scope $scope): array
(string) $node->name,
))
->identifier('constant.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip((string) $node->name)
->build(),
];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Exceptions/CaughtExceptionExistenceRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function processNode(Node $node, Scope $scope): array
$errors[] = RuleErrorBuilder::message(sprintf('Caught class %s not found.', $className))
->line($class->getLine())
->identifier('class.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip($className)
->build();
continue;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Functions/CallToNonExistentFunctionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public function processNode(Node $node, Scope $scope): array
}

return [
RuleErrorBuilder::message(sprintf('Function %s not found.', (string) $node->name))->identifier('function.notFound')->discoveringSymbolsTip()->build(),
RuleErrorBuilder::message(sprintf('Function %s not found.', (string) $node->name))->identifier('function.notFound')->discoveringSymbolsTip((string) $node->name)->build(),
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to do (string) $node->name) only once.

Copy link
Author

Choose a reason for hiding this comment

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

If I understand your comment below, you actually don't want the argument at all in these cases, since here the symbols are for functions, not classes. Right?

];
}

Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Methods/StaticMethodCallCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public function check(
'Call to static method %s() on an unknown class %s.',
$methodName,
$className,
))->identifier('class.notFound')->discoveringSymbolsTip()->build(),
))->identifier('class.notFound')->discoveringSymbolsTip($className)->build(),
],
null,
];
Expand Down
4 changes: 2 additions & 2 deletions src/Rules/Namespaces/ExistingNamesInGroupUseRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private function checkConstant(Node\Name $name): ?IdentifierRuleError
{
if (!$this->reflectionProvider->hasConstant($name, null)) {
return RuleErrorBuilder::message(sprintf('Used constant %s not found.', (string) $name))
->discoveringSymbolsTip()
->discoveringSymbolsTip((string) $name)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the passed argument in cases where it's not a class name.

Copy link
Author

Choose a reason for hiding this comment

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

So don't do what you asked me to do in the issue?

Yes, all 22 calls should be updated.

Copy link
Author

Choose a reason for hiding this comment

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

(I did ask about non-classes in a comment on the issue.)

One more clarification. I assume that although we're calling the new argument $className, what we want to test is the name of the thing which was not found, even if that "thing" isn't a class (for example, an interface, contstant, trait, etc.). Right?

->line($name->getLine())
->identifier('constant.notFound')
->build();
Expand All @@ -86,7 +86,7 @@ private function checkFunction(Node\Name $name): ?IdentifierRuleError
{
if (!$this->reflectionProvider->hasFunction($name, null)) {
return RuleErrorBuilder::message(sprintf('Used function %s not found.', (string) $name))
->discoveringSymbolsTip()
->discoveringSymbolsTip((string) $name)
->line($name->getLine())
->identifier('function.notFound')
->build();
Expand Down
4 changes: 2 additions & 2 deletions src/Rules/Namespaces/ExistingNamesInUseRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private function checkConstants(array $uses): array
$errors[] = RuleErrorBuilder::message(sprintf('Used constant %s not found.', (string) $use->name))
->line($use->name->getLine())
->identifier('constant.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip((string) $use->name)
->build();
}

Expand All @@ -91,7 +91,7 @@ private function checkFunctions(array $uses): array
$errors[] = RuleErrorBuilder::message(sprintf('Used function %s not found.', (string) $use->name))
->line($use->name->getLine())
->identifier('function.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip((string) $use->name)
->build();
} elseif ($this->checkFunctionNameCase) {
$functionReflection = $this->reflectionProvider->getFunction($use->name, null);
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/PhpDoc/InvalidPhpDocVarTagTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public function processNode(Node $node, Scope $scope): array
$referencedClass,
))
->identifier('class.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip($referencedClass)
->build();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Properties/AccessStaticPropertiesRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private function processSingleProperty(Scope $scope, StaticPropertyFetch $node,
$name,
$class,
))
->discoveringSymbolsTip()
->discoveringSymbolsTip($class)
->identifier('class.notFound')
->build(),
];
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Properties/ExistingClassesInPropertiesRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public function processNode(Node $node, Scope $scope): array
$propertyReflection->getDeclaringClass()->getDisplayName(),
$node->getName(),
$referencedClass,
))->identifier('class.notFound')->discoveringSymbolsTip()->build();
))->identifier('class.notFound')->discoveringSymbolsTip($referencedClass)->build();
}

if ($this->checkClassCaseSensitivity) {
Expand Down
6 changes: 5 additions & 1 deletion src/Rules/RuleErrorBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use function implode;
use function is_file;
use function sprintf;
use function substr;

/**
* @api
Expand Down Expand Up @@ -182,8 +183,11 @@ public function addTip(string $tip): self
* @phpstan-this-out self<T&TipRuleError>
* @return self<T&TipRuleError>
*/
public function discoveringSymbolsTip(): self
public function discoveringSymbolsTip(?string $className = null): self
{
if (isset($className) && substr($className, 0, 7) === 'Drupal/') {
Copy link
Member

Choose a reason for hiding this comment

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

A test would be nice, I'm pretty sure this code is wrong (It's Drupal\, not Drupal/) :) And please use str_starts_with.

The test should be written for one of the impacted rules, it doesn't matter which one. An error tip is tested with a third array item (https://phpstan.org/developing-extensions/rules#testing-the-rule) - besides the message and the line.

Copy link
Author

Choose a reason for hiding this comment

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

You're right about the backslash. My mistake.

And please use str_starts_with.

Hmm. I was under the impression that PHPStan supports PHP >= 7.2. The str_starts_with() function was only introduced in PHP 8.

I'll see what I can come up with for a test.

return $this->tip('Learn more at https://github.com/mglaman/phpstan-drupal');
}
return $this->tip('Learn more at https://phpstan.org/user-guide/discovering-symbols');
}

Expand Down
2 changes: 1 addition & 1 deletion src/Rules/RuleLevelHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ public function findTypeToCheck(
$errors[] = RuleErrorBuilder::message(sprintf($unknownClassErrorPattern, $referencedClass))
->line($var->getLine())
->identifier('class.notFound')
->discoveringSymbolsTip()
->discoveringSymbolsTip($referencedClass)
->build();
}

Expand Down
Loading