Page MenuHomePhabricator

Allow PHP version to be customized with `ArcanistXHPASTLinter`
ClosedPublic

Authored by joshuaspence on Jun 16 2014, 5:42 PM.
Tags
None
Referenced Files
F14393719: D9576.id22963.diff
Sun, Dec 22, 12:41 AM
Unknown Object (File)
Fri, Dec 20, 8:02 PM
Unknown Object (File)
Thu, Dec 12, 2:56 AM
Unknown Object (File)
Sat, Dec 7, 12:39 PM
Unknown Object (File)
Fri, Dec 6, 12:00 PM
Unknown Object (File)
Tue, Dec 3, 4:45 PM
Unknown Object (File)
Tue, Dec 3, 4:45 PM
Unknown Object (File)
Tue, Dec 3, 4:45 PM
Subscribers

Details

Summary

Fixes T5385. Provide a flexible means of setting a minimum PHP version for the ArcanistXHPASTLinter, instead of relying on ArcanistXHPASTLinter::LINT_PHP_53_FEATURES and ArcanistXHPASTLinter::LINT_PHP_54_FEATURES.

Test Plan

Fixed up and ran unit tests.

Diff Detail

Repository
rARC Arcanist
Branch
xhpast-version
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/lint/linter/ArcanistXHPASTLinter.php:425XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 1144
Build 1144: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Allow PHP version to be customized with `ArcanistXHPASTLinter`.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
src/lint/linter/ArcanistXHPASTLinter.php
192

Should maybe attempt to validate if $value looks like a valid PHP version.

368–370

Should probably look at whether LINT_PHP_53_FEATURES and/or LINT_PHP_54_FEATURES are being used (i.e. is a non-disabled severity set) and then:

  • Raise a deprecation warning.
  • Set $this->version if it is not already set.
401–408

This is probably wrong now and needs to be given some more thought.

joshuaspence edited edge metadata.
  • Raise deprecation warnings on LINT_PHP_53_FEATURES and LINT_PHP_54_FEATURES.
  • Set xhpast.php-version if it is not set but LINT_PHP_53_FEATURES or LINT_PHP_54_FEATURES is being used.

I don't really have any good ideas about the Windows issue beyond, like, adding xhpast.php-version.windows. :/

src/lint/linter/ArcanistXHPASTLinter.php
368–388

I think it's fine to get rid of these entirely, I strongly suspect only Phabricator is using them.

src/lint/linter/ArcanistXHPASTLinter.php
368–388

We use them where I work. I think leave them there for a short period of time.

joshuaspence edited edge metadata.
  • Add xhpast.php-version.windows.
epriestley edited edge metadata.
epriestley added inline comments.
src/lint/linter/ArcanistXHPASTLinter.php
396

Prefer Filesystem::readFile() to get a more useful, explicit exception on failure.

This revision is now accepted and ready to land.Jun 17 2014, 11:17 AM