Changeset View
Changeset View
Standalone 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. | |||||
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 ActionsThis 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 | |||||
I personally prefer the Boy Scout Motto style of lint enforcement, but this is a reasonable default.