From 25ee39b657b89f907e47be04b7c43542cfa8ee4e Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 27 May 2020 10:49:31 -0700 Subject: [PATCH 1/6] In the "cpplint" binding, raise messages on "line 0" without a line Summary: Ref T13543. The "cpplint.py" script may emit messages on line 0, but Arcanist doesn't accept these messages. This is a small piece of a whole set of broader issues, but stop the bleeding for now. Test Plan: - Ran `arc lint example.h` on a file with no `#ifndef` guard, and `cpplint` configured. - Cpplint raised a message at line "0". - Before change: arc choked when trying to render this. - After change: arc survives rendering. Maniphest Tasks: T13543 Differential Revision: https://secure.phabricator.com/D21289 --- src/lint/linter/ArcanistCpplintLinter.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/lint/linter/ArcanistCpplintLinter.php b/src/lint/linter/ArcanistCpplintLinter.php index f9c6ddf5..4559ec47 100644 --- a/src/lint/linter/ArcanistCpplintLinter.php +++ b/src/lint/linter/ArcanistCpplintLinter.php @@ -48,12 +48,22 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter { $message = new ArcanistLintMessage(); $message->setPath($path); - $message->setLine($matches[1]); $message->setCode($matches[3]); $message->setName($matches[3]); $message->setDescription($matches[2]); $message->setSeverity($severity); + // NOTE: "cpplint" raises some messages which apply to the whole file, + // like "no #ifndef guard found". It raises these messages on line 0. + + // Arcanist messages should have a "null" line, not a "0" line, if they + // aren't bound to a particular line number. + + $line = (int)$matches[1]; + if ($line > 0) { + $message->setLine($line); + } + $messages[] = $message; } From e69aa326038857867f1b5aedab120466b7c14044 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 27 May 2020 11:24:01 -0700 Subject: [PATCH 2/6] Fix an issue when rendering a lint message which removes whitespace at the end of a file Summary: Ref T13543. If a file ends in spaces and no newline, we'll emit a message suggesting removal of the spaces. This will effectively remove the line, but the code will then attempt to highlight text within the line. Prior to D21044 this continued without raising an error and produced a reasonable result, but it now fatals. Insetad, don't try to highlight lines which no longer exist. Test Plan: See T13543 for details. Maniphest Tasks: T13543 Differential Revision: https://secure.phabricator.com/D21290 --- src/lint/renderer/ArcanistConsoleLintRenderer.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/lint/renderer/ArcanistConsoleLintRenderer.php b/src/lint/renderer/ArcanistConsoleLintRenderer.php index 5b99e7c5..6f6daf33 100644 --- a/src/lint/renderer/ArcanistConsoleLintRenderer.php +++ b/src/lint/renderer/ArcanistConsoleLintRenderer.php @@ -146,11 +146,16 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer { $char - 1, strlen($original)); - $new_lines[$start - 1] = substr_replace( - $new_lines[$start - 1], - $this->highlightText($replacement), - $char - 1, - strlen($replacement)); + // See T13543. The message may have completely removed this line: for + // example, if it trimmed trailing spaces from the end of a file. If + // the line no longer exists, don't try to highlight it. + if (isset($new_lines[$start - 1])) { + $new_lines[$start - 1] = substr_replace( + $new_lines[$start - 1], + $this->highlightText($replacement), + $char - 1, + strlen($replacement)); + } } // If lines at the beginning of the changed line range are actually the From fce72b9c89d7fa64ddeb3f9da7fc96ff07322abf Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 27 May 2020 12:07:49 -0700 Subject: [PATCH 3/6] Make lint tests handle paths better and distinguish between "0" and "null" more carefully Summary: Ref T13543. Currently, the `cpplint` tests do not function because `cpplint` is passed a path which does not end in a suffix it recognizes. Change the tempfile / path code to pass `linter path/to/example.c`-style linters a path they expect. Then, correct some older code which was playing it fast-and-loose with "null" vs "0". Test Plan: Ran `arc unit --everything`, got a clean bill of health on all the linters I have installed. (This is probably not all tests, since I have only a subset of linters installed locally that we have code for.) Maniphest Tasks: T13543 Differential Revision: https://secure.phabricator.com/D21291 --- src/lint/linter/ArcanistTextLinter.php | 3 +- .../__tests__/ArcanistLinterTestCase.php | 154 ++++++++++++------ .../chmod/empty_executable.lint-test | 2 +- .../__tests__/cpplint/googlestyle.lint-test | 17 +- .../__tests__/filename/bad_filename.lint-test | 2 +- src/lint/linter/__tests__/php/fatal.lint-test | 2 +- .../linter/__tests__/php/syntax.lint-test | 2 +- .../linter/__tests__/ruby/hello.lint-test | 2 +- .../linter/__tests__/text/empty.lint-test | 2 +- .../linter/__tests__/xhpast/empty.lint-test | 2 +- 10 files changed, 122 insertions(+), 66 deletions(-) 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 @@ Date: Thu, 28 May 2020 07:24:37 -0700 Subject: [PATCH 4/6] Mark the wildcard argument to "arc liberate" as a path argument for shell completion Summary: This is a path argument, and shell completion should suggest tabs. Test Plan: Typed "arc liberate s", got path suggestions. Differential Revision: https://secure.phabricator.com/D21292 --- src/workflow/ArcanistLiberateWorkflow.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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), ); } From a0c346bf63ecb958b7e37788a32353cfe698187a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 28 May 2020 07:26:06 -0700 Subject: [PATCH 5/6] Add a support class to simplify typechecking list-of-objects return values Summary: With some frequency, code wants to assert that some "$o->m()" returns a list of objects of type X, possibly with unique values for some "getKey()"-style method result. Existing checks via `PhutilTypeMap` and `assert_instances_of()` aren't quite powerful enough to do this while producing an easily understandable error state. We want to know that the error arose from a call to "$o->m()" in particular. Test Plan: Currently used elsewhere, in Piledriver code. Differential Revision: https://secure.phabricator.com/D21293 --- src/__phutil_library_map__.php | 2 + src/utils/PhutilArrayCheck.php | 251 +++++++++++++++++++++++++++++++++ 2 files changed, 253 insertions(+) create mode 100644 src/utils/PhutilArrayCheck.php 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/utils/PhutilArrayCheck.php b/src/utils/PhutilArrayCheck.php new file mode 100644 index 00000000..0cefc637 --- /dev/null +++ b/src/utils/PhutilArrayCheck.php @@ -0,0 +1,251 @@ +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_object($object) && !is_string($object)) { + throw new Exception( + pht( + 'Expected an object or string 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_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)); + } + +} From 0da1a2e17d921dc27ce9afa76b123cb4c8b73b17 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 28 May 2020 11:07:51 -0700 Subject: [PATCH 6/6] Allow PhutilArrayCheck to accept a list of objects as a context Summary: This simplifies merging a list-of-lists, which is a common pattern when asking a list of extensions to each provide some kind of list of items. Test Plan: Used elsewhere in Piledriver. Differential Revision: https://secure.phabricator.com/D21294 --- src/utils/PhutilArrayCheck.php | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/utils/PhutilArrayCheck.php b/src/utils/PhutilArrayCheck.php index 0cefc637..1b8ce131 100644 --- a/src/utils/PhutilArrayCheck.php +++ b/src/utils/PhutilArrayCheck.php @@ -29,10 +29,24 @@ final class PhutilArrayCheck } public function setContext($object, $method) { - if (!is_object($object) && !is_string($object)) { + 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 or string for "object" context, got "%s".', + 'Expected an object, string, or list of objects for "object" '. + 'context, got "%s".', phutil_describe_type($object))); } @@ -219,7 +233,20 @@ final class PhutilArrayCheck $method = $context['method']; $argv = $context['argv']; - if (is_object($object)) { + 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),