From be803ce5777ce12a08dc617e88d521f9b851475e Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Mon, 12 May 2014 04:46:56 -0700 Subject: [PATCH] Remove the `getConfig` and `setConfig` method. Summary: It seems that there is a lot of overlap between `getConfig` / `setConfig` and `getLinterConfigurationOptions` / `setLinterConfigurationValue` respectively. Test Plan: `arc lint` and `arc unit`. Reviewers: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Differential Revision: https://secure.phabricator.com/D9067 --- src/lint/engine/ComprehensiveLintEngine.php | 2 +- src/lint/linter/ArcanistLesscLinter.php | 22 +++++++++-- src/lint/linter/ArcanistLinter.php | 10 ----- src/lint/linter/ArcanistXHPASTLinter.php | 38 +++++++++++++++++-- .../__tests__/ArcanistLinterTestCase.php | 5 ++- .../__tests__/xhpast/switches.lint-test | 2 +- 6 files changed, 59 insertions(+), 20 deletions(-) diff --git a/src/lint/engine/ComprehensiveLintEngine.php b/src/lint/engine/ComprehensiveLintEngine.php index 9d46cfb4..a56d5f7b 100644 --- a/src/lint/engine/ComprehensiveLintEngine.php +++ b/src/lint/engine/ComprehensiveLintEngine.php @@ -33,7 +33,7 @@ final class ComprehensiveLintEngine extends ArcanistLintEngine { $py_paths = preg_grep('/\.py$/', $paths); $linters[] = id(new ArcanistPyFlakesLinter())->setPaths($py_paths); $linters[] = id(new ArcanistPEP8Linter()) - ->setConfig(array('flags' => array($this->getPEP8WithTextOptions()))) + ->setFlags(array($this->getPEP8WithTextOptions())) ->setPaths($py_paths); $linters[] = id(new ArcanistRubyLinter()) diff --git a/src/lint/linter/ArcanistLesscLinter.php b/src/lint/linter/ArcanistLesscLinter.php index c267389e..cc881fc9 100644 --- a/src/lint/linter/ArcanistLesscLinter.php +++ b/src/lint/linter/ArcanistLesscLinter.php @@ -16,6 +16,9 @@ final class ArcanistLesscLinter extends ArcanistExternalLinter { const LINT_PARSE_ERROR = 6; const LINT_SYNTAX_ERROR = 7; + private $strictMath = false; + private $strictUnits = false; + public function getInfoName() { return pht('Less'); } @@ -54,6 +57,19 @@ final class ArcanistLesscLinter extends ArcanistExternalLinter { ); } + public function setLinterConfigurationValue($key, $value) { + switch ($key) { + case 'lessc.strict-math': + $this->strictMath = $value; + return; + case 'lessc.strict-units': + $this->strictUnits = $value; + return; + } + + return parent::setLinterConfigurationValue($key, $value); + } + public function getLintNameMap() { return array( self::LINT_RUNTIME_ERROR => pht('Runtime Error'), @@ -104,10 +120,8 @@ final class ArcanistLesscLinter extends ArcanistExternalLinter { return array( '--lint', '--no-color', - '--strict-math='. - ($this->getConfig('lessc.strict-math') ? 'on' : 'off'), - '--strict-units='. - ($this->getConfig('lessc.strict-units') ? 'on' : 'off')); + '--strict-math='.($this->strictMath ? 'on' : 'off'), + '--strict-units='.($this->strictUnits ? 'on' : 'off')); } protected function parseLinterOutput($path, $err, $stdout, $stderr) { diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php index 90d3b46b..f599505a 100644 --- a/src/lint/linter/ArcanistLinter.php +++ b/src/lint/linter/ArcanistLinter.php @@ -24,7 +24,6 @@ abstract class ArcanistLinter { private $customSeverityMap = array(); private $customSeverityRules = array(); - private $config = array(); /* -( Human Readable Information )---------------------------------------- */ @@ -89,15 +88,6 @@ abstract class ArcanistLinter { return $this; } - public function setConfig(array $config) { - $this->config = $config; - return $this; - } - - protected function getConfig($key, $default = null) { - return idx($this->config, $key, $default); - } - public function getActivePath() { return $this->activePath; } diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php index e74cb71d..8234d9df 100644 --- a/src/lint/linter/ArcanistXHPASTLinter.php +++ b/src/lint/linter/ArcanistXHPASTLinter.php @@ -44,6 +44,9 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { const LINT_REUSED_ITERATOR_REFERENCE = 39; const LINT_KEYWORD_CASING = 40; + private $naminghook; + private $switchhook; + public function getLintNameMap() { return array( self::LINT_PHP_SYNTAX_ERROR => 'PHP Syntax Error!', @@ -129,6 +132,32 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { ); } + public function getLinterConfigurationOptions() { + return parent::getLinterConfigurationOptions() + array( + 'xhpast.naminghook' => array( + 'type' => 'optional ArcanistXHPASTLintNamingHook', + 'help' => pht(''), + ), + 'xhpast.switchhook' => array( + 'type' => 'optional ArcanistXHPASTLintSwitchHook', + 'help' => pht(''), + ), + ); + } + + public function setLinterConfigurationValue($key, $value) { + switch ($key) { + case 'xhpast.naminghook': + $this->naminghook = $value; + return; + case 'xhpast.switchhook': + $this->switchhook = $value; + return; + } + + return parent::setLinterConfigurationValue($key, $value); + } + public function getCacheVersion() { $version = '4'; $path = xhpast_get_binary_path(); @@ -464,8 +493,9 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { $hook_obj = null; $working_copy = $this->getEngine()->getWorkingCopy(); if ($working_copy) { - $hook_class = $working_copy->getProjectConfig('lint.xhpast.switchhook'); - $hook_class = $this->getConfig('switchhook', $hook_class); + $hook_class = $this->switchhook + ? $this->switchhook + : $this->getDeprecatedConfiguration('lint.xhpast.switchhook'); if ($hook_class) { $hook_obj = newv($hook_class, array()); assert_instances_of(array($hook_obj), 'ArcanistXHPASTLintSwitchHook'); @@ -1673,7 +1703,9 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter { if ($working_copy) { // If a naming hook is configured, give it a chance to override the // default results for all the symbol names. - $hook_class = $working_copy->getProjectConfig('lint.xhpast.naminghook'); + $hook_class = $this->naminghook + ? $this->naminghook + : $working_copy->getProjectConfig('lint.xhpast.naminghook'); if ($hook_class) { $hook_obj = newv($hook_class, array()); foreach ($names as $k => $name_attrs) { diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php index 6e221ad4..d3592ea4 100644 --- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php @@ -96,7 +96,10 @@ abstract class ArcanistLinterTestCase extends ArcanistPhutilTestCase { $path_name = idx($config, 'path', $path); $linter->addPath($path_name); $linter->addData($path_name, $data); - $linter->setConfig(idx($config, 'config', array())); + $config = idx($config, 'config', array()); + foreach ($config as $key => $value) { + $linter->setLinterConfigurationValue($key, $value); + } $engine->addLinter($linter); $engine->addFileData($path_name, $data); diff --git a/src/lint/linter/__tests__/xhpast/switches.lint-test b/src/lint/linter/__tests__/xhpast/switches.lint-test index b32d55cd..dda2c082 100644 --- a/src/lint/linter/__tests__/xhpast/switches.lint-test +++ b/src/lint/linter/__tests__/xhpast/switches.lint-test @@ -94,4 +94,4 @@ warning:71:3 warning:75:3 ~~~~~~~~~~ ~~~~~~~~~~ -{"config":{"switchhook":"ArcanistXHPASTLintTestSwitchHook"}} +{"config":{"xhpast.switchhook":"ArcanistXHPASTLintTestSwitchHook"}}