diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index defdbb03..d4f29121 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -612,6 +612,7 @@ phutil_register_library_map(array( 'PhutilArgumentUsageException' => 'parser/argument/exception/PhutilArgumentUsageException.php', 'PhutilArgumentWorkflow' => 'parser/argument/workflow/PhutilArgumentWorkflow.php', 'PhutilArray' => 'utils/PhutilArray.php', + 'PhutilArrayCheck' => 'utils/PhutilArrayCheck.php', 'PhutilArrayTestCase' => 'utils/__tests__/PhutilArrayTestCase.php', 'PhutilArrayWithDefaultValue' => 'utils/PhutilArrayWithDefaultValue.php', 'PhutilAsanaFuture' => 'future/asana/PhutilAsanaFuture.php', @@ -1619,6 +1620,7 @@ phutil_register_library_map(array( 'ArrayAccess', 'Iterator', ), + 'PhutilArrayCheck' => 'Phobject', 'PhutilArrayTestCase' => 'PhutilTestCase', 'PhutilArrayWithDefaultValue' => 'PhutilArray', 'PhutilAsanaFuture' => 'FutureProxy', 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 @@ -261,8 +261,7 @@ final class ArcanistTextLinter extends ArcanistLinter { 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, ''); } 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 @@ -64,8 +64,6 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { $contents, array(null, null)); - $basename = basename($file); - if ($config) { $config = phutil_json_decode($config); } else { @@ -87,6 +85,14 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { $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; @@ -97,7 +103,6 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { } $dir = dirname($full_path); - $path = basename($full_path); $working_copy = ArcanistWorkingCopyIdentity::newFromRootAndConfigFile( $dir, @@ -106,26 +111,25 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { $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), @@ -187,9 +191,20 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { $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); @@ -256,46 +271,14 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { } 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))); } } @@ -312,15 +295,78 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { /** * 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 +~~~~~~~~~~ +#include "library.cpp" +#include + void main() { } ~~~~~~~~~~ -warning:0: -warning:2: -warning:5: +{ + "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 @@ -4,4 +4,4 @@ function f() { $this = "cannot be re-assigned"; } ~~~~~~~~~ -error:4:0:PHP2:Fatal Error +error:4::PHP2:Fatal Error diff --git a/src/lint/linter/__tests__/php/syntax.lint-test b/src/lint/linter/__tests__/php/syntax.lint-test index 279fb7af..098a9f68 100644 --- a/src/lint/linter/__tests__/php/syntax.lint-test +++ b/src/lint/linter/__tests__/php/syntax.lint-test @@ -4,4 +4,4 @@ function f() { this is bad syntax; } ~~~~~~~~~ -error:4:0:PHP1:Parse Error +error:4::PHP1:Parse Error diff --git a/src/lint/linter/__tests__/ruby/hello.lint-test b/src/lint/linter/__tests__/ruby/hello.lint-test index f6693b6c..a9bf9016 100644 --- a/src/lint/linter/__tests__/ruby/hello.lint-test +++ b/src/lint/linter/__tests__/ruby/hello.lint-test @@ -1,4 +1,4 @@ def hello() puts "hello world" ~~~~~~~~~~ -error:2:0:RUBY:Syntax Error +error:2::RUBY:Syntax Error diff --git a/src/lint/linter/__tests__/text/empty.lint-test b/src/lint/linter/__tests__/text/empty.lint-test index 72fb27ca..d4bf5989 100644 --- a/src/lint/linter/__tests__/text/empty.lint-test +++ b/src/lint/linter/__tests__/text/empty.lint-test @@ -1,4 +1,4 @@ ~~~~~~~~~~ -error:0:0:TXT10:Empty File +error:::TXT10:Empty File diff --git a/src/lint/linter/__tests__/xhpast/empty.lint-test b/src/lint/linter/__tests__/xhpast/empty.lint-test index 22cca1cd..c0b9b003 100644 --- a/src/lint/linter/__tests__/xhpast/empty.lint-test +++ b/src/lint/linter/__tests__/xhpast/empty.lint-test @@ -1,4 +1,4 @@ instancesOf = $instances_of; + return $this; + } + + public function getInstancesOf() { + return $this->instancesOf; + } + + public function setUniqueMethod($unique_method) { + $this->uniqueMethod = $unique_method; + return $this; + } + + public function getUniqueMethod() { + return $this->uniqueMethod; + } + + public function setContext($object, $method) { + if (is_array($object)) { + foreach ($object as $idx => $value) { + if (!is_object($value)) { + throw new Exception( + pht( + 'Expected an object, string, or list of objects for "object" '. + 'context. Got a list ("%s"), but the list item at index '. + '"%s" (with type "%s") is not an object.', + phutil_describe_type($object), + $idx, + phutil_describe_type($value))); + } + } + } else if (!is_object($object) && !is_string($object)) { + throw new Exception( + pht( + 'Expected an object, string, or list of objects for "object" '. + 'context, got "%s".', + phutil_describe_type($object))); + } + + if (!is_string($method)) { + throw new Exception( + pht( + 'Expected a string for "method" context, got "%s".', + phutil_describe_type($method))); + } + + $argv = func_get_args(); + $argv = array_slice($argv, 2); + + $this->context = array( + 'object' => $object, + 'method' => $method, + 'argv' => $argv, + ); + + return $this; + } + + public function checkValues($maps) { + foreach ($maps as $idx => $map) { + $maps[$idx] = $this->checkValue($map); + } + + $unique = $this->getUniqueMethod(); + if ($unique === null) { + $result = array(); + + foreach ($maps as $map) { + foreach ($map as $value) { + $result[] = $value; + } + } + } else { + $items = array(); + foreach ($maps as $idx => $map) { + foreach ($map as $key => $value) { + $items[$key][$idx] = $value; + } + } + + foreach ($items as $key => $values) { + if (count($values) === 1) { + continue; + } + $this->raiseValueException( + pht( + 'Unexpected return value from calls to "%s(...)". More than one '. + 'object returned a value with unique key "%s". This key was '. + 'returned by objects with indexes: %s.', + $unique, + $key, + implode(', ', array_keys($values)))); + } + + $result = array(); + foreach ($items as $key => $values) { + $result[$key] = head($values); + } + } + + return $result; + } + + public function checkValue($items) { + if (!$this->context) { + throw new PhutilInvalidStateException('setContext'); + } + + if (!is_array($items)) { + $this->raiseValueException( + pht( + 'Expected value to be a list, got "%s".', + phutil_describe_type($items))); + } + + $instances_of = $this->getInstancesOf(); + if ($instances_of !== null) { + foreach ($items as $idx => $item) { + if (!($item instanceof $instances_of)) { + $this->raiseValueException( + pht( + 'Expected value to be a list of objects which are instances of '. + '"%s", but item with index "%s" is "%s".', + $instances_of, + $idx, + phutil_describe_type($item))); + } + } + } + + $unique = $this->getUniqueMethod(); + if ($unique !== null) { + if ($instances_of === null) { + foreach ($items as $idx => $item) { + if (!is_object($item)) { + $this->raiseValueException( + pht( + 'Expected value to be a list of objects to support calling '. + '"%s" to generate unique keys, but item with index "%s" is '. + '"%s".', + $unique, + $idx, + phutil_describe_type($item))); + } + } + } + + $map = array(); + + foreach ($items as $idx => $item) { + $key = call_user_func(array($item, $unique)); + + if (!is_string($key) && !is_int($key)) { + $this->raiseValueException( + pht( + 'Expected method "%s->%s()" to return a string or integer for '. + 'use as a unique key, got "%s" from object at index "%s".', + get_class($item), + $unique, + phutil_describe_type($key), + $idx)); + } + + $key = phutil_string_cast($key); + + $map[$key][$idx] = $item; + } + + $result = array(); + foreach ($map as $key => $values) { + if (count($values) === 1) { + $result[$key] = head($values); + continue; + } + + $classes = array(); + foreach ($values as $value) { + $classes[] = get_class($value); + } + $classes = array_fuse($classes); + + if (count($classes) === 1) { + $class_display = head($classes); + } else { + $class_display = sprintf( + '[%s]', + implode(', ', $classes)); + } + + $index_display = array(); + foreach ($values as $idx => $value) { + $index_display[] = pht( + '"%s" (%s)', + $idx, + get_class($value)); + } + $index_display = implode(', ', $index_display); + + $this->raiseValueException( + pht( + 'Expected method "%s->%s()" to return a unique key, got "%s" '. + 'from %s object(s) at indexes: %s.', + $class_display, + $unique, + $key, + phutil_count($values), + $index_display)); + } + + $items = $result; + } + + return $items; + } + + private function raiseValueException($message) { + $context = $this->context; + + $object = $context['object']; + $method = $context['method']; + $argv = $context['argv']; + + if (is_array($object)) { + $classes = array(); + foreach ($object as $item) { + $classes[] = get_class($item); + } + $classes = array_fuse($classes); + $n = count($object); + + $object_display = sprintf( + '[%s]<%d>->%s', + implode(', ', $classes), + $n, + $method); + } else if (is_object($object)) { + $object_display = sprintf( + '%s->%s', + get_class($object), + $method); + } else { + $object_display = sprintf( + '%s::%s', + $object, + $method); + } + + if (count($argv)) { + $call_display = sprintf( + '%s(...)', + $object_display); + } else { + $call_display = sprintf( + '%s()', + $object_display); + } + + throw new Exception( + pht( + 'Unexpected return value from call to "%s": %s.', + $call_display, + $message)); + } + +} diff --git a/src/workflow/ArcanistLiberateWorkflow.php b/src/workflow/ArcanistLiberateWorkflow.php index c8aa224f..1a85cbbc 100644 --- a/src/workflow/ArcanistLiberateWorkflow.php +++ b/src/workflow/ArcanistLiberateWorkflow.php @@ -29,7 +29,8 @@ EOTEXT ->setHelp( pht('Perform a clean rebuild, ignoring caches. Thorough, but slow.')), $this->newWorkflowArgument('argv') - ->setWildcard(true), + ->setWildcard(true) + ->setIsPathArgument(true), ); }