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 @@ -128,78 +128,87 @@ $this->compareTransform($xform, $after_lint); } - private function compareLint($file, $expect, ArcanistLintResult $result) { - $seen = array(); + private function compareLint($file, $expect, ArcanistLintResult $results) { + $expected_results = new ArcanistLintResult(); + $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); + if (!$raised) { + $raised = array('No messages.'); } + $raised = sprintf( + "%s:\n%s", + pht('Actually raised'), + implode("\n", $raised)); + $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); - $expect = array_fill_keys($expect, true); - $seen = array_fill_keys($seen, true); + $message = id(new ArcanistLintMessage()) + ->setSeverity(idx($parts, 0)); - if (!$raised) { - $raised = array(pht('No messages.')); - } - $raised = sprintf( - "%s:\n%s", - pht('Actually raised'), - implode("\n", $raised)); + if (idx($parts, 1)) { + $message->setLine((int)idx($parts, 1)); + } + if (idx($parts, 2)) { + $message->setChar((int)idx($parts, 2)); + } + if (idx($parts, 3)) { + $message->setCode(idx($parts, 3)); + } - 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)); + $expected_results->addMessage($message); } - foreach (array_diff_key($seen, $expect) as $surprising => $ignored) { - $message = $message_map[$surprising]; - $message_info = $message->getDescription(); + if ( + ArcanistLinterTestCase::compareLintResults($expected_results, $results) || + ArcanistLinterTestCase::compareLintResults($results, $expected_results)) { + + $expected = 'Expected to raise'; + if ($expected_results) { + foreach ($expected_results->getMessages() 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 ($results) { + foreach ($results->getMessages() 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)); } } @@ -213,4 +222,95 @@ pht('File as patched by lint did not match the expected patched file.')); } + /** + * Compare two @{class:ArcanistLintResult} instances. + * + * @param ArcanistLintResult $results1 [description] + * @param ArcanistLintResult $results2 [description] + * @return [type] [description] + */ + private static function compareLintResults( + ArcanistLintResult $results1, + ArcanistLintResult $results2) { + + $diff = array(); + + $messages1 = $results1->getMessages(); + $messages2 = $results2->getMessages(); + + foreach ($messages1 as $x) { + $found = false; + + // This relies on the sorted order returned by ArcanistLintMessage. + while ( + $x->getLine() > head($messages2)->getLine() || + ( + $x->getLine() === head($messages2)->getLine() && + $x->getChar() > head($messages2)->getChar())) { + array_shift($messages2); + } + + foreach ($messages2 as $y) { + if ($x->getLine() < $y->getLine()) { + break; + } else if ($x->getLine() > $y->getLine()) { + continue; + } + + if ($x->getChar() < $y->getChar()) { + break; + } else if ($x->getChar() > $y->getChar()) { + continue; + } + + if ($x->getSeverity() != $y->getSeverity()) { + continue; + } + + if (!ArcanistLinterTestCase::compareLintMessageDetail( + $x->getCode(), + $y->getCode())) { + + continue; + } + + if (!ArcanistLinterTestCase::compareLintMessageDetail( + $x->getName(), + $y->getName())) { + + continue; + } + + $found = true; + break; + } + + if (!$found) { + $diff[] = $x; + } + } + + return $diff; + } + + /** + * 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 compareLintMessageDetail($x, $y) { + if ($x === null || $y === null) { + return true; + } else if ($x === $y) { + return true; + } else { + return false; + } + } + }