Skip to content

Add FQCN support to implementsInterface #144

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

Merged

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Sep 26, 2022

Closes #17
Closes #18

The idea behind this is amazingly simple. If the $class arg is a known interface we can simply use is_subclass_of to check if the given object or FQCN implements that interface.

@herndlm herndlm force-pushed the add-fqcn-support-to-implements-interface branch from 2f904da to e10744b Compare September 26, 2022 20:18
@herndlm herndlm marked this pull request as draft September 26, 2022 21:20
@herndlm herndlm force-pushed the add-fqcn-support-to-implements-interface branch from e10744b to 5da43bc Compare September 26, 2022 21:26
@herndlm herndlm marked this pull request as ready for review September 26, 2022 21:27
@herndlm herndlm force-pushed the add-fqcn-support-to-implements-interface branch from 5da43bc to 03ceb86 Compare October 3, 2022 10:37
@herndlm
Copy link
Contributor Author

herndlm commented Oct 4, 2022

If you want a simple fix for a long outstanding bug, this is it 😊

@ondrejmirtes
Copy link
Member

Can you please remove the changes in src/ and see what the tests do since we already depend on PHPStan 1.9.0 here and the method has these annotations? https://github.com/webmozarts/assert/blob/11cb2199493b2f8a3b53e7f19068fc6aac760991/src/Assert.php#L1566-L1577

Thanks!

@herndlm
Copy link
Contributor Author

herndlm commented Oct 5, 2022

interesting, indeed it understands more now, I get those failures

1) PHPStan\Type\WebMozartAssert\AssertTypeSpecifyingExtensionTest::testFileAsserts with data set "/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php:41" ('type', '/Users/herndlm/Development/so...ct.php', PHPStan\Type\Constant\ConstantStringType Object (...), PHPStan\Type\Generic\GenericClassStringType Object (...), 41)
Expected type class-string<PHPStan\Type\WebMozartAssert\ObjectFoo>|PHPStan\Type\WebMozartAssert\ObjectFoo, got type class-string<PHPStan\Type\WebMozartAssert\ObjectFoo> in /Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php on line 41.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'class-string<PHPStan\Type\WebMozartAssert\ObjectFoo>|PHPStan\Type\WebMozartAssert\ObjectFoo'
+'class-string<PHPStan\Type\WebMozartAssert\ObjectFoo>'

phar:///Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/vendor/phpstan/phpstan/phpstan.phar/src/Testing/TypeInferenceTestCase.php:58
/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/AssertTypeSpecifyingExtensionTest.php:35
phpvfscomposer:///Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/vendor/phpunit/phpunit/phpunit:97

2) PHPStan\Type\WebMozartAssert\AssertTypeSpecifyingExtensionTest::testFileAsserts with data set "/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php:44" ('type', '/Users/herndlm/Development/so...ct.php', PHPStan\Type\Constant\ConstantStringType Object (...), PHPStan\Type\UnionType Object (...), 44)
Expected type class-string<PHPStan\Type\WebMozartAssert\ObjectFoo>|PHPStan\Type\WebMozartAssert\ObjectFoo|null, got type class-string<PHPStan\Type\WebMozartAssert\ObjectFoo>|null in /Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php on line 44.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'class-string<PHPStan\Type\WebMozartAssert\ObjectFoo>|PHPStan\Type\WebMozartAssert\ObjectFoo|null'
+'class-string<PHPStan\Type\WebMozartAssert\ObjectFoo>|null'

phar:///Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/vendor/phpstan/phpstan/phpstan.phar/src/Testing/TypeInferenceTestCase.php:58
/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/AssertTypeSpecifyingExtensionTest.php:35
phpvfscomposer:///Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/vendor/phpunit/phpunit/phpunit:97

3) PHPStan\Type\WebMozartAssert\AssertTypeSpecifyingExtensionTest::testFileAsserts with data set "/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php:50" ('type', '/Users/herndlm/Development/so...ct.php', PHPStan\Type\Constant\ConstantStringType Object (...), PHPStan\Type\NeverType Object (...), 50)
Expected type PHPStan\Type\WebMozartAssert\ObjectFoo, got type *NEVER* in /Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php on line 50.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'PHPStan\Type\WebMozartAssert\ObjectFoo'
+'*NEVER*'

phar:///Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/vendor/phpstan/phpstan/phpstan.phar/src/Testing/TypeInferenceTestCase.php:58
/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/AssertTypeSpecifyingExtensionTest.php:35
phpvfscomposer:///Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/vendor/phpunit/phpunit/phpunit:97

4) PHPStan\Type\WebMozartAssert\AssertTypeSpecifyingExtensionTest::testFileAsserts with data set "/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php:53" ('type', '/Users/herndlm/Development/so...ct.php', PHPStan\Type\Constant\ConstantStringType Object (...), PHPStan\Type\Generic\GenericClassStringType Object (...), 53)
Expected type mixed, got type class-string<PHPStan\Type\WebMozartAssert\ObjectTest> in /Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php on line 53.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'mixed'
+'class-string<PHPStan\Type\WebMozartAssert\ObjectTest>'

phar:///Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/vendor/phpstan/phpstan/phpstan.phar/src/Testing/TypeInferenceTestCase.php:58
/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/AssertTypeSpecifyingExtensionTest.php:35
phpvfscomposer:///Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/vendor/phpunit/phpunit/phpunit:97

5) PHPStan\Type\WebMozartAssert\AssertTypeSpecifyingExtensionTest::testFileAsserts with data set "/Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php:56" ('type', '/Users/herndlm/Development/so...ct.php', PHPStan\Type\Constant\ConstantStringType Object (...), PHPStan\Type\Generic\GenericClassStringType Object (...), 56)
Expected type mixed, got type class-string<PHPStan\Type\WebMozartAssert\Unknown> in /Users/herndlm/Development/source/git-forks/phpstan-webmozart-assert/tests/Type/WebMozartAssert/data/object.php on line 56.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'mixed'
+'class-string<PHPStan\Type\WebMozartAssert\Unknown>'

looks like objects are not working because of the class-string assert or is that ok? and the last 2 failures are kind of expected I guess, the first is using a class instead of an interface (which I explicitly forbid via reflection) and the second is an unknown class. the only real thing bothering me is the *NEVER* for the object hmm

@ondrejmirtes
Copy link
Member

The annotation and the implementation suggests to me that only strings can ever work there...

@herndlm
Copy link
Contributor Author

herndlm commented Oct 5, 2022

annotation yeah, class_implements() does support objects though :/

@herndlm
Copy link
Contributor Author

herndlm commented Oct 5, 2022

anyways, I think it would still close the linked issues by using only phpstan 1.9.0 assert annotations, I'll adapt this

@ondrejmirtes
Copy link
Member

Oh I get it now, yeah, objects are indeed supported. Please try to send a PR to the library :)

@ondrejmirtes ondrejmirtes merged commit f4f29ab into phpstan:1.2.x Oct 5, 2022
@ondrejmirtes
Copy link
Member

Until then, our extension is superior :)

@herndlm herndlm deleted the add-fqcn-support-to-implements-interface branch October 5, 2022 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants