Adding class templates to PhabricatorPolicyAwareQuery and
PhabricatorCursorPagedPolicyAwareQuery to make execute() have the right
return signature, when the query classes are also annotated. Otherwise, the
return type is inferred as PhabricatorPolicyInterface as before.
Details
Details
- Reviewers
- None
- Group Reviewers
O1: Blessed Committers
Add some annotations to query classes and see that $query->execute() infers
the more specific return type.
Diff Detail
Diff Detail
- Repository
- rP Phorge
- Branch
- query_templates (branched from master)
- Lint
Lint Passed - Unit
Test Failures - Build Status
Buildable 2025 Build 2025: arc lint + arc unit
| Time | Test | |
|---|---|---|
| 3 ms | PhabricatorAphrontViewTestCase::testHasChildren EXCEPTION (RuntimeException): Code coverage needs to be enabled in php.ini by setting 'xdebug.mode' to 'coverage'
#0 [internal function]: PhutilErrorHandler::handleError(2, '...', '...', 708)
#1 /home/thorn/third_party/arcanist/src/unit/engine/phutil/PhutilTestCase.php(708): xdebug_start_code_coverage(3)
| |
| 0 ms | PhabricatorCelerityTestCase::testCelerityMaps EXCEPTION (RuntimeException): Code coverage needs to be enabled in php.ini by setting 'xdebug.mode' to 'coverage'
#0 [internal function]: PhutilErrorHandler::handleError(2, '...', '...', 708)
#1 /home/thorn/third_party/arcanist/src/unit/engine/phutil/PhutilTestCase.php(708): xdebug_start_code_coverage(3)
| |
| 0 ms | PhabricatorConduitTestCase::testConduitMethods EXCEPTION (RuntimeException): Code coverage needs to be enabled in php.ini by setting 'xdebug.mode' to 'coverage'
#0 [internal function]: PhutilErrorHandler::handleError(2, '...', '...', 708)
#1 /home/thorn/third_party/arcanist/src/unit/engine/phutil/PhutilTestCase.php(708): xdebug_start_code_coverage(3)
| |
| 0 ms | PhabricatorInfrastructureTestCase::testApplicationsInstalled EXCEPTION (RuntimeException): Code coverage needs to be enabled in php.ini by setting 'xdebug.mode' to 'coverage'
#0 [internal function]: PhutilErrorHandler::handleError(2, '...', '...', 708)
#1 /home/thorn/third_party/arcanist/src/unit/engine/phutil/PhutilTestCase.php(708): xdebug_start_code_coverage(3)
| |
| 0 ms | PhabricatorInfrastructureTestCase::testRejectMySQLNonUTF8Queries EXCEPTION (RuntimeException): Code coverage needs to be enabled in php.ini by setting 'xdebug.mode' to 'coverage'
#0 [internal function]: PhutilErrorHandler::handleError(2, '...', '...', 708)
#1 /home/thorn/third_party/arcanist/src/unit/engine/phutil/PhutilTestCase.php(708): xdebug_start_code_coverage(3)
| |
| View Full Test Results (12 Failed) |
Event Timeline
Comment Actions
Hmm.. clearly I need to set xdebug.mode correctly, but I'm not sure why it wasn't. It's been fine before.
Comment Actions
Yes to the idea however a related elephant in the room (IMHO) are those non-standard types like list or map or wild being replaced, no clue if that has any effects on...something™ (Diviner docs? Code?) as I haven't managed to understand the consequences of code in https://we.phorge.it/source/arcanist/browse/master/src/parser/PhutilTypeSpec.php yet. (Technically speaking map and list seem to be just arrays, indeed.)
Comment Actions
Oh, that's a good thought to check that this wouldn't mess up anything like Diviner. I don't think it will. Of course, changing these code comments won't do anything to runtime behavior, but does inform static analyzers of types. I've spent a good amount of time looking through the code to see if there's any tooling or applications that depend on these custom type names and I can't find anything, so it's probably safe.
Re: PhutilTypeSpec: it isn't involved in Diviner or anything except runtime shape validation as far as I can tell. I also examined the diviner code, and it looks like it's just simply not that sophisticated about its docblock parsing anyway so it doesn't even try to interpret as far as I can see so we're not losing or gaining anything there anyway.