diff --git a/src/lint/ArcanistLintMessage.php b/src/lint/ArcanistLintMessage.php --- a/src/lint/ArcanistLintMessage.php +++ b/src/lint/ArcanistLintMessage.php @@ -72,7 +72,7 @@ } public function setLine($line) { - $this->line = $line; + $this->line = (int) $line; return $this; } @@ -81,7 +81,7 @@ } public function setChar($char) { - $this->char = $char; + $this->char = (int) $char; return $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 @@ -165,78 +165,123 @@ $this->compareTransform($xform, $after_lint); } - private function compareLint($file, $expect, ArcanistLintResult $result) { - $seen = array(); - $raised = array(); - $message_map = array(); - - foreach ($result->getMessages() as $message) { - $sev = $message->getSeverity(); - $line = $message->getLine(); - $char = $message->getChar(); - $code = $message->getCode(); - $name = $message->getName(); - $message_key = $sev.':'.$line.':'.$char; - $message_map[$message_key] = $message; - $seen[] = $message_key; - $raised[] = sprintf( - ' %s: %s %s', - pht('%s at line %d, char %d', $sev, $line, $char), - $code, - $name); - } + 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 $key => $expected) { - $expect[$key] = head(explode(' ', $expected)); - } + foreach ($expect as $result) { + $parts = explode(':', $result); + + $message = new ArcanistLintMessage(); + + if ($severity = idx($parts, 0)) { + $message->setSeverity($severity); + } - $expect = array_fill_keys($expect, true); - $seen = array_fill_keys($seen, true); + if ($line = idx($parts, 1)) { + $message->setLine((int) $line); + } + if ($char = idx($parts, 2)) { + $message->setChar((int) $char); + } + if ($code = idx($parts, 3)) { + $message->setCode($code); + } - if (!$raised) { - $raised = array(pht('No messages.')); + $expected_results->addMessage($message); } - $raised = sprintf( - "%s:\n%s", - pht('Actually raised'), - implode("\n", $raised)); - foreach (array_diff_key($expect, $seen) as $missing => $ignored) { - list($sev, $line, $char) = explode(':', $missing); - $this->assertFailure( - pht( - "In '%s', expected lint to raise %s on line %d at char %d, ". - "but no %s was raised. %s", - $file, - $sev, - $line, - $char, - $sev, - $raised)); + $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 (!ArcanistLinterTestCase::compareLintMessageProperty( + $expected_message->getSeverity(), + $actual_message->getSeverity())) { + + continue; + } + + if (!ArcanistLinterTestCase::compareLintMessageProperty( + $expected_message->getLine(), + $actual_message->getLine())) { + + continue; + } + + if (!ArcanistLinterTestCase::compareLintMessageProperty( + $expected_message->getChar(), + $actual_message->getChar())) { + + continue; + } + + if (!ArcanistLinterTestCase::compareLintMessageProperty( + $expected_message->getCode(), + $actual_message->getCode())) { + + continue; + } + + $found = true; + unset($surprising[$ii]); + } + + if (!$found) { + $missing[] = $expected_message; + } } - foreach (array_diff_key($seen, $expect) as $surprising => $ignored) { - $message = $message_map[$surprising]; - $message_info = $message->getDescription(); + if ($missing || $surprising) { + $expected = 'Expected to raise'; + 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 .= sprintf("\n %s", pht('No messages')); + } + + $actual = 'Actually raised'; + 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 .= sprintf("\n %s", pht('No messages')); + } - list($sev, $line, $char) = explode(':', $surprising); $this->assertFailure( sprintf( - "%s:\n\n%s\n\n%s", - pht( - "In '%s', lint raised %s on line %d at char %d, ". - "but nothing was expected", - $file, - $sev, - $line, - $char), - $message_info, - $raised)); + "%s\n\n%s\n\n%s", + pht("Lint failed for '%s'.", $file), + $expected, + $actual)); } } @@ -250,4 +295,18 @@ 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; + } + }