1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-28 08:20:56 +01:00

[Wilds] Make "arc" survive all tests with no failures

Summary:
Ref T13098. The lint part of this barely gets all the pieces cobbled together, but all the tests do actually run now, minus a handful of skipped tests. Major ideas here:

Lint, Unit and (soon) Build are becoming "Operations" where an "Overseer" manages a list of "Engines" that read configuration from a ".arc<operation>" file, operate on a working copy, may operate on a subset of files (possibly selected by examining recent changes in the working copy), produce some kind of result (test outcomes, lint messages, build artifacts) and then the results are sent to a list of one or more "Sinks" (console display, files, Harbormaster, patchers). All three workflows share a meaningful amount of code in terms of doing most of these things in roughly the same way.

A lot of Lint logic is currently based around passing paths around as strings, then calling a lot of `$thing->getParentObject()->getInformationForPath($path_as_string)`. This tends to be pretty bulky and somewhat fragile. I'm moving toward passing an `ArcanistWorkingCopyPath $path` around instead, which has methods with a better defined scope like `$path->getData()`, `$path->getMimeType()`, `$path->isBinary()`, and so on. This requires us to build fewer objects to run tests.

`arc lint` itself won't run yet, and I don't plan to make it run for a bit (I have //some// of a patch, but it's not really there yet). Instead, I want to focus on getting coverage for existing flows (particularly `alias`) and then getting Windows support online now that it can have test coverage.

This change is less about any of what it actually does and more about powering through to make `arc unit` a useful tool for building other changes.

Test Plan:
Flawless!

{F5910428}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13098

Differential Revision: https://secure.phabricator.com/D19716
This commit is contained in:
epriestley 2018-09-27 10:23:18 -07:00
parent 529b21844b
commit ee756592af
20 changed files with 286 additions and 587 deletions

View file

@ -126,14 +126,12 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'ArcanistBackoutWorkflow' => 'ArcanistWorkflow',
'ArcanistBaseCommitParser' => 'Phobject',
'ArcanistBaseCommitParserTestCase' => 'PhutilTestCase',
'ArcanistBaseXHPASTLinter' => 'ArcanistFutureLinter',
'ArcanistBaseXHPASTLinter' => 'ArcanistLinter',
'ArcanistBinaryExpressionSpacingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistBinaryExpressionSpacingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistBinaryNumericScalarCasingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
@ -1182,14 +1179,12 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'ArcanistLanguageConstructParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistLanguageConstructParenthesesXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistLiberateWorkflow' => 'ArcanistWorkflow',
'ArcanistLintEngine' => 'Phobject',
'ArcanistLintEngine' => 'ArcanistOperationEngine',
'ArcanistLintMessage' => 'Phobject',
'ArcanistLintMessageTestCase' => 'PhutilTestCase',
'ArcanistLintPatcher' => 'Phobject',
@ -1439,7 +1434,6 @@ phutil_register_library_map(array(
'ArcanistSetting' => 'Phobject',
'ArcanistSettings' => 'Phobject',
'ArcanistShellCompleteWorkflow' => 'ArcanistWorkflow',
'ArcanistSingleLintEngine' => 'ArcanistLintEngine',
'ArcanistSlownessXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistSlownessXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistSpellingLinter' => 'ArcanistLinter',
@ -1487,7 +1481,6 @@ phutil_register_library_map(array(
'ArcanistUnitSink' => 'Phobject',
'ArcanistUnitTestResult' => 'Phobject',
'ArcanistUnitTestResultTestCase' => 'PhutilTestCase',
'ArcanistUnitTestableLintEngine' => 'ArcanistLintEngine',
'ArcanistUnitWorkflow' => 'ArcanistWorkflow',
'ArcanistUnnecessaryFinalModifierXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistUnnecessaryFinalModifierXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
@ -1518,6 +1511,7 @@ phutil_register_library_map(array(
'ArcanistWorkflowInformation' => 'Phobject',
'ArcanistWorkingCopy' => 'Phobject',
'ArcanistWorkingCopyConfigurationSource' => 'ArcanistFilesystemConfigurationSource',
'ArcanistWorkingCopyPath' => 'Phobject',
'ArcanistWorkingCopyStateRef' => 'ArcanistRef',
'ArcanistXHPASTLintNamingHook' => 'Phobject',
'ArcanistXHPASTLintNamingHookTestCase' => 'PhutilTestCase',

View file

@ -1,55 +0,0 @@
<?php
/**
* Basic lint engine which just applies several linters based on the file types.
*/
final class ArcanistComprehensiveLintEngine extends ArcanistLintEngine {
public function buildLinters() {
$linters = array();
$paths = $this->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');
}
}

View file

@ -1,194 +0,0 @@
<?php
final class ArcanistConfigurationDrivenLintEngine extends ArcanistLintEngine {
public function buildLinters() {
$working_copy = $this->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<regex>',
'linters' => 'map<string, map<string, wild>>',
));
} 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<regex>',
'exclude' => 'optional regex | list<regex>',
) + $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;
}
}

View file

@ -1,71 +0,0 @@
<?php
/**
* Run a single linter on every path unconditionally. This is a glue engine for
* linters like @{class:ArcanistScriptAndRegexLinter}, if you are averse to
* writing a phutil library. Your linter will receive every path, including
* paths which have been moved or deleted.
*
* Set which linter should be run by configuring `lint.engine.single.linter` in
* `.arcconfig` or user config.
*/
final class ArcanistSingleLintEngine extends ArcanistLintEngine {
public function buildLinters() {
$key = 'lint.engine.single.linter';
$linter_name = $this->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);
}
}

View file

@ -1,32 +0,0 @@
<?php
/**
* Lint engine for use in constructing test cases. See
* @{class:ArcanistLinterTestCase}.
*/
final class ArcanistUnitTestableLintEngine extends ArcanistLintEngine {
protected $linters = array();
public function addLinter($linter) {
$this->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;
}
}

View file

@ -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();

View file

@ -41,12 +41,10 @@ final class ArcanistChmodLinter extends ArcanistLinter {
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 @@ final class ArcanistChmodLinter extends ArcanistLinter {
// 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 @@ final class ArcanistChmodLinter extends ArcanistLinter {
}
}
/**
* 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;
}
}

View file

@ -35,8 +35,10 @@ final class ArcanistFilenameLinter extends ArcanistLinter {
);
}
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(

View file

@ -32,9 +32,8 @@ final class ArcanistGeneratedLinter extends ArcanistLinter {
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();
}
}

View file

@ -33,8 +33,8 @@ final class ArcanistJSONLinter extends ArcanistLinter {
return false;
}
public function lintPath($path) {
$data = $this->getData($path);
protected function lintPath(ArcanistWorkingCopyPath $path) {
$data = $path->getData();
try {
id(new PhutilJSONParser())->parse($data);

View file

@ -158,6 +158,19 @@ abstract class ArcanistLinter extends Phobject {
/* -( 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 @@ abstract class ArcanistLinter extends Phobject {
}
/**
* 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 @@ abstract class ArcanistLinter extends Phobject {
$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 @@ abstract class ArcanistLinter extends Phobject {
}
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 @@ abstract class ArcanistLinter extends Phobject {
$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 @@ abstract class ArcanistLinter extends Phobject {
}
final protected function isCodeEnabled($code) {
// TOOLSETS: Restore this.
return true;
$severity = $this->getLintMessageSeverity($code);
return $this->getEngine()->isSeverityEnabled($severity);
}

View file

@ -31,8 +31,8 @@ final class ArcanistMergeConflictLinter extends ArcanistLinter {
);
}
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

View file

@ -32,8 +32,8 @@ final class ArcanistNoLintLinter extends ArcanistLinter {
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();
}

View file

@ -54,8 +54,11 @@ final class ArcanistSpellingLinter extends ArcanistLinter {
}
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 @@ final class ArcanistSpellingLinter extends ArcanistLinter {
);
}
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 @@ final class ArcanistSpellingLinter extends ArcanistLinter {
}
}
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 @@ final class ArcanistSpellingLinter extends ArcanistLinter {
}
}
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);

View file

@ -90,10 +90,11 @@ final class ArcanistTextLinter extends ArcanistLinter {
);
}
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 @@ final class ArcanistTextLinter extends ArcanistLinter {
$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 @@ final class ArcanistTextLinter extends ArcanistLinter {
}
}
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 @@ final class ArcanistTextLinter extends ArcanistLinter {
}
}
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 @@ final class ArcanistTextLinter extends ArcanistLinter {
}
}
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 @@ final class ArcanistTextLinter extends ArcanistLinter {
}
}
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 @@ final class ArcanistTextLinter extends ArcanistLinter {
}
}
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 @@ final class ArcanistTextLinter extends ArcanistLinter {
}
}
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 @@ final class ArcanistTextLinter extends ArcanistLinter {
'');
}
protected function lintEOFWhitespace($path) {
$data = $this->getData($path);
private function lintEOFWhitespace(ArcanistWorkingCopyPath $path) {
$data = $path->getData();
$matches = null;
$preg = preg_match(

View file

@ -128,10 +128,21 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
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 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
return;
}
$root = $tree->getRootNode();
foreach ($this->rules as $rule) {
if ($this->isCodeEnabled($rule->getLintID())) {
$rule->setLinter($this);

View file

@ -91,44 +91,30 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase {
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 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase {
}
$result = reset($results);
$patcher = ArcanistLintPatcher::newFromArcanistLintResult($result);
$after_lint = $patcher->getModifiedFileContent();
} catch (PhutilTestTerminatedException $ex) {

View file

@ -17,8 +17,8 @@ final class ArcanistPHPCompatibilityXHPASTLinterRule
}
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));
}

View file

@ -1,52 +1,12 @@
<?php
/**
* Manages lint execution. When you run 'arc lint' or 'arc diff', Arcanist
* attempts to run lint rules using a lint engine.
*
* Lint engines are high-level strategic classes which do not contain any
* actual linting rules. Linting rules live in `Linter` classes. The lint
* engine builds and configures linters.
*
* Most modern linters can be configured with an `.arclint` file, which is
* managed by the builtin @{class:ArcanistConfigurationDrivenLintEngine}.
* Consult the documentation for more information on these files.
*
* In the majority of cases, you do not need to write a custom lint engine.
* For example, to add new rules for a new language, write a linter instead.
* However, if you have a very advanced or specialized use case, you can write
* a custom lint engine by extending this class; custom lint engines are more
* powerful but much more complex than the builtin engines.
*
* The lint engine is given a list of paths (generally, the paths that you
* modified in your change) and determines which linters to run on them. The
* linters themselves are responsible for actually analyzing file text and
* finding warnings and errors. For example, if the modified paths include some
* JS files and some Python files, you might want to run JSLint on the JS files
* and PyLint on the Python files.
*
* You can also run multiple linters on a single file. For instance, you might
* run one linter on all text files to make sure they don't have trailing
* whitespace, or enforce tab vs space rules, or make sure there are enough
* curse words in them.
*
* You can test an engine like this:
*
* arc lint --engine YourLintEngineClassName --lintall some_file.py
*
* ...which will show you all the lint issues raised in the file.
*
* See @{article@phabricator:Arcanist User Guide: Customizing Lint, Unit Tests
* and Workflows} for more information about configuring lint engines.
*/
abstract class ArcanistLintEngine extends Phobject {
abstract class ArcanistLintEngine
extends ArcanistOperationEngine {
protected $workingCopy;
protected $paths = array();
protected $fileData = array();
protected $charToLine = array();
protected $lineToFirstChar = array();
private $cachedResults;
private $cacheVersion;
private $repositoryVersion;
@ -60,35 +20,16 @@ abstract class ArcanistLintEngine extends Phobject {
private $linterResources = array();
public function __construct() {}
final public function setConfigurationManager(
ArcanistConfigurationManager $configuration_manager) {
$this->configurationManager = $configuration_manager;
return $this;
final public function getUnitEngineType() {
return $this->getPhobjectClassConstant('ENGINETYPE');
}
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;
}
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 @@ abstract class ArcanistLintEngine extends Phobject {
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;
}

View file

@ -0,0 +1,129 @@
<?php
final class ArcanistWorkingCopyPath
extends Phobject {
private $path;
private $mode;
private $data;
private $binary;
private $dataAsLines;
private $charMap;
private $lineMap;
public function setPath($path) {
$this->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);
}
}