Skip to content

Warn about missing class synopses when running gen_stub.php --replace-classsynopses #9472

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

Merged
merged 3 commits into from
Sep 5, 2022

Conversation

kocsismate
Copy link
Member

Running ./build/gen_stub.php --replace-classsynopses ./ ../doc-en/ locally shows the following output now:

Warning: Missing class synopsis page for Attribute
Warning: Missing class synopsis page for ReturnTypeWillChange
Warning: Missing class synopsis page for AllowDynamicProperties
Warning: Missing class synopsis page for SensitiveParameter
Warning: Missing class synopsis page for SensitiveParameterValue
Warning: Missing class synopsis page for ClosedGeneratorException
Warning: Missing class synopsis page for InternalIterator
Warning: Missing class synopsis page for com_safearray_proxy
Warning: Missing class synopsis page for DOMNameSpaceNode
Warning: Missing class synopsis page for LibXMLError
Warning: Missing class synopsis page for PDORow
Warning: Missing class synopsis page for Random\Engine\Mt19937
Warning: Missing class synopsis page for Random\Engine\PcgOneseq128XslRr64
Warning: Missing class synopsis page for Random\Engine\Xoshiro256StarStar
Warning: Missing class synopsis page for Random\Engine\Secure
Warning: Missing class synopsis page for Random\Engine
Warning: Missing class synopsis page for Random\CryptoSafeEngine
Warning: Missing class synopsis page for Random\Randomizer
Warning: Missing class synopsis page for Random\RandomError
Warning: Missing class synopsis page for Random\BrokenRandomEngineError
Warning: Missing class synopsis page for Random\RandomException
Warning: Missing class synopsis page for XMLParser

Do we want to document the Attribute classes themselves? if not, I'll exclude them too.

@kocsismate kocsismate requested a review from cmb69 September 2, 2022 15:14
@kocsismate kocsismate changed the base branch from master to PHP-8.2 September 2, 2022 15:14
@kocsismate kocsismate changed the title Warn when a class synopsis page doesn't exist Warn about missing class synopses when running gen_stub.php --replace-classsynopses Sep 2, 2022
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you! This looks really helpful.

Warning: Missing class synopsis page for XMLParser

Hmm, https://www.php.net/manual/en/class.xmlparser.php is there.

Do we want to document the Attribute classes themselves?

In my opinion, we should.

@cmb69
Copy link
Member

cmb69 commented Sep 2, 2022

Warning: Missing class synopsis page for PDORow

What's that?

@kocsismate
Copy link
Member Author

Hmm, https://www.php.net/manual/en/class.xmlparser.php is there.

php/doc-en#1795

In my opinion, we should.

Agreed!

What's that?

According to bf48daa, it seems like that this class is for some short of laziness. I'm not really sure if it really works because there is no meaningful test for it.

@cmb69
Copy link
Member

cmb69 commented Sep 2, 2022

According to bf48daa, it seems like that this class is for some short of laziness.

Ah, thanks! So this is a support class for PDO::FETCH_LAZY; looks like there are some tests for that. Anyhow, I think it's good to document class, so we can clarify that it should not be used in userland code (which is fortunately already forbidden).

@kocsismate
Copy link
Member Author

kocsismate commented Sep 5, 2022

I slightly changed the implementation: now, the validation is only shown when the --verify flag is set. This is because otherwise false-positive warnings might be emitted when class synopses in a specific subdirectory are the target, like below:

 ./build/gen_stub.php --replace-classsynopses ./ext/reflection ./Zend ../doc-en/reference/reflection

The input arguments have to cover all members of the targeted class hierarchy (thus we need to include ./Zend as well), and in this case, no class synopsis page of any Zend classes would be found. In order to prevent such scenarios, I think it's a good idea to be able to opt into the validation.

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you!

now, the validation is only shown when the --verify flag is set.

Good idea!

@kocsismate kocsismate merged commit c547fc1 into php:PHP-8.2 Sep 5, 2022
@kocsismate kocsismate deleted the stub-sync-validation branch September 5, 2022 12:21
kocsismate added a commit that referenced this pull request Sep 5, 2022
* PHP-8.2:
  Add support for validation of missing class synopses (#9472)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants