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

Conditionally add return type for WP_Http::request #264

Closed
wants to merge 2 commits into from

Conversation

IanDelMar
Copy link
Contributor

Removes duplicate return tag for WP_Http::request()

@szepeviktor
Copy link
Member

Hello guys!
This has turned into a wrong direction.

How to handle ever-growing number of PHPStan tags in core???

@szepeviktor
Copy link
Member

How about not adding a tag that already exists?
Could we do that?
(and write a log line)

@IanDelMar
Copy link
Contributor Author

We can. But then it is lost for each previous version as well.

@szepeviktor
Copy link
Member

But then it is lost for each previous version as well.

Why? I do not understand.

Only not add our tag when one already exists in the current version.

@IanDelMar
Copy link
Contributor Author

IanDelMar commented Nov 25, 2024

Ah, you mean not adding a tag if the visitor added a tag automatically? We can avoid it that way, but at the cost of losing the ability to override incorrect tags that were added automatically.

@szepeviktor
Copy link
Member

to override incorrect tags that were added automatically.

That is better!

So if there is already a @phpstan-* tag in core we overwrite that: core's tag will be removed, not ours.

@szepeviktor
Copy link
Member

core's tag will be removed, not ours.

The opposite of this PR.

@IanDelMar
Copy link
Contributor Author

IanDelMar commented Nov 25, 2024

Not sure what you mean by

So if there is already a @phpstan-* tag in core we overwrite that: core's tag will be removed, not ours.

There is no @phpstan-* tag in core. They are all generated by the visitor.

As far as I can tell, in the case of WP_Http::request, the auto-generated tag is more accurate than the one generated from functionMap.php.

@szepeviktor
Copy link
Member

I see!!

"Autogenerated" means we read @return { @type ... } and autogenerate a phpstan tag.

Okay. Then drop the one from functionMap.php.

@szepeviktor
Copy link
Member

but no $wp_version please

@IanDelMar
Copy link
Contributor Author

Okay. Then drop the one from functionMap.php.

That's what I did. We can implement the same behavior via the visitor, but I won't have time for this this week.

@IanDelMar
Copy link
Contributor Author

#265

@IanDelMar IanDelMar closed this Nov 25, 2024
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.

2 participants