From ca98fc175f93062152d8e4eb58441201cca92313 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 15 Feb 2011 14:57:24 -0800 Subject: [PATCH] Add lint support to 'arc svn-hook-pre-commit'. Summary: The biggest blocker on getting rid of arc in trunk is that the lint rules in the commit hooks are still running the old version. Push arc commit hook support toward some reasonable state of approximately working. Test Plan: Reviewers: CC: --- src/lint/engine/base/ArcanistLintEngine.php | 22 +++++- src/lint/engine/phutil/PhutilLintEngine.php | 5 -- src/lint/engine/phutil/__init__.php | 4 +- src/workflow/amend/ArcanistAmendWorkflow.php | 4 ++ .../ArcanistSvnHookPreCommitWorkflow.php | 69 +++++++++++++++++-- src/workflow/svn-hook-pre-commit/__init__.php | 5 ++ .../ArcanistWorkingCopyIdentity.php | 43 +++++++----- 7 files changed, 119 insertions(+), 33 deletions(-) diff --git a/src/lint/engine/base/ArcanistLintEngine.php b/src/lint/engine/base/ArcanistLintEngine.php index d3291ab7..2f4071b3 100644 --- a/src/lint/engine/base/ArcanistLintEngine.php +++ b/src/lint/engine/base/ArcanistLintEngine.php @@ -27,6 +27,7 @@ abstract class ArcanistLintEngine { private $minimumSeverity = null; private $changedLines = array(); + private $commitHookMode = false; public function __construct() { @@ -59,6 +60,16 @@ abstract class ArcanistLintEngine { return idx($this->changedLines, $path); } + public function setFileData($data) { + $this->fileData = $data + $this->fileData; + return $this; + } + + public function setCommitHookMode($mode) { + $this->commitHookMode = $mode; + return $this; + } + protected function loadData($path) { if (!isset($this->fileData[$path])) { $disk_path = $this->getFilePathOnDisk($path); @@ -67,6 +78,15 @@ abstract class ArcanistLintEngine { return $this->fileData[$path]; } + public function pathExists($path) { + if ($this->getCommitHookMode()) { + return (idx($this->fileData, $path) !== null); + } else { + $disk_path = $this->getFilePathOnDisk($path); + return Filesystem::pathExists($disk_path); + } + } + public function getFilePathOnDisk($path) { return Filesystem::resolvePath( $path, @@ -79,7 +99,7 @@ abstract class ArcanistLintEngine { } public function getCommitHookMode() { - return false; + return $this->commitHookMode; } public function run() { diff --git a/src/lint/engine/phutil/PhutilLintEngine.php b/src/lint/engine/phutil/PhutilLintEngine.php index 6d16e4eb..d393c8c9 100644 --- a/src/lint/engine/phutil/PhutilLintEngine.php +++ b/src/lint/engine/phutil/PhutilLintEngine.php @@ -93,9 +93,4 @@ class PhutilLintEngine extends ArcanistLintEngine { return $linters; } - public function pathExists($path) { - $disk_path = $this->getFilePathOnDisk($path); - return Filesystem::pathExists($disk_path); - } - } diff --git a/src/lint/engine/phutil/__init__.php b/src/lint/engine/phutil/__init__.php index c9a4bd11..77ce9ef9 100644 --- a/src/lint/engine/phutil/__init__.php +++ b/src/lint/engine/phutil/__init__.php @@ -7,14 +7,12 @@ phutil_require_module('arcanist', 'lint/engine/base'); +phutil_require_module('arcanist', 'lint/linter/apachelicense'); phutil_require_module('arcanist', 'lint/linter/filename'); phutil_require_module('arcanist', 'lint/linter/generated'); phutil_require_module('arcanist', 'lint/linter/phutilmodule'); phutil_require_module('arcanist', 'lint/linter/text'); phutil_require_module('arcanist', 'lint/linter/xhpast'); -phutil_require_module('arcanist', 'lint/linter/apachelicense'); - -phutil_require_module('phutil', 'filesystem'); phutil_require_source('PhutilLintEngine.php'); diff --git a/src/workflow/amend/ArcanistAmendWorkflow.php b/src/workflow/amend/ArcanistAmendWorkflow.php index e8801099..afa5c605 100644 --- a/src/workflow/amend/ArcanistAmendWorkflow.php +++ b/src/workflow/amend/ArcanistAmendWorkflow.php @@ -95,6 +95,10 @@ EOTEXT } } + // TODO: The old 'arc amend' had a check here to see if you were running + // 'arc amend' with an explicit revision but HEAD already had another + // revision in it. Maybe this is worth restoring? + $conduit = $this->getConduit(); $message = $conduit->callMethodSynchronous( 'differential.getcommitmessage', diff --git a/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php b/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php index 02ccdcb6..13d14289 100644 --- a/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php +++ b/src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php @@ -20,7 +20,7 @@ class ArcanistSvnHookPreCommitWorkflow extends ArcanistBaseWorkflow { public function getCommandHelp() { return phutil_console_format(<<getConfig('lint_engine'); + if (!$lint_engine) { + return 0; + } + + PhutilSymbolLoader::loadClass($lint_engine); + + $engine = newv($lint_engine, array()); + $engine->setWorkingCopy($working_copy); + $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ERROR); + $engine->setPaths(array_keys($data)); + $engine->setFileData($data); + $engine->setCommitHookMode(true); + + $results = $engine->run(); + + $renderer = new ArcanistLintRenderer(); + $failures = array(); + foreach ($results as $result) { + if (!$result->getMessages()) { + continue; + } + $failures[] = $result; + } + + if ($failures) { + $at = "@"; + $msg = phutil_console_format( + "\n**LINT ERRORS**\n\n". + "This changeset has lint errors. You must fix all lint errors before ". + "you can commit.\n\n". + "You can add '{$at}bypass-lint' to your commit message to disable ". + "lint checks for this commit, or '{$at}nolint' to the file with ". + "errors to disable lint for that file.\n\n"); + echo phutil_console_wrap($msg); + foreach ($failures as $result) { + echo $renderer->renderLintResult($result); + } + return 1; + } + + return 99; } } diff --git a/src/workflow/svn-hook-pre-commit/__init__.php b/src/workflow/svn-hook-pre-commit/__init__.php index e3f69614..f80fe11c 100644 --- a/src/workflow/svn-hook-pre-commit/__init__.php +++ b/src/workflow/svn-hook-pre-commit/__init__.php @@ -7,10 +7,15 @@ phutil_require_module('arcanist', 'exception/usage'); +phutil_require_module('arcanist', 'lint/renderer'); +phutil_require_module('arcanist', 'lint/severity'); phutil_require_module('arcanist', 'workflow/base'); +phutil_require_module('arcanist', 'workingcopyidentity'); phutil_require_module('phutil', 'console'); phutil_require_module('phutil', 'future/exec'); +phutil_require_module('phutil', 'symbols'); +phutil_require_module('phutil', 'utils'); phutil_require_source('ArcanistSvnHookPreCommitWorkflow.php'); diff --git a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php b/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php index 2d8fd2e9..fbde4ff6 100644 --- a/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php +++ b/src/workingcopyidentity/ArcanistWorkingCopyIdentity.php @@ -31,29 +31,38 @@ class ArcanistWorkingCopyIdentity { continue; } $proj_raw = Filesystem::readFile($config_file); - $proj = json_decode($proj_raw, true); - if (!is_array($proj)) { - throw new Exception( - "Unable to parse '.arcconfig' file in '{$dir}'. The file contents ". - "should be valid JSON."); - } - $required_keys = array( - 'project_id', - ); - foreach ($required_keys as $key) { - if (!array_key_exists($key, $proj)) { - throw new Exception( - "Required key '{$key}' is missing from '.arcconfig' file in ". - "'{$dir}'."); - } - } - $config = $proj; + $config = self::parseRawConfigFile($proj_raw); $project_root = $dir; break; } return new ArcanistWorkingCopyIdentity($project_root, $config); } + public static function newFromRootAndConfigFile($root, $config_raw) { + $config = self::parseRawConfigFile($config_raw); + return new ArcanistWorkingCopyIdentity($root, $config); + } + + private static function parseRawConfigFile($raw_config) { + $proj = json_decode($raw_config, true); + if (!is_array($proj)) { + throw new Exception( + "Unable to parse '.arcconfig' file in '{$dir}'. The file contents ". + "should be valid JSON."); + } + $required_keys = array( + 'project_id', + ); + foreach ($required_keys as $key) { + if (!array_key_exists($key, $proj)) { + throw new Exception( + "Required key '{$key}' is missing from '.arcconfig' file in ". + "'{$dir}'."); + } + } + return $proj; + } + protected function __construct($root, array $config) { $this->projectRoot = $root; $this->projectConfig = $config;