Paths

Table of Contentst

Differential D26060

PhpDoc: Replace non-standard dict type with array
AcceptedPublic
Actions

Authored by aklapper on Wed, Jun 4, 07:55.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Summary

Make static code analysis more correct (which does not also mean less noisy) by replacing "dict" type in PhpDoc with what they actually are: an array.
The "dict" type is not mentioned in arcanist/src/parser/PhutilTypeSpec.php either, thus no side effects.

See D26059 for the Phorge counterpart.

Test Plan

Grep and read the code, use static code analysis.

Diff Detail

Repository

rARC Arcanist

Branch

phpdocKillListType (branched from master)

Lint

Lint Passed

Unit

Test Failures

Build Status

Buildable 2054
Build 2054: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	855 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(87): ArcanistBundleTestCase->runGitRepositoryTests(Object(PhutilDirectoryFixture)) #1 /var/www/html/phorge/arcanist/src/unit/engine/phutil/PhutilTestCase.php(639): ArcanistBundleTestCase->testGitRepository()
	124 ms	ArcanistXMLLinterTestCase::testLinter Lint emitted an unexpected set of messages for file "languages-6.lint-test". MISSING MESSAGES Message with severity "error" at "4:1" (XML5)
	87 ms	AbstractDirectedGraphTestCase::testCyclicGraph 1 assertion(s) passed.
	75 ms	AbstractDirectedGraphTestCase::testEdgeLoadFailure 1 assertion(s) passed.
	85 ms	AbstractDirectedGraphTestCase::testNonTreeGraph 1 assertion(s) passed.
		View Full Test Results (2 Failed · 271 Passed · 32 Skipped)

Event Timeline

aklapper created this revision.Wed, Jun 4, 07:55

Owners added a reviewer: O1: Blessed Committers.Wed, Jun 4, 07:55

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptWed, Jun 4, 07:55

aklapper requested review of this revision.Wed, Jun 4, 07:55

Harbormaster completed remote builds in B2053: Diff 3149.Wed, Jun 4, 07:55

Harbormaster completed remote builds in B2054: Diff 3150.Wed, Jun 4, 08:34

P.S. why is your local unit test failing?

Cannot reproduce:

arc unit src/parser/__tests__/ArcanistBundleTestCase.php

This revision is now accepted and ready to land.Wed, Jun 4, 10:12

In D26060#28141, @valerio.bozzolan wrote:
P.S. why is your local unit test failing?

Cannot reproduce:
arc unit src/parser/__tests__/ArcanistBundleTestCase.php

That is a very good question to which I currently have no answer. But it also happens on git master for me so I don't blame my patch for it. :P

diff --git a/image_2.png b/image.png
copy from image_2.png
copy to image.png
index 229920a50597be0644f4b994d213fc7042050289..229920a50597be0644f4b994d213fc7042050289
GIT binary patch
literal 108
zc%17D@N?(olHy`uVBq!ia0y~yU|<1Z4kiW$2DQnYnhXpKk|nMYCBgY=CFO}lsSJ)O
z`AMk?p1FzXsX?iUDV2pMQ*9U+82CJ0978H@B_$}xGO$Q6Fqkkf@+2Od!oa}5;OXk;
Jvd$@?2>`UB82A7H

literal 108
zc%17D@N?(olHy`uVBq!ia0y~yU|<1Z4kiW$2DQnYnhXpKk|nMYCBgY=CFO}lsSJ)O
z`AMk?p1FzXsX?iUDV2pMQ*9U+82CJ0978H@B_$}xGO$Q6Fqkkf@+2Od!oa}5;OXk;
Jvd$@?2>`UB82A7H

diff --git a/image_2.png b/image_2.png
index 229920a50597be0644f4b994d213fc7042050289..69ac76e69fabc49d97339547b91cf748ab703052
GIT binary patch
literal 113
zc%17D@N?(olHy`uVBq!ia0y~yU|<1Z4kiW$2DQnYnhXpKk|nMYCBgY=CFO}lsSJ)O
z`AMk?p1FzXsX?iUDV2pMQ*9U+7=%4t978H@CH*<yz~IB<Qpmu>!(e=ok=^x?;WY*Z
O1_n=8KbLh*2~7Zgh8*nx

literal 108
zc%17D@N?(olHy`uVBq!ia0y~yU|<1Z4kiW$2DQnYnhXpKk|nMYCBgY=CFO}lsSJ)O
z`AMk?p1FzXsX?iUDV2pMQ*9U+82CJ0978H@B_$}xGO$Q6Fqkkf@+2Od!oa}5;OXk;
Jvd$@?2>`UB82A7H

Unrelated troubleshooting but what about this?

LC_ALL=C arc unit src/parser/__tests__/ArcanistBundleTestCase.php

Nah, LC_ALL=C won't change anything. (My system is already in variants of English.)

Revision Contents
Changeset List

Path

Size

Coverage (All)

Coverage (Touched)

Packages

src/

channel/

PhutilChannel.php

4 lines

78%

O1: Blessed Committers

error/

PhutilErrorHandler.php

2 lines

51%

O1: Blessed Committers

phlog.php

2 lines

O1: Blessed Committers

filesystem/

PhutilDeferredLog.php

2 lines

83%

O1: Blessed Committers

future/

exec/

ExecFuture.php

2 lines

65%

O1: Blessed Committers

lexer/

PhutilLexer.php

2 lines

65%

O1: Blessed Committers

lint/

ArcanistLintMessage.php

2 lines

52%

O1: Blessed Committers

engine/

ArcanistLintEngine.php

4 lines

63%

O1: Blessed Committers

linter/

ArcanistScriptAndRegexLinter.php

4 lines

O1: Blessed Committers

moduleutils/

PhutilLibraryMapBuilder.php

14 lines

47%

O1: Blessed Committers

parser/

PhutilJSON.php

6 lines

74%

O1: Blessed Committers

PhutilSimpleOptions.php

8 lines

94%

O1: Blessed Committers

argument/

PhutilArgumentSpecification.php

2 lines

85%

O1: Blessed Committers

repository/

api/

ArcanistGitAPI.php

2 lines

18%

O1: Blessed Committers

parser/

ArcanistMercurialParser.php

4 lines

72%

O1: Blessed Committers

symbols/

PhutilSymbolLoader.php

10 lines

54%

O1: Blessed Committers

utils/

AbstractDirectedGraph.php

6 lines

86%

O1: Blessed Committers

utils.php

16 lines

66%

O1: Blessed Committers

workflow/

ArcanistDiffWorkflow.php

2 lines

O1: Blessed Committers

ArcanistWorkflow.php

2 lines

O1: Blessed Committers

Diff 3150

View Options

src/channel/PhutilChannel.php

View Options

src/error/PhutilErrorHandler.php

View Options

src/error/phlog.php

View Options

src/filesystem/PhutilDeferredLog.php

View Options

src/future/exec/ExecFuture.php

View Options

src/lexer/PhutilLexer.php

View Options

src/lint/ArcanistLintMessage.php

View Options

src/lint/engine/ArcanistLintEngine.php

View Options

src/lint/linter/ArcanistScriptAndRegexLinter.php

View Options

src/moduleutils/PhutilLibraryMapBuilder.php

View Options

src/parser/PhutilJSON.php

View Options

src/parser/PhutilSimpleOptions.php

View Options

src/parser/argument/PhutilArgumentSpecification.php

View Options

src/repository/api/ArcanistGitAPI.php

View Options

src/repository/parser/ArcanistMercurialParser.php

View Options

src/symbols/PhutilSymbolLoader.php

View Options

src/utils/AbstractDirectedGraph.php

View Options

src/utils/utils.php

View Options

src/workflow/ArcanistDiffWorkflow.php

View Options

src/workflow/ArcanistWorkflow.php

PhpDoc: Replace non-standard dict type with arrayAcceptedPublicActions

Details

Diff Detail

Unit TestsFailedView All

Event Timeline

Revision ContentsChangeset List

Diff 3150

src/channel/PhutilChannel.php

src/error/PhutilErrorHandler.php

src/error/phlog.php

src/filesystem/PhutilDeferredLog.php

src/future/exec/ExecFuture.php

src/lexer/PhutilLexer.php

src/lint/ArcanistLintMessage.php

src/lint/engine/ArcanistLintEngine.php

src/lint/linter/ArcanistScriptAndRegexLinter.php

src/moduleutils/PhutilLibraryMapBuilder.php

src/parser/PhutilJSON.php

src/parser/PhutilSimpleOptions.php

src/parser/argument/PhutilArgumentSpecification.php

src/repository/api/ArcanistGitAPI.php

src/repository/parser/ArcanistMercurialParser.php

src/symbols/PhutilSymbolLoader.php

src/utils/AbstractDirectedGraph.php

src/utils/utils.php

src/workflow/ArcanistDiffWorkflow.php

src/workflow/ArcanistWorkflow.php

PhpDoc: Replace non-standard dict type with array
AcceptedPublic
Actions

Unit TestsFailed
View All

Revision Contents
Changeset List