Details

Reviewers

Group Reviewers

Commits

rARC4f5203375e0b: Add a linter rule to detect implict method visibility

Summary

I'm not sure whether this is in any coding standards, but it seems to be an unspoken rule that methods should have a visiblity modifier (public, protected or private) explicitly specified. According to the PHP documentation, methods declared without any explicit visibility keyword are defined as public.

Test Plan

Added a test case.

Diff Detail

Repository

rARC Arcanist

Branch

master

Lint

Lint Passed

Unit

Test Failures

Build Status

Buildable 3569
Build 3577: [Placeholder Plan] Wait for 30 Seconds

Unit TestsFailed
View All

	Time	Test
	4,124 ms	ArcanistXHPASTLinterTestCase::testLinter
	435 ms	ArcanistCSSLintLinterTestCase::testLinter
	111 ms	ArcanistCSSLintLinterTestCase::testVersion
	220 ms	ArcanistClosureLinterTestCase::testLinter
	63 ms	ArcanistClosureLinterTestCase::testVersion
		View Full Test Results (1 Failed · 32 Passed · 14 Skipped)

Event Timeline

joshuaspence updated this revision to Diff 25655.Oct 12 2014, 12:29 PM

joshuaspence retitled this revision from to Add a linter rule to detect implict method visibility.

joshuaspence updated this object.

joshuaspence edited the test plan for this revision. (Show Details)

joshuaspence added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptOct 12 2014, 12:29 PM

Herald added subscribers: epriestley, Korvin. · View Herald Transcript

Harbormaster completed remote builds in B2809: Diff 25655.Oct 12 2014, 12:30 PM

I think the unit test is not working as expected because of a bug in XHPAST. See https://secure.phabricator.com/xhpast/view/621/ for an example. It seems that in public function bar() {}, T_FUNCTION belongs to n_METHOD_DECLARATION but in function baz() {} this is not the case.