Add inner-bucket tag ordering to DocBlockTagOrderSniff#60
Merged
Conversation
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.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in innerOrder configuration to DocBlockTagOrderSniff to enforce deterministic ordering of tags within a tag-type bucket (e.g. ordering @method lines by CRUD prefixes, or alphabetizing @property association lists), alongside new fixtures and tests.
Changes:
- Add
innerOrderproperty and subject-extraction/scoring logic to enable within-bucket ordering. - Emit a separate
InnerOrderInvalidfixable error code for inner-order violations. - Add 4 new fixture pairs and corresponding PHPUnit tests covering property/method recipes and edge cases.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniff.php |
Implements inner-bucket ordering (detection + fixer) and introduces InnerOrderInvalid. |
tests/PhpCollective/Sniffs/Commenting/DocBlockTagOrderSniffTest.php |
Adds tests for inner ordering recipes, edge cases, and combined outer+inner fixes. |
tests/_data/DocBlockTagOrder/inner-property.*.php |
Fixture for alphabetizing @property bucket. |
tests/_data/DocBlockTagOrder/inner-method.*.php |
Fixture for CRUD-priority + alphabetical inner ordering in @method bucket. |
tests/_data/DocBlockTagOrder/inner-method-edges.*.php |
Fixture for edge-case @method line shapes (generics/unions/static/malformed). |
tests/_data/DocBlockTagOrder/inner-combined.*.php |
Fixture for combined outer bucket ordering + inner ordering in one fixer pass. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…bjects to bottom - 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends
DocBlockTagOrderSniffwith an opt-ininnerOrderproperty that orders tags within a tag-type bucket. Step 1 (PR #59) ordered the buckets themselves; this is Step 2: ordering within them.Default
innerOrder = []keeps existing behavior unchanged. Configured via XML:Two recipes
newEmptyEntity,newEntity,get,save, ...) to the top in a configured order; custom methods float to the bottom alphabetically.Scoping for CakePHP entities
The empty-pattern recipe is not appropriate for entity column docblocks. CakePHP entities list their properties in DB schema order (
idfirst, logical fields together, timestamps last), and that order is meaningful for review and stays in sync with re-runs of cakephp-ide-helper. Alphabetizing breaks column groupings (lat/lng/location,beginning/end,lft/rght) and creates infinite churn against IDE-helper regeneration.The separate
InnerOrderInvaliderror code makes this easy to scope per path without losing bucket-level ordering on the same files:Tables, views, controllers still receive the inner ordering. Tested on a real CakePHP sandbox: 51 files flagged initially; scoping the exclude leaves 18 net-positive fixes — for example, a randomly-placed
findmethod annotation pulled up to its canonical bake slot in Table classes — and skips ~30 entity rewrites that would have destroyed schema-meaningful column order.How it works
Within each configured bucket, every tag is scored by first-matching prefix (lowest wins; unmatched gets
PHP_INT_MAXand floats to the bottom), then sorted alphabetically by extracted subject within each score class.Subject extraction handles return types with generics, union types, the
staticmodifier, missing return types, and trailing-bareword malformed lines.A separate
InnerOrderInvaliderror code (alongside the existingOrderInvalid) lets users suppress one without losing the other — see the entity-scoping recipe above.Refs cakephp/cakephp-codesniffer#434.