Document valid plugin ID format

Good question. Let's put this into the plugin system component for now to see if we can get an answer. For the moment assigning to tim.plunkett, one of the plugin system maintainers.

The plugin system only required plugin IDs to be strings. However, you are advised to make sure all IDs are valid machine names (alphanumeric, plus underscores). Note that plugin IDs contain colons when you use core's derivative decorators. This means that such plugin IDs cannot be used as config IDs, for instance, as those are used as filenames and those cannot contain colons.

Is this something we should throw an exception about in addition to documenting?

Yes, colons should only be in derivatives.
We could document this as matching the rules for function names, variables, and "other labels":

Variable names follow the same rules as other labels in PHP. A valid variable name starts with a letter or underscore, followed by any number of letters, numbers, or underscores. As a regular expression, it would be expressed thus: '[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*'

Still following, just unassigning

Updating the IS with a concrete proposal

#2920682: Add config validation for plugin IDs

The use of plugin id strings containing uppercase, spaces or special chars is causing an issue for ultimate_cron - see #2968661: Queue names can contain special characters.

> [a-z_][a-z_0-9-]*

This allows hyphens AFAICT.

Are there any cases where a form ID is derived from a plugin ID? If that's the case, then a hyphen would break form alter hooks.

We also have quite a few plugins in core that would violate the lowercase rule, e.g.:

 * @Constraint(
 *   id = "BookOutline",
 *   label = @Translation("Book outline.", context = "Validation"),
 * )

AFAICT the plugins with uppercase IDs are all either Constraint or Archiver plugins.

We can't change plugin names because of BC, so we'd need to devise a way to allow some plugin managers to opt out of the ID format validation.

There are plugins in some contrib modules that violate the "machine_name standards". For instance the SendGrid integration module has a queue worker with an id of "SendGridResendQueue". Also as a test case, I have created a queue worker with an id of "Testbed Queue: #1" which works perfectly well under the core queue subsystem.

I propose the documentation also suggests prefixing the plugin ID with module machine name to avoid ID clashing.

Looks like a bit of a stale issue?

We already use "." in custom code, and I think some other chars like "+", "|", "/", etc. could be semantically useful, especially for building up combined IDs.

Just wanted to mention the use of the slash character '/' in plugin IDs can cause issues when rendering URLs with the plugin ID, see related issue #3426088: InvalidParameterException: Parameter "queueName" for route "queue_ui.inspect" must match "[^/]++" ("queue_unique/mymodule_entity_update" given) to generate a corresponding URL, where naming a queue with the plugin ID prefixed with 'queue_unique/' ends up breaking contrib module Queue UI when it tries generating path with path: admin/config/system/queue-ui/inspect/{queueName}.

The suggestion at #15 certainly makes sense and would help preventing these types of issues.

Thanks in advance for your help and feedbacks !

What about the maximum length of a plugin id?

IMO it would be awesome if the plugin class's FQCN could be used as a plugin id because the id is just one additional magic string that you have to remember or should not forget updating if you change it in source. But that is not possible currently, the idea falling apart here and there...

Opened #3440178: Support FQCN as plugin id and reported #3440170: Forked DocParser incorrectly parses ::class notations.

This issue began as a task to improve documentation. Then from #6 it changed to discussing what characters should be allowed, such as a PHP label or a Drupal machine name. Eventually that was followed by examples in the wild of using strings that may not pass those restrictions. Given, that this should go back to simply improving the documentation.

I have updated the issue summary to return this to documenting the plugin ID.

As a doc only change I cancelled the pipeline once linting was complete.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Looking good, just a few small changes needed.

Though are we sure we don't want to say that machine name-type IDs are recommended? Hmm I guess now hooks don't form function names, it doesn't matter so much.

> Define an annotation class for your plugin type

Whoooaaa that's another bug!

@joachim, Thanks!

I tried to state in the issue summary that this issue is not for changing the docs to use attributes instead of annotations. This is solely to improve the plugin ID.

> I tried to state in the issue summary that this issue is not for changing the docs to use attributes instead of annotations. This is solely to improve the plugin ID.

Agreed -- I filed a new bug for that: #3613569: plugin docs still talk about annotations.

Changes made for the feedback from @joachim.