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

Abstract methods declared by traits and implemented by parent classes shouldn't raise errors #3406

Closed
Slamdunk opened this issue Jun 8, 2020 · 14 comments

Comments

@Slamdunk
Copy link
Contributor

Slamdunk commented Jun 8, 2020

Bug report

Abstract methods declared by traits and implemented by parent classes shouldn't raise errors, at least not on Level 0.

Code snippet that reproduces the problem

abstract class AbstractFoo {
	public function myFoo(): void {}
}
trait TraitFoo {
	abstract public function myFoo(): void;
}
final class ClassFoo extends AbstractFoo {
	use TraitFoo;
}

https://phpstan.org/r/12ccc2ea-b367-471b-9da9-7efd57b012a4

Expected output

No errors, at least not on Level 0, as this is valid code: https://3v4l.org/BOAEE

@danrot
Copy link

danrot commented Jun 8, 2020

Any reason this should raise an error at all? IMO this is valid code, and it is completely valid to use traits like that, isn't it?

@ghost
Copy link

ghost commented Jun 8, 2020

Any reason this should raise an error at all? IMO this is valid code, and it is completely valid to use traits like that, isn't it?

Sure, it is valid and I am facing the same issue. Lots of valid code is now marked as error

@ondrejmirtes
Copy link
Member

Yes, it’s valid code, it’s just that this edge case wasn’t covered by any test. It will be fixed soon.

@marc-mabe
Copy link
Contributor

I have the same issue with methods declared in traits : https://phpstan.org/r/1c7d4b55-9b31-4b71-94c9-caa14c699553

@freshp
Copy link

freshp commented Jun 8, 2020

I got the same error with phpstan version 0.12.26.

I could not ignore these type of error, whether with --generate-baseline or by regex.

ignoreErrors:
    - '#Non-abstract class .* contains abstract method getView\(\) from class#'

The error: "cannot be ignored, use excludes_analyse instead." occured instead.

At the moment I have to exclude the files completly.

Does it belong to the same error?

At the moment I downgrade to 0.12.25 to prevent these error.

@phil-davis
Copy link

Does it belong to the same error?

I had this kind of thing start being reported by 0.12.26 and it was a genuine problem. The trait had code like:

trait LegacyDependencyCheckPolyfill {

	/**
	 * @return string
	 */
	abstract public function getStorageClass();
...

And the class had:

class DAV extends ExternalBackend {
	use LegacyDependencyCheckPolyfill;
...

and the error report is correct - someone who tries to instantiate that DAV class directly will find that there is no implementation of getStorageClass() available.

So when you have a "real" class that ends up having just an abstract method (not a real implementation of the method), the problem is being correctly reported.

For me, the error report is new, because IMO phpstan now analyses the content of traits properly.

@SamMousa
Copy link

SamMousa commented Jun 8, 2020

But if ExternalBackend has an implementation it should not result in an error, which it does currently.

@ondrejmirtes
Copy link
Member

Traits are so weird! Look at this example:

https://3v4l.org/1aQa0

Normal method from trait shadows the method from the parent class but abstract method from trait does not shadow the method from the parent class.

@Slamdunk
Copy link
Contributor Author

Slamdunk commented Jun 8, 2020

Normal method from trait shadows the method from the parent class

That's ok as trait are copy-paste

but abstract method from trait does not shadow the method from the parent class.

Abstract methods in trait are just nonsense in fact, even thou are legit.
I would consider them errors in phpstan-strict-rules

@ondrejmirtes
Copy link
Member

ondrejmirtes added a commit that referenced this issue Jun 8, 2020
ondrejmirtes added a commit that referenced this issue Jun 8, 2020
phpstan/phpstan-src@5d76184 Detect void parameter typehint
phpstan/phpstan-src@4329b07 Check default property value against native type
phpstan/phpstan-src@be132b7 Fix sapi_windows_set_ctrl_handler signature
phpstan/phpstan-src@3359422 Fix signature of XSLTProcessor::transformToXml
phpstan/phpstan-src@7ae1393 Don't merge deprecated tag with parents
phpstan/phpstan-src@9d87da8 Overriding method rule - check final
phpstan/phpstan-src@406b5e4 Overriding method rule - visibility
phpstan/phpstan-src@093ae3f OverridingMethodRule - RuleErrorBuilder
phpstan/phpstan-src@437e99c Overriding method rule - static/non-static
phpstan/phpstan-src@0b9e9a3 Rename test file
phpstan/phpstan-src@f7210dc Overriding method rule - do not complain about this case
phpstan/phpstan-src@ac0cfd0 Overriding method rule - check parameter type compatibility
phpstan/phpstan-src@6d5c6fe Fix
phpstan/phpstan-src@f3a2833 Fix test
phpstan/phpstan-src@f478b91 Refactoring
phpstan/phpstan-src@ada27eb OverridingMethodRule - check return type covariance
phpstan/phpstan-src@487f2fa Check abstract method in non-abstract class
phpstan/phpstan-src@3ee81d1 Unification - show class display name in messages
phpstan/phpstan-src@50d2c39 Check missing method implementations in non-abstract class
phpstan/phpstan-src@414784c Fix
phpstan/phpstan-src@41ab4a7 Fix
phpstan/phpstan-src@004a8b7 Refactoring
phpstan/phpstan-src@c11ab15 array_map -> foreach
phpstan/phpstan-src@1eedc10 Get native type from reflection
phpstan/phpstan-src@fe72e26 Default value property type does not observe strict types - it's always strict
phpstan/phpstan-src@8fdeba5 MethodSignatureRule - do not report duplicates already reported by OverridingMethodRule
phpstan/phpstan-src@3ae5e83 Fix undetected generator function with static reflection
phpstan/phpstan-src@1236c17 Fix
phpstan/phpstan-src@4576346 Fix
phpstan/phpstan-src@3950454 Fix detecting overriding final constructor
phpstan/phpstan-src@64618be Fix overriden variadic parameter
phpstan/phpstan-src@bf94fac Do not generate baseline when internal errors occured
phpstan/phpstan-src@6f9f9aa CommandHelper - executing bootstrap file extracted into a private method
phpstan/phpstan-src@aad1bf8 Introduce bootstrapFiles config option; deprecate bootstrap config option
phpstan/phpstan-src@b971fc6 Introduce scanFiles
phpstan/phpstan-src@d0e9bf2 Deprecate autoload_files
phpstan/phpstan-src@27db54c Introduce scanDirectories
phpstan/phpstan-src@61f4b18 Deprecate autoload_directories
phpstan/phpstan-src@e7b7030 Autoloading is gone from user-facing messages
phpstan/phpstan-src@35341d0 Resolve relative paths in scanFiles and scanDirectories
phpstan/phpstan-src@a3e644a Detect nonexistent/wrong scanFiles and scanDirectories
phpstan/phpstan-src@61f7a97 Function with `yield from` wasn't recognized as generator
phpstan/phpstan-src@c2135cb Fix finfo constructor
phpstan/phpstan-src@7aabe35 Fix magic constants as default parameter values
phpstan/phpstan-src@3af7eaf Parameter with a default value followed by a variadic parameter is also optional
phpstan/phpstan-src@b79c08d AbstractMethodInNonAbstractClassRule - fix abstract method from trait that overshadowed method from parent class
phpstan/phpstan-src@29b9470 Print deprecation notice only when running the AnalyseCommand
phpstan/phpstan-src@31844b1 Another test for #3406
phpstan/phpstan-src@98cdb30 In case of multiple comments, use the last one as reflection getDocComment
phpstan/phpstan-src@7738778 Fix CS
ondrejmirtes added a commit that referenced this issue Jun 10, 2020
phpstan/phpstan-src@0b9e9a3 Rename test file
phpstan/phpstan-src@f7210dc Overriding method rule - do not complain about this case
phpstan/phpstan-src@ac0cfd0 Overriding method rule - check parameter type compatibility
phpstan/phpstan-src@6d5c6fe Fix
phpstan/phpstan-src@f3a2833 Fix test
phpstan/phpstan-src@f478b91 Refactoring
phpstan/phpstan-src@ada27eb OverridingMethodRule - check return type covariance
phpstan/phpstan-src@487f2fa Check abstract method in non-abstract class
phpstan/phpstan-src@3ee81d1 Unification - show class display name in messages
phpstan/phpstan-src@50d2c39 Check missing method implementations in non-abstract class
phpstan/phpstan-src@414784c Fix
phpstan/phpstan-src@41ab4a7 Fix
phpstan/phpstan-src@004a8b7 Refactoring
phpstan/phpstan-src@c11ab15 array_map -> foreach
phpstan/phpstan-src@1eedc10 Get native type from reflection
phpstan/phpstan-src@fe72e26 Default value property type does not observe strict types - it's always strict
phpstan/phpstan-src@8fdeba5 MethodSignatureRule - do not report duplicates already reported by OverridingMethodRule
phpstan/phpstan-src@3ae5e83 Fix undetected generator function with static reflection
phpstan/phpstan-src@1236c17 Fix
phpstan/phpstan-src@4576346 Fix
phpstan/phpstan-src@3950454 Fix detecting overriding final constructor
phpstan/phpstan-src@64618be Fix overriden variadic parameter
phpstan/phpstan-src@bf94fac Do not generate baseline when internal errors occured
phpstan/phpstan-src@6f9f9aa CommandHelper - executing bootstrap file extracted into a private method
phpstan/phpstan-src@aad1bf8 Introduce bootstrapFiles config option; deprecate bootstrap config option
phpstan/phpstan-src@b971fc6 Introduce scanFiles
phpstan/phpstan-src@d0e9bf2 Deprecate autoload_files
phpstan/phpstan-src@27db54c Introduce scanDirectories
phpstan/phpstan-src@61f4b18 Deprecate autoload_directories
phpstan/phpstan-src@e7b7030 Autoloading is gone from user-facing messages
phpstan/phpstan-src@35341d0 Resolve relative paths in scanFiles and scanDirectories
phpstan/phpstan-src@a3e644a Detect nonexistent/wrong scanFiles and scanDirectories
phpstan/phpstan-src@61f7a97 Function with `yield from` wasn't recognized as generator
phpstan/phpstan-src@c2135cb Fix finfo constructor
phpstan/phpstan-src@7aabe35 Fix magic constants as default parameter values
phpstan/phpstan-src@3af7eaf Parameter with a default value followed by a variadic parameter is also optional
phpstan/phpstan-src@b79c08d AbstractMethodInNonAbstractClassRule - fix abstract method from trait that overshadowed method from parent class
phpstan/phpstan-src@29b9470 Print deprecation notice only when running the AnalyseCommand
phpstan/phpstan-src@31844b1 Another test for #3406
phpstan/phpstan-src@98cdb30 In case of multiple comments, use the last one as reflection getDocComment
phpstan/phpstan-src@7738778 Fix CS
phpstan/phpstan-src@4ba83e8 template type: correctly resolve isSuperTypeOf
phpstan/phpstan-src@a599eaa Fix variadic parameters by not making them always ArrayType
phpstan/phpstan-src@f8512a7 Use runtime reflection for Generator
phpstan/phpstan-src@d063193 Fix CS
phpstan/phpstan-src@988cea9 Updated phpdoc-parser
phpstan/phpstan-src@3a2a666 fix PharData::offsetGet return type
phpstan/phpstan-src@03a630e Fixed getting constant on self:: with duplicate classes
phpstan/phpstan-src@3b2c806 ExistingClassInClassExtendsRule - do not complain about `@final`
phpstan/phpstan-src@bd36d6d README - note about the result cache
@dktapps
Copy link
Contributor

dktapps commented Jun 14, 2020

This is still a problem with latest dev-master when the inheritance is not direct. See https://github.com/dktapps/phpstan-traits-abstract-bug-test

@ondrejmirtes
Copy link
Member

@dktapps This issue is closed. Can you report this as a new issue?

@dktapps
Copy link
Contributor

dktapps commented Jun 14, 2020

Sure, working on it right now.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants