CLI-1731: ACLI documentation expose MEO commands that should be hidden#1970
Open
jigar-shah-acquia wants to merge 3 commits intoacquia:mainfrom
Open
CLI-1731: ACLI documentation expose MEO commands that should be hidden#1970jigar-shah-acquia wants to merge 3 commits intoacquia:mainfrom
jigar-shah-acquia wants to merge 3 commits intoacquia:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses CLI-1731 by ensuring hidden commands are excluded from generated ACLI documentation, and adds targeted unit tests to lock in the intended “hidden in docs” and API namespace visibility behavior.
Changes:
- Update docs generation to skip commands marked as hidden in the Symfony JSON descriptor output.
- Refactor API list-command generation to use a helper for “namespace has at least one visible command”.
- Add/extend PHPUnit coverage to verify hidden-command exclusion and API namespace list-command creation behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Command/Self/MakeDocsCommand.php |
Filters out hidden commands during JSON docs dump via isCommandHiddenInDocs() helper. |
tests/phpunit/src/Commands/Self/MakeDocsCommandTest.php |
Adds tests to validate hidden-command handling and exclusion from dumped docs/index. |
src/Command/Api/ApiCommandHelper.php |
Refactors namespace visibility check into namespaceHasVisibleCommand() and gates list-command creation on visibility. |
tests/phpunit/src/Commands/Api/ApiCommandHelperTest.php |
Adds new tests for list-command creation rules based on namespace visibility/hidden state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1970 +/- ##
============================================
+ Coverage 92.30% 92.33% +0.02%
- Complexity 1921 1927 +6
============================================
Files 122 122
Lines 7009 7015 +6
============================================
+ Hits 6470 6477 +7
+ Misses 539 538 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/1970/acli.phar |
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.
Motivation
Fixes #CLI-1731 ACLI documentation expose MEO commands that should be hidden
Proposed changes
src/Command/Api/ApiCommandHelper.phpgenerateApiListCommands(): Replaced the inner loop that usedbreakwith a call to a new helper. Visibility is now computed vianamespaceHasVisibleCommand().namespaceHasVisibleCommand(array $apiCommands, string $namespace): bool: Encapsulates “does this namespace have at least one non-hidden command?” and uses earlyreturn trueinstead ofbreak, removing the Break_ mutation at that site.src/Command/Self/MakeDocsCommand.phpexecute(): Replaced inline$command['hidden'] ?? falsewithself::isCommandHiddenInDocs($command).isCommandHiddenInDocs(array $command): bool: Public static helper that returns$command['hidden'] ?? falseso the “hidden in docs” rule is testable and the FalseValue/Coalesce mutations can be killed by direct unit tests.hiddenkey are still included; commands withhidden => trueare still excluded.Alternatives considered
N/A
Testing steps
./bin/acli ckc./vendor/bin/phpunit./vendor/bin/phpunit tests/phpunit/src/Commands/Api/ApiCommandHelperTest.php tests/phpunit/src/Commands/Api/ApiListCommandTest.php./vendor/bin/phpunit tests/phpunit/src/Commands/Self/MakeDocsCommandTest.php./bin/acli apiand./bin/acli api:accounts(or another namespace) and confirm list output and subcommands are unchanged.ApiCommandHelper.php(e.g. LogicalAnd, ConcatOperandRemoval, Break_) andMakeDocsCommand.php(FalseValue, Coalesce) are killed../bin/acli self:make-docs --dump=/tmp/acli-docsand inspectindex.jsonand presence/absence ofself:make-docs.json.