Paths

Table of Contentst

Differential D10687

Add a linter rule to detect implict method visibility
ClosedPublic
Actions

Authored by joshuaspence on Oct 12 2014, 12:29 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Sat, Apr 6, 12:18 PM

	Unknown Object (File)
	Fri, Apr 5, 11:18 PM

	Unknown Object (File)
	Fri, Mar 29, 5:16 AM

	Unknown Object (File)
	Feb 15 2024, 5:05 PM

	Unknown Object (File)
	Feb 14 2024, 1:24 AM

	Unknown Object (File)
	Jan 28 2024, 12:18 AM

	Unknown Object (File)
	Jan 27 2024, 10:06 PM

	Unknown Object (File)
	Jan 25 2024, 1:30 PM

Subscribers

richardvanvelzen

Details

Reviewers

Group Reviewers

Blessed 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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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.

Handle properties as well

Harbormaster completed remote builds in B2810: Diff 25656.Oct 12 2014, 12:49 PM

Rebase

epriestley accepted this revision.Jan 4 2015, 11:27 PM

epriestley edited edge metadata.

This revision is now accepted and ready to land.Jan 4 2015, 11:27 PM

This doesn't actually work yet :P

I need to make some changes to XHPAST I think :(

joshuaspence mentioned this in D11215: Fix a minor XHPAST bug.Jan 5 2015, 12:25 AM

joshuaspence added a parent revision: D11215: Fix a minor XHPAST bug.Jan 5 2015, 12:25 AM

Working now (after D11215)

Closed by commit rARC4f5203375e0b: Add a linter rule to detect implict method visibility (authored by joshuaspence, committed by joshuaspence). · Explain WhyJan 6 2015, 8:42 PM

This revision was automatically updated to reflect the committed changes.

joshuaspence mentioned this in rPHU00fe30dd9ebf: Fix a minor XHPAST bug.

richardvanvelzen added a subscriber: richardvanvelzen.Jan 7 2015, 12:40 PM

richardvanvelzen added inline comments.

src/lint/linter/ArcanistXHPASTLinter.php
3014	Shouldn't `abstract` or `static` methods have an explicit visibility?

joshuaspence mentioned this in D11267: Improvements to `LINT_IMPLICIT_VISIBILITY`.Jan 7 2015, 1:26 PM

joshuaspence mentioned this in rARCa232f7d5b874: Improvements to `LINT_IMPLICIT_VISIBILITY`.Jan 7 2015, 8:33 PM

joshuaspence mentioned this in rP698b7f9ea39c: Explicitly declare method/property visibility.Jan 12 2015, 12:52 AM

Revision Contents
Changeset List

Path

Size

src/

lint/

linter/

ArcanistXHPASTLinter.php

66 lines

__tests__/

xhpast/

decl-parens-hug-closing.lint-test

10 lines

implicit-visibility.lint-test

24 lines

Diff 27043

src/lint/linter/ArcanistXHPASTLinter.php

Loading...

src/lint/linter/tests/xhpast/decl-parens-hug-closing.lint-test

Loading...

src/lint/linter/tests/xhpast/implicit-visibility.lint-test

Loading...