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

Enable DisallowArrayTypeHintSyntax sniff #143

Closed
wants to merge 2 commits into from
Closed

Enable DisallowArrayTypeHintSyntax sniff #143

wants to merge 2 commits into from

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Dec 5, 2019

Disallows usage of array type hint syntax (eg. int[], bool[][]) in phpDocs in favour of generic type hint syntax (eg. array, array<array>).

  • added new sniff
  • fixed violations in all tests
  • added type-hints test to cover this (+2 errors, autoloading and string[] violation)

If this passes, I can drop all traversableTypeHints settings from #139 (review)

@simPod simPod requested a review from a team as a code owner December 5, 2019 09:37
Copy link
Member

@ostrolucky ostrolucky left a comment

Choose a reason for hiding this comment

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

PHPStorm does not understand this, that's a no go

Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

Agree with @ostrolucky here: until current tooling understands the generic syntax, we shouldn't enforce this in doctrine/cs.

@simPod
Copy link
Contributor Author

simPod commented Dec 5, 2019

Makes sense. Thanks

@simPod simPod closed this Dec 5, 2019
@lcobucci
Copy link
Member

lcobucci commented Dec 5, 2019

PHPStorm does understand that syntax: https://blog.jetbrains.com/phpstorm/2018/09/phpstorm-2018-3-eap-183-2635-12/ - that's just for reference not that we must enforce it now 😄

@ostrolucky
Copy link
Member

Only on parser level. You will not get $foo-> autocompletion for such annotations

@simPod
Copy link
Contributor Author

simPod commented Dec 5, 2019

Yea, don't know why they published the article, it's deceptive. PHPStorm does not complain about the syntax, but that's it 😄

@simPod
Copy link
Contributor Author

simPod commented Dec 5, 2019

This one is in their top 10 most voted issues for phpstorm but from information they gave me it's not even planned yet https://youtrack.jetbrains.com/issue/WI-43843

@Ocramius
Copy link
Member

Ocramius commented Dec 5, 2019

In my case, I started adding two sets of annotations (to be removed later on), such as @var for the IDE, @psalm-var for the actual type:

/**
 * @var Collection|Foo[] - notw that this type is invalid
 * @psalm-var Collection<String, Foo>
 */

I'm hoping for the IDE to catch up, and then I'd do a find/replace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants