#61399 closed enhancement (fixed)
HTML API: Add type annotations
Reported by: | jonsurrell | Owned by: | jonsurrell |
---|---|---|---|
Milestone: | 6.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | HTML API | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
The HTML API benefits from adding type annotations where possible and supported. It makes development easier and helps to discover tricky bugs.
Change History (44)
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#1
- Keywords has-patch added
This ticket was mentioned in βPR #6754 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#2
This applies three fixes that appeared as part of adding type annotations to the HTML API in #6753.
- Fix a bug where a string was passed instead of a token
- Fix Tag Processor bug where null was used as int
- Fix return type error in tag processor get_token_name
Trac ticket: https://core.trac.wordpress.org/ticket/61399
---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See βGitHub Pull Requests for Code Review in the Core Handbook for more details.
β@dmsnell commented on βPR #6754:
7 months ago
#5
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#6
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#7
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#8
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#10
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#11
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#12
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#13
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
7 months ago
#14
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
#15
@
7 months ago
@dmsnell and I talked about this. @dmsnell shared concerns around this causing crashes on sites and wanting to be careful. These are valid.
We discussed introducing type annotations on places that should be safe, meaning that they can't introduce new crashes.
- Private methods may be safe to annotate completely.
- Return types should be safe to annotate.
- Public method arguments are more risky to type and we'll avoid them for now. It's relatively easy to pass values of the wrong type and introduce runtime errors.
I'll adapt βPR#6753 accordingly.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#16
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#17
- Keywords has-unit-tests added
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#18
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Closes βhttps://github.com/WordPress/wordpress-develop/pull/7031 (supersedes)
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#19
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#20
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#21
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#22
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#23
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#24
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#25
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
β@jonsurrell commented on βPR #6753:
6 months ago
#26
I've removed most of the type annotations from the parameters of public APIs. I did this to address concerns about the potential to break sites due to type errors from plugins that are not actionable in any way for the sites. I've maintained the type annotations on the private APIs and internal classes.
@dmsnell What do you think about the low-level API like the token map? Should I remove types in the same way there as well?
This ticket was mentioned in βPR #7031 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#27
Trac ticket: Core-61399
β@dmsnell commented on βPR #7031:
6 months ago
#29
Thank you, @sirreal. I updated the patch with some Optional.
and Default
declarations.
Merged in [58742]
βhttps://github.com/wordpress/wordpress-develop/commit/48c0c968468b54ec2909317e1a9384c0943c0dd3
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#30
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#31
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
β@dmsnell commented on βPR #6753:
6 months ago
#32
@sirreal I think all of this as is is probably fine, but I want to run this against the Gutenberg repository first to ensure that nothing basic is broken, since we're introducing new intentional-breaking behaviors.
β@dmsnell commented on βPR #6753:
6 months ago
#33
Backport contains these anticipated updates in the following Gutenberg PR.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#34
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#35
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
β@dmsnell commented on βPR #6753:
6 months ago
#37
This ticket was mentioned in βPR #6753 on βWordPress/wordpress-develop by β@jonsurrell.
6 months ago
#38
Trac ticket: Core-61399
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the array
type where the @param
or @return
annotations are more expressive, or a mixed
type instead of a type union.
I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, that's a smaller patch that can be landed independently.
Includes βhttps://github.com/WordPress/wordpress-develop/pull/7031.
β@TobiasBg commented on βPR #6753:
6 months ago
#39
@dmsnell @sirreal:
I see 2-3 $case_sensitivity
parameters (2nd/3rd parameters of a function) that could also get a string
annotation?
Or is this similar to $termination_list
not getting array
as the PHPDoc has the more specific string[]
(which is intentional from what I understand from the PR description)?
I would actually vote for adding these as well, as the annotations not only serve documentation purposes but also prevent wrong parameter types from being passed to the function.
β@dmsnell commented on βPR #6753:
6 months ago
#40
@TobiasBg some of the annotations were removed to prevent site crashes when improperly called. I think we are still gathering experience knowing how to handle this scenario. the PHDDoc comment adds the types, right? so IDE's should still type-check, autocomplete, and auto-document the parameters.
I've seen plenty of well-intentioned efforts to add type annotations only to realize that something was a mistake and now WordPress crashes because the types were wrong. I'm sensitive to avoid doing this out of over-eagerness.
β@TobiasBg commented on βPR #6753:
6 months ago
#41
@dmsnell: Yes, PHPDocs (correctly) adds the types for syntax highlighting, autocomplete, documentation, use in static analysis, etc. But these are pretty much "optional". Only the annotations let PHP raise an error message if a parameter has the wrong type.
And of course that's why type annotations should be added with care (and e.g. prevent with custom type checks in the code) to prevent crashes! Thus, excluding public-facing functions from this totally makes sense!
I was more referring to "inconsistencies": E.g. in [https://core.trac.wordpress.org/changeset/58769/trunk/src/wp-includes/class-wp-token-map.php trunk/src/wp-includes/class-wp-token-map.php]
:
In the line
{{{PHP
public function contains( string $word, string $case_sensitivity = 'case-sensitive' ): bool {
}}}
both parameters (including $case_sensitivity
) got an annotation. But a few lines below, in the line
{{{PHP
public function read_token(Β string $text, int $offset = 0, &$matched_token_byte_length = null, $case_sensitivity = 'case-sensitive' ): ?stringΒ {
}}}
only the first parameter got an annotation while the same $case_sensitivity
parameter did not (but in both cases, it's PHPdoc'ed as string).
(I'm fine with the call-by-reference parameter not getting one, as I don't know what consequences that might have.)
Similarly, in trunk/src/wp-includes/html-api/class-wp-html-open-elements.php, there's the line
{{{PHP
public function has_element_in_specific_scope( string $tag_name, $termination_list ): bool {
}}}
where only the first parameter got an annotation.
β@dmsnell commented on βPR #6753:
6 months ago
#42
@TobiasBg happy to review a patch!
Add type annotations in the HTML API where possible. In this PR I've avoided adding annotations where the supported annotations are worse that PHPDoc annotations.
For example, I've avoided using the
array
type where the@param
or@return
annotations are more expressive, or amixed
type instead of a type union.I've extracted some bugfixes that arose as part of this work to βhttps://github.com/WordPress/wordpress-develop/pull/6754, they can be landed independently and as bugfixes for WordPress 6.6.
Trac ticket: https://core.trac.wordpress.org/ticket/61399