Enforce @since tags in block-library/src//.php files #59700

anton-vlasenko · 2024-03-08T00:23:53Z

What?

This PR introduces a new sniff called FunctionCommentSinceTagSniff to ensure that PHP functions have a valid @since tag in the docblock.

The sniff skips checking files in __experimental block-library packages.

Fixes #55777.

Why?

@since tags are helpful for tracking origins and how long things have been in Core.

How?

Defines a new sniff class FunctionCommentSinceTagSniff within the GutenbergCS\Gutenberg\Sniffs\Commenting namespace.
Adds unit tests for the FunctionCommentSinceTagSniff sniff.
Updates existing block-library functions so that they have valid @since tags.

Testing Instructions

Try to define a PHP function somewhere in the block-library/src/*/*.php without a @since tag.
Make sure that CI checks fail.

Testing Instructions for Keyboard

Screenshots or Screencast

github-actions · 2024-03-08T00:47:29Z

Flaky tests detected in 9bf6c1868644024f75f08d5b5a3da7ffbf614f4a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8264199919
📝 Reported issues:

[Flaky Test] User Mention: should insert mention between two other words #48774 in /test/e2e/specs/editor/various/autocomplete-and-mentions.spec.js

github-actions · 2024-03-12T20:22:12Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: anton-vlasenko <[email protected]>
Co-authored-by: gziolo <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: getdave <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: kebbet <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

anton-vlasenko · 2024-03-12T20:23:29Z

@kebbet @gziolo I would appreciate your feedback and thoughts.
Thank you.

anton-vlasenko · 2024-03-12T20:24:01Z

packages/block-library/src/media-text/index.php

@@ -8,6 +8,8 @@
 /**
 * Renders the `core/media-text` block on server.
 *
+ * @since 6.6.0


This is the only place I'm not sure about. These functions haven't been included in WordPress Core yet.

anton-vlasenko · 2024-03-12T20:24:46Z

packages/block-library/src/media-text/index.php

@@ -54,6 +56,8 @@ function render_block_core_media_text( $attributes, $content ) {

 /**
 * Registers the `core/media-text` block renderer on server.
+ *
+ * @since 6.6.0


This is the only place I'm not sure about. These functions haven't been included in WordPress Core yet.

I don't see it in WordPress core, so that would be correct. The PHP file was added 5 days ago 👍🏻

gziolo

Impressive detective work adding all missing @since annotations. It must have taken some time to track down the proper versions. I'm not going to validate these numbers and will put trust in you and the collective knowledge of the community that can always follow-up to correct them 😄

The code looks good. I'm starting to get more familiar with PHP sniffs and more open to approve 😄

I left a comment with a suggestion to cover more files that I know we sync with WP core, but it isn't a blocker. In practice, it would require some changes to when to run is_experimantal_package, as it only applies to the packages/block-library. So maybe, another PR.

gziolo · 2024-03-13T09:07:46Z

packages/block-library/src/media-text/index.php

@@ -54,6 +56,8 @@ function render_block_core_media_text( $attributes, $content ) {

 /**
 * Registers the `core/media-text` block renderer on server.
+ *
+ * @since 6.6.0


I don't see it in WordPress core, so that would be correct. The PHP file was added 5 days ago 👍🏻

gziolo · 2024-03-13T09:36:07Z

phpcs.xml.dist

@@ -172,4 +172,8 @@
 			</property>
 		</properties>
 	</rule>
+	<rule ref="Gutenberg.Commenting.FunctionCommentSinceTag">
+		<!-- The sniff ensures that functions have a valid @since tag but skips checking experimental packages. -->


Nice!

In another PR, we should also include /pacakages/block-serialization-default-parser/.+/*\.php$.

https://github.com/WordPress/wordpress-develop/blob/b1b97f96727f137210b942f00f85fc6449033d1d/tools/webpack/packages.js#L128-L135

There are two more special core blocks:

widgets/src/blocks/legacy-widget/index.php

widgets/src/blocks/widget-group/index.php

https://github.com/WordPress/wordpress-develop/blob/b1b97f96727f137210b942f00f85fc6449033d1d/tools/webpack/blocks.js#L38-L48

@youknowriad, what do we miss here? Should we discuss in a separate issue what else we could copy from Gutenberg to the core and put under more pressure to follow WP coding standards? 😄

It is interesting that @getdave opened a discussion about this PR yesterday #59786

Yeah, so these files would be good to address next but I wonder if all the php files in Gutenberg should have these rules applied (Manual backports include a lot of copy/pasting of php files)

Thank you for your feedback, @gziolo. Your review is greatly appreciated.
I'd prefer to implement checking files in /pacakages/block-serialization-default-parser and other folders in a follow-up issue as this would allow for incremental improvements while ensuring that the enforcement of @since tags in the Block Library is already in place.

Follow-up issue: #59826

Yes, it is definitely another issue/PR. We better introduce these improvements gradually so we can start catching issues for covered files.

...hp/gutenberg-coding-standards/Gutenberg/Tests/Commenting/FunctionCommentSinceTagUnitTest.php

.../php/gutenberg-coding-standards/Gutenberg/Sniffs/Commenting/FunctionCommentSinceTagSniff.php

kebbet · 2024-03-13T10:10:15Z

What a detectives work! Great with progress on the @since-tag, and creating a sniff for it! Wow! 👏🏼

youknowriad · 2024-03-13T10:55:02Z

Awesome work here.

2. Don't check for validness of the docblock. This sniff is not designed to do it.

…0 (comment).

swissspidy

A custom sniff for enforcing @since tags sounds really useful!

I definitely encourage you to contribute this to WPCS, as this is important for WordPress core as well and even plugins and themes. It's the right thing to do.

See WordPress/WordPress-Coding-Standards#1994

anton-vlasenko · 2024-03-13T19:11:33Z

Great work. I can't believe you found all the @since releases for al those 🤯

I'm of the opinion we should ship and if we find any that are inaccurate we can modify them as needed.

I would encourage you to leave your opinion on the discussion I raised about the need to implement these exact types of automated linting in the project. It would be hugely valuable if you were able to continue down this route.

Thanks again

Thank you, @getdave!
I'm glad you think we should proceed with this PR.
I will look into the issue you mentioned.

A custom sniff for enforcing @since tags sounds really useful!

I definitely encourage you to contribute this to WPCS, as this is important for WordPress core as well and even plugins and themes. It's the right thing to do.

See WordPress/WordPress-Coding-Standards#1994

Thank you for your suggestion, @swissspidy.
I've already attempted to submit some of my other sniffs to WPCS.
However, the sniff I tried to submit was Gutenberg-specific, and I received feedback from the WPCS maintainers stating that it doesn't belong in the WPCS package (meaning it won't be useful for other WordPress plugins/sites).
I agree with that assessment.
This particular sniff is also Gutenberg-specific, as it checks whether the current block is experimental.
However, if I ever work on a sniff that is more universally applicable, I will consider submitting it to WPCS.
I think that's great idea.

swissspidy · 2024-03-13T19:34:25Z

@anton-vlasenko Yes obviously the sniffs would need to be made generic. In this instance for example there is no reason to include GB-specific stuff in it. I would just make the sniff enforce @since annotations for all functions. You can then either a) create a custom sniff in GB that extends the generic one or b) just exclude experimental block files in the config file. That seems pretty straightforward.

Agan, especially core itself would be really benefit from this and expanding the scope of the contribution would demonstrate the unity of the project instead of working in isolation. Just figured I'd chime in to share a core perspective here.

If you don't have the capacity for this I totally understand of course, so maybe we can start with leaving a comment on that issue.

getdave · 2024-03-14T09:23:34Z

Agan, especially core itself would be really benefit from this and expanding the scope of the contribution would demonstrate the unity of the project instead of working in isolation. Just figured I'd chime in to share a core perspective here.

Yes please 👍 The more we can unify the standards then the easier it will make syncing code from Gutenberg repo to the WP Core repo.

Personally I feel like exploring these opportunities would be a great use of contributor time as - based on my experience in this cycle - I see we all spend too much time on resolving coding standards issues which should be automated.

It's my opinion that we should consider them the same codebase in many ways. Therefore the more alignment we can bring the better. The more of this that is automated the better.

Again I'd love to hear from folks on #59786.

anton-vlasenko · 2024-03-19T13:58:23Z

Agan, especially core itself would be really benefit from this and expanding the scope of the contribution would demonstrate the unity of the project instead of working in isolation. Just figured I'd chime in to share a core perspective here.

@swissspidy I will think about submitting this sniff to WPCS when it becomes more stable. You've made me reconsider my viewpoint on the matter, and I agree that Core could benefit from it. Thank you for sharing your thoughts.

anton-vlasenko · 2024-03-19T14:00:18Z

Agan, especially core itself would be really benefit from this and expanding the scope of the contribution would demonstrate the unity of the project instead of working in isolation. Just figured I'd chime in to share a core perspective here.

Thank you for sharing your thoughts, @getdave. I appreciate it. I will consider submitting this sniff to WPCS after #59826 is done.

@SInCE

* Enforce @SInCE tags in block-library/src/*/*.php files Co-authored-by: anton-vlasenko <[email protected]> Co-authored-by: gziolo <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: getdave <[email protected]>

anton-vlasenko changed the title ~~Enforcec @since tag~~ Enforce @since tag in block-library/src/*/*.php files Mar 12, 2024

anton-vlasenko force-pushed the try/enforce-since-tag-sniff branch from 5121966 to 763dbb9 Compare March 12, 2024 20:17

anton-vlasenko self-assigned this Mar 12, 2024

anton-vlasenko added the [Type] Code Quality Issues or PRs that relate to code quality label Mar 12, 2024

anton-vlasenko linked an issue Mar 12, 2024 that may be closed by this pull request

Tagging all functions in the wp-includes/blocks/ directory with @since tags #55777

Closed

anton-vlasenko requested review from gziolo, hellofromtonya, ironprogrammer and azaozz March 12, 2024 20:19

anton-vlasenko marked this pull request as ready for review March 12, 2024 20:22

anton-vlasenko requested review from artemiomorales, michalczaplinski, geriux and ajitbohra as code owners March 12, 2024 20:22

anton-vlasenko commented Mar 12, 2024

View reviewed changes

anton-vlasenko changed the title ~~Enforce @since tag in block-library/src/*/*.php files~~ Enforce @since tags in block-library/src/*/*.php files Mar 12, 2024

anton-vlasenko added [Type] Enhancement A suggestion for improvement. [Package] Block library /packages/block-library and removed [Type] Enhancement A suggestion for improvement. labels Mar 12, 2024

gziolo approved these changes Mar 13, 2024

View reviewed changes

anton-vlasenko added 17 commits March 13, 2024 18:27

Add @SInCE tags.

eb64f10

Add @SInCE tags.

c1239b1

First version of the unit test for FunctionCommentSniff.

274ea9e

Commit WIP.

f90077a

Fix bug in the sniff.

882919a

1. Rename the sniff.

85a7d5c

2. Don't check for validness of the docblock. This sniff is not designed to do it.

Rename the sniff.

da4d5b3

Add a comment.

4d47446

Add @SInCE tags.

940a233

Fix CS.

78d49d3

Fix comment.

6191411

Use double quotes for consistency.

940cf56

Rename is_experimental_package() to is_experimental_block() per #5970…

b9157b1

…0 (comment).

Rename is_experimental_package() to is_experimental_block() per #5970…

b2647be

…0 (comment).

Rename is_experimental_package() to is_experimental_block() per #5970…

2b3c026

…0 (comment).

Add a comment per #59700 (comment).

6a65a13

Fix grammar.

236eb2d

anton-vlasenko force-pushed the try/enforce-since-tag-sniff branch from bb34d07 to 236eb2d Compare March 13, 2024 17:28

swissspidy reviewed Mar 13, 2024

View reviewed changes

anton-vlasenko disabled auto-merge March 13, 2024 17:52

anton-vlasenko enabled auto-merge (squash) March 13, 2024 17:53

anton-vlasenko merged commit 86b166c into trunk Mar 13, 2024
57 checks passed

anton-vlasenko deleted the try/enforce-since-tag-sniff branch March 13, 2024 18:01

github-actions bot added this to the Gutenberg 18.0 milestone Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enforce @since tags in block-library/src//.php files #59700

Enforce @since tags in block-library/src//.php files #59700

anton-vlasenko commented Mar 8, 2024 •

edited

Loading

github-actions bot commented Mar 8, 2024 •

edited

Loading

github-actions bot commented Mar 12, 2024 •

edited

Loading

anton-vlasenko commented Mar 12, 2024

anton-vlasenko Mar 12, 2024 •

edited

Loading

anton-vlasenko Mar 12, 2024

gziolo Mar 13, 2024

gziolo left a comment

gziolo Mar 13, 2024

gziolo Mar 13, 2024 •

edited

Loading

youknowriad Mar 13, 2024

anton-vlasenko Mar 13, 2024 •

edited

Loading

gziolo Mar 13, 2024

kebbet commented Mar 13, 2024

youknowriad commented Mar 13, 2024

swissspidy left a comment

anton-vlasenko commented Mar 13, 2024 •

edited

Loading

swissspidy commented Mar 13, 2024

getdave commented Mar 14, 2024

anton-vlasenko commented Mar 19, 2024

anton-vlasenko commented Mar 19, 2024

Enforce @since tags in block-library/src/*/*.php files #59700

Enforce @since tags in block-library/src/*/*.php files #59700

Conversation

anton-vlasenko commented Mar 8, 2024 • edited Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or Screencast

github-actions bot commented Mar 8, 2024 • edited Loading

github-actions bot commented Mar 12, 2024 • edited Loading

anton-vlasenko commented Mar 12, 2024

anton-vlasenko Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

anton-vlasenko Mar 12, 2024

Choose a reason for hiding this comment

gziolo Mar 13, 2024

Choose a reason for hiding this comment

gziolo left a comment

Choose a reason for hiding this comment

gziolo Mar 13, 2024

Choose a reason for hiding this comment

gziolo Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

youknowriad Mar 13, 2024

Choose a reason for hiding this comment

anton-vlasenko Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

gziolo Mar 13, 2024

Choose a reason for hiding this comment

kebbet commented Mar 13, 2024

youknowriad commented Mar 13, 2024

swissspidy left a comment

Choose a reason for hiding this comment

anton-vlasenko commented Mar 13, 2024 • edited Loading

swissspidy commented Mar 13, 2024

getdave commented Mar 14, 2024

anton-vlasenko commented Mar 19, 2024

anton-vlasenko commented Mar 19, 2024

Enforce @since tags in block-library/src//.php files #59700

Enforce @since tags in block-library/src//.php files #59700

anton-vlasenko commented Mar 8, 2024 •

edited

Loading

github-actions bot commented Mar 8, 2024 •

edited

Loading

github-actions bot commented Mar 12, 2024 •

edited

Loading

anton-vlasenko Mar 12, 2024 •

edited

Loading

gziolo Mar 13, 2024 •

edited

Loading

anton-vlasenko Mar 13, 2024 •

edited

Loading

anton-vlasenko commented Mar 13, 2024 •

edited

Loading