Correct PHPDoc @return value of methods that can return null
ClosedPublic
Actions

Authored by aklapper on Sat, Aug 24, 20:41.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Commits

rARCc01198d55f6d: Correct PHPDoc @return value of methods that can return null

Summary

Make the PHPDoc @return say so when the method can also return null instead of an, array, string, or int.
(In case of getCommandHelp(), return an empty string as child implementations do return strings.)

Test Plan

Read the code; run static code analysis.

Diff Detail

Repository

rARC Arcanist

Branch

phpdocCanReturnNull

Lint

Lint Passed

Unit

Test Failures

Build Status

Buildable 1565
Build 1565: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	689 ms	ArcanistBundleTestCase::testGitRepository EXCEPTION (Exception): Expected patch and actual patch for 5dec8bf28557f078d1987c4e8cfb53d08310f522 differ. Wrote actual patch to '/var/www/html/phorge/arcanist/src/parser/__tests__/patches//5dec8bf28557f078d1987c4e8cfb53d08310f522.gitpatch.real'. #0 /var/www/html/phorge/arcanist/src/parser/__tests__/ArcanistBundleTestCase.php(85): ArcanistBundleTestCase->runGitRepositoryTests(Object(PhutilDirectoryFixture)) #1 /var/www/html/phorge/arcanist/src/unit/engine/phutil/PhutilTestCase.php(639): ArcanistBundleTestCase->testGitRepository()
	76 ms	AbstractDirectedGraphTestCase::testCyclicGraph 1 assertion(s) passed.
	76 ms	AbstractDirectedGraphTestCase::testEdgeLoadFailure 1 assertion(s) passed.
	76 ms	AbstractDirectedGraphTestCase::testNonTreeGraph 1 assertion(s) passed.
	76 ms	AbstractDirectedGraphTestCase::testNoncyclicGraph 1 assertion(s) passed.
		View Full Test Results (1 Failed · 210 Passed)

Event Timeline

aklapper created this revision.Sat, Aug 24, 20:41

Owners added a reviewer: O1: Blessed Committers.Sat, Aug 24, 20:41

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptSat, Aug 24, 20:41

Harbormaster completed remote builds in B1565: Diff 2424.Sat, Aug 24, 20:41

aklapper requested review of this revision.Sat, Aug 24, 20:41

aklapper retitled this revision from Correct PHPDoc @return value that method can return null to Correct PHPDoc @return value of methods that can return null.Sun, Aug 25, 12:47

valerio.bozzolan added inline comments.Tue, Sep 3, 05:26

src/workflow/ArcanistWorkflow.php
293	I agree but probably we should update also something else, like this check that originally was a "not empty, not null, not objects without a toString()" check and instead now it's just "not null": https://we.phorge.it/source/arcanist/browse/master/src/workflow/ArcanistWorkflow.php;995072b31fff5c7d77bcb353d25df819bfd3798a$157 (that line was changed during b50a646a3f49) The question may be: is there any Workflow out of there without an help command? If the answer is "maybe yes" let's update `ArcanistWorkflow.php` and adopt: `phutil_nonempty_stringlike()` - if an help may return an object with __toString() (that renders text?) `phutil_nonempty_string()` - if we are sure that it only returns null + string