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

Annotate parameter type/return type #1368

Open
mokraemer opened this issue Nov 20, 2021 · 7 comments
Open

Annotate parameter type/return type #1368

mokraemer opened this issue Nov 20, 2021 · 7 comments

Comments

@mokraemer
Copy link

I understand davfs currently supports older php releases. But for a start, it would be great to annotate parameter and return types as @param or @return so e.g. phan can check the correct usage.

@phil-davis
Copy link
Contributor

https://github.com/sabre-io/dav/blob/master/composer.json
"php": "^7.1.0 || ^8.0",

A lot of parameter and return types are supported already in PHP 7.1. So we could declare those in a lot of places already.

@staabm @DeepDiver1975 @evert and anyone. do we want to moved towards "strongly-typed" code?

IMO there is a benefit that callers quickly discover when they have code that passes "not the right type". But for backward-compatibility it can make things fail that used to work because the types were being coerced, and the data being passed was actually OK.

And if strict_types=1 is specified, then wrong code really "explodes".

@staabm
Copy link
Member

staabm commented Nov 21, 2021

Adding native types can be a bc break for code which get subclassed in e.g. apps or plugins.

Adding phpdoc-types cannot break things and leads to similar benefits. As a first step IMO we should only add phpdoc types.

In a future major version we can use this type information and generate proper native types based on phpdocs with rector or similar tooling

@mokraemer
Copy link
Author

Yepp, that is what I want to suggest. If you want to go this way, you may want to use https://github.com/phan/phan which can help you check all the types.

@staabm
Copy link
Member

staabm commented Nov 21, 2021

Our github action already contains a phpstan check (which is similar to phan)

Feel free to contribute

@mokraemer
Copy link
Author

Our github action already contains a phpstan check (which is similar to phan)

Feel free to contribute

I already work on other (opensource) projects. So my time is limited, None of them ueses git, everytime I use it, I end up rm -rf as the only solution. I can add patches, if you like ;)

@evert
Copy link
Member

evert commented Nov 21, 2021

I feel this project is more yours than mine, so my vote shouldn't really count!

However, my opinion has always been stricter is better!

@phil-davis
Copy link
Contributor

However, my opinion has always been stricter is better!

I agree with that. I wish all languages had been strict at the start.

As Marcus said, adding the types to parameters and/or returns of public methods of classes makes incompatibilities for those who extend a class. So we need to leave that for a major version.

"But for a start, it would be great to annotate parameter and return types as @param or @return so e.g. phan can check the correct usage." @mokraemer feel free to make PRs to do this.

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

No branches or pull requests

4 participants