Differential D18644 Diff 44777 src/workflow/ArcanistLintWorkflow.php

Changeset View

Standalone View

src/workflow/ArcanistLintWorkflow.php

<?php		<?php

/**		/**
* Runs lint rules on changes.		* Runs lint rules on changes.
*/		*/
final class ArcanistLintWorkflow extends ArcanistWorkflow {		final class ArcanistLintWorkflow extends ArcanistWorkflow {

const RESULT_OKAY = 0;		const RESULT_OKAY = 0;
const RESULT_WARNINGS = 1;		const RESULT_WARNINGS = 1;
const RESULT_ERRORS = 2;		const RESULT_ERRORS = 2;
const RESULT_SKIP = 3;		const RESULT_SKIP = 3;

const DEFAULT_SEVERITY = ArcanistLintSeverity::SEVERITY_ADVICE;		const DEFAULT_SEVERITY = ArcanistLintSeverity::SEVERITY_ADVICE;

private $unresolvedMessages;		private $unresolvedMessages;
private $shouldLintAll;
private $shouldAmendChanges = false;		private $shouldAmendChanges = false;
private $shouldAmendWithoutPrompt = false;		private $shouldAmendWithoutPrompt = false;
private $shouldAmendAutofixesWithoutPrompt = false;		private $shouldAmendAutofixesWithoutPrompt = false;
private $engine;		private $engine;

public function getWorkflowName() {		public function getWorkflowName() {
return 'lint';		return 'lint';
}		}
Show All 31 Lines	EOTEXT
}		}

public function getArguments() {		public function getArguments() {
return array(		return array(
'lintall' => array(		'lintall' => array(
'help' => pht(		'help' => pht(
'Show all lint warnings, not just those on changed lines. When '.		'Show all lint warnings, not just those on changed lines. When '.
'paths are specified, this is the default behavior.'),		'paths are specified, this is the default behavior.'),
'conflicts' => array(
'only-changed' => true,
),
),
'only-changed' => array(
'help' => pht(
'Show lint warnings just on changed lines. When no paths are '.
'specified, this is the default.'),
'conflicts' => array(
'lintall' => true,
),
),		),
'rev' => array(		'rev' => array(
'param' => 'revision',		'param' => 'revision',
'help' => pht('Lint changes since a specific revision.'),		'help' => pht('Lint changes since a specific revision.'),
'supports' => array(		'supports' => array(
'git',		'git',
'hg',		'hg',
),		),
▲ Show 20 Lines • Show All 89 Lines • ▼ Show 20 Lines	public function run() {
if ($everything && $paths) {		if ($everything && $paths) {
throw new ArcanistUsageException(		throw new ArcanistUsageException(
pht(		pht(
'You can not specify paths with %s. The %s flag lints every file.',		'You can not specify paths with %s. The %s flag lints every file.',
'--everything',		'--everything',
'--everything'));		'--everything'));
}		}

if ($rev && $paths) {		if ($rev !== null) {
throw new ArcanistUsageException(		$this->parseBaseCommitArgument(array($rev));
pht('Specify either %s or paths.', '--rev'));
}		}

		// Sometimes, we hide low-severity messages which occur on lines which
		// were not changed. This is the default behavior when you run "arc lint"
		// with no arguments: if you touched a file, but there was already some
		// minor warning about whitespace or spelling elsewhere in the file, you
		// don't need to correct it.
		amckinleyUnsubmitted Not Done Inline Actions I personally prefer the Boy Scout Motto style of lint enforcement, but this is a reasonable default. amckinley: I personally prefer the Boy Scout Motto style of lint enforcement, but this is a reasonable…

		// In other modes, notably "arc lint <file>", this is not the defualt
		// behavior. If you ask us to lint a specific file, we show you all the
		// lint messages in the file.

// NOTE: When the user specifies paths, we imply --lintall and show all		// You can change this behavior with various flags, including "--lintall",
// warnings for the paths in question. This is easier to deal with for		// "--rev", and "--everything".
// us and less confusing for users.
$this->shouldLintAll = $paths ? true : false;
if ($this->getArgument('lintall')) {		if ($this->getArgument('lintall')) {
$this->shouldLintAll = true;		$lint_all = true;
} else if ($this->getArgument('only-changed')) {		} else if ($rev !== null) {
$this->shouldLintAll = false;		$lint_all = false;
		} else if ($paths \|\| $everything) {
		$lint_all = true;
		} else {
		$lint_all = false;
}		}

if ($everything) {		if ($everything) {
$paths = iterator_to_array($this->getRepositoryAPI()->getAllFiles());		$paths = iterator_to_array($this->getRepositoryAPI()->getAllFiles());
$this->shouldLintAll = true;
} else {		} else {
$paths = $this->selectPathsForWorkflow($paths, $rev);		$paths = $this->selectPathsForWorkflow($paths, $rev);
}		}

$this->engine = $engine;		$this->engine = $engine;

$engine->setMinimumSeverity(		$engine->setMinimumSeverity(
$this->getArgument('severity', self::DEFAULT_SEVERITY));		$this->getArgument('severity', self::DEFAULT_SEVERITY));

// Propagate information about which lines changed to the lint engine.		// Propagate information about which lines changed to the lint engine.
// This is used so that the lint engine can drop warning messages		// This is used so that the lint engine can drop warning messages
// concerning lines that weren't in the change.		// concerning lines that weren't in the change.
$engine->setPaths($paths);		$engine->setPaths($paths);
if (!$this->shouldLintAll) {		if ($lint_all) {
		epriestleyAuthorUnsubmitted Not Done Inline Actions This condition got flipped and I didn't catch it in my initial poking, but D18651 has a fix for it. epriestley: This condition got flipped and I didn't catch it in my initial poking, but D18651 has a fix for…
foreach ($paths as $path) {		foreach ($paths as $path) {
// Note that getChangedLines() returns null to indicate that a file		// Note that getChangedLines() returns null to indicate that a file
// is binary or a directory (i.e., changed lines are not relevant).		// is binary or a directory (i.e., changed lines are not relevant).
$engine->setPathChangedLines(		$engine->setPathChangedLines(
$path,		$path,
$this->getChangedLines($path, 'new'));		$this->getChangedLines($path, 'new'));
}		}
}		}
▲ Show 20 Lines • Show All 232 Lines • Show Last 20 Lines