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

Change message 'typehint' to 'type' and 'typehint type' to 'type'. #270

Closed
wants to merge 11 commits into from
Closed

Change message 'typehint' to 'type' and 'typehint type' to 'type'. #270

wants to merge 11 commits into from

Conversation

Danack
Copy link
Contributor

@Danack Danack commented Jul 6, 2020

Not sure if you've chosen to keep 'typehint' rather than 'type', but it is my own personal windmill so here is a PR to avoid using legacy description of types.

Some of them were changed from:

Return typehint of anonymous function has invalid type %s

To:

Return type of anonymous function has invalid type %s

But they might be better as:

Return type of anonymous function is invalid type %s

But I didn't do that change.

Also, didn't touch the internal classnames as they don't get shown to endusers in the error messages.

@ondrejmirtes
Copy link
Member

This wouldn't be a good idea to do in minor version, but I'll keep this open and merge it once I open 1.0-dev. Thanks.

@Danack
Copy link
Contributor Author

Danack commented Jul 13, 2020

I'll keep this open and merge it once I open 1.0-dev.

Ooo. A 1.0? la-dee-dah.

Going to close and reopen to retrigger the build, as apparently github actions don't trigger off new commits....which seems odd.

@ondrejmirtes
Copy link
Member

Hi, I just opened 1.0-dev, if you rebase this and make sure the nomenclature is correct everywhere, I'll merge it. Thanks.

@Danack
Copy link
Contributor Author

Danack commented Sep 13, 2021

Going to close and reopen to retrigger the build, as apparently github actions don't trigger off new commits....which seems odd.

That still seems odd.

@Danack Danack closed this Sep 13, 2021
@Danack Danack reopened this Sep 13, 2021
@Danack
Copy link
Contributor Author

Danack commented Sep 13, 2021

That appears to be updated.

I think the phrase Return type of anonymous function has invalid type %s. is inelegant. Is it okay to change it to Anonymous function has invalid return type %s. ?

$functionName
),
sprintf(
'Return typehint of function %s() has invalid type %%s.',
'Return type of function %s() has invalid type %%s.',
Copy link
Contributor

@staabm staabm Sep 13, 2021

Choose a reason for hiding this comment

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

maybe

Suggested change
'Return type of function %s() has invalid type %%s.',
'Function %s() has invalid return type %%s.',

(same for the method case below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would match the suggestion I have for anonymous functions.

@ondrejmirtes
Copy link
Member

I think the phrase Return type of anonymous function has invalid type %s. is inelegant

Yes, please, update all the four rules: about closures, about arrow functions, about functions, and about methods :)

@Danack
Copy link
Contributor Author

Danack commented Sep 14, 2021

Something broken in the CI:

Previously working - https://github.com/phpstan/phpstan-src/runs/3588423148?check_suite_focus=true#step:6:182

Broken now - https://github.com/phpstan/phpstan-src/runs/3599628069?check_suite_focus=true#step:6:182

It's not obviously my fault. Any suggestions?

@ondrejmirtes
Copy link
Member

I guess the problem is related to: composer/composer#10102

@ondrejmirtes
Copy link
Member

I've rebased it and some relevant tests are now failing.

@ondrejmirtes
Copy link
Member

Thank you! Merged as: e1ccc85

@Danack
Copy link
Contributor Author

Danack commented Sep 16, 2021

@ondrejmirtes fyi - I think there's one more to do:

  •    "message": "Return type of method Levels\\Typehints\\Foo::doFoo() has invalid type Levels\\Typehints\\Ipsum.",
    
  •    "message": "Method Levels\\Typehints\\Foo::doFoo() has invalid return type Levels\\Typehints\\Ipsum.",
    

in file tests/PHPStan/Levels/data/typehints-0.json

However, I've just spent 15 minutes trying to figure out what is touching whitespace automatically in that file, as something keeps adding an extra line at the end of that file.

@ondrejmirtes
Copy link
Member

However, I've just spent 15 minutes trying to figure out what is touching whitespace automatically in that file

That's me when I edit the file manually on my machine 😂 But since PHPUnit compares JSON against JSON, it doesn't matter.

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