Drupal.Classes.FullyQualifiedNamespace sniff doesn't work with attributes

Thanks for reporting! Can you post some test code where this triggers? Then we can try to reproduce here.

I've tried this again with Action Link plugins. Here's my code after using Rector and adding linebreaks so each unimported class is on its own line:

<?php

namespace Drupal\action_link_test_plugins\Plugin\StateAction;

use Drupal\action_link\Entity\ActionLinkInterface;
use Drupal\action_link\Plugin\StateAction\StateActionBase;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Session\AccountInterface;

/**
 * Test action which is always usable.
 */
#[\Drupal\action_link\Attribute\StateAction(
  id: 'test_always',
  label: new \Drupal\Core\StringTranslation\TranslatableMarkup('Test Always'),
  description: new \Drupal\Core\StringTranslation\TranslatableMarkup('Test Always'),
  directions: [
    'change' => 'change',
  ]
)]
class TestAlways extends StateActionBase {
SNIP

phpcs:

vendor/bin/phpcs --standard=Drupal  -s --sniffs=Drupal.Classes.FullyQualifiedNamespace web/modules/contrib/action_link/tests/modules/action_link_test_plugins/src/Plugin/StateAction/TestAlways.php

FILE: /Users/joachim/Sites/drupal-contrib/web/modules/contrib/action_link/tests/modules/action_link_test_plugins/src/Plugin/StateAction/TestAlways.php
-------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------
 15 | ERROR | [x] Namespaced classes/interfaces/traits should be referenced with use statements
    |       |     (Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing)
 16 | ERROR | [x] Namespaced classes/interfaces/traits should be referenced with use statements
    |       |     (Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing)
-------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------------------------------------------------------

It's finding the two uses of Drupal\Core\StringTranslation\TranslatableMarkup, but not the attribute class.

Similarly, phpcbf imports Drupal\Core\StringTranslation\TranslatableMarkup but doesn't change the attribute class.

Thanks, pushed a fix!

This is breaking Drush command classes where we use to have partially namespaces:

#[CLI\Command(name: 'cas:set-cas-username')]

This is breaking Drush command classes where we use to have partially namespaces:

Yes, I now have upwards of 50 PHPCS failures in some of my projects because of this.

• As of 7 Jan PHPCS jobs on GitLabCI reported no errors for my Drush command classes. (See https://git.drupalcode.org/project/regcode/-/jobs/3931521)
• As of 8 Jan I now get many of these errors: Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing (See https://git.drupalcode.org/project/regcode/-/jobs/3947317)

Specifically, I have a lot of Drush commands across a lot of projects. I converted them all to using Attributes a while back.

The Drush standard usage and all the Drush core commands and documentation use this pattern to define Drush commands:

use Drush\Attributes as CLI;

final class RulesDrushCommands extends DrushCommands {

  #[CLI\Command(name: 'rules:list', aliases: ['rules-list', 'rlst'])]
  #[CLI\Help(description: 'Lists all the active and inactive rules for your site.')]
  #[CLI\Argument(name: 'type', description: "Type of Rule. Either 'rule' or 'component'.")]
  #[CLI\Usage(name: 'drush rules:list', description: 'Lists both Reaction Rules and Rules Components.')]
  #[CLI\Usage(name: 'drush rules:list component', description: 'Lists only Rules Components.')]
  #[CLI\Usage(name: 'drush rules:list --fields=machine-name', description: 'Lists just the machine names.')]
  #[CLI\Usage(name: 'drush rules:list --fields=machine-name --pipe', description: 'Outputs machine names in a format suitable for piping.')]
  #[CLI\FieldLabels(labels: ['machine-name' => 'Rule', 'label' => 'Label', 'event' => 'Event', 'active' => 'Active'])]
  #[CLI\DefaultFields(fields: ['machine-name', 'label', 'event', 'active'])]
  public function listAll(string $type = '', array $options = ['format' => 'table', 'fields' => '']): RowsOfFields {
    :
    :
  }

But this now fails PHPCS testing.

Changing this in all my Drush commands will mean several hundred lines of code need to be changed, and this will be a persistent problem because as I said all the core Drush commands which people use as examples, and all the Drush documentation, uses this pattern with is now deemed invalid.

Very sorry about this flagging Drush attributes. I was hoping to catch this error before the Coder release by running it on an internal codebase, but the composer install of Coder 8.3.x-dev installed an outdated version. Note to self: never trust composer with dev versions and verify that it actually has installed the latest commit.

Now back to the issue about fully qualified names in PHP attributes: The usual Drupal coding standards forbid fully or partially qualified names. "Specify a single class per use statement. Do not specify multiple classes in a single use statement." https://www.drupal.org/docs/develop/coding-standards/namespaces →

So the coding standards in PHP attributes from Drush violate the Drupal coding standards by using partially qualified names.

However, since this Drush standard is established and used so widely I think that we should revert this issue. Coder is technically correct, but I think we need a coding standards discussion first. Then we should write down PHP attributes coding standards, especially if fully or partially namespaced classes are allowed there.

Another reason to revert this is Symfony. They do weird aliasing with use statements as well, see 🐛 False Postive: Aliased PHP 8.0 Attributes on Class properties leading to "Namespaced classes/interfaces/traits should be referenced with use statements" Active .

So for now I think PHP attributes are undefined and lawless territory regarding use statements and namespaces, so Coder should not flag it.

Sorry Joachim, at least you can use the fixer in Coder version 8.3.27.

Workarounds for now:
use the Slevomat sniff:

  <rule ref="Drupal">
    <!-- @todo Disabling this sniff until PHP attributes are ignored. -->
    <exclude name="Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing" />
  </rule>
  
  <!-- @todo Using this sniff while
    Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing is disabled. -->
  <rule ref="SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly">
    <properties>
      <property name="allowFullyQualifiedGlobalClasses" value="true"/>
      <property name="allowFullyQualifiedGlobalFunctions" value="true"/>
      <property name="allowFullyQualifiedGlobalConstants" value="true"/>
      <property name="allowFallbackGlobalFunctions" value="true"/>
      <property name="allowFallbackGlobalConstants" value="true"/>
    </properties>
  </rule>
  <rule ref="SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedName">
    <exclude-pattern>*.api.php</exclude-pattern>
  </rule>
  <rule ref="SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly.ReferenceViaFullyQualifiedNameWithoutNamespace">
    <severity>0</severity>
  </rule>

I will check how to revert and cover this with test cases.

And here is the revert with more test cases: https://github.com/pfrenssen/coder/pull/258

Let me know if you have any objections, will merge this soonish.

> Sorry Joachim, at least you can use the fixer in Coder version 8.3.27.

Thanks! :)

I can report it works perfectly for converting plugins to us attributes! :)

What's the reasoning for the partial namespace import for Drush command attributes? Why not import the whole thing, or alias them if there's risk of a name collision?

> What's the reasoning for the partial namespace import for Drush command attributes? Why not import the whole thing, or alias them if there's risk of a name collision?

I'd say because the attribute name themself are very generic (Command, Help, Usage, ...) and CLI gives them context. Technically it works correctly, but it's just against the common usage that drush itself has established, so maybe it just needs an exception for that.

Also, it sometimes takes a tall wall of Attributes to specify a command. That wall gets unnecessarily wide if you need a full namespace in every line. That would be so tedious to read.