Problem/Motivation

Since PHP 8.0 native attributes introduced https://www.php.net/manual/en/language.attributes.overview.php
Working on PHP 8.2 compatibility it's required to by-pass #[\AllowDynamicProperties] before class names see #3275858-4: View's ResultRow uses deprecated dynamic properties

Steps to reproduce

see https://www.drupal.org/pift-ci-job/2363989

Adding the attribute before class makes coder fail

 /**
  * A class representing a view result row.
  */
 #[\AllowDynamicProperties]
 class ResultRow {

Missing class doc comment (Drupal.Commenting.ClassComment.Missing)

Proposed resolution

Fix ClassCommentSniff to correctly skip over PHP attributes.

Remaining tasks

Create a pull request on Github and add a test case

User interface changes

no

API changes

no

Issue fork coder-3292573

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#3275858: View's ResultRow uses deprecated dynamic properties
andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: » #3275851: [META] Fix PHP 8.2 dynamic property deprecations
andypost’s picture

andypost
he/him
Primary language Russian
commented

Related issue is https://github.com/squizlabs/PHP_CodeSniffer/issues/3489

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Closed (works as designed)

This is phpstan issue (

TwoD made their first commit to this issue’s fork.

TwoD opened merge request !16

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Closed (works as designed) » Needs review

Thank you! It looks promising to allow checking core attributes

twod’s picture

twod
Primary language Swedish
Location Sweden
commented

Wow, you're fast hehe! I was just previewing the comment below:

I'm reopening this because it seems we can work around it in the sniffs.
Phpstan accepts this just fine so not sure why that was mentioned in #5.

I tested this with squizlabs/php_codesniffer 3.7.1 and now it correctly ignores attributes.

/**
 * Some class.
 */
#[Attrib1]
#[Attrib2]
class MyClass() {
}

(including multi-line attribute containers).

Whitespace errors are reported and fixable for newlines both before and after the attribute(s).

/**
 * Some class.
 */

#[Attrib1]
#[Attrib2]

class MyClass() {
}
andypost’s picture

andypost
he/him
Primary language Russian
commented

Now it needs a sniff to warn about #[\AllowDynamicProperties] attribute (we have in core for 5 classes atm) to catch it

andypost’s picture

andypost
he/him
Primary language Russian
commented

Since 3.6.3 phpcs works properly with attributes
https://github.com/squizlabs/PHP_CodeSniffer/issues/3448#event-5446855243

twod’s picture

twod
Primary language Swedish
Location Sweden
commented

I could perhaps take a stab at adding that if it's not already in progress somewhere else, but would that not be a separate issue?

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs tests

Thanks, updating PHPCS is a good idea, we can also set a new minimum version in our composer.json for the next release.

Please create a pull request against https://github.com/pfrenssen/coder so that we see our test cases run. Please also add a test case in ClassCommentUnitTest.inc so that we verify the fix.

klausi closed merge request !16

klausi’s picture

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

This was already fixed at #3322615: `Drupal.Commenting.ClassComment.Missing` Appears when Using PHP 8 Class Attributes, closing as duplicate.