Add attribute sniffs

[michnovka]

Add new attribute sniffs to doctrine standard

[derrabus]

Our test fixtures should be updated to reflect the proposed change.

[michnovka]

@derrabus have a look if I did it properly

[simPod]

You'll also need to adapt the report summary

A TOTAL OF 429 ERRORS AND 0 WARNINGS WERE FOUND IN 46 FILES

and all the patches

[kukulich]

I think SlevomatCodingStandard.Attributes.RequireAttributeAfterDocComment should be added too so PHPStan (psalm maybe too) works as expected, see nikic/PHP-Parser#762

[michnovka]

is there some tool which generates the patches? seems tedious to do this by hand...

[michnovka]

I am trying the makefile update-compatibility-patch-XX but this gives me

error: patch failed: tests/expected_report.txt:42
error: tests/expected_report.txt: patch does not apply
make: *** [/home/superuser/scripts/doctrine-coding-standard/Makefile:12: update-compatibility-patch-72] Error 1

[michnovka]

I am sorry, I tried to make the patches work, but I only created mess. I have forced pushed back the latest version which has expected_report.txt for PHP8.1 working fine, please somebody do the dirty work and finish the patches for 7.2-8.0.

@kukulich I have also added the SlevomatCodingStandard.Attributes.RequireAttributeAfterDocComment and relevant test

[greg0ire]

Yeah, updating the patches is a pain, I'll give it a try. Also, I'll retarget to 11.0, which is the correct branch according to https://www.doctrine-project.org/projects/doctrine-coding-standard/en/stable/reference/index.html#testing

[greg0ire]

Also, I'll retarget to 11.0,

Ugh I should have started with that 🤦

[simPod]

I wonder why there are so many changes 🤔 something is wrong. Should I clean it up?

[greg0ire]

Maybe what I did was wrong? How do you usually proceed? What I did was:

1. edit the patch to remove the hunk about the expected report, so that it applies
2. apply the patch
3. set the correct php version in phpcs.xml.dist
4. run vendor/bin/phpcs $(find tests/input/* | sort) --report=summary --report-file=tests/expected_report.txt

[simPod]

Depends, I sometimes do it by hand. I have re-applied patches on 11.x branch and there are little changes only.

I guess this must be run on related php version vendor/bin/phpcs $(find tests/input/* | sort) --report=summary --report-file=tests/expected_report.txt

[michnovka]

@simPod I experienced the same issue, this is why I asked for somebody to finish it. Yes, it seems PHP version running the phpcs is relevant. I guess it also installs different versions of composer dependencies, especially for e.g. 7.2

[simPod]

I believe nobody here likes the patches but that's the best we've got. I'll look into it.

[kukulich]

Yes, there are sniffs that are enabled/disabled by detected PHP version - and it’s intentional here.

[greg0ire]

I forced the php version in step 3 of my process (otherwise the tests would be red)

[simPod]

I have opened in as #297 #298 since I cannot push here. We can either merge that (I kept the author) or someone with permissions do some commit shuffling.

[greg0ire]

You mean #298

[simPod]

Yes. I have done +1 but somebody opened a PR meanwhile :D