diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -126,14 +126,12 @@ 'ArcanistCommitUpstreamHardpointLoader' => 'loader/ArcanistCommitUpstreamHardpointLoader.php', 'ArcanistCommitWorkflow' => 'workflow/ArcanistCommitWorkflow.php', 'ArcanistCompilerLintRenderer' => 'lint/renderer/ArcanistCompilerLintRenderer.php', - 'ArcanistComprehensiveLintEngine' => 'lint/engine/ArcanistComprehensiveLintEngine.php', 'ArcanistConcatenationOperatorXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistConcatenationOperatorXHPASTLinterRule.php', 'ArcanistConcatenationOperatorXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistConcatenationOperatorXHPASTLinterRuleTestCase.php', 'ArcanistConduitCall' => 'conduit/ArcanistConduitCall.php', 'ArcanistConduitEngine' => 'conduit/ArcanistConduitEngine.php', 'ArcanistConduitException' => 'conduit/ArcanistConduitException.php', 'ArcanistConfigOption' => 'config/option/ArcanistConfigOption.php', - 'ArcanistConfigurationDrivenLintEngine' => 'lint/engine/ArcanistConfigurationDrivenLintEngine.php', 'ArcanistConfigurationEngine' => 'config/ArcanistConfigurationEngine.php', 'ArcanistConfigurationEngineExtension' => 'config/ArcanistConfigurationEngineExtension.php', 'ArcanistConfigurationManager' => 'configuration/ArcanistConfigurationManager.php', @@ -264,7 +262,7 @@ 'ArcanistLanguageConstructParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLanguageConstructParenthesesXHPASTLinterRule.php', 'ArcanistLanguageConstructParenthesesXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistLanguageConstructParenthesesXHPASTLinterRuleTestCase.php', 'ArcanistLiberateWorkflow' => 'workflow/ArcanistLiberateWorkflow.php', - 'ArcanistLintEngine' => 'lint/engine/ArcanistLintEngine.php', + 'ArcanistLintEngine' => 'lint/operation/ArcanistLintEngine.php', 'ArcanistLintMessage' => 'lint/ArcanistLintMessage.php', 'ArcanistLintMessageTestCase' => 'lint/__tests__/ArcanistLintMessageTestCase.php', 'ArcanistLintPatcher' => 'lint/ArcanistLintPatcher.php', @@ -383,7 +381,6 @@ 'ArcanistSetting' => 'configuration/ArcanistSetting.php', 'ArcanistSettings' => 'configuration/ArcanistSettings.php', 'ArcanistShellCompleteWorkflow' => 'toolset/workflow/ArcanistShellCompleteWorkflow.php', - 'ArcanistSingleLintEngine' => 'lint/engine/ArcanistSingleLintEngine.php', 'ArcanistSlownessXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistSlownessXHPASTLinterRule.php', 'ArcanistSlownessXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistSlownessXHPASTLinterRuleTestCase.php', 'ArcanistSpellingLinter' => 'lint/linter/ArcanistSpellingLinter.php', @@ -431,7 +428,6 @@ 'ArcanistUnitSink' => 'unit/sink/ArcanistUnitSink.php', 'ArcanistUnitTestResult' => 'unit/ArcanistUnitTestResult.php', 'ArcanistUnitTestResultTestCase' => 'unit/__tests__/ArcanistUnitTestResultTestCase.php', - 'ArcanistUnitTestableLintEngine' => 'lint/engine/ArcanistUnitTestableLintEngine.php', 'ArcanistUnitWorkflow' => 'workflow/ArcanistUnitWorkflow.php', 'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistUnnecessaryFinalModifierXHPASTLinterRule.php', 'ArcanistUnnecessaryFinalModifierXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistUnnecessaryFinalModifierXHPASTLinterRuleTestCase.php', @@ -462,6 +458,7 @@ 'ArcanistWorkflowInformation' => 'toolset/ArcanistWorkflowInformation.php', 'ArcanistWorkingCopy' => 'workingcopy/ArcanistWorkingCopy.php', 'ArcanistWorkingCopyConfigurationSource' => 'config/source/ArcanistWorkingCopyConfigurationSource.php', + 'ArcanistWorkingCopyPath' => 'workingcopy/ArcanistWorkingCopyPath.php', 'ArcanistWorkingCopyStateRef' => 'ref/ArcanistWorkingCopyStateRef.php', 'ArcanistXHPASTLintNamingHook' => 'lint/linter/xhpast/ArcanistXHPASTLintNamingHook.php', 'ArcanistXHPASTLintNamingHookTestCase' => 'lint/linter/xhpast/__tests__/ArcanistXHPASTLintNamingHookTestCase.php', @@ -1128,7 +1125,7 @@ 'ArcanistBackoutWorkflow' => 'ArcanistWorkflow', 'ArcanistBaseCommitParser' => 'Phobject', 'ArcanistBaseCommitParserTestCase' => 'PhutilTestCase', - 'ArcanistBaseXHPASTLinter' => 'ArcanistFutureLinter', + 'ArcanistBaseXHPASTLinter' => 'ArcanistLinter', 'ArcanistBinaryExpressionSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistBinaryExpressionSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistBinaryNumericScalarCasingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', @@ -1182,14 +1179,12 @@ 'ArcanistCommitUpstreamHardpointLoader' => 'ArcanistHardpointLoader', 'ArcanistCommitWorkflow' => 'ArcanistWorkflow', 'ArcanistCompilerLintRenderer' => 'ArcanistLintRenderer', - 'ArcanistComprehensiveLintEngine' => 'ArcanistLintEngine', 'ArcanistConcatenationOperatorXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistConcatenationOperatorXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistConduitCall' => 'Phobject', 'ArcanistConduitEngine' => 'Phobject', 'ArcanistConduitException' => 'Exception', 'ArcanistConfigOption' => 'Phobject', - 'ArcanistConfigurationDrivenLintEngine' => 'ArcanistLintEngine', 'ArcanistConfigurationEngine' => 'Phobject', 'ArcanistConfigurationEngineExtension' => 'Phobject', 'ArcanistConfigurationManager' => 'Phobject', @@ -1320,7 +1315,7 @@ 'ArcanistLanguageConstructParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistLanguageConstructParenthesesXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistLiberateWorkflow' => 'ArcanistWorkflow', - 'ArcanistLintEngine' => 'Phobject', + 'ArcanistLintEngine' => 'ArcanistOperationEngine', 'ArcanistLintMessage' => 'Phobject', 'ArcanistLintMessageTestCase' => 'PhutilTestCase', 'ArcanistLintPatcher' => 'Phobject', @@ -1439,7 +1434,6 @@ 'ArcanistSetting' => 'Phobject', 'ArcanistSettings' => 'Phobject', 'ArcanistShellCompleteWorkflow' => 'ArcanistWorkflow', - 'ArcanistSingleLintEngine' => 'ArcanistLintEngine', 'ArcanistSlownessXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistSlownessXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistSpellingLinter' => 'ArcanistLinter', @@ -1487,7 +1481,6 @@ 'ArcanistUnitSink' => 'Phobject', 'ArcanistUnitTestResult' => 'Phobject', 'ArcanistUnitTestResultTestCase' => 'PhutilTestCase', - 'ArcanistUnitTestableLintEngine' => 'ArcanistLintEngine', 'ArcanistUnitWorkflow' => 'ArcanistWorkflow', 'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistUnnecessaryFinalModifierXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', @@ -1518,6 +1511,7 @@ 'ArcanistWorkflowInformation' => 'Phobject', 'ArcanistWorkingCopy' => 'Phobject', 'ArcanistWorkingCopyConfigurationSource' => 'ArcanistFilesystemConfigurationSource', + 'ArcanistWorkingCopyPath' => 'Phobject', 'ArcanistWorkingCopyStateRef' => 'ArcanistRef', 'ArcanistXHPASTLintNamingHook' => 'Phobject', 'ArcanistXHPASTLintNamingHookTestCase' => 'PhutilTestCase', diff --git a/src/lint/engine/ArcanistComprehensiveLintEngine.php b/src/lint/engine/ArcanistComprehensiveLintEngine.php deleted file mode 100644 --- a/src/lint/engine/ArcanistComprehensiveLintEngine.php +++ /dev/null @@ -1,55 +0,0 @@ -getPaths(); - - foreach ($paths as $key => $path) { - if (preg_match('@^externals/@', $path)) { - // Third-party stuff lives in /externals/; don't run lint engines - // against it. - unset($paths[$key]); - } - } - - $text_paths = preg_grep('/\.(php|css|hpp|cpp|l|y|py|pl)$/', $paths); - $linters[] = id(new ArcanistGeneratedLinter())->setPaths($text_paths); - $linters[] = id(new ArcanistNoLintLinter())->setPaths($text_paths); - $linters[] = id(new ArcanistTextLinter())->setPaths($text_paths); - - $linters[] = id(new ArcanistFilenameLinter())->setPaths($paths); - - $linters[] = id(new ArcanistXHPASTLinter()) - ->setPaths(preg_grep('/\.php$/', $paths)); - - $py_paths = preg_grep('/\.py$/', $paths); - $linters[] = id(new ArcanistPyFlakesLinter())->setPaths($py_paths); - $linters[] = id(new ArcanistPEP8Linter()) - ->setFlags($this->getPEP8WithTextOptions()) - ->setPaths($py_paths); - - $linters[] = id(new ArcanistRubyLinter()) - ->setPaths(preg_grep('/\.rb$/', $paths)); - - $linters[] = id(new ArcanistJSHintLinter()) - ->setPaths(preg_grep('/\.js$/', $paths)); - - return $linters; - } - - 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 array('--ignore=E101,E501,W291,W292,W293'); - } - -} diff --git a/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php b/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php deleted file mode 100644 --- a/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php +++ /dev/null @@ -1,194 +0,0 @@ -getWorkingCopy(); - $config_path = $working_copy->getProjectPath('.arclint'); - - if (!Filesystem::pathExists($config_path)) { - throw new ArcanistUsageException( - pht( - "Unable to find '%s' file to configure linters. Create an ". - "'%s' file in the root directory of the working copy.", - '.arclint', - '.arclint')); - } - - $data = Filesystem::readFile($config_path); - $config = null; - try { - $config = phutil_json_decode($data); - } catch (PhutilJSONParserException $ex) { - throw new PhutilProxyException( - pht( - "Expected '%s' file to be a valid JSON file, but ". - "failed to decode '%s'.", - '.arclint', - $config_path), - $ex); - } - - $linters = $this->loadAvailableLinters(); - - try { - PhutilTypeSpec::checkMap( - $config, - array( - 'exclude' => 'optional regex | list', - 'linters' => 'map>', - )); - } catch (PhutilTypeCheckException $ex) { - throw new PhutilProxyException( - pht("Error in parsing '%s' file.", $config_path), - $ex); - } - - $global_exclude = (array)idx($config, 'exclude', array()); - - $built_linters = array(); - $all_paths = $this->getPaths(); - foreach ($config['linters'] as $name => $spec) { - $type = idx($spec, 'type'); - if ($type !== null) { - if (empty($linters[$type])) { - throw new ArcanistUsageException( - pht( - "Linter '%s' specifies invalid type '%s'. ". - "Available linters are: %s.", - $name, - $type, - implode(', ', array_keys($linters)))); - } - - $linter = clone $linters[$type]; - $linter->setEngine($this); - $more = $linter->getLinterConfigurationOptions(); - - foreach ($more as $key => $option_spec) { - PhutilTypeSpec::checkMap( - $option_spec, - array( - 'type' => 'string', - 'help' => 'string', - )); - $more[$key] = $option_spec['type']; - } - } else { - // We'll raise an error below about the invalid "type" key. - $linter = null; - $more = array(); - } - - try { - PhutilTypeSpec::checkMap( - $spec, - array( - 'type' => 'string', - 'include' => 'optional regex | list', - 'exclude' => 'optional regex | list', - ) + $more); - } catch (PhutilTypeCheckException $ex) { - throw new PhutilProxyException( - pht( - "Error in parsing '%s' file, for linter '%s'.", - '.arclint', - $name), - $ex); - } - - foreach ($more as $key => $value) { - if (array_key_exists($key, $spec)) { - try { - $linter->setLinterConfigurationValue($key, $spec[$key]); - } catch (Exception $ex) { - throw new PhutilProxyException( - pht( - "Error in parsing '%s' file, in key '%s' for linter '%s'.", - '.arclint', - $key, - $name), - $ex); - } - } - } - - $include = (array)idx($spec, 'include', array()); - $exclude = (array)idx($spec, 'exclude', array()); - - $console = PhutilConsole::getConsole(); - $console->writeLog( - "%s\n", - pht("Examining paths for linter '%s'.", $name)); - $paths = $this->matchPaths( - $all_paths, - $include, - $exclude, - $global_exclude); - $console->writeLog( - "%s\n", - pht("Found %d matching paths for linter '%s'.", count($paths), $name)); - - $linter->setPaths($paths); - $built_linters[] = $linter; - } - - return $built_linters; - } - - private function loadAvailableLinters() { - return id(new PhutilClassMapQuery()) - ->setAncestorClass('ArcanistLinter') - ->setUniqueMethod('getLinterConfigurationName', true) - ->execute(); - } - - private function matchPaths( - array $paths, - array $include, - array $exclude, - array $global_exclude) { - - $console = PhutilConsole::getConsole(); - - $match = array(); - foreach ($paths as $path) { - $keep = false; - if (!$include) { - $keep = true; - } else { - foreach ($include as $rule) { - if (preg_match($rule, $path)) { - $keep = true; - break; - } - } - } - - if (!$keep) { - continue; - } - - if ($exclude) { - foreach ($exclude as $rule) { - if (preg_match($rule, $path)) { - continue 2; - } - } - } - - if ($global_exclude) { - foreach ($global_exclude as $rule) { - if (preg_match($rule, $path)) { - continue 2; - } - } - } - - $match[] = $path; - } - - return $match; - } - -} diff --git a/src/lint/engine/ArcanistSingleLintEngine.php b/src/lint/engine/ArcanistSingleLintEngine.php deleted file mode 100644 --- a/src/lint/engine/ArcanistSingleLintEngine.php +++ /dev/null @@ -1,71 +0,0 @@ -getConfigurationManager() - ->getConfigFromAnySource($key); - - if (!$linter_name) { - throw new ArcanistUsageException( - pht( - "You must configure '%s' with the name of a linter ". - "in order to use %s.", - $key, - __CLASS__)); - } - - if (!class_exists($linter_name)) { - throw new ArcanistUsageException( - pht( - "Linter '%s' configured in '%s' does not exist!", - $linter_name, - $key)); - } - - if (!is_subclass_of($linter_name, 'ArcanistLinter')) { - throw new ArcanistUsageException( - pht( - "Linter `%s` configured in '%s' MUST be a subclass of `%s`.", - $linter_name, - $key, - 'ArcanistLinter')); - } - - // Filter the affected paths. - $paths = $this->getPaths(); - foreach ($paths as $key => $path) { - if (!$this->pathExists($path)) { - // Don't lint removed files. In more complex linters it is sometimes - // appropriate to lint removed files so you can raise a warning like - // "you deleted X, but forgot to delete Y!", but most linters do not - // operate correctly on removed files. - unset($paths[$key]); - continue; - } - $disk = $this->getFilePathOnDisk($path); - if (is_dir($disk)) { - // Don't lint directories. (In SVN, they can be directly modified by - // changing properties on them, and may appear as modified paths.) - unset($paths[$key]); - continue; - } - } - - $linter = newv($linter_name, array()); - $linter->setPaths($paths); - - return array($linter); - } - -} diff --git a/src/lint/engine/ArcanistUnitTestableLintEngine.php b/src/lint/engine/ArcanistUnitTestableLintEngine.php deleted file mode 100644 --- a/src/lint/engine/ArcanistUnitTestableLintEngine.php +++ /dev/null @@ -1,32 +0,0 @@ -linters[] = $linter; - return $this; - } - - public function addFileData($path, $data) { - $this->fileData[$path] = $data; - return $this; - } - - public function pathExists($path) { - if (idx($this->fileData, $path) !== null) { - return true; - } - return parent::pathExists($path); - } - - public function buildLinters() { - return $this->linters; - } - -} diff --git a/src/lint/linter/ArcanistBaseXHPASTLinter.php b/src/lint/linter/ArcanistBaseXHPASTLinter.php --- a/src/lint/linter/ArcanistBaseXHPASTLinter.php +++ b/src/lint/linter/ArcanistBaseXHPASTLinter.php @@ -3,7 +3,7 @@ /** * @task sharing Sharing Parse Trees */ -abstract class ArcanistBaseXHPASTLinter extends ArcanistFutureLinter { +abstract class ArcanistBaseXHPASTLinter extends ArcanistLinter { private $futures = array(); private $trees = array(); diff --git a/src/lint/linter/ArcanistChmodLinter.php b/src/lint/linter/ArcanistChmodLinter.php --- a/src/lint/linter/ArcanistChmodLinter.php +++ b/src/lint/linter/ArcanistChmodLinter.php @@ -41,12 +41,10 @@ return true; } - public function lintPath($path) { - $engine = $this->getEngine(); - - if (is_executable($engine->getFilePathOnDisk($path))) { - if ($engine->isBinaryFile($path)) { - $mime = Filesystem::getMimeType($engine->getFilePathOnDisk($path)); + protected function lintPath(ArcanistWorkingCopyPath $path) { + if ($path->isExecutable()) { + if ($path->isBinary()) { + $mime = $path->getMimeType(); switch ($mime) { // Archives @@ -102,7 +100,7 @@ // Path is a binary file, which makes it a valid executable. return; } - } else if ($this->getShebang($path)) { + } else if ($this->hasShebang($path->getData())) { // Path contains a shebang, which makes it a valid executable. return; } else { @@ -114,21 +112,19 @@ } } - /** - * Returns the path's shebang. - * - * @param string - * @return string|null - */ - private function getShebang($path) { - $line = head(phutil_split_lines($this->getEngine()->loadData($path), true)); - - $matches = array(); - if (preg_match('/^#!(.*)$/', $line, $matches)) { - return $matches[1]; - } else { - return null; + private function hasShebang($data) { + $lines = phutil_split_lines($data); + if (!$lines) { + return false; } + + $line = head($lines); + + if (preg_match('/^#!(.*)$/', $line)) { + return true; + } + + return false; } } diff --git a/src/lint/linter/ArcanistFilenameLinter.php b/src/lint/linter/ArcanistFilenameLinter.php --- a/src/lint/linter/ArcanistFilenameLinter.php +++ b/src/lint/linter/ArcanistFilenameLinter.php @@ -35,8 +35,10 @@ ); } - public function lintPath($path) { - if (!preg_match('@^[a-z0-9./\\\\_-]+$@i', $path)) { + protected function lintPath(ArcanistWorkingCopyPath $path) { + $path_name = $path->getPath(); + + if (!preg_match('@^[a-z0-9./\\\\_-]+$@i', $path_name)) { $this->raiseLintAtPath( self::LINT_BAD_FILENAME, pht( diff --git a/src/lint/linter/ArcanistGeneratedLinter.php b/src/lint/linter/ArcanistGeneratedLinter.php --- a/src/lint/linter/ArcanistGeneratedLinter.php +++ b/src/lint/linter/ArcanistGeneratedLinter.php @@ -32,9 +32,8 @@ return false; } - public function lintPath($path) { - $data = $this->getData($path); - if (preg_match('/@'.'generated/', $data)) { + protected function lintPath(ArcanistWorkingCopyPath $path) { + if (preg_match('/@'.'generated/', $path->getData())) { $this->stopAllLinters(); } } diff --git a/src/lint/linter/ArcanistJSONLinter.php b/src/lint/linter/ArcanistJSONLinter.php --- a/src/lint/linter/ArcanistJSONLinter.php +++ b/src/lint/linter/ArcanistJSONLinter.php @@ -33,8 +33,8 @@ return false; } - public function lintPath($path) { - $data = $this->getData($path); + protected function lintPath(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); try { id(new PhutilJSONParser())->parse($data); diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php --- a/src/lint/linter/ArcanistLinter.php +++ b/src/lint/linter/ArcanistLinter.php @@ -158,6 +158,19 @@ /* -( Executing Linters )-------------------------------------------------- */ + final public function lintPaths(array $paths) { + assert_instances_of($paths, 'ArcanistWorkingCopyPath'); + + $this->messages = array(); + + foreach ($paths as $path) { + $this->setActivePath($path); + $this->lintPath($path); + } + + return $this->getLintMessages(); + } + /** * Hook called before a list of paths are linted. * @@ -177,19 +190,7 @@ } - /** - * Hook called for each path to be linted. - * - * Linters which are not parallelizable can do work here. - * - * Linters which are parallelizable may want to ignore this callback and - * implement @{method:willLintPaths} and @{method:didLintPaths} instead. - * - * @param string Path to lint. - * @return void - * @task exec - */ - public function lintPath($path) { + protected function lintPath(ArcanistWorkingCopyPath $path) { return; } @@ -255,9 +256,7 @@ $path = $this->getActivePath(); } - list($line, $char) = $this->getEngine()->getLineAndCharFromOffset( - $path, - $offset); + list($line, $char) = $path->getLineAndCharFromOffset($offset); return array( 'path' => $path, @@ -388,10 +387,6 @@ } final protected function addLintMessage(ArcanistLintMessage $message) { - $root = $this->getProjectRoot(); - $path = Filesystem::resolvePath($message->getPath(), $root); - $message->setPath(Filesystem::readablePath($path, $root)); - $this->messages[] = $message; return $message; } @@ -439,7 +434,7 @@ $line = null; $char = null; } else { - list($line, $char) = $engine->getLineAndCharFromOffset($path, $offset); + list($line, $char) = $path->getLineAndCharFromOffset($offset); } return $this->raiseLintAtLine( @@ -462,6 +457,9 @@ } final protected function isCodeEnabled($code) { + // TOOLSETS: Restore this. + return true; + $severity = $this->getLintMessageSeverity($code); return $this->getEngine()->isSeverityEnabled($severity); } diff --git a/src/lint/linter/ArcanistMergeConflictLinter.php b/src/lint/linter/ArcanistMergeConflictLinter.php --- a/src/lint/linter/ArcanistMergeConflictLinter.php +++ b/src/lint/linter/ArcanistMergeConflictLinter.php @@ -31,8 +31,8 @@ ); } - public function lintPath($path) { - $lines = phutil_split_lines($this->getData($path), false); + protected function lintPath(ArcanistWorkingCopyPath $path) { + $lines = $path->getDataAsLines(); foreach ($lines as $lineno => $line) { // An unresolved merge conflict will contain a series of seven diff --git a/src/lint/linter/ArcanistNoLintLinter.php b/src/lint/linter/ArcanistNoLintLinter.php --- a/src/lint/linter/ArcanistNoLintLinter.php +++ b/src/lint/linter/ArcanistNoLintLinter.php @@ -32,8 +32,8 @@ return false; } - public function lintPath($path) { - $data = $this->getData($path); + protected function lintPath(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); if (preg_match('/@'.'nolint/', $data)) { $this->stopAllLinters(); } diff --git a/src/lint/linter/ArcanistSpellingLinter.php b/src/lint/linter/ArcanistSpellingLinter.php --- a/src/lint/linter/ArcanistSpellingLinter.php +++ b/src/lint/linter/ArcanistSpellingLinter.php @@ -54,8 +54,11 @@ } public function loadDictionary($path) { - $root = $this->getProjectRoot(); - $path = Filesystem::resolvePath($path, $root); + // TOOLSETS: Restore the ability to reference resources relative to the + // project root. + + // $root = $this->getProjectRoot(); + // $path = Filesystem::resolvePath($path, $root); $dict = phutil_json_decode(Filesystem::readFile($path)); PhutilTypeSpec::checkMap( @@ -102,7 +105,7 @@ ); } - public function lintPath($path) { + protected function lintPath(ArcanistWorkingCopyPath $path) { // TODO: This is a bit hacky. If no dictionaries were specified, then add // the default dictionary. if (!$this->dictionaries) { @@ -119,8 +122,13 @@ } } - private function checkExactWord($path, $word, $correction) { - $text = $this->getData($path); + private function checkExactWord( + ArcanistWorkingCopyPath $path, + $word, + $correction) { + + $text = $path->getData(); + $matches = array(); $num_matches = preg_match_all( '#\b'.preg_quote($word, '#').'\b#i', @@ -145,8 +153,13 @@ } } - private function checkPartialWord($path, $word, $correction) { - $text = $this->getData($path); + private function checkPartialWord( + ArcanistWorkingCopyPath $path, + $word, + $correction) { + + $text = $path->getData(); + $pos = 0; while ($pos < strlen($text)) { $next = stripos($text, $word, $pos); diff --git a/src/lint/linter/ArcanistTextLinter.php b/src/lint/linter/ArcanistTextLinter.php --- a/src/lint/linter/ArcanistTextLinter.php +++ b/src/lint/linter/ArcanistTextLinter.php @@ -90,10 +90,11 @@ ); } - public function lintPath($path) { + protected function lintPath(ArcanistWorkingCopyPath $path) { $this->lintEmptyFile($path); - if (!strlen($this->getData($path))) { + $data = $path->getData(); + if (!strlen($data)) { // If the file is empty, don't bother; particularly, don't require // the user to add a newline. return; @@ -124,32 +125,37 @@ $this->lintEOFWhitespace($path); } - protected function lintEmptyFile($path) { - $data = $this->getData($path); + private function lintEmptyFile(ArcanistWorkingCopyPath $path) { + // If this file has any content, it isn't empty. + $data = $path->getData(); + if (!preg_match('/^\s*$/', $data)) { + return; + } // It is reasonable for certain file types to be completely empty, // so they are excluded here. - switch ($filename = basename($this->getActivePath())) { - case '__init__.py': - return; + $basename = $path->getBasename(); - default: - if (strlen($filename) && $filename[0] == '.') { - return; - } + // Allow empty "__init__.py", as this is legitimate in Python. + if ($basename === '__init__.py') { + return; } - if (preg_match('/^\s*$/', $data)) { - $this->raiseLintAtPath( - self::LINT_EMPTY_FILE, - pht("Empty files usually don't serve any useful purpose.")); - $this->stopAllLinters(); + // Allow empty ".gitkeep" and similar files. + if (isset($filename[0]) && $filename[0] == '.') { + return; } + + $this->raiseLintAtPath( + self::LINT_EMPTY_FILE, + pht('Empty files usually do not serve any useful purpose.')); + + $this->stopAllLinters(); } - protected function lintNewlines($path) { - $data = $this->getData($path); - $pos = strpos($this->getData($path), "\r"); + private function lintNewlines(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); + $pos = strpos($data, "\r"); if ($pos !== false) { $this->raiseLintAtOffset( @@ -165,8 +171,10 @@ } } - protected function lintTabs($path) { - $pos = strpos($this->getData($path), "\t"); + private function lintTabs(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); + $pos = strpos($data, "\t"); + if ($pos !== false) { $this->raiseLintAtOffset( $pos, @@ -176,11 +184,12 @@ } } - protected function lintLineLength($path) { - $lines = explode("\n", $this->getData($path)); + private function lintLineLength(ArcanistWorkingCopyPath $path) { + $lines = $path->getDataAsLines(); $width = $this->maxLineLength; foreach ($lines as $line_idx => $line) { + $line = rtrim($line, "\n"); if (strlen($line) > $width) { $this->raiseLintAtLine( $line_idx + 1, @@ -196,8 +205,9 @@ } } - protected function lintEOFNewline($path) { - $data = $this->getData($path); + private function lintEOFNewline(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); + if (!strlen($data) || $data[strlen($data) - 1] != "\n") { $this->raiseLintAtOffset( strlen($data), @@ -208,8 +218,8 @@ } } - protected function lintCharset($path) { - $data = $this->getData($path); + private function lintCharset(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); $matches = null; $bad = '[^\x09\x0A\x20-\x7E]'; @@ -240,8 +250,8 @@ } } - protected function lintTrailingWhitespace($path) { - $data = $this->getData($path); + private function lintTrailingWhitespace(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); $matches = null; $preg = preg_match_all( @@ -268,8 +278,8 @@ } } - protected function lintBOFWhitespace($path) { - $data = $this->getData($path); + private function lintBOFWhitespace(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); $matches = null; $preg = preg_match( @@ -293,8 +303,8 @@ ''); } - protected function lintEOFWhitespace($path) { - $data = $this->getData($path); + private function lintEOFWhitespace(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); $matches = null; $preg = preg_match( diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php --- a/src/lint/linter/ArcanistXHPASTLinter.php +++ b/src/lint/linter/ArcanistXHPASTLinter.php @@ -128,10 +128,21 @@ return count($this->rules); } - protected function resolveFuture($path, Future $future) { - $tree = $this->getXHPASTTreeForPath($path); - if (!$tree) { - $ex = $this->getXHPASTExceptionForPath($path); + protected function lintPath(ArcanistWorkingCopyPath $path) { + $data = $path->getData(); + + try { + $future = PhutilXHPASTBinary::getParserFuture($data); + + $tree = XHPASTTree::newFromDataAndResolvedExecFuture( + $data, + $future->resolve()); + + $root = $tree->getRootNode(); + + $root->buildSelectCache(); + $root->buildTokenCache(); + } catch (Exception $ex) { if ($ex instanceof XHPASTSyntaxErrorException) { $this->raiseLintAtLine( $ex->getErrorLine(), @@ -148,8 +159,6 @@ return; } - $root = $tree->getRootNode(); - foreach ($this->rules as $rule) { if ($this->isCodeEnabled($rule->getLintID())) { $rule->setLinter($this); diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php --- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php @@ -91,44 +91,30 @@ Filesystem::writeFile($tmp, $data); $full_path = (string)$tmp; - $mode = idx($config, 'mode'); + $mode = idx($config, 'mode', 0644); if ($mode) { Filesystem::changePermissions($tmp, octdec($mode)); } - $dir = dirname($full_path); - $path = basename($full_path); + $path_name = idx($config, 'path', $basename); - $working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile( - $dir, - null, - pht('Unit Test')); - $configuration_manager = new ArcanistConfigurationManager(); - $configuration_manager->setWorkingCopyIdentity($working_copy); - - - $engine = new ArcanistUnitTestableLintEngine(); - $engine->setWorkingCopy($working_copy); - $engine->setConfigurationManager($configuration_manager); - - $path_name = idx($config, 'path', $path); - $engine->setPaths(array($path_name)); - - $linter->addPath($path_name); - $linter->addData($path_name, $data); + $path = id(new ArcanistWorkingCopyPath()) + ->setPath($path_name) + ->setMode($mode) + ->setData($data); foreach (idx($config, 'config', array()) as $key => $value) { $linter->setLinterConfigurationValue($key, $value); } - $engine->addLinter($linter); - $engine->addFileData($path_name, $data); + $messages = $linter->lintPaths(array($path)); - $results = $engine->run(); - $this->assertEqual( - 1, - count($results), - pht('Expect one result returned by linter.')); + $result = id(new ArcanistLintResult()) + ->setData($data); + foreach ($messages as $message) { + $result->addMessage($message); + } + $results = array($result); $assert_stopped = idx($config, 'stopped'); if ($assert_stopped !== null) { @@ -141,6 +127,7 @@ } $result = reset($results); + $patcher = ArcanistLintPatcher::newFromArcanistLintResult($result); $after_lint = $patcher->getModifiedFileContent(); } catch (PhutilTestTerminatedException $ex) { diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php --- a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php @@ -17,8 +17,8 @@ } if ($compat_info === null) { - $target = phutil_get_library_root('phutil'). - '/../resources/php_compat_info.json'; + $arcanist_root = dirname(phutil_get_library_root('arcanist')); + $target = $arcanist_root.'/resources/php/php_compat_info.json'; $compat_info = phutil_json_decode(Filesystem::readFile($target)); } diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/operation/ArcanistLintEngine.php rename from src/lint/engine/ArcanistLintEngine.php rename to src/lint/operation/ArcanistLintEngine.php --- a/src/lint/engine/ArcanistLintEngine.php +++ b/src/lint/operation/ArcanistLintEngine.php @@ -1,52 +1,12 @@ configurationManager = $configuration_manager; - return $this; - } - - final public function getConfigurationManager() { - return $this->configurationManager; - } - - final public function setWorkingCopy( - ArcanistWorkingCopyIdentity $working_copy) { - $this->workingCopy = $working_copy; - return $this; - } - - final public function getWorkingCopy() { - return $this->workingCopy; - } - - final public function setPaths($paths) { - $this->paths = $paths; - return $this; + final public function getUnitEngineType() { + return $this->getPhobjectClassConstant('ENGINETYPE'); } - public function getPaths() { - return $this->paths; + public static function getAllLintEngines() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getLintEngineType') + ->execute(); } final public function setPathChangedLines($path, $changed) { @@ -378,33 +319,6 @@ return $this->results[$path]; } - final 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); - } - protected function getCacheVersion() { return 1; } diff --git a/src/workingcopy/ArcanistWorkingCopyPath.php b/src/workingcopy/ArcanistWorkingCopyPath.php new file mode 100644 --- /dev/null +++ b/src/workingcopy/ArcanistWorkingCopyPath.php @@ -0,0 +1,129 @@ +path = $path; + return $this; + } + + public function getPath() { + return $this->path; + } + + public function setData($data) { + $this->data = $data; + return $this; + } + + public function getData() { + if ($this->data === null) { + throw new Exception( + pht( + 'No data provided for path "%s".', + $this->getDescription())); + } + + return $this->data; + } + + public function getDataAsLines() { + if ($this->dataAsLines === null) { + $lines = phutil_split_lines($this->getData()); + $this->dataAsLines = $lines; + } + + return $this->dataAsLines; + } + + public function setMode($mode) { + $this->mode = $mode; + return $this; + } + + public function getMode() { + if ($this->mode === null) { + throw new Exception( + pht( + 'No mode provided for path "%s".', + $this->getDescription())); + } + + return $this->mode; + } + + public function isExecutable() { + $mode = $this->getMode(); + return (bool)($mode & 0111); + } + + public function isBinary() { + if ($this->binary === null) { + $data = $this->getData(); + $is_binary = ArcanistDiffUtils::isHeuristicBinaryFile($data); + $this->binary = $is_binary; + } + + return $this->binary; + } + + public function getMimeType() { + if ($this->mimeType === null) { + // TOOLSETS: This is not terribly efficient on real repositories since + // it re-writes files which are often already on disk, but is good for + // unit tests. + + $tmp = new TempFile(); + Filesystem::writeFile($tmp, $this->getData()); + $mime = Filesystem::getMimeType($tmp); + + $this->mimeType = $mime; + } + + return $this->mimeType; + } + + + public function getBasename() { + return basename($this->getPath()); + } + + public function getLineAndCharFromOffset($offset) { + if ($this->charMap === null) { + $char_map = array(); + $line_map = array(); + + $lines = $this->getDataAsLines(); + + $line_number = 0; + $line_start = 0; + foreach ($lines as $line) { + $len = strlen($line); + $line_map[] = $line_start; + $line_start += $len; + for ($ii = 0; $ii < $len; $ii++) { + $char_map[] = $line_number; + } + $line_number++; + } + + $this->charMap = $char_map; + $this->lineMap = $line_map; + } + + $line = $this->charMap[$offset]; + $char = $offset - $this->lineMap[$line]; + + return array($line, $char); + } + +}