Page MenuHomePhabricator
Create Task
Maniphest T7170

Support beta versions of PHP Code Sniffer
Open, Needs TriagePublic
Actions

Description

Using arcanist on git commit 59640f7eae3f7c0a352c18e784fb55acb7967991, the following error can be seen:

$  bin/arc unit --everything --no-coverage
   FAIL  ArcanistPhpcsLinterTestCase::testVersion
Assertion failed, expected 'true' (at ArcanistExternalLinterTestCase.php:10): Failed to parse version from command.

ACTUAL VALUE
false

I am using PHP_CodeSniffer 1.5.0RC4, so it seems that regexp in src/lint/linter/ArcanistPhpcsLinter.php does not match

  • $ phpcs --version
  • PHP_CodeSniffer version 1.5.0RC4 (beta) by Squiz (http://www.squiz.net)

Here is a quick and dirty patch:

diff --git a/src/lint/linter/ArcanistPhpcsLinter.php b/src/lint/linter/ArcanistPhpcsLinter.php
index 8475c55..5364ec3 100644
--- a/src/lint/linter/ArcanistPhpcsLinter.php
+++ b/src/lint/linter/ArcanistPhpcsLinter.php
@@ -88,7 +88,7 @@ final class ArcanistPhpcsLinter extends ArcanistExternalLinter {
     list($stdout) = execx('%C --version', $this->getExecutableCommand());
 
     $matches = array();
-    $regex = '/^PHP_CodeSniffer version (?P<version>\d+\.\d+\.\d+)\b/';
+    $regex = '/^PHP_CodeSniffer version (?P<version>\d+\.\d+\.\d+)(RC\d)?\b/';
     if (preg_match($regex, $stdout, $matches)) {
       return $matches['version'];
     } else {

List of all release, beta, and alpha versions:
http://pear.php.net/package/PHP_CodeSniffer/download

Related Objects

Event Timeline

beber created this task.Feb 5 2015, 1:45 AM
beber raised the priority of this task from to Needs Triage.
beber updated the task description. (Show Details)
beber added a subscriber: beber.
chad renamed this task from Test failure with git 59640f7eae3f7c0a352c18e784fb55acb7967991 to Support beta versions of PHP Code Sniffer.Feb 5 2015, 1:57 AM
chad added a project: Lint.
chad added a subscriber: chad.

Updating PHP Code Sniffer makes the error go away, correct?

beber added a comment.Feb 5 2015, 1:59 AM

I did not change the PHP Code Sniffer release, I applied the patch proposed. But this is not the only one issue mentionned.

chad added a comment.Feb 5 2015, 1:59 AM

If you have multiple bugs, please file them separately.

chad added a subscriber: joshuaspence.Feb 5 2015, 9:12 PM
beber updated the task description. (Show Details)Feb 8 2015, 8:36 PM
beber updated the task description. (Show Details)
chad updated the task description. (Show Details)Feb 8 2015, 8:40 PM
joshuaspence added a comment.Feb 8 2015, 8:41 PM

Does the ArcanistPhpcsLinterTestCase::testLinter test execute successfully?

beber added a comment.Feb 8 2015, 8:45 PM
In T7170#95394, @joshuaspence wrote:

Does the ArcanistPhpcsLinterTestCase::testLinter test execute successfully?

I can confirm that, yes ArcanistPhpcsLinterTestCase::testLinter return is PASS.

joshuaspence added a comment.Feb 8 2015, 8:53 PM

OK, so I have a couple of thoughts:

  • The getVersion() stuff isn't widely used. I believe that this is only used for caching linter results (which is an advanced option which is disabled by default and documented as "probably not working"). There are plans to extend this functionality in the future, but it hasn't seen much demand.
  • Generally, in terms of external linters, we try to remain conpatible with the latest stable version. I'm not sure how much value there is in supporting release candidates.
  • I wouldn't expect this to cause any issues from a users perspective.

This is maybe a weak argument for having some kind of PhutilVersionNumber parser.

epriestley moved this task from Backlog to Bindings on the Lint board.Dec 22 2015, 12:47 PM
epriestley mentioned this in T10038: Plan the mid-term pathway for unit-test/linter bindings.Dec 22 2015, 12:59 PM
Phabricator · Built by Phacility