diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php index 424c4c6a..41e1153e 100644 --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/engine/ArcanistLintEngine.php @@ -1,348 +1,348 @@ workingCopy = $working_copy; return $this; } public function getWorkingCopy() { return $this->workingCopy; } public function setPaths($paths) { $this->paths = $paths; return $this; } public function getPaths() { return $this->paths; } public function setPathChangedLines($path, $changed) { if ($changed === null) { $this->changedLines[$path] = null; } else { $this->changedLines[$path] = array_fill_keys($changed, true); } return $this; } public function getPathChangedLines($path) { 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; } public function setHookAPI(ArcanistHookAPI $hook_api) { $this->hookAPI = $hook_api; return $this; } public function getHookAPI() { return $this->hookAPI; } public function setEnableAsyncLint($enable_async_lint) { $this->enableAsyncLint = $enable_async_lint; return $this; } public function getEnableAsyncLint() { return $this->enableAsyncLint; } public function loadData($path) { if (!isset($this->fileData[$path])) { if ($this->getCommitHookMode()) { $this->fileData[$path] = $this->getHookAPI() ->getCurrentFileData($path); } else { $disk_path = $this->getFilePathOnDisk($path); $this->fileData[$path] = Filesystem::readFile($disk_path); } } return $this->fileData[$path]; } public function pathExists($path) { if ($this->getCommitHookMode()) { $file_data = $this->loadData($path); return ($file_data !== null); } else { $disk_path = $this->getFilePathOnDisk($path); return Filesystem::pathExists($disk_path); } } public function getFilePathOnDisk($path) { return Filesystem::resolvePath( $path, $this->getWorkingCopy()->getProjectRoot()); } public function setMinimumSeverity($severity) { $this->minimumSeverity = $severity; return $this; } public function getCommitHookMode() { return $this->commitHookMode; } public function run() { $stopped = array(); $linters = $this->buildLinters(); if (!$linters) { throw new ArcanistNoEffectException("No linters to run."); } $have_paths = false; foreach ($linters as $linter) { if ($linter->getPaths()) { $have_paths = true; break; } } if (!$have_paths) { throw new ArcanistNoEffectException("No paths are lintable."); } $exceptions = array(); foreach ($linters as $linter_name => $linter) { try { $linter->setEngine($this); if (!$linter->canRun()) { continue; } $paths = $linter->getPaths(); foreach ($paths as $key => $path) { // Make sure each path has a result generated, even if it is empty // (i.e., the file has no lint messages). $result = $this->getResultForPath($path); if (isset($stopped[$path])) { unset($paths[$key]); } } $paths = array_values($paths); if ($paths) { $linter->willLintPaths($paths); foreach ($paths as $path) { $linter->willLintPath($path); $linter->lintPath($path); if ($linter->didStopAllLinters()) { $stopped[$path] = true; } } } $minimum = $this->minimumSeverity; foreach ($linter->getLintMessages() as $message) { if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) { continue; } if (!$this->isRelevantMessage($message)) { continue; } $result = $this->getResultForPath($message->getPath()); $result->addMessage($message); } } catch (Exception $ex) { if (!is_string($linter_name)) { $linter_name = get_class($linter); } $exceptions[$linter_name] = $ex; } } foreach ($this->results as $path => $result) { $disk_path = $this->getFilePathOnDisk($path); $result->setFilePathOnDisk($disk_path); if (isset($this->fileData[$path])) { $result->setData($this->fileData[$path]); } else if ($disk_path && Filesystem::pathExists($disk_path)) { // TODO: this may cause us to, e.g., load a large binary when we only // raised an error about its filename. We could refine this by looking // through the lint messages and doing this load only if any of them // have original/replacement text or something like that. try { $this->fileData[$path] = Filesystem::readFile($disk_path); $result->setData($this->fileData[$path]); } catch (FilesystemException $ex) { // Ignore this, it's noncritical that we access this data and it // might be unreadable or a directory or whatever else for plenty // of legitimate reasons. } } } if ($exceptions) { throw new PhutilAggregateException('Some linters failed:', $exceptions); } return $this->results; } public function getResults() { return $this->results; } abstract protected function buildLinters(); private function isRelevantMessage($message) { // When a user runs "arc lint", we default to raising only warnings on // lines they have changed (errors are still raised anywhere in the // file). The list of $changed lines may be null, to indicate that the // path is a directory or a binary file so we should not exclude // warnings. $changed = $this->getPathChangedLines($message->getPath()); if ($changed === null || $message->isError() || !$message->getLine()) { return true; } $last_line = $message->getLine(); if ($message->getOriginalText()) { $last_line += substr_count($message->getOriginalText(), "\n"); } for ($l = $message->getLine(); $l <= $last_line; $l++) { if (!empty($changed[$l])) { return true; } } return false; } private function getResultForPath($path) { if (empty($this->results[$path])) { $result = new ArcanistLintResult(); $result->setPath($path); $this->results[$path] = $result; } return $this->results[$path]; } public function getLineAndCharFromOffset($path, $offset) { if (!isset($this->charToLine[$path])) { $char_to_line = array(); $line_to_first_char = array(); $lines = explode("\n", $this->loadData($path)); $line_number = 0; $line_start = 0; foreach ($lines as $line) { $len = strlen($line) + 1; // Account for "\n". $line_to_first_char[] = $line_start; $line_start += $len; for ($ii = 0; $ii < $len; $ii++) { $char_to_line[] = $line_number; } $line_number++; } $this->charToLine[$path] = $char_to_line; $this->lineToFirstChar[$path] = $line_to_first_char; } $line = $this->charToLine[$path][$offset]; $char = $offset - $this->lineToFirstChar[$path][$line]; return array($line, $char); } public function getPostponedLinters() { return $this->postponedLinters; } public function setPostponedLinters(array $linters) { $this->postponedLinters = $linters; return $this; } protected function getPEP8WithTextOptions() { // E101 is subset of TXT2 (Tab Literal). // E501 is same as TXT3 (Line Too Long). // W291 is same as TXT6 (Trailing Whitespace). // W292 is same as TXT4 (File Does Not End in Newline). // W293 is same as TXT6 (Trailing Whitespace). return '--ignore=E101,E501,W291,W292,W293'; } } diff --git a/src/unit/engine/PhpunitTestEngine.php b/src/unit/engine/PhpunitTestEngine.php index 5ed4fc5b..7dd5b936 100644 --- a/src/unit/engine/PhpunitTestEngine.php +++ b/src/unit/engine/PhpunitTestEngine.php @@ -1,410 +1,410 @@ projectRoot = $this->getWorkingCopy()->getProjectRoot(); $this->affectedTests = array(); foreach ($this->getPaths() as $path) { $path = Filesystem::resolvePath($path, $this->projectRoot); // TODO: add support for directories // Users can call phpunit on the directory themselves if (is_dir($path)) { continue; } // Not sure if it would make sense to go further if // it is not a .php file if (substr($path, -4) != '.php') { continue; } if (substr($path, -8) == 'Test.php') { // Looks like a valid test file name. $this->affectedTests[$path] = $path; continue; } if ($test = $this->findTestFile($path)) { $this->affectedTests[$path] = $test; } } if (empty($this->affectedTests)) { throw new ArcanistNoEffectException('No tests to run.'); } $this->prepareConfigFile(); $futures = array(); $tmpfiles = array(); foreach ($this->affectedTests as $class_path => $test_path) { $json_tmp = new TempFile(); $clover_tmp = null; $clover = null; if ($this->getEnableCoverage() !== false) { $clover_tmp = new TempFile(); $clover = csprintf('--coverage-clover %s', $clover_tmp); } $config = $this->configFile ? csprintf('-c %s', $this->configFile) : null; $futures[$test_path] = new ExecFuture('phpunit %C --log-json %s %C %s', $config, $json_tmp, $clover, $test_path); $tmpfiles[$test_path] = array( 'json' => $json_tmp, 'clover' => $clover_tmp, ); } $results = array(); foreach (Futures($futures)->limit(4) as $test => $future) { list($err, $stdout, $stderr) = $future->resolve(); $results[] = $this->parseTestResults($test_path, $tmpfiles[$test]['json'], $tmpfiles[$test]['clover']); } return array_mergev($results); } /** * We need this non-sense to make json generated by phpunit * valid. * * @param string $json_tmp Path to JSON report * * @return array JSON decoded array */ private function getJsonReport($json_tmp) { $json = Filesystem::readFile($json_tmp); if (empty($json)) { throw new Exception('JSON report file is empty, ' . 'it probably means that phpunit failed to run tests. ' . 'Try running arc unit with --trace option and then run ' . 'generated phpunit command yourself, you might get the ' . 'answer.' ); } $json = preg_replace('/}{\s*"/', '},{"', $json); $json = '[' . $json . ']'; $json = json_decode($json); if (!is_array($json)) { throw new Exception('JSON could not be decoded'); } return $json; } /** * Parse test results from phpunit json report * * @param string $path Path to test * @param string $json_path Path to phpunit json report * @param string $clover_tmp Path to phpunit clover report * * @return array */ private function parseTestResults($path, $json_tmp, $clover_tmp) { $test_results = Filesystem::readFile($json_tmp); $report = $this->getJsonReport($json_tmp); // coverage is for all testcases in the executed $path $coverage = array(); if ($this->getEnableCoverage() !== false) { $coverage = $this->readCoverage($clover_tmp); } $results = array(); foreach ($report as $event) { if ('test' != $event->event) { continue; } $status = ArcanistUnitTestResult::RESULT_PASS; $user_data = ''; if ('fail' == $event->status) { $status = ArcanistUnitTestResult::RESULT_FAIL; $user_data .= $event->message . "\n"; foreach ($event->trace as $trace) { $user_data .= sprintf("\n%s:%s", $trace->file, $trace->line); } } else if ('error' == $event->status) { if ('Skipped Test' == $event->message) { $status = ArcanistUnitTestResult::RESULT_SKIP; $user_data .= $event->message; } else if ('Incomplete Test' == $event->message) { $status = ArcanistUnitTestResult::RESULT_SKIP; $user_data .= $event->message; } else { $status = ArcanistUnitTestResult::RESULT_BROKEN; $user_data .= $event->message; foreach ($event->trace as $trace) { $user_data .= sprintf("\n%s:%s", $trace->file, $trace->line); } } } $name = preg_replace('/ \(.*\)/', '', $event->test); $result = new ArcanistUnitTestResult(); $result->setName($name); $result->setResult($status); $result->setDuration($event->time); $result->setCoverage($coverage); $result->setUserData($user_data); $results[] = $result; } return $results; } /** * Red the coverage from phpunit generated clover report * * @param string $path Path to report * * @return array */ private function readCoverage($path) { $test_results = Filesystem::readFile($path); if (empty($test_results)) { throw new Exception('Clover coverage XML report file is empty, ' . 'it probably means that phpunit failed to run tests. ' . 'Try running arc unit with --trace option and then run ' . 'generated phpunit command yourself, you might get the ' . 'answer.' ); } $coverage_dom = new DOMDocument(); $coverage_dom->loadXML($test_results); $reports = array(); $files = $coverage_dom->getElementsByTagName('file'); foreach ($files as $file) { $class_path = $file->getAttribute('name'); if (empty($this->affectedTests[$class_path])) { continue; } $test_path = $this->affectedTests[$file->getAttribute('name')]; // get total line count in file $line_count = count(file($class_path)); $coverage = ''; $start_line = 1; $lines = $file->getElementsByTagName('line'); for ($ii = 0; $ii < $lines->length; $ii++) { $line = $lines->item($ii); for (; $start_line < $line->getAttribute('num'); $start_line++) { $coverage .= 'N'; } if ($line->getAttribute('type') != 'stmt') { $coverage .= 'N'; } else { if ((int) $line->getAttribute('count') == 0) { $coverage .= 'U'; } else if ((int) $line->getAttribute('count') > 0) { $coverage .= 'C'; } } $start_line++; } for (; $start_line <= $line_count; $start_line++) { $coverage .= 'N'; } $len = strlen($this->projectRoot . DIRECTORY_SEPARATOR); $class_path = substr($class_path, $len); $reports[$class_path] = $coverage; } return $reports; } /** * Search for test cases for a given file in a large number of "reasonable" * locations. See @{method:getSearchLocationsForTests} for specifics. * * TODO: Add support for finding tests in testsuite folders from * phpunit.xml configuration. * * @param string PHP file to locate test cases for. * @return string|null Path to test cases, or null. */ private function findTestFile($path) { $root = $this->projectRoot; $path = Filesystem::resolvePath($path, $root); $file = basename($path); $possible_files = array( $file, substr($file, 0, -4).'Test.php', ); $search = self::getSearchLocationsForTests($path); foreach ($search as $search_path) { foreach ($possible_files as $possible_file) { $full_path = $search_path.$possible_file; if (!Filesystem::pathExists($full_path)) { // If the file doesn't exist, it's clearly a miss. continue; } if (!Filesystem::isDescendant($full_path, $root)) { // Don't look above the project root. continue; } if (Filesystem::resolvePath($full_path) == $path) { // Don't return the original file. continue; } return $full_path; } } return null; } /** * Get places to look for PHP Unit tests that cover a given file. For some * file "/a/b/c/X.php", we look in the same directory: * * /a/b/c/ * * We then look in all parent directories for a directory named "tests/" * (or "Tests/"): * * /a/b/c/tests/ * /a/b/tests/ * /a/tests/ * /tests/ * * We also try to replace each directory component with "tests/": * * /a/b/tests/ * /a/tests/c/ * /tests/b/c/ * * We also try to add "tests/" at each directory level: * * /a/b/c/tests/ * /a/b/tests/c/ * /a/tests/b/c/ * /tests/a/b/c/ * * This finds tests with a layout like: * * docs/ * src/ * tests/ * * ...or similar. This list will be further pruned by the caller; it is * intentionally filesystem-agnostic to be unit testable. * * @param string PHP file to locate test cases for. * @return list List of directories to search for tests in. */ public static function getSearchLocationsForTests($path) { $file = basename($path); $dir = dirname($path); $test_dir_names = array('tests', 'Tests'); $try_directories = array(); // Try in the current directory. $try_directories[] = array($dir); // Try in a tests/ directory anywhere in the ancestry. foreach (Filesystem::walkToRoot($dir) as $parent_dir) { if ($parent_dir == '/') { // We'll restore this later. $parent_dir = ''; } foreach ($test_dir_names as $test_dir_name) { $try_directories[] = array($parent_dir, $test_dir_name); } } // Try replacing each directory component with 'tests/'. $parts = trim($dir, DIRECTORY_SEPARATOR); $parts = explode(DIRECTORY_SEPARATOR, $parts); foreach (array_reverse(array_keys($parts)) as $key) { foreach ($test_dir_names as $test_dir_name) { $try = $parts; $try[$key] = $test_dir_name; array_unshift($try, ''); $try_directories[] = $try; } } // Try adding 'tests/' at each level. foreach (array_reverse(array_keys($parts)) as $key) { foreach ($test_dir_names as $test_dir_name) { $try = $parts; $try[$key] = $test_dir_name.DIRECTORY_SEPARATOR.$try[$key]; array_unshift($try, ''); $try_directories[] = $try; } } $results = array(); foreach ($try_directories as $parts) { $results[implode(DIRECTORY_SEPARATOR, $parts).DIRECTORY_SEPARATOR] = true; } return array_keys($results); } /** * Tries to find and update phpunit configuration file * based on phpunit_config option in .arcconfig */ private function prepareConfigFile() { $project_root = $this->projectRoot . DIRECTORY_SEPARATOR; if ($config = $this->getWorkingCopy()->getConfig('phpunit_config')) { if (Filesystem::pathExists($project_root . $config)) { $this->configFile = $project_root . $config; } else { throw new Exception('PHPUnit configuration file was not ' . 'found in ' . $project_root . $config); } } } } diff --git a/src/workflow/ArcanistSvnHookPreCommitWorkflow.php b/src/workflow/ArcanistSvnHookPreCommitWorkflow.php index a89a8a6e..3ec3af08 100644 --- a/src/workflow/ArcanistSvnHookPreCommitWorkflow.php +++ b/src/workflow/ArcanistSvnHookPreCommitWorkflow.php @@ -1,231 +1,231 @@ 'svnargs', ); } public function shouldShellComplete() { return false; } public function run() { $svnargs = $this->getArgument('svnargs'); $repository = $svnargs[0]; $transaction = $svnargs[1]; list($commit_message) = execx( 'svnlook log --transaction %s %s', $transaction, $repository); if (strpos($commit_message, '@bypass-lint') !== false) { return 0; } // TODO: Do stuff with commit message. list($changed) = execx( 'svnlook changed --transaction %s %s', $transaction, $repository); $paths = array(); $changed = explode("\n", trim($changed)); foreach ($changed as $line) { $matches = null; preg_match('/^..\s*(.*)$/', $line, $matches); $paths[$matches[1]] = strlen($matches[1]); } $resolved = array(); $failed = array(); $missing = array(); $found = array(); asort($paths); foreach ($paths as $path => $length) { foreach ($resolved as $rpath => $root) { if (!strncmp($path, $rpath, strlen($rpath))) { $resolved[$path] = $root; continue 2; } } $config = $path; if (basename($config) == '.arcconfig') { $resolved[$config] = $config; continue; } $config = rtrim($config, '/'); $last_config = $config; do { if (!empty($missing[$config])) { break; } else if (!empty($found[$config])) { $resolved[$path] = $found[$config]; break; } list($err) = exec_manual( 'svnlook cat --transaction %s %s %s', $transaction, $repository, $config ? $config.'/.arcconfig' : '.arcconfig'); if ($err) { $missing[$path] = true; } else { $resolved[$path] = $config ? $config.'/.arcconfig' : '.arcconfig'; $found[$config] = $resolved[$path]; break; } $config = dirname($config); if ($config == '.') { $config = ''; } if ($config == $last_config) { break; } $last_config = $config; } while (true); if (empty($resolved[$path])) { $failed[] = $path; } } if ($failed && $resolved) { $failed_paths = ' '.implode("\n ", $failed); $resolved_paths = ' '.implode("\n ", array_keys($resolved)); throw new ArcanistUsageException( "This commit includes a mixture of files in Arcanist projects and ". "outside of Arcanist projects. A commit which affects an Arcanist ". "project must affect only that project.\n\n". "Files in projects:\n\n". $resolved_paths."\n\n". "Files not in projects:\n\n". $failed_paths); } if (!$resolved) { // None of the affected paths are beneath a .arcconfig file. return 0; } $groups = array(); foreach ($resolved as $path => $project) { $groups[$project][] = $path; } if (count($groups) > 1) { $message = array(); foreach ($groups as $project => $group) { $message[] = "Files underneath '{$project}':\n\n"; $message[] = " ".implode("\n ", $group)."\n\n"; } $message = implode('', $message); throw new ArcanistUsageException( "This commit includes a mixture of files from different Arcanist ". "projects. A commit which affects an Arcanist project must affect ". "only that project.\n\n". $message); } $config_file = key($groups); $project_root = dirname($config_file); $paths = reset($groups); list($config) = execx( 'svnlook cat --transaction %s %s %s', $transaction, $repository, $config_file); $working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile( $project_root, $config, $config_file." (svnlook: {$transaction} {$repository})"); $repository_api = new ArcanistSubversionHookAPI( $project_root, $transaction, $repository); - $lint_engine = $working_copy->getConfig('lint_engine'); + $lint_engine = $working_copy->getConfig('lint.engine'); if (!$lint_engine) { return 0; } $engine = newv($lint_engine, array()); $engine->setWorkingCopy($working_copy); $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ERROR); $engine->setPaths($paths); $engine->setCommitHookMode(true); $engine->setHookAPI($repository_api); try { $results = $engine->run(); } catch (ArcanistNoEffectException $no_effect) { // Nothing to do, bail out. return 0; } $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); $renderer = new ArcanistLintConsoleRenderer(); foreach ($failures as $result) { echo $renderer->renderLintResult($result); } return 1; } return 0; } }