Mark SqlWalker methods as not deprecated #10540
Conversation
|
Would it make sense to have a dedicated sub-interface for the concept that SqlWalker implements, which seems to be more specific than being a tree walker? If so, would that solve the issue as well? |
|
As long as that new interface extends TreeWalker (which is needed for BC) which contains the deprecated methods, the issue of having a non-deprecated method overriding/implementing a deprecated method would still be there. And introducing an interface for the SqlWalker would be a bad idea. If AST nodes were written against a SqlWalkerInterface instead of the SqlWalker for their conversion to SQL, adding new features in DQL would be a BC break any time it would require adding a new method in the SqlWalker to handle some new kind of AST node. |
|
Interesting, I didn't know about that annotation. @stof Can you make PHPCS happy please?
No, we have no use for a |
|
@stof thank you for the explanations, makes sense |
|
@derrabus this annotation is quite new (the solution was suggested by Ondrej a while ago but the implementation was done today) |
phpstan treats implementations of deprecated methods of an interface as being deprecated themselves by default. However, SqlWalker does not intend to deprecate all those methods that are deprecated in TreeWalker, as they are moved down. Marking them as not deprecated will avoid reporting usages of deprecated APIs when implementing custom DQL functions for instance.
stof
force-pushed
the
stof-sqlwalker-not-deprecated
branch
from
27d8331 to
8838564
Compare
|
@derrabus I fixed the order of phpdoc annotations to make phpcs happy. |
* 2.14.x: Ignore the cache dir of PHPUnit 10 (doctrine#10546) Make data providers static (doctrine#10544) Bump dev tools (doctrine#10541) Mark SqlWalker methods as not deprecated (doctrine#10540) docs: consistency order for docblock in association mapping (doctrine#10534) Correct use of PHP attribute fix typo in faq.rst (doctrine#10526) fix: use executeStatement in SchemaTool (doctrine#10516) Write a test in a more specific way Put up a warning sign that mapping may not be inherited from transient classes (doctrine#10392) Avoid unnecessary information in query hints to improve query cache hit ratio
* 2.14.x: Ignore the cache dir of PHPUnit 10 (#10546) Make data providers static (#10544) Bump dev tools (#10541) Mark SqlWalker methods as not deprecated (#10540) docs: consistency order for docblock in association mapping (#10534) Correct use of PHP attribute fix typo in faq.rst (#10526) fix: use executeStatement in SchemaTool (#10516) Write a test in a more specific way Put up a warning sign that mapping may not be inherited from transient classes (#10392) Avoid unnecessary information in query hints to improve query cache hit ratio
* 2.15.x: Ignore the cache dir of PHPUnit 10 (doctrine#10546) Make data providers static (doctrine#10545) Make data providers static (doctrine#10544) Bump dev tools (doctrine#10541) Mark SqlWalker methods as not deprecated (doctrine#10540) docs: consistency order for docblock in association mapping (doctrine#10534) Correct use of PHP attribute fix typo in faq.rst (doctrine#10526) fix: use executeStatement in SchemaTool (doctrine#10516) Write a test in a more specific way Put up a warning sign that mapping may not be inherited from transient classes (doctrine#10392) Avoid unnecessary information in query hints to improve query cache hit ratio
phpstan treats implementations of deprecated methods of an interface as being deprecated themselves by default. However, SqlWalker does not intend to deprecate all those methods that are deprecated in TreeWalker, as they are moved down. Marking them as not deprecated will avoid reporting usages of deprecated APIs when implementing custom DQL functions for instance.
See phpstan/phpstan#7422 for the discussion about that kind of cases in the phpstan repository, which contains links justifying the current behavior of phpstan.
Without this
@not-deprecated(or with it but without the dev version of phpstan as support for it is merged but not released yet), I get such error in my project when implementing custom DQL functions (which work with the SqlWalker API to generate the SQL):But while it will indeed be removed from the TreeWalker interface in 3.0, it won't be removed from SqlWalker.