Page MenuHomePhabricator

Allow PHP version to be customized with `ArcanistXHPASTLinter`
ClosedPublic

Authored by joshuaspence on Jun 16 2014, 5:42 PM.

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
Lint
Lint Skipped
Unit
Tests Skipped

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
191

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

369–371

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.
509–516

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
369–389

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

src/lint/linter/ArcanistXHPASTLinter.php
369–389

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
397

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