Missing check for ordering of comment tags [#2605986]

Problem/Motivation

The software does not report problems with comment tag ordering e.g. when a @throws tag occurs before a @return tag in the function comment block (see issue #2599524: Fixing order of documentation sections for /core/lib/Drupal for an example in D8 core).

Proposed resolution

The proposed resolution is to enhance DocCommentSniff.php to check the ordering of tags.

Comment	File	Size	Author
#4	2605986-4-add-check-for-order.patch	2.32 KB	blackra
#2	2605986-2-add-check-for-order.patch	1.79 KB	blackra

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

1 November 2015 at 22:39

blackra created an issue. See original summary.

Comment #2

blackra CreditAttribution: blackra as a volunteer commented 2 November 2015 at 00:41

Title:	No check for ordering of comment tags	» Missing check for ordering of comment tags
Issue summary:	View changes
Status:	Active	» Needs review

File	Size
2605986-2-add-check-for-order.patch	1.79 KB

I have updated the issue summary and uploaded a patch. The patch has been manually tested against some of the files affected by issue #2599524: Fixing order of documentation sections for /core/lib/Drupal before and after the fix for that issue.

Comment #3

blackra CreditAttribution: blackra as a volunteer commented 2 November 2015 at 18:16

Status:

Needs review

» Needs work

I have found a problem with this patch. It does not pick up tags followed by punctuation.

Example:

@todo: Fix something

I don't know what doxygen does in these circumstances either, but the ordering is independent of whether this is a permitted construction.

Comment #4

blackra CreditAttribution: blackra as a volunteer commented 3 November 2015 at 06:28

Status:

Needs work

» Needs review

File	Size
2605986-4-add-check-for-order.patch	2.32 KB

1 file was hidden/shown/deleted

File	Size
2605986-2-add-check-for-order.patch	1.79 KB

Here is a patch that deals with @todo:

Comment #5

klausi

he/him||they/them

German

🇦🇹 Vienna

CreditAttribution: klausi commented 6 November 2015 at 15:21

Status:

Needs review

» Needs work

+++ b/coder_sniffer/Drupal/Sniffs/Commenting/DocCommentSniff.php
@@ -451,7 +451,22 @@ class Drupal_Sniffs_Commenting_DocCommentSniff implements PHP_CodeSniffer_Sniff
+        // The tags for which the relative order is defined

Inline comments should end with a dot.

+++ b/coder_sniffer/Drupal/Sniffs/Commenting/DocCommentSniff.php
@@ -451,7 +451,22 @@ class Drupal_Sniffs_Commenting_DocCommentSniff implements PHP_CodeSniffer_Sniff
+            '@see',
+            '@todo',
+            '@Plugin',

not sure @Plugin should be in here, since that is an annotation, not a doc tag. I think we should remove that for now.

+++ b/coder_sniffer/Drupal/Sniffs/Commenting/DocCommentSniff.php
@@ -465,6 +480,32 @@ class Drupal_Sniffs_Commenting_DocCommentSniff implements PHP_CodeSniffer_Sniff
+            }
+            else {

we use PHPCS coding standards for our sniffs to be more in line with PHPCS upstream code so the else should be on the same line as }.

+++ b/coder_sniffer/Drupal/Sniffs/Commenting/DocCommentSniff.php
@@ -465,6 +480,32 @@ class Drupal_Sniffs_Commenting_DocCommentSniff implements PHP_CodeSniffer_Sniff
+            if (array_key_exists($baseTagName, $orderedTags)) {

why array_key_exists()? Can we use isset() instead?

+++ b/coder_sniffer/Drupal/Sniffs/Commenting/DocCommentSniff.php
@@ -465,6 +480,32 @@ class Drupal_Sniffs_Commenting_DocCommentSniff implements PHP_CodeSniffer_Sniff
+                }
+                else {

else here again.

Then we need test cases. We need to check if we have covered this well enough in good.php. And we need additional checks in the DocCoomentUnitTest* files.