The following valid deprecation fails:

 * @deprecated in commerce:8.x-2.7 and will be removed from commerce:9.x-3.0.
 * Use CurrencyFormatterInterface instead.
 * @see https://www.drupal.org/node/2971732</blockquote>

With the following warnings:

-----------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 1 LINE
-----------------------------------------------------------------------------
 8 | WARNING | The deprecation in-version 'commerce:8.x-2.0' does not match the standard: drupal:n.n.n or project:n.x-n.n
 8 | WARNING | The deprecation removal-version 'commerce:9.x-3.0' does not match the standard: drupal:n.n.n or project:n.x-n.n
-----------------------------------------------------------------------------

That's caused by the regexp for matching the project version that's missing a + quantifier.

Comments

jsacksick created an issue. See original summary.

jsacksick’s picture

Status: Active » Needs review
jonathan1055’s picture

Hi jsacksick,
Thanks for reporting this. I don't know how I missed that qualifier when I was writing the sniff, as I had my own local file of test case passes which included that type of case. And it fails now!

I confirm that that the addition of + fixes the problem (and also my local test data). The addition of the test case shows that this is now allowed and does not raise the warning.

Jonathan

jonathan1055’s picture

Title: Project version is not correctly matched in deprecated checks. » Contrib project version is not correctly matched in @deprecated tag
Issue summary: View changes
Related issues: +#2908391: Add a rule for expected format of @deprecated and @trigger_error() text

Renamed the issue title as this only affects contrib deprecations not core deprecations.

I have just worked out how I missed this case. The regex was originally
preg_match('/^\w+:\d{1,2}\.x\-\d{1,2}\.\d{1,2}$/', $version)
which was working fine, but was too generous as it allowed uppercase. I had to change it to force all lower-case, so it became
preg_match('/^[a-z\d_]:\d{1,2}\.x\-\d{1,2}\.\d{1,2}$/', $version)
and that's where the + got lost. I knew it was working OK originally :-)

  • klausi committed 641f087 on 8.x-3.x authored by jsacksick
    fix(Deprecated): Fix project version not correctly matched in deprecated...
klausi’s picture

Status: Needs review » Fixed

Thanks a lot for the quick follow up!

You 2 are definitely my favourite 2 Johnathans this month, contributing so generously here :-)

jonathan1055’s picture

Hey! I did not know that jsacksick was also a Jonathan. Good work.

Thanks Klausi for merging in the fix quickly too.

wim leers’s picture

Just ran into the same problem at #2870435: Support additional stream wrappers. The fix is not yet in a release.

wim leers’s picture

klausi’s picture

We have a bit of a fallout with this rule like in #3050416: @see for @deprecated tags too strict., so I would wait for another 1-2 weeks if other problems come up that we need to fix - and then make a release.

wim leers’s picture

👍 Thanks for the info!

gábor hojtsy’s picture

There is also #3024461: Adopt consistent deprecation format for core and contrib deprecation messages where there is again no agreement on the right formats, so either coder needs to roll back the checks or we need more eyes on #3024461: Adopt consistent deprecation format for core and contrib deprecation messages to agree on consistent formats for trigger_error() and @deprecated.

Status: Fixed » Closed (fixed)

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