Problem/Motivation

As discovered by @xjm in #3327853-34: [10.0.x and 9.5.x backports] Don't allow {@inheritDoc} annotation in PHPDocBlocks there are a few occurrences of @inheritdoc around (Note the missing curly braces).

Since we already have Slevomat rules to check for the unwanted {@inheritDoc} and @inheritDoc, let's add another one to check for @inheritdoc.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 3331633-12-9.5.x.patch1.3 KBlongwave
#6 3331633.10.1.x-3.patch1.73 KBspokje
#6 3331633.10.0.x-3.patch1.37 KBspokje
#5 3331633-test-only.patch369 bytesspokje
#3 3331633.10.0.x-3.patch1.37 KBspokje
#3 3331633.10.1.x-3.patch1.73 KBspokje
#2 3331633-test-only.patch1.01 KBspokje

Comments

Spokje created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new 3331633-test-only.patch1.01 KB
spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new 3331633.10.1.x-3.patch1.73 KB
new 3331633.10.0.x-3.patch1.37 KB
spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Needs review
spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new 3331633-test-only.patch369 bytes
spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new 3331633.10.0.x-3.patch1.37 KB
new 3331633.10.1.x-3.patch1.73 KB

Re-uploading the original, unchanged patches from #3 to make sure that if we reach RTBC with this one, the bi-daily retesting doesn't fail on the (correctly) failing test-only patch.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Thanks!

  • catch committed abd595d5 on 10.1.x 
    Issue #3331633 by Spokje: Don't allow @inheritdoc (no curly braces)...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x, thanks!

Status: Fixed » Closed (fixed)

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

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Closed (fixed) » Patch (to be ported)

Reopening this for backport, as the 10.0.x and 9.5.x updated deps jobs are failing:

FILE: ...rupal/KernelTests/Core/Database/DriverSpecificKernelTestBase.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 27 | ERROR | [x] @inheritdoc found. Did you mean {@inheritdoc}?
    |       |     (Drupal.Commenting.DocComment.InheritDocWithoutBraces)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...pal/KernelTests/Core/Database/DriverSpecificDatabaseTestBase.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 14 | ERROR | [x] @inheritdoc found. Did you mean {@inheritdoc}?
    |       |     (Drupal.Commenting.DocComment.InheritDocWithoutBraces)
 19 | ERROR | [x] @inheritdoc found. Did you mean {@inheritdoc}?
    |       |     (Drupal.Commenting.DocComment.InheritDocWithoutBraces)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 10.1.x-dev » 9.5.x-dev
Status: Patch (to be ported) » Needs review
StatusFileSize
new 3331633-12-9.5.x.patch1.3 KB

Sent #6 for another run on 10.0.x, here's a 9.5.x patch.

spokje’s picture

spokje
Primary language Dutch
commented

Looks like the original patch was only committed to 10.1.x.

Also Coder now seems to check for it in the latest release, which is the reason tests now fails on updated deps runs on 9.5.x and 10.0.x.

We also get the "interesting" situation that 10.1.x does the checks using a Slevomat rule, whilst the checks in 9/10 are done through a coder sniff.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

@Spokje I guess once we update deps in 10.1.x it will be done by both rules? So it doesn't really matter that much I think, we could clean up one but there are far more important things to work on.

spokje’s picture

spokje
Primary language Dutch
commented

@longwave That should work (probably), and agreed there's much more important work to do.
Just wanted to mention it (and think that test-only changes should always be backported, but that's a different issue)

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

Ah, forgot the most important part of my previous comments: RTBC

  • catch committed 3fd342d7 on 9.5.x 
    Issue #3331633 by Spokje, longwave, catch: Don't allow @inheritdoc (no...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Whoops didn't realise this needed backporting. Committed/pushed the 10.0.x and 9.5.x patches, thanks!

  • catch committed 225378cd on 10.0.x 
    Issue #3331633 by Spokje, longwave, catch: Don't allow @inheritdoc (no...

Status: Fixed » Closed (fixed)

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