Page MenuHomePhabricator

Make it easier to parse external linter versions
Needs ReviewPublic

Authored by joshuaspence on May 20 2019, 1:57 AM.

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Summary

Currently pretty much every external linter has copy-pasted the following code (with minor variations):

public function getVersion() {
  list($stdout) = execx('%C --version', $this->getExecutableCommand());

  $matches = null;
  if (!preg_match('/^v(?P<version>\d+\.\d+\.\d+)$/', $stdout, $matches)) {
    return null;
  }

  return $matches['version'];
}

We can reduce the boilerplate code here by providing a simpler interface consisting of getVersionFlags and parseVersionOutput, similar to getMandatoryFlags and parseLinterOutput. This change also makes it possible for me to proxy the getVersion method in ArcanistDockerContainerLinterProxy.

Test Plan

Installed a bunch of the external linters and ran unit tests.

Diff Detail

Repository
rARC Arcanist
Branch
lint2
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 22903
Build 31420: Run Core Tests
Build 31419: arc lint + arc unit

Event Timeline

joshuaspence created this revision.May 20 2019, 1:57 AM
joshuaspence requested review of this revision.May 20 2019, 1:57 AM
joshuaspence added inline comments.May 20 2019, 2:05 AM
src/lint/linter/ArcanistExternalLinter.php
401

I'm open to suggestions for a better name for this method.

408

I'm open to suggestions for a better name for this method.

Remove unrelated change

Semi-related thought: the current behavior of getVersion (return a string if successful, false if unable to determine the version or null if no version exists) is rather awkward and I feel like we should throw some sort of ArcanistUnableToDetermineLinterVersionException instead of returning false.

joshuaspence edited the summary of this revision. (Show Details)May 22 2019, 12:32 AM
joshuaspence edited the summary of this revision. (Show Details)May 22 2019, 3:24 AM
joshuaspence updated this revision to Diff 48989.EditedMay 22 2019, 3:26 AM

Implement parseVersionOutput in ArcanistExternalLinter so that it can be called from within another instance of ArcanistExternalLinter

joshuaspence edited the summary of this revision. (Show Details)May 23 2019, 10:10 PM