Skip to content

Fixes and improvements for class synopsis generation #10098

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 4 commits into from
Dec 16, 2022

Conversation

kocsismate
Copy link
Member

@kocsismate kocsismate commented Dec 13, 2022

  • Replace another root XML element format to the "canonical" one: gen_stub would modify the order of the attributes in some cases (e.g. reference/spl/outofrangeexception.xml), so another replacement rule is added to the list
  • Remove the superfluous closing parenthesis from class synopsis page includes: Fixes php/doc-en@89eaa41#r92906240
  • Always include the constructor on the class manual pages: As default constructors are sometimes documented, sometimes not, gen_stub.php sometimes generates false positive diffs with the docs (e.g. AllowDynamicProperties::__construct() is not currently documented, but it has a stub. Conversely, SplStack::__construct() is documented, but it doesn't have a stub). Therefore, the only sensible way to have deterministic rules is to always document default constructors.

Comment on lines 2771 to 2774
$includeElement = $this->createIncludeElement(
$doc,
"xmlns(db=http://docbook.org/ns/docbook) xpointer(id('$classReference')/db:refentry/db:refsect1[@role='description']/descendant::db:constructorsynopsis[not(@role='procedural')])"
);
Copy link
Member

Choose a reason for hiding this comment

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

Does this create a fallback element that doesn't warn if it doesn't exist?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it does:

php-src/build/gen_stub.php

Lines 2962 to 2974 in 002d54d

private function createIncludeElement(DOMDocument $doc, string $query): DOMElement
{
$includeElement = $doc->createElement("xi:include");
$attr = $doc->createAttribute("xpointer");
$attr->value = $query;
$includeElement->appendChild($attr);
$fallbackElement = $doc->createElement("xi:fallback");
$includeElement->appendChild(new DOMText("\n "));
$includeElement->appendChild($fallbackElement);
$includeElement->appendChild(new DOMText("\n "));
return $includeElement;
}

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Other than the comment question LGTM

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!

Comment on lines 2771 to 2774
$includeElement = $this->createIncludeElement(
$doc,
"xmlns(db=http://docbook.org/ns/docbook) xpointer(id('$classReference')/db:refentry/db:refsect1[@role='description']/descendant::db:constructorsynopsis[not(@role='procedural')])"
);
Copy link
Member

Choose a reason for hiding this comment

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

Yes, it does:

php-src/build/gen_stub.php

Lines 2962 to 2974 in 002d54d

private function createIncludeElement(DOMDocument $doc, string $query): DOMElement
{
$includeElement = $doc->createElement("xi:include");
$attr = $doc->createAttribute("xpointer");
$attr->value = $query;
$includeElement->appendChild($attr);
$fallbackElement = $doc->createElement("xi:fallback");
$includeElement->appendChild(new DOMText("\n "));
$includeElement->appendChild($fallbackElement);
$includeElement->appendChild(new DOMText("\n "));
return $includeElement;
}

@kocsismate
Copy link
Member Author

kocsismate commented Dec 16, 2022

Just before merging, I realized that the recent role attribute related changes have not been available in PHP 8.2, so I included the backport into this PR.

@kocsismate kocsismate merged commit 6aa5e58 into php:PHP-8.2 Dec 16, 2022
@kocsismate kocsismate deleted the methodsynopsis-fix branch December 16, 2022 12:18
kocsismate added a commit to kocsismate/php-src that referenced this pull request Dec 18, 2022
Until php#10098 default constructors were sometimes documented, sometimes omitted. The above PR made adding documentation for constructor of all non-abstract classes required. However, this is not desired when such a class inherits a constructor from its parent. The current PR fixes the behavior the following way:
- documents inherited constructors (along with destructors)
- makes it possible to generate/replace the methodsynopsis of implicit default constructors which doesn't have a stub
kocsismate added a commit to kocsismate/php-src that referenced this pull request Dec 18, 2022
Until php#10098 default constructors were sometimes documented, sometimes omitted. The above PR made adding documentation for constructor of all non-abstract classes required. However, this is not desired when such a class inherits a constructor from its parent. The current PR fixes the behavior the following way:
- documents inherited constructors (along with destructors)
- makes it possible to generate/replace the methodsynopsis of implicit default constructors which don't have a stub counterpart
kocsismate added a commit to kocsismate/php-src that referenced this pull request Dec 18, 2022
Until php#10098 default constructors were sometimes documented, sometimes omitted. The above PR made adding documentation for constructor of all non-abstract classes required. However, this is not desired when such a class inherits a constructor from its parent. The current PR fixes the behavior the following way:
- documents inherited constructors (along with destructors)
- makes it possible to generate/replace the methodsynopsis of implicit default constructors which don't have a stub counterpart
kocsismate added a commit that referenced this pull request Jan 1, 2023
Until #10098 default constructors were sometimes documented, sometimes omitted. The above PR made adding documentation for constructor of all non-abstract classes required. However, this is not desired when such a class inherits a constructor from its parent. The current PR fixes the behavior the following way:
- documents inherited constructors (along with destructors)
- makes it possible to generate/replace the methodsynopsis of implicit default constructors which don't have a stub counterpart
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.

3 participants