Problem/Motivation

When having a class with Aliased PHP Attributes attached to properties we get the following phpcs error

Namespaced classes/interfaces/traits should be referenced with use statements

<?php

use Symfony\Component\Validator\Constraints as Assert;

class Foo {

  #[Assert\NotBlank]
  /**
   * Bar property.
   *
   * @var bool
   */
  private bool $bar;

This is a very commong Symfony community drive way to add assertion using a Namespace alias Assert.

Steps to reproduce

run phpcs against

<?php

use Symfony\Component\Validator\Constraints as Assert;

/**
 * Foo class description.
 */
class Foo {

  #[Assert\NotBlank]
  /**
   * Bar property.
   *
   * @var bool
   */
  private bool $bar;

}

Workaround

A working workaround for now is to remove the namespace alias.

<?php

use Symfony\Component\Validator\Constraints\NotBlank;

/**
 * Foo class description.
 */
class Foo {

  #[NotBlank]
  /**
   * Bar property.
   *
   * @var bool
   */
  private bool $bar;

}

Proposed resolution

  • Having a sniffer that handle PHP 8.0 Attribute properly

Remaining tasks

Comments

wengerk created an issue. See original summary.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Active

Thanks for reporting!

Not sure we should do something here as the Drupal coding standards say: "PHP allows classes to be aliased when they are imported into a namespace. In general that should only be done to avoid a name collision." https://www.drupal.org/docs/develop/coding-standards/namespaces#s-class-...

In your example there is no name collision. I don't think it is good practice from Symfony to alias "Constraints" to "Assert", which can be super confusing as there is no Assert namespace anywhere.

Would like to hear more opinions on this!

wengerk’s picture

wengerk
he/him
Primary language French
Location Lausanne
commented

Many thanks for your consideration and answer.
I totally understand your point of view.
I still reckon using aliases for Attribute is a smart move, just like Symfony does. Let me proceed with why.

There's this discussion in the community (https://github.com/symfony/symfony/discussions/45525) that backs up this convention.
I'll sum up one of the responses below for easier reading:

Having alias provides additional context to what you're doing, especially when you're in a class that might be using attributes from multiple packages. Suppose you have a class User with validation constraints and Serialization, it wouldn't be all that farfetched to have a property definition like this one:

class User
{
    #[Assert\Email]
    #[Serializer\Groups(['list'])]
    public string $email;
}

With the aliased imports, you know where each of the attributes is coming from and what their purpose is (validation constraint, serialization group configuration, etc ...). If you take away the imports, there's a bit more ambiguity with each attribute:

class User
{
    #[Email]
    #[Groups(['list'])]
    public string $email;
}

Hope this helps explain why having aliases makes sense.
Keep up the awesome work with the drupal/coder!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Thanks a lot for the explanation.

I would argue that your example is an indication of a code smell. Never name a class "Email", that is a very bad name on its own as it is very ambiguous. You should not solve bad naming by using aliases all over the place when your class is used.

Yes, that would mean repeating the namespace in the class name, but that is totally fine and much better. Don't optimize your fully qualified class name for shortness; it still should convey meaning on its own.

What would be a good example for the Email class name here? In my opinion: "Symfony\Component\Validator\Constraints\EmailConstraint", then "EmailConstraint" is much clearer that this is a validation constraint.

Same for serialization groups. Is "Groups" a good class name? Probably not. I like "SerializationGroups" much more.

Doing that consistently suddenly all your aliasing problems go away because you now actually have good class names that mean something.

So much for my rant on this topic :-) I'm still open to support aliasing in Coder for attributes, since we probably can't convince the Symfony folks to use meaningful class names.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Active » Needs review

Turns out that Drush commands are also defined with partial namespaces, this caused some problems in #3483583: Drupal.Classes.FullyQualifiedNamespace sniff doesn't work with attributes and that is ok.

The conclusion is now that we don't have a standard yet, so Coder will allow any namespacing in PHP attributes.

Pull request: https://github.com/pfrenssen/coder/pull/258 , will merge this soonish.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Version: 8.3.22 » 8.3.x-dev
Status: Needs review » Fixed

The pull request was merged, thanks for the input!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.