diff --git a/src/lint/linter/ArcanistTextLinter.php b/src/lint/linter/ArcanistTextLinter.php index c7c4af52..b10c5c46 100644 --- a/src/lint/linter/ArcanistTextLinter.php +++ b/src/lint/linter/ArcanistTextLinter.php @@ -1,319 +1,318 @@ array( 'type' => 'optional int', 'help' => pht( 'Adjust the maximum line length before a warning is raised. By '. 'default, a warning is raised on lines exceeding 80 characters.'), ), ); return $options + parent::getLinterConfigurationOptions(); } public function setMaxLineLength($new_length) { $this->maxLineLength = $new_length; return $this; } public function setLinterConfigurationValue($key, $value) { switch ($key) { case 'text.max-line-length': $this->setMaxLineLength($value); return; } return parent::setLinterConfigurationValue($key, $value); } public function getLinterName() { return 'TXT'; } public function getLinterConfigurationName() { return 'text'; } public function getLintSeverityMap() { return array( self::LINT_LINE_WRAP => ArcanistLintSeverity::SEVERITY_WARNING, self::LINT_TRAILING_WHITESPACE => ArcanistLintSeverity::SEVERITY_AUTOFIX, self::LINT_BOF_WHITESPACE => ArcanistLintSeverity::SEVERITY_AUTOFIX, self::LINT_EOF_WHITESPACE => ArcanistLintSeverity::SEVERITY_AUTOFIX, ); } public function getLintNameMap() { return array( self::LINT_DOS_NEWLINE => pht('DOS Newlines'), self::LINT_TAB_LITERAL => pht('Tab Literal'), self::LINT_LINE_WRAP => pht('Line Too Long'), self::LINT_EOF_NEWLINE => pht('File Does Not End in Newline'), self::LINT_BAD_CHARSET => pht('Bad Charset'), self::LINT_TRAILING_WHITESPACE => pht('Trailing Whitespace'), self::LINT_BOF_WHITESPACE => pht('Leading Whitespace at BOF'), self::LINT_EOF_WHITESPACE => pht('Trailing Whitespace at EOF'), self::LINT_EMPTY_FILE => pht('Empty File'), ); } public function lintPath($path) { $this->lintEmptyFile($path); if (!strlen($this->getData($path))) { // If the file is empty, don't bother; particularly, don't require // the user to add a newline. return; } if ($this->didStopAllLinters()) { return; } $this->lintNewlines($path); $this->lintTabs($path); if ($this->didStopAllLinters()) { return; } $this->lintCharset($path); if ($this->didStopAllLinters()) { return; } $this->lintLineLength($path); $this->lintEOFNewline($path); $this->lintTrailingWhitespace($path); $this->lintBOFWhitespace($path); $this->lintEOFWhitespace($path); } protected function lintEmptyFile($path) { $data = $this->getData($path); // 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; default: if (strlen($filename) && $filename[0] == '.') { return; } } if (preg_match('/^\s*$/', $data)) { $this->raiseLintAtPath( self::LINT_EMPTY_FILE, pht("Empty files usually don't serve any useful purpose.")); $this->stopAllLinters(); } } protected function lintNewlines($path) { $data = $this->getData($path); $pos = strpos($this->getData($path), "\r"); if ($pos !== false) { $this->raiseLintAtOffset( 0, self::LINT_DOS_NEWLINE, pht('You must use ONLY Unix linebreaks ("%s") in source code.', '\n'), $data, str_replace("\r\n", "\n", $data)); if ($this->isMessageEnabled(self::LINT_DOS_NEWLINE)) { $this->stopAllLinters(); } } } protected function lintTabs($path) { $pos = strpos($this->getData($path), "\t"); if ($pos !== false) { $this->raiseLintAtOffset( $pos, self::LINT_TAB_LITERAL, pht('Configure your editor to use spaces for indentation.'), "\t"); } } protected function lintLineLength($path) { $lines = explode("\n", $this->getData($path)); $width = $this->maxLineLength; foreach ($lines as $line_idx => $line) { if (strlen($line) > $width) { $this->raiseLintAtLine( $line_idx + 1, 1, self::LINT_LINE_WRAP, pht( 'This line is %s characters long, but the '. 'convention is %s characters.', new PhutilNumber(strlen($line)), $width), $line); } } } protected function lintEOFNewline($path) { $data = $this->getData($path); if (!strlen($data) || $data[strlen($data) - 1] != "\n") { $this->raiseLintAtOffset( strlen($data), self::LINT_EOF_NEWLINE, pht('Files must end in a newline.'), '', "\n"); } } protected function lintCharset($path) { $data = $this->getData($path); $matches = null; $bad = '[^\x09\x0A\x20-\x7E]'; $preg = preg_match_all( "/{$bad}(.*{$bad})?/", $data, $matches, PREG_OFFSET_CAPTURE); if (!$preg) { return; } foreach ($matches[0] as $match) { list($string, $offset) = $match; $this->raiseLintAtOffset( $offset, self::LINT_BAD_CHARSET, pht( 'Source code should contain only ASCII bytes with ordinal '. 'decimal values between 32 and 126 inclusive, plus linefeed. '. 'Do not use UTF-8 or other multibyte charsets.'), $string); } if ($this->isMessageEnabled(self::LINT_BAD_CHARSET)) { $this->stopAllLinters(); } } protected function lintTrailingWhitespace($path) { $data = $this->getData($path); $matches = null; $preg = preg_match_all( '/[[:blank:]]+$/m', $data, $matches, PREG_OFFSET_CAPTURE); if (!$preg) { return; } foreach ($matches[0] as $match) { list($string, $offset) = $match; $this->raiseLintAtOffset( $offset, self::LINT_TRAILING_WHITESPACE, pht( 'This line contains trailing whitespace. Consider setting '. - 'up your editor to automatically remove trailing whitespace, '. - 'you will save time.'), + 'up your editor to automatically remove trailing whitespace.'), $string, ''); } } protected function lintBOFWhitespace($path) { $data = $this->getData($path); $matches = null; $preg = preg_match( '/^\s*\n/', $data, $matches, PREG_OFFSET_CAPTURE); if (!$preg) { return; } list($string, $offset) = $matches[0]; $this->raiseLintAtOffset( $offset, self::LINT_BOF_WHITESPACE, pht( 'This file contains leading whitespace at the beginning of the file. '. 'This is unnecessary and should be avoided when possible.'), $string, ''); } protected function lintEOFWhitespace($path) { $data = $this->getData($path); $matches = null; $preg = preg_match( '/(?<=\n)\s+$/', $data, $matches, PREG_OFFSET_CAPTURE); if (!$preg) { return; } list($string, $offset) = $matches[0]; $this->raiseLintAtOffset( $offset, self::LINT_EOF_WHITESPACE, pht('This file contains unnecessary trailing whitespace.'), $string, ''); } } diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php index a6dc69b5..85144bc2 100644 --- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php @@ -1,326 +1,372 @@ getLinter(); $files = id(new FileFinder($root)) ->withType('f') ->withSuffix('lint-test') ->find(); $test_count = 0; foreach ($files as $file) { $this->lintFile($root.$file, $linter); $test_count++; } $this->assertTrue( ($test_count > 0), pht( 'Expected to find some %s tests in directory %s!', '.lint-test', $root)); } private function lintFile($file, ArcanistLinter $linter) { $linter = clone $linter; $contents = Filesystem::readFile($file); $contents = preg_split('/^~{4,}\n/m', $contents); if (count($contents) < 2) { throw new Exception( pht( "Expected '%s' separating test case and results.", '~~~~~~~~~~')); } list($data, $expect, $xform, $config) = array_merge( $contents, array(null, null)); - $basename = basename($file); - if ($config) { $config = phutil_json_decode($config); } else { $config = array(); } PhutilTypeSpec::checkMap( $config, array( 'config' => 'optional map', 'mode' => 'optional string', 'path' => 'optional string', 'stopped' => 'optional bool', )); $exception = null; $after_lint = null; $messages = null; $exception_message = false; $caught_exception = false; try { + $path_name = idx($config, 'path'); + + if ($path_name !== null) { + $basename = basename($path_name); + } else { + $basename = basename($file); + } + $tmp = new TempFile($basename); Filesystem::writeFile($tmp, $data); $full_path = (string)$tmp; $mode = idx($config, 'mode'); if ($mode) { Filesystem::changePermissions($tmp, octdec($mode)); } $dir = dirname($full_path); - $path = basename($full_path); $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)); + $engine->setPaths(array($basename)); $linter->setEngine($engine); - $linter->addPath($path_name); - $linter->addData($path_name, $data); + $linter->addPath($basename); + $linter->addData($basename, $data); foreach (idx($config, 'config', array()) as $key => $value) { $linter->setLinterConfigurationValue($key, $value); } $engine->addLinter($linter); - $engine->addFileData($path_name, $data); + $engine->addFileData($basename, $data); $results = $engine->run(); + $this->assertEqual( 1, count($results), pht('Expect one result returned by linter.')); $assert_stopped = idx($config, 'stopped'); if ($assert_stopped !== null) { $this->assertEqual( $assert_stopped, $linter->didStopAllLinters(), $assert_stopped ? pht('Expect linter to be stopped.') : pht('Expect linter to not be stopped.')); } $result = reset($results); $patcher = ArcanistLintPatcher::newFromArcanistLintResult($result); $after_lint = $patcher->getModifiedFileContent(); } catch (PhutilTestTerminatedException $ex) { throw $ex; } catch (Exception $exception) { $caught_exception = true; if ($exception instanceof PhutilAggregateException) { $caught_exception = false; foreach ($exception->getExceptions() as $ex) { if ($ex instanceof ArcanistUsageException || $ex instanceof ArcanistMissingLinterException) { $this->assertSkipped($ex->getMessage()); } else { $caught_exception = true; } } } else if ($exception instanceof ArcanistUsageException || $exception instanceof ArcanistMissingLinterException) { $this->assertSkipped($exception->getMessage()); } $exception_message = $exception->getMessage()."\n\n". $exception->getTraceAsString(); } $this->assertEqual(false, $caught_exception, $exception_message); $this->compareLint($basename, $expect, $result); $this->compareTransform($xform, $after_lint); } private function compareLint($file, $expect, ArcanistLintResult $results) { $expected_results = new ArcanistLintResult(); $expect = trim($expect); if ($expect) { $expect = explode("\n", $expect); } else { $expect = array(); } foreach ($expect as $result) { $parts = explode(':', $result); $message = new ArcanistLintMessage(); $severity = idx($parts, 0); - $line = idx($parts, 1); - $char = idx($parts, 2); - $code = idx($parts, 3); + $line = idx($parts, 1); + if ($line === '') { + $line = null; + } + + $char = idx($parts, 2); + if ($char === '') { + $char = null; + } + + $code = idx($parts, 3); + if ($code === '') { + $code = null; + } if ($severity !== null) { $message->setSeverity($severity); } if ($line !== null) { $message->setLine($line); } if ($char !== null) { $message->setChar($char); } if ($code !== null) { $message->setCode($code); } $expected_results->addMessage($message); } $missing = array(); $surprising = $results->getMessages(); // TODO: Make this more efficient. foreach ($expected_results->getMessages() as $expected_message) { $found = false; foreach ($results->getMessages() as $ii => $actual_message) { if (!self::compareLintMessageProperty( $expected_message->getSeverity(), $actual_message->getSeverity())) { continue; } if (!self::compareLintMessageProperty( $expected_message->getLine(), $actual_message->getLine())) { continue; } if (!self::compareLintMessageProperty( $expected_message->getChar(), $actual_message->getChar())) { continue; } if (!self::compareLintMessageProperty( $expected_message->getCode(), $actual_message->getCode())) { continue; } $found = true; unset($surprising[$ii]); } if (!$found) { $missing[] = $expected_message; } } if ($missing || $surprising) { - $expected = pht('EXPECTED MESSAGES'); - if ($missing) { - foreach ($missing as $message) { - $expected .= sprintf( - "\n %s: %s %s", - pht( - '%s at line %d, char %d', - $message->getSeverity(), - $message->getLine(), - $message->getChar()), - $message->getCode(), - $message->getName()); - } - } else { - $expected .= "\n ".pht('No messages'); - } - - $actual = pht('UNEXPECTED MESSAGES'); - if ($surprising) { - foreach ($surprising as $message) { - $actual .= sprintf( - "\n %s: %s %s", - pht( - '%s at line %d, char %d', - $message->getSeverity(), - $message->getLine(), - $message->getChar()), - $message->getCode(), - $message->getName()); - } - } else { - $actual .= "\n ".pht('No messages'); - } - $this->assertFailure( sprintf( - "%s\n\n%s\n\n%s", - pht("Lint failed for '%s'.", $file), - $expected, - $actual)); + "%s\n%s%s", + pht( + 'Lint emitted an unexpected set of messages for file "%s".', + $file), + $this->renderMessages(pht('MISSING MESSAGES'), $missing), + $this->renderMessages(pht('SURPLUS MESSAGES'), $surprising))); } } private function compareTransform($expected, $actual) { if (!strlen($expected)) { return; } $this->assertEqual( $expected, $actual, pht('File as patched by lint did not match the expected patched file.')); } /** * Compare properties of @{class:ArcanistLintMessage} instances. * - * The expectation is that if one (or both) of the properties is null, then - * we don't care about its value. - * * @param wild * @param wild * @return bool */ private static function compareLintMessageProperty($x, $y) { - return $x === null || $y === null || $x === $y; + if ($x === null) { + return true; + } + + return ($x === $y); + } + + private function renderMessages($header, array $messages) { + if (!$messages) { + $display = tsprintf( + "%s\n", + pht('(No messages.)')); + } else { + $lines = array(); + foreach ($messages as $message) { + $line = $message->getLine(); + if ($line === null) { + $display_line = pht(''); + } else { + $display_line = $line; + } + + $char = $message->getChar(); + if ($char === null) { + $display_char = pht(''); + } else { + $display_char = $char; + } + + $code = $message->getCode(); + $name = $message->getName(); + if ($code !== null && $name !== null) { + $display_code = pht('%s: %s', $code, $name); + } else if ($code !== null) { + $display_code = pht('%s', $code); + } else { + $display_code = null; + } + + $severity = $message->getSeverity(); + + if ($display_code === null) { + $display_message = pht( + 'Message with severity "%s" at "%s:%s"', + $severity, + $display_line, + $display_char); + } else { + $display_message = pht( + 'Message with severity "%s" at "%s:%s" (%s)', + $severity, + $display_line, + $display_char, + $display_code); + } + + $lines[] = tsprintf( + " %s\n", + $display_message); + } + $display = implode('', $lines); + } + + return tsprintf( + "%s\n%B\n", + $header, + $display); } } diff --git a/src/lint/linter/__tests__/chmod/empty_executable.lint-test b/src/lint/linter/__tests__/chmod/empty_executable.lint-test index cf8980b6..695d3b8e 100644 --- a/src/lint/linter/__tests__/chmod/empty_executable.lint-test +++ b/src/lint/linter/__tests__/chmod/empty_executable.lint-test @@ -1,5 +1,5 @@ ~~~~~~~~~~ -warning:0:0:CHMOD1:Invalid Executable +warning:::CHMOD1:Invalid Executable ~~~~~~~~~~ ~~~~~~~~~~ {"mode": "0755"} diff --git a/src/lint/linter/__tests__/cpplint/googlestyle.lint-test b/src/lint/linter/__tests__/cpplint/googlestyle.lint-test index 5417f073..0c5b1b79 100644 --- a/src/lint/linter/__tests__/cpplint/googlestyle.lint-test +++ b/src/lint/linter/__tests__/cpplint/googlestyle.lint-test @@ -1,10 +1,21 @@ #include "library.cpp" #include void main() { } +~~~~~~~~~ +warning +warning:2 +warning:5 ~~~~~~~~~~ -warning:0: -warning:2: -warning:5: +#include "library.cpp" +#include + +void main() +{ +} +~~~~~~~~~~ +{ + "path": "example.cpp" +} diff --git a/src/lint/linter/__tests__/filename/bad_filename.lint-test b/src/lint/linter/__tests__/filename/bad_filename.lint-test index 12ec5757..28b7e560 100644 --- a/src/lint/linter/__tests__/filename/bad_filename.lint-test +++ b/src/lint/linter/__tests__/filename/bad_filename.lint-test @@ -1,5 +1,5 @@ ~~~~~~~~~~ -error:0:0:NAME1:Bad Filename +error:::NAME1:Bad Filename ~~~~~~~~~~ ~~~~~~~~~~ {"path": "bad@filename"} diff --git a/src/lint/linter/__tests__/php/fatal.lint-test b/src/lint/linter/__tests__/php/fatal.lint-test index 6d3c7b82..04bd5c86 100644 --- a/src/lint/linter/__tests__/php/fatal.lint-test +++ b/src/lint/linter/__tests__/php/fatal.lint-test @@ -1,7 +1,7 @@