Fix stale/inaccurate PHPDoc in core/API/Proxy, ResponseBuilder, Access#24736
Open
sgiehl wants to merge 3 commits into
Open
Fix stale/inaccurate PHPDoc in core/API/Proxy, ResponseBuilder, Access#24736sgiehl wants to merge 3 commits into
sgiehl wants to merge 3 commits into
Conversation
Remove claims about API call performance logging via a "log_api_call" config setting; no such logging or config key exists anywhere in the codebase, so this was stale documentation of removed functionality. Also correct registerClass()'s misleading param example (it takes a fully qualified class name, not a bare module name), fix the $requiredParameters shape description on the two getXRequestParametersArray() methods (it's a map to ['default'=>.., 'type'=>..], not a flat name=>default pair), and add missing types to two untyped params.
decorateExceptionWithDebugTrace() can return the original $e unchanged, which may be a Throwable that isn't an Exception (eg. an Error); widen the return type to match. Also document the previously-untyped $shouldPrintBacktrace constructor param. The return type fix resolves a PHPStan finding that was previously suppressed in the baseline.
isUserHasSomeWriteAccess() and checkUserHasWriteAccess() had docblocks copy-pasted from their admin/view counterparts, describing "admin access" and "VIEW or ADMIN access" respectively even though both methods check write access. Also complete getRoleForSite()'s return value example (it can also return 'write') and add a missing param type on throwNoAccessException().
sgiehl
added
not-in-changelog
![github-actions[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🤖 Codex Review: ✅ No findings
Summary
This PR is a narrow PHPDoc cleanup for core/API/Proxy, core/API/ResponseBuilder, and core/Access, plus removal of a now-obsolete PHPStan baseline entry. I found no blocking, medium, or polish issues in the reviewed diff.
Executable validation is delegated to CI per the review policy; the read-only inspection supports the branch intent and I consider it merge-ready from review.
Findings Overview
| Severity | Count |
|---|---|
| 🚫 Blocking | 0 |
| 0 | |
| 💬 Low / Polish | 0 |
✅ No inline findings to place.
Diagnostics
Detailed review diagnostics are available in the codex-review-output workflow artifact.
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
1 participant
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
Part of an ongoing pass to fix PHPDoc comments across
core/andplugins/(excluding@api-tagged classes/methods andAPI.phpclasses) that no longer match the actual code behavior, or whose@param/@returnannotations are wrong/incomplete.This PR covers:
core/API/Proxy.php: removed a stale claim about API-call performance logging via alog_api_callconfig setting that doesn't exist anywhere in the codebase; fixed a misleadingregisterClass()param example; corrected the$requiredParametersshape description used by two methods; added missing types on two untyped params.core/API/ResponseBuilder.php: wideneddecorateExceptionWithDebugTrace()'s return type since it can return the originalThrowableunchanged, not justException; documented a previously-untyped constructor param.core/Access.php: fixed two copy-pasted docblocks —isUserHasSomeWriteAccess()was documented as checking "admin access" andcheckUserHasWriteAccess()was documented as checking "VIEW or ADMIN access", both copied from sibling methods; completedgetRoleForSite()'s return value example; added a missing param type.One
phpstan-baseline.neonentry was removed because theResponseBuilderreturn type fix resolved a real, previously-suppressed PHPStan finding.core/API/NoDefaultValue.php,core/API/Request.php(class-level@api, out of scope),core/Access/CapabilitiesProvider.php,core/Access/Capability.php,core/Access/Role.php,core/Access/Role/Admin.php, andcore/Access/Role/View.phpneeded no changes.Checklist