Skip to content

Connectors: Remove redundant helper and inline sorting where needed#11227

Closed
gziolo wants to merge 9 commits intoWordPress:trunkfrom
gziolo:add/connectors-cleanup-redundant-helper
Closed

Connectors: Remove redundant helper and inline sorting where needed#11227
gziolo wants to merge 9 commits intoWordPress:trunkfrom
gziolo:add/connectors-cleanup-redundant-helper

Conversation

@gziolo
Copy link
Member

@gziolo gziolo commented Mar 11, 2026

Trac ticket: https://core.trac.wordpress.org/ticket/64791

Follow-up for #11175 (comment) (https://core.trac.wordpress.org/changeset/61943).

Summary

  • Removes the private _wp_connectors_get_connector_settings() wrapper, which only called wp_get_connectors() and applied ksort(). Callers now use wp_get_connectors() directly, with the sort applied in _wp_connectors_get_connector_script_module_data() — the only place where ordering matters.
  • Expands the WordPress-style @return array shape for both wp_get_connector() and wp_get_connectors(), documenting all fields including logo_url, authentication, and plugin.
  • Adds @phpstan-return inline array shapes to wp_get_connector() and wp_get_connectors() using non-empty-string and literal union types ('ai_provider', 'api_key'|'none') for improved static analysis. Note: @phpstan-import-type is not used due to a known PHPStan limitation.
  • Makes logo_url and credentials_url truly optional (no longer stored as null when absent), consistent with how optional fields are handled elsewhere in register().
  • Simplifies get_all_registered() @return to a one-liner, avoiding a duplicate shape on an internal method; @phpstan-return array<string, Connector> covers static analysis.
  • Renames the test class from wpConnectorsGetConnectorSettings to wpGetConnectors, targeting wp_get_connectors() directly.

Test plan

  • Run npm run test:php -- --filter=connectors and confirm all 53 tests pass.
  • Run npm run env:composer -- lint -- src/wp-includes/connectors.php src/wp-includes/class-wp-connector-registry.php and confirm no PHPCS violations.
  • Run npm run typecheck:php and confirm no new PHPStan errors.

🤖 Generated with Claude Code

@github-actions
Copy link

github-actions bot commented Mar 11, 2026

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.

Core Committers: Use this line as a base for the props when committing in SVN:

Props gziolo, westonruter.

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

@github-actions
Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@gziolo
Copy link
Member Author

gziolo commented Mar 11, 2026

@westonruter, I would appreciate your guidance on how to handle PHPDoc and PHPStan for wp_get_connectors() that wraps the same method from get_all_registered from WP_Connector_Registry.

@gziolo
Copy link
Member Author

gziolo commented Mar 11, 2026

@westonruter, thank you for all the insights. I still need to do a round of cleanup to align PHPDoc shapes for the connector array.

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

Pre-LGTM pending the array shapes updates in phpdoc.

@gziolo
Copy link
Member Author

gziolo commented Mar 11, 2026

It's really hard to draw the line of when to include the entire shape of the return value. Maybe it's enough to combine the PHPStan type and a very generic array as @return. The only place where we must document the entire shape is WP_Connectors_Registry::register. That's something we can decide on later, though.

gziolo and others added 9 commits March 12, 2026 07:07
Removes the private `_wp_connectors_get_connector_settings()` wrapper,
which only called `wp_get_connectors()` and applied `ksort()`. Callers
now use `wp_get_connectors()` directly, and the alphabetical sort is
applied in `_wp_connectors_get_connector_script_module_data()` just
before the data is passed to JavaScript, which is the only place
where ordering matters.

Expands the `@return` PHPDoc on `wp_get_connectors()` with the full
array shape that was previously documented on the removed helper.

Updates tests accordingly: renames `wpConnectorsGetConnectorSettings`
to `wpGetConnectors` targeting `wp_get_connectors()` directly.

Props gziolo.
Fixes #64791.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…stered().

Expands the terse `@return array<string, array>` on
`WP_Connector_Registry::get_all_registered()` to the full WordPress-style
array shape, matching `wp_get_connectors()`. Adds
`@phpstan-import-type Connector from WP_Connector_Registry` and
`@phpstan-return array<string, Connector>` to `wp_get_connectors()`,
matching the PHPStan annotation already present on the class method.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The `logo_url?: string|null` field present in the PHPStan `Connector`
type was absent from the WordPress-style `@return` array shape on both
`wp_get_connectors()` and `WP_Connector_Registry::get_all_registered()`.
Adds it after `description` to match the PHPStan field order, and
adjusts column alignment across all sibling `@type` entries.

Props gziolo.
Fixes #64791.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Removes the duplicate untyped `@param array` tag from
`_wp_connectors_get_connector_script_module_data()` left over after
applying the `@param array<string, mixed>` suggestion.

In `wpGetConnectors.php`, adds `void` return types to all test methods
for PHPStan level 8 compliance, and uses `?? null` when accessing
optional array keys (`setting_name`, `credentials_url`) so PHPStan
does not complain about potentially missing offsets.

Props gziolo, westonruter.
Fixes #64791.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
- Expand @return shape for wp_get_connector() to match wp_get_connectors()
- Use non-empty-string and literal types in @phpstan-return annotations
- Remove |null from optional fields (logo_url, credentials_url); store
  them only when non-empty, consistent with the existing logo_url pattern
- Fix PHPDoc alignment across modified blocks
- Update test to reflect credentials_url being truly optional

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ation.

- Simplify get_all_registered() @return to a one-liner, keeping @phpstan-return
  for static analysis — avoids maintaining a duplicate expanded shape on an
  internal method marked "do not use directly"
- Order $authentication before $plugin consistently across all PHPDoc blocks,
  matching the actual construction order in register()
- Fix @return array|null typo on get_all_registered() which never returns null

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gziolo gziolo force-pushed the add/connectors-cleanup-redundant-helper branch from fadd350 to f22aae6 Compare March 12, 2026 06:07
@@ -18,11 +18,11 @@
* @phpstan-type Connector array{
* name: string,
Copy link
Member

Choose a reason for hiding this comment

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

Just noting this is not consistent with the return value of wp_get_connector() which uses non-empty-string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, I'm updating in the svn commit directly.

pento pushed a commit that referenced this pull request Mar 12, 2026
Remove `_wp_connectors_get_connector_settings()` and inline `ksort()`
into `_wp_connectors_get_connector_script_module_data()`. Expand
`@return` and `@phpstan-return` array shapes for `wp_get_connector()`
and `wp_get_connectors()`. Make `logo_url` and `credentials_url` truly
optional. Rename test class to `wpGetConnectors`.

Developed in #11227.

Follow-up to [61943].

Props gziolo, westonruter.
Fixes #64791.



git-svn-id: https://develop.svn.wordpress.org/trunk@61983 602fd350-edb4-49c9-b593-d223f7449a82
@github-actions
Copy link

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 61983
GitHub commit: 09f7588

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.

@github-actions github-actions bot closed this Mar 12, 2026
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Mar 12, 2026
Remove `_wp_connectors_get_connector_settings()` and inline `ksort()`
into `_wp_connectors_get_connector_script_module_data()`. Expand
`@return` and `@phpstan-return` array shapes for `wp_get_connector()`
and `wp_get_connectors()`. Make `logo_url` and `credentials_url` truly
optional. Rename test class to `wpGetConnectors`.

Developed in WordPress/wordpress-develop#11227.

Follow-up to [61943].

Props gziolo, westonruter.
Fixes #64791.


Built from https://develop.svn.wordpress.org/trunk@61983


git-svn-id: http://core.svn.wordpress.org/trunk@61265 1a063a9b-81f0-0310-95a4-ce76da25c4cd
@gziolo gziolo deleted the add/connectors-cleanup-redundant-helper branch March 12, 2026 08:05
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