Skip to content

PHPORM-395: Fix autocompletion for compound search operators#2832

Closed
alcaeus wants to merge 1 commit intodoctrine:2.12.xfrom
alcaeus:phporm-395-fix-search-compound-autocomplete
Closed

PHPORM-395: Fix autocompletion for compound search operators#2832
alcaeus wants to merge 1 commit intodoctrine:2.12.xfrom
alcaeus:phporm-395-fix-search-compound-autocomplete

Conversation

@alcaeus
Copy link
Copy Markdown
Member

@alcaeus alcaeus commented Sep 25, 2025

Q A
Type bug
BC Break no
Fixed issues

Summary

For some reason, PhpStorm didn't like the @return annotation on the method, which should've told it that the result of a text() operator is a special Text instance that allows calling other operators. To work around this, using the compounded classes is necessary, which is what's actually being returned. Since this effectively narrows the return type (as that class extends the original operator class but implements an extra interface) we can do this in the trait without running into issues.

@alcaeus alcaeus requested a review from GromNaN September 25, 2025 11:52
@GromNaN
Copy link
Copy Markdown
Member

GromNaN commented Sep 25, 2025

I just verified that this fixes the autocompletion issue in PHPStorm.

I understand why you didn't do this initially, as you shouldn't depend on an internal class in the API of a public class. Therefore, classes like CompoundedAutocomplete should no longer be marked internal.

NB: It seems we have a new range of errors to exclude from the phpstan analysis.

@alcaeus
Copy link
Copy Markdown
Member Author

alcaeus commented Sep 25, 2025

Therefore, classes like CompoundedAutocomplete should no longer be marked internal.

Users shouldn't ever need to depend on those classes, unless they store the return value of somewhere within the chain, which I generally wouldn't recommend. No objection, I just don't think it's necessary.

NB: It seems we have a new range of errors to exclude from the phpstan analysis.

Created #2833 to fix those separately.

@GromNaN GromNaN added the Bug label Sep 25, 2025
@GromNaN GromNaN added this to the 2.12.2 milestone Sep 25, 2025
Copy link
Copy Markdown
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

Sorry, I found a better solution: #2836

*/
abstract protected function addOperator(SearchOperator $operator): SearchOperator;

/** return Autocomplete&CompoundSearchOperatorInterface */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just seen the missing @ before return.

@alcaeus
Copy link
Copy Markdown
Member Author

alcaeus commented Sep 26, 2025

Closing in favour of #2836.

@alcaeus alcaeus closed this Sep 26, 2025
@alcaeus alcaeus deleted the phporm-395-fix-search-compound-autocomplete branch September 26, 2025 07:17
@GromNaN GromNaN removed this from the 2.12.2 milestone Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants