From 32622cc68a25552479c8d15b4ce8aff6fad8e9ed Mon Sep 17 00:00:00 2001 From: mscherer Date: Sat, 9 May 2026 18:37:24 +0200 Subject: [PATCH 1/2] Add inner-bucket tag ordering to DocBlockTagOrderSniff Extends the sniff with an opt-in innerOrder property that sorts tags within a tag-type bucket (e.g. ordering of method tags within the method group) using user-configured prefix patterns plus alphabetical tiebreaker. Default is empty - existing behavior unchanged unless configured. Two recipes covered by tests: - method tag CRUD ordering: prefix list lifts well-known operations (newEmptyEntity, newEntity, get, save, ...) to the top in a configured order, custom methods float to the bottom alphabetically. - property tag alphabetization: empty pattern list sorts the entire bucket alphabetically, which is the typical recipe for association lists generated by IDE helpers. Subject extraction handles return-type expressions with generics and union types, the static modifier, missing return types, and trailing- bareword malformed lines. Errors use a separate InnerOrderInvalid code so they can be suppressed independently from the existing OrderInvalid bucket-level errors. --- .../Commenting/DocBlockTagOrderSniff.php | 371 +++++++++++++++++- .../Commenting/DocBlockTagOrderSniffTest.php | 79 ++++ .../DocBlockTagOrder/inner-combined.after.php | 19 + .../inner-combined.before.php | 19 + .../inner-method-edges.after.php | 18 + .../inner-method-edges.before.php | 18 + .../DocBlockTagOrder/inner-method.after.php | 23 ++ .../DocBlockTagOrder/inner-method.before.php | 23 ++ .../DocBlockTagOrder/inner-property.after.php | 17 + .../inner-property.before.php | 17 + 10 files changed, 595 insertions(+), 9 deletions(-) create mode 100644 tests/_data/DocBlockTagOrder/inner-combined.after.php create mode 100644 tests/_data/DocBlockTagOrder/inner-combined.before.php create mode 100644 tests/_data/DocBlockTagOrder/inner-method-edges.after.php create mode 100644 tests/_data/DocBlockTagOrder/inner-method-edges.before.php create mode 100644 tests/_data/DocBlockTagOrder/inner-method.after.php create mode 100644 tests/_data/DocBlockTagOrder/inner-method.before.php create mode 100644 tests/_data/DocBlockTagOrder/inner-property.after.php create mode 100644 tests/_data/DocBlockTagOrder/inner-property.before.php diff --git a/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php b/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php index 9042f3e..2039c83 100644 --- a/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php +++ b/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php @@ -65,6 +65,48 @@ class DocBlockTagOrderSniff extends AbstractSniff '@mixin', ]; + /** + * Within-tag ordering: ordered list of name prefixes per tag type. + * + * Defaults empty - no inner ordering is applied unless configured. + * + * Configurable via XML: + * + * ``` xml + * + * + * + * + * + * + * + * + * + * + * ``` + * + * Each value is a comma-separated list of prefix patterns. A trailing `*` matches any + * suffix; a pattern without `*` matches only the exact subject. Tags matching no pattern + * float to the bottom of their bucket. Within each score class (matched-by-same-pattern, + * or all-unmatched), tags are sorted alphabetically by extracted subject. + * + * An empty value means "no prefix priority - alphabetize everything in this bucket," + * which is the typical recipe for `@property` association lists. + * + * Subject extraction per tag: + * - `@method` -> method name (after optional `static` + return type) + * - `@property[-read|-write]` -> variable name minus leading `$` + * - `@mixin`/`@extends`/`@implements` -> last segment of the FQCN + * - `@template` -> template name token + * + * Note: PHPCS may convert empty XML `value=""` into `null` when populating array + * properties; both forms are normalized to "no prefixes, alphabetize the bucket." + * + * @var array|null> + */ + public array $innerOrder = []; + /** * @inheritDoc */ @@ -100,10 +142,61 @@ public function process(File $phpcsFile, $stackPtr): void $tags = $this->readTags($phpcsFile, $docBlockStartIndex, $docBlockEndIndex); $tags = $this->checkAnnotationTagOrder($tags, $order); + if (!$isFunction) { + $tags = $this->checkInnerOrder($tags); + } $this->fixOrder($phpcsFile, $docBlockStartIndex, $docBlockEndIndex, $tags, $order); } + /** + * Marks tags that violate within-bucket inner ordering with an `innerError` flag. + * + * Walks tags grouped by tag-type bucket. Within each bucket whose tag is configured + * in `$innerOrder`, computes a sort key (score, subject) and flags every tag whose + * key is less than its predecessor's. + * + * @param array> $tags + * + * @return array> + */ + protected function checkInnerOrder(array $tags): array + { + if (!$this->innerOrder) { + return $tags; + } + + $byBucket = []; + foreach ($tags as $i => $tag) { + $byBucket[$tag['tag']][] = $i; + } + + foreach ($byBucket as $tagName => $indexes) { + if (!array_key_exists($tagName, $this->innerOrder)) { + continue; + } + $prefixes = $this->normalizeInnerPrefixes($this->innerOrder[$tagName]); + + $previousKey = null; + foreach ($indexes as $idx) { + $value = ltrim(substr((string)$tags[$idx]['content'], strlen($tagName))); + $subject = $this->extractSubject($tagName, $value); + $score = $subject === null ? PHP_INT_MAX : $this->scoreSubject($subject, $prefixes); + $key = [$score, (string)$subject]; + if ($previousKey !== null && $key < $previousKey) { + $tags[$idx]['innerError'] = sprintf( + 'Inner order of %s tag wrong (subject "%s").', + $tagName, + (string)$subject, + ); + } + $previousKey = $key; + } + } + + return $tags; + } + /** * @param array> $tags * @param array $orderList @@ -247,19 +340,40 @@ protected function getContent(array $tokens, int $start, int $end): string */ protected function fixOrder(File $phpcsFile, int $docBlockStartIndex, int $docBlockEndIndex, array $tags, array $orderList): void { - $errors = []; + $bucketErrors = []; + $innerErrors = []; foreach ($tags as $i => $tag) { if (isset($tag['error'])) { - $errors[$i] = $tag['error']; + $bucketErrors[$i] = $tag['error']; + } + if (isset($tag['innerError'])) { + $innerErrors[$i] = $tag['innerError']; } } - if (!$errors) { + if (!$bucketErrors && !$innerErrors) { return; } - $fix = $phpcsFile->addFixableError('Invalid order of tags: ' . implode(', ', $errors), $docBlockEndIndex, 'OrderInvalid'); - if (!$fix) { + $fixBucket = false; + if ($bucketErrors) { + $fixBucket = $phpcsFile->addFixableError( + 'Invalid order of tags: ' . implode(', ', $bucketErrors), + $docBlockEndIndex, + 'OrderInvalid', + ); + } + + $fixInner = false; + if ($innerErrors) { + $fixInner = $phpcsFile->addFixableError( + 'Invalid inner order of tags: ' . implode(', ', $innerErrors), + $docBlockEndIndex, + 'InnerOrderInvalid', + ); + } + + if (!$fixBucket && !$fixInner) { return; } @@ -272,17 +386,49 @@ protected function fixOrder(File $phpcsFile, int $docBlockStartIndex, int $docBl $newOrder = []; foreach ($tags as $tag) { $tagOrder = $order[$tag['tag']] ?? -1; - $newOrder[$tagOrder][] = $this->getContent($tokens, $tag['start'], $tag['end']); + $newOrder[$tagOrder][] = [ + 'tag' => (string)$tag['tag'], + 'content' => $this->getContent($tokens, $tag['start'], $tag['end']), + ]; } ksort($newOrder); if (isset($newOrder[-1])) { - ksort($newOrder[-1]); + usort( + $newOrder[-1], + static fn (array $a, array $b): int => strcmp($a['content'], $b['content']), + ); + } + + if ($fixInner) { + foreach ($newOrder as $tagOrder => $entries) { + $tagName = $entries[0]['tag']; + if (!array_key_exists($tagName, $this->innerOrder)) { + continue; + } + $prefixes = $this->normalizeInnerPrefixes($this->innerOrder[$tagName]); + + usort($entries, function (array $a, array $b) use ($tagName, $prefixes): int { + $sa = $this->extractSubjectFromLine($a['content'], $tagName); + $sb = $this->extractSubjectFromLine($b['content'], $tagName); + $scoreA = $sa === null ? PHP_INT_MAX : $this->scoreSubject($sa, $prefixes); + $scoreB = $sb === null ? PHP_INT_MAX : $this->scoreSubject($sb, $prefixes); + if ($scoreA !== $scoreB) { + return $scoreA <=> $scoreB; + } + + return strcmp((string)$sa, (string)$sb); + }); + + $newOrder[$tagOrder] = $entries; + } } $content = ''; - foreach ($newOrder as $tagGroup) { - $content .= implode('', $tagGroup); + foreach ($newOrder as $entries) { + foreach ($entries as $entry) { + $content .= $entry['content']; + } } $firstTagTokenIndex = $tags[0]['start']; @@ -297,6 +443,213 @@ protected function fixOrder(File $phpcsFile, int $docBlockStartIndex, int $docBl $phpcsFile->fixer->endChangeset(); } + /** + * Extracts the subject from a full line (e.g. ` * @method \Foo save(...)`) by + * locating the tag name and parsing what comes after. + * + * @param string $lineContent + * @param string $tagName + * + * @return string|null + */ + protected function extractSubjectFromLine(string $lineContent, string $tagName): ?string + { + $position = strpos($lineContent, $tagName); + if ($position === false) { + return null; + } + + $value = ltrim(substr($lineContent, $position + strlen($tagName))); + + return $this->extractSubject($tagName, $value); + } + + /** + * Extracts the orderable subject from a tag's content for inner-ordering purposes. + * + * Returns null when the tag type is not handled or the content is malformed. + * + * @param string $tagName Tag name including leading "@" (e.g. "@method", "@property"). + * @param string $content Raw tag content as it appears in the docblock (the part after + * the tag name, leading whitespace stripped). + * + * @return string|null + */ + protected function extractSubject(string $tagName, string $content): ?string + { + $content = ltrim($content); + + switch ($tagName) { + case '@property': + case '@property-read': + case '@property-write': + return $this->extractMethodOrPropertySubject($content, true); + case '@method': + return $this->extractMethodOrPropertySubject($content, false); + case '@mixin': + case '@extends': + case '@implements': + if (preg_match('/^\\\\?([A-Za-z0-9_\\\\]+)/', $content, $m) === 1) { + $segments = explode('\\', $m[1]); + $last = end($segments); + + return $last === '' ? null : $last; + } + + return null; + case '@template': + if (preg_match('/^([A-Za-z_][A-Za-z0-9_]*)/', $content, $m) === 1) { + return $m[1]; + } + + return null; + } + + return null; + } + + /** + * Walks a `@method` or `@property*` content string: skips an optional `static` keyword, + * skips one type expression (with depth tracking for `<>`, `()`, `[]`), then returns + * the next bareword. For `@property*`, also strips a leading `$` from the subject. + * + * @param string $content + * @param bool $isProperty + * + * @return string|null + */ + protected function extractMethodOrPropertySubject(string $content, bool $isProperty): ?string + { + $length = strlen($content); + $i = 0; + + if (substr($content, $i, 7) === 'static ') { + $i += 7; + $i += strspn($content, " \t", $i); + } + + $startsWithName = preg_match('/\A([A-Za-z_][A-Za-z0-9_]*)/', substr($content, $i), $m) === 1; + if ($startsWithName) { + $name = $m[1]; + $afterName = $i + strlen($name); + $afterName += strspn($content, " \t", $afterName); + $nextChar = $afterName < $length ? $content[$afterName] : ''; + $isMethodWithoutType = !$isProperty && $nextChar === '('; + $isPropertyWithoutType = $isProperty && $nextChar === '$'; + if ($isMethodWithoutType) { + return $name; + } + if ($isPropertyWithoutType) { + $j = $afterName + 1; + if (preg_match('/\A([A-Za-z_][A-Za-z0-9_]*)/', substr($content, $j), $m2) === 1) { + return $m2[1]; + } + + return null; + } + } + + $depthAngle = 0; + $depthParen = 0; + $depthBracket = 0; + while ($i < $length) { + $c = $content[$i]; + if ($depthAngle === 0 && $depthParen === 0 && $depthBracket === 0 && ($c === ' ' || $c === "\t")) { + break; + } + if ($c === '<') { + $depthAngle++; + } elseif ($c === '>') { + if ($depthAngle > 0) { + $depthAngle--; + } + } elseif ($c === '(') { + $depthParen++; + } elseif ($c === ')') { + if ($depthParen > 0) { + $depthParen--; + } + } elseif ($c === '[') { + $depthBracket++; + } elseif ($c === ']') { + if ($depthBracket > 0) { + $depthBracket--; + } + } + $i++; + } + + $i += strspn($content, " \t", $i); + + if ($isProperty) { + if ($i < $length && $content[$i] === '$') { + $i++; + } + if (preg_match('/\A([A-Za-z_][A-Za-z0-9_]*)/', substr($content, $i), $m) === 1) { + return $m[1]; + } + + return null; + } + + if (preg_match('/\A([A-Za-z_][A-Za-z0-9_]*)/', substr($content, $i), $m) === 1) { + return $m[1]; + } + + return null; + } + + /** + * Scores a subject against an ordered prefix list. Lower score wins. + * Unmatched subjects return PHP_INT_MAX so they float to the bottom. + * + * @param string $subject + * @param array $prefixes + * + * @return int + */ + protected function scoreSubject(string $subject, array $prefixes): int + { + foreach ($prefixes as $i => $pattern) { + if ($pattern === '') { + continue; + } + if (str_ends_with($pattern, '*')) { + $needle = substr($pattern, 0, -1); + if ($needle === '' || str_starts_with($subject, $needle)) { + return $i; + } + } elseif ($subject === $pattern) { + return $i; + } + } + + return PHP_INT_MAX; + } + + /** + * Normalizes the configured `$innerOrder` entry for a tag into an ordered prefix list. + * Empty inputs (null, "", [""]) become [] so every subject is unmatched and the bucket + * sorts purely alphabetically. + * + * @param array|string|null $value + * + * @return array + */ + protected function normalizeInnerPrefixes(string|array|null $value): array + { + if ($value === null || $value === '') { + return []; + } + if (is_string($value)) { + $value = array_map('trim', explode(',', $value)); + } else { + $value = array_map(static fn ($v): string => trim((string)$v), $value); + } + + return array_values(array_filter($value, static fn (string $v): bool => $v !== '')); + } + /** * @param array $orderList * diff --git a/tests/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniffTest.php b/tests/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniffTest.php index 4f02afa..bc76e37 100644 --- a/tests/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniffTest.php +++ b/tests/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniffTest.php @@ -81,4 +81,83 @@ public function testDocBlockTagOrderNormalizesBlankLineSeparators(): void $this->prefix = null; } + + /** + * Empty pattern list for a tag means "alphabetize everything in this bucket." + * Typical recipe for @property association lists. + * + * @return void + */ + public function testDocBlockTagOrderInnerOrderProperty(): void + { + $this->prefix = 'inner-property.'; + + $sniff = new DocBlockTagOrderSniff(); + $sniff->innerOrder = ['@property' => '']; + + $this->assertSnifferCanFixErrors($sniff, 1); + + $this->prefix = null; + } + + /** + * @method CRUD recipe: prefix-priority with alphabetical tiebreak, + * unmatched custom methods float to the bottom alphabetically. + * + * @return void + */ + public function testDocBlockTagOrderInnerOrderMethodCrud(): void + { + $this->prefix = 'inner-method.'; + + $sniff = new DocBlockTagOrderSniff(); + $sniff->innerOrder = [ + '@method' => 'newEmptyEntity,newEntity,newEntities,get,findOrCreate,find*,patchEntity,patchEntities,save,saveOrFail,saveMany*,delete,deleteOrFail,deleteMany*', + ]; + + $this->assertSnifferCanFixErrors($sniff, 1); + + $this->prefix = null; + } + + /** + * Edge-case shapes for @method: generic / union return types, `static` modifier, + * missing return type, trailing-bareword malformed line. + * + * @return void + */ + public function testDocBlockTagOrderInnerOrderMethodEdges(): void + { + $this->prefix = 'inner-method-edges.'; + + $sniff = new DocBlockTagOrderSniff(); + $sniff->innerOrder = [ + '@method' => 'newEmptyEntity,newEntity,newEntities,get,findOrCreate,find*,patchEntity,patchEntities,save,saveOrFail,saveMany*,delete,deleteOrFail,deleteMany*', + ]; + + $this->assertSnifferCanFixErrors($sniff, 1); + + $this->prefix = null; + } + + /** + * Combined: bucket reordering + inner ordering for both @method and @property + * applied in one fixer pass. + * + * @return void + */ + public function testDocBlockTagOrderInnerOrderCombined(): void + { + $this->prefix = 'inner-combined.'; + + $sniff = new DocBlockTagOrderSniff(); + $sniff->innerOrder = [ + '@method' => 'newEmptyEntity,newEntity,newEntities,get,findOrCreate,find*,patchEntity,patchEntities,save,saveOrFail,saveMany*,delete,deleteOrFail,deleteMany*', + '@property' => '', + ]; + + $this->assertSnifferCanFixErrors($sniff, 2); + + $this->prefix = null; + } } diff --git a/tests/_data/DocBlockTagOrder/inner-combined.after.php b/tests/_data/DocBlockTagOrder/inner-combined.after.php new file mode 100644 index 0000000..c8ee040 --- /dev/null +++ b/tests/_data/DocBlockTagOrder/inner-combined.after.php @@ -0,0 +1,19 @@ + + * @property \Foo $Boards + * @property \Foo $StaffMembers + * @method \Foo newEmptyEntity() + * @method \Foo get(mixed $pk) + * @method \Foo save(\Foo $entity) + * @mixin \BehaviorOne + * @mixin \BehaviorTwo + */ +class FixMe +{ +} diff --git a/tests/_data/DocBlockTagOrder/inner-combined.before.php b/tests/_data/DocBlockTagOrder/inner-combined.before.php new file mode 100644 index 0000000..84aabd4 --- /dev/null +++ b/tests/_data/DocBlockTagOrder/inner-combined.before.php @@ -0,0 +1,19 @@ + + * @method \Foo get(mixed $pk) + * @mixin \BehaviorTwo + */ +class FixMe +{ +} diff --git a/tests/_data/DocBlockTagOrder/inner-method-edges.after.php b/tests/_data/DocBlockTagOrder/inner-method-edges.after.php new file mode 100644 index 0000000..4bc6ddc --- /dev/null +++ b/tests/_data/DocBlockTagOrder/inner-method-edges.after.php @@ -0,0 +1,18 @@ + save(\Foo $entity) + * @method \Cake\Datasource\ResultSetInterface<\Foo>|false saveMany(iterable $entities) + * @method \Foo customMethod(\Foo $entity) + * @method \Foo somethingMalformed + */ +class FixMe +{ +} diff --git a/tests/_data/DocBlockTagOrder/inner-method-edges.before.php b/tests/_data/DocBlockTagOrder/inner-method-edges.before.php new file mode 100644 index 0000000..29d49af --- /dev/null +++ b/tests/_data/DocBlockTagOrder/inner-method-edges.before.php @@ -0,0 +1,18 @@ +|false saveMany(iterable $entities) + * @method get(mixed $primaryKey) + * @method \Foo\Container save(\Foo $entity) + * @method \Foo somethingMalformed + * @method \Foo customMethod(\Foo $entity) + */ +class FixMe +{ +} diff --git a/tests/_data/DocBlockTagOrder/inner-method.after.php b/tests/_data/DocBlockTagOrder/inner-method.after.php new file mode 100644 index 0000000..694b3a5 --- /dev/null +++ b/tests/_data/DocBlockTagOrder/inner-method.after.php @@ -0,0 +1,23 @@ + newEntities(array $data, array $options = []) + * @method \Foo\Entity get(mixed $primaryKey, array $finder = 'all') + * @method \Foo\Entity findOrCreate(\Cake\ORM\Query\SelectQuery|callable|array $search, ?callable $callback = null) + * @method \Foo\Entity patchEntity(\Foo\Entity $entity, array $data, array $options = []) + * @method \Foo\Entity save(\Foo\Entity $entity, array $options = []) + * @method \Foo\Entity|false saveMany(iterable $entities, array $options = []) + * @method \Foo\Entity saveManyOrFail(iterable $entities, array $options = []) + * @method bool delete(\Foo\Entity $entity, array $options = []) + * @method array<\Foo\Entity> deleteMany(iterable $entities, array $options = []) + * @method \Foo\Entity customDomainMethod(\Foo\Entity $entity) + */ +class FixMe +{ +} diff --git a/tests/_data/DocBlockTagOrder/inner-method.before.php b/tests/_data/DocBlockTagOrder/inner-method.before.php new file mode 100644 index 0000000..97c4bec --- /dev/null +++ b/tests/_data/DocBlockTagOrder/inner-method.before.php @@ -0,0 +1,23 @@ + deleteMany(iterable $entities, array $options = []) + * @method static array<\Foo\Entity> newEntities(array $data, array $options = []) + * @method \Foo\Entity customDomainMethod(\Foo\Entity $entity) + */ +class FixMe +{ +} diff --git a/tests/_data/DocBlockTagOrder/inner-property.after.php b/tests/_data/DocBlockTagOrder/inner-property.after.php new file mode 100644 index 0000000..ebbfb85 --- /dev/null +++ b/tests/_data/DocBlockTagOrder/inner-property.after.php @@ -0,0 +1,17 @@ + Date: Tue, 12 May 2026 01:52:53 +0200 Subject: [PATCH 2/2] Address review: skip -1 mixed bucket in inner-order fix, push null subjects to bottom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The -1 bucket in fixOrder() holds tags not present in the outer order list, which means it can contain a mix of tag types. The inner-order code path treats `$entries[0]['tag']` as the bucket's tag name and applies that tag's prefix list to every entry, which produces wrong subject extraction across mixed types. Skip the -1 bucket entirely — the alphabetical-by-content sort applied earlier already gives those entries a stable order. - The inner-order sort key in checkInnerOrder() previously paired `[$score, (string)$subject]`; for null subjects (malformed/ unparseable lines) (string)null === '' sorts before every real string within the same score class. Insert a presence flag in the tuple so null subjects sort last, matching the "unmatched floats to the bottom" contract. - The corresponding comparator in fixOrder() had the same hazard; rewrite to explicitly push null-subject entries to the bottom of their score class and to fall back to original line content as the stable tiebreaker when both subjects are null. - Note in the property docblock that innerOrder is intentionally class/interface/trait-only — per-method @param/@return tags are positional and have no within-bucket subject to reorder against. --- .../Commenting/DocBlockTagOrderSniff.php | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php b/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php index 2039c83..569b855 100644 --- a/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php +++ b/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php @@ -94,6 +94,10 @@ class DocBlockTagOrderSniff extends AbstractSniff * An empty value means "no prefix priority - alphabetize everything in this bucket," * which is the typical recipe for `@property` association lists. * + * Only applied on class/interface/trait docblocks, not on function/method + * docblocks - per-method tags like `@param`/`@return` are positional and have + * no meaningful within-bucket subject. + * * Subject extraction per tag: * - `@method` -> method name (after optional `static` + return type) * - `@property[-read|-write]` -> variable name minus leading `$` @@ -182,7 +186,10 @@ protected function checkInnerOrder(array $tags): array $value = ltrim(substr((string)$tags[$idx]['content'], strlen($tagName))); $subject = $this->extractSubject($tagName, $value); $score = $subject === null ? PHP_INT_MAX : $this->scoreSubject($subject, $prefixes); - $key = [$score, (string)$subject]; + // Tuple second slot pushes null-subject (malformed/unparseable) + // entries to the bottom of their score class; without it they'd + // sort before real subjects because (string)null === ''. + $key = [$score, $subject === null ? 1 : 0, (string)$subject]; if ($previousKey !== null && $key < $previousKey) { $tags[$idx]['innerError'] = sprintf( 'Inner order of %s tag wrong (subject "%s").', @@ -402,6 +409,14 @@ protected function fixOrder(File $phpcsFile, int $docBlockStartIndex, int $docBl if ($fixInner) { foreach ($newOrder as $tagOrder => $entries) { + // The -1 bucket holds tags not present in the outer order list, + // so its entries can be a mix of tag types. Applying a single + // tag's inner-order sort across that mix would extract subjects + // with the wrong tag name and reorder unrelated tags. The + // alphabetical-by-content sort applied above already covers it. + if ($tagOrder === -1) { + continue; + } $tagName = $entries[0]['tag']; if (!array_key_exists($tagName, $this->innerOrder)) { continue; @@ -416,8 +431,21 @@ protected function fixOrder(File $phpcsFile, int $docBlockStartIndex, int $docBl if ($scoreA !== $scoreB) { return $scoreA <=> $scoreB; } + // Push null subjects (malformed/unparseable) to the bottom + // of the score class instead of letting (string)null === '' + // pull them above real subjects. Fall back to original line + // content when both are null so the order remains stable. + if ($sa === null && $sb === null) { + return strcmp($a['content'], $b['content']); + } + if ($sa === null) { + return 1; + } + if ($sb === null) { + return -1; + } - return strcmp((string)$sa, (string)$sb); + return strcmp($sa, $sb); }); $newOrder[$tagOrder] = $entries;