From b50a646a3f49c8b842cf0764c59ea2c38c2f9567 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 9 Dec 2021 15:57:08 -0800 Subject: [PATCH] Provide additional Arcanist PHP 8.1 fixes Summary: Ref T13588. I pointed my local `php` at PHP 8.1 and this is what I've hit so far; all these cases seem very unlikely to have any subtle behavior. Test Plan: Ran various `arc` workflows under PHP 8.1. Maniphest Tasks: T13588 Differential Revision: https://secure.phabricator.com/D21742 --- .../ArcanistConfigurationManager.php | 2 +- src/filesystem/PhutilDirectoryFixture.php | 2 ++ src/future/exec/ExecFuture.php | 22 +++++++++++++++---- src/lint/linter/ArcanistXMLLinter.php | 6 ++++- .../__tests__/ArcanistLinterTestCase.php | 10 +++++++++ ...canistPHPCompatibilityXHPASTLinterRule.php | 4 +++- ...tParentMemberReferenceXHPASTLinterRule.php | 7 +++++- ...istSelfMemberReferenceXHPASTLinterRule.php | 7 +++++- src/parser/ArcanistBundle.php | 5 ++--- src/parser/ArcanistDiffParser.php | 21 ++++++++++-------- src/parser/PhutilBugtraqParser.php | 2 +- src/parser/PhutilEmailAddress.php | 3 ++- src/parser/PhutilURI.php | 9 ++++++-- src/parser/aast/api/AASTNodeList.php | 6 +++++ src/serviceprofiler/PhutilServiceProfiler.php | 6 ++++- .../renderer/ArcanistUnitConsoleRenderer.php | 6 +++-- src/utils/PhutilArray.php | 10 +++++++++ src/utils/PhutilCallbackFilterIterator.php | 1 + src/workflow/ArcanistDiffWorkflow.php | 11 ++++++---- src/workflow/ArcanistWorkflow.php | 2 +- support/lib/extract-symbols.php | 8 ++++++- 21 files changed, 116 insertions(+), 34 deletions(-) diff --git a/src/configuration/ArcanistConfigurationManager.php b/src/configuration/ArcanistConfigurationManager.php index f1391a8b..68ebde4e 100644 --- a/src/configuration/ArcanistConfigurationManager.php +++ b/src/configuration/ArcanistConfigurationManager.php @@ -258,7 +258,7 @@ final class ArcanistConfigurationManager extends Phobject { } public function getUserConfigurationFileLocation() { - if (strlen($this->customArcrcFilename)) { + if ($this->customArcrcFilename !== null) { return $this->customArcrcFilename; } diff --git a/src/filesystem/PhutilDirectoryFixture.php b/src/filesystem/PhutilDirectoryFixture.php index 8eb4c14a..5564b7bb 100644 --- a/src/filesystem/PhutilDirectoryFixture.php +++ b/src/filesystem/PhutilDirectoryFixture.php @@ -28,6 +28,8 @@ final class PhutilDirectoryFixture extends Phobject { } public function getPath($to_file = null) { + $to_file = phutil_string_cast($to_file); + return $this->path.'/'.ltrim($to_file, '/'); } diff --git a/src/future/exec/ExecFuture.php b/src/future/exec/ExecFuture.php index 7aadd61a..ddf9e515 100644 --- a/src/future/exec/ExecFuture.php +++ b/src/future/exec/ExecFuture.php @@ -192,11 +192,18 @@ final class ExecFuture extends PhutilExecutableFuture { * @task interact */ public function read() { - $stdout = $this->readStdout(); + $stdout_value = $this->readStdout(); + + $stderr = $this->stderr; + if ($stderr === null) { + $stderr_value = ''; + } else { + $stderr_value = substr($stderr, $this->stderrPos); + } $result = array( - $stdout, - (string)substr($this->stderr, $this->stderrPos), + $stdout_value, + $stderr_value, ); $this->stderrPos = $this->getStderrBufferLength(); @@ -209,7 +216,14 @@ final class ExecFuture extends PhutilExecutableFuture { $this->updateFuture(); // Sync } - $result = (string)substr($this->stdout, $this->stdoutPos); + $stdout = $this->stdout; + + if ($stdout === null) { + $result = ''; + } else { + $result = substr($stdout, $this->stdoutPos); + } + $this->stdoutPos = $this->getStdoutBufferLength(); return $result; diff --git a/src/lint/linter/ArcanistXMLLinter.php b/src/lint/linter/ArcanistXMLLinter.php index b48b19bc..1d7971ab 100644 --- a/src/lint/linter/ArcanistXMLLinter.php +++ b/src/lint/linter/ArcanistXMLLinter.php @@ -27,7 +27,11 @@ final class ArcanistXMLLinter extends ArcanistLinter { } public function getCacheVersion() { - return LIBXML_VERSION; + if (defined('LIBXML_VERSION')) { + return LIBXML_VERSION; + } else { + return 'unavailable'; + } } public function lintPath($path) { diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php index 85144bc2..19d9ce10 100644 --- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php +++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php @@ -51,6 +51,13 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { private function lintFile($file, ArcanistLinter $linter) { $linter = clone $linter; + if (!$linter->canRun()) { + $this->assertSkipped( + pht( + 'Linter "%s" can not run.', + get_class($linter))); + } + $contents = Filesystem::readFile($file); $contents = preg_split('/^~{4,}\n/m', $contents); if (count($contents) < 2) { @@ -283,9 +290,12 @@ abstract class ArcanistLinterTestCase extends PhutilTestCase { } private function compareTransform($expected, $actual) { + $expected = phutil_string_cast($expected); + if (!strlen($expected)) { return; } + $this->assertEqual( $expected, $actual, diff --git a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php index 743d1484..0b52eed4 100644 --- a/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistPHPCompatibilityXHPASTLinterRule.php @@ -155,7 +155,9 @@ final class ArcanistPHPCompatibilityXHPASTLinterRule if ($this->windowsVersion) { $windows = idx($compat_info['functions_windows'], $name); - if ($windows === false) { + if ($windows === null) { + // This function has no special Windows considerations. + } else if ($windows === false) { $this->raiseLintAtNode( $node, pht( diff --git a/src/lint/linter/xhpast/rules/ArcanistParentMemberReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistParentMemberReferenceXHPASTLinterRule.php index 0183461b..ab18e683 100644 --- a/src/lint/linter/xhpast/rules/ArcanistParentMemberReferenceXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistParentMemberReferenceXHPASTLinterRule.php @@ -55,7 +55,12 @@ final class ArcanistParentMemberReferenceXHPASTLinterRule } } - if (version_compare($this->version, '5.4.0', '>=') || !$in_closure) { + $version_target = $this->version; + if ($version_target === null) { + $version_target = phpversion(); + } + + if (version_compare($version_target, '5.4.0', '>=') || !$in_closure) { $this->raiseLintAtNode( $class_ref, pht( diff --git a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php index 8db13ca7..8c1ed2e4 100644 --- a/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php +++ b/src/lint/linter/xhpast/rules/ArcanistSelfMemberReferenceXHPASTLinterRule.php @@ -42,7 +42,12 @@ final class ArcanistSelfMemberReferenceXHPASTLinterRule } } - if (version_compare($this->version, '5.4.0', '>=') || !$in_closure) { + $version_target = $this->version; + if (!$version_target) { + $version_target = phpversion(); + } + + if (version_compare($version_target, '5.4.0', '>=') || !$in_closure) { $this->raiseLintAtNode( $class_ref, pht( diff --git a/src/parser/ArcanistBundle.php b/src/parser/ArcanistBundle.php index 6d72047b..4617c9b6 100644 --- a/src/parser/ArcanistBundle.php +++ b/src/parser/ArcanistBundle.php @@ -639,8 +639,7 @@ final class ArcanistBundle extends Phobject { $old_path = $change->getOldPath(); $type = $change->getType(); - if (!strlen($old_path) || - $type == ArcanistDiffChangeType::TYPE_ADD) { + if ($old_path === '' || $type == ArcanistDiffChangeType::TYPE_ADD) { $old_path = null; } @@ -1023,7 +1022,7 @@ final class ArcanistBundle extends Phobject { if ($is_64bit) { for ($count = 4; $count >= 0; $count--) { $val = $accum % 85; - $accum = $accum / 85; + $accum = (int)($accum / 85); $slice .= $map[$val]; } } else { diff --git a/src/parser/ArcanistDiffParser.php b/src/parser/ArcanistDiffParser.php index 1d213846..9a708fd2 100644 --- a/src/parser/ArcanistDiffParser.php +++ b/src/parser/ArcanistDiffParser.php @@ -217,7 +217,7 @@ final class ArcanistDiffParser extends Phobject { $line = $this->nextLine(); } - if (strlen($message)) { + if ($message !== null && strlen($message)) { // If we found a message during pre-parse steps, add it to the resulting // changes here. $change = $this->buildChange(null) @@ -582,10 +582,13 @@ final class ArcanistDiffParser extends Phobject { $ok = false; $match = null; - foreach ($patterns as $pattern) { - $ok = preg_match('@^'.$pattern.'@', $line, $match); - if ($ok) { - break; + + if ($line !== null) { + foreach ($patterns as $pattern) { + $ok = preg_match('@^'.$pattern.'@', $line, $match); + if ($ok) { + break; + } } } @@ -774,7 +777,7 @@ final class ArcanistDiffParser extends Phobject { $this->nextLine(); $this->parseGitBinaryPatch(); $line = $this->getLine(); - if (preg_match('/^literal/', $line)) { + if ($line !== null && preg_match('/^literal/', $line)) { // We may have old/new binaries (change) or just a new binary (hg add). // If there are two blocks, parse both. $this->parseGitBinaryPatch(); @@ -920,11 +923,11 @@ final class ArcanistDiffParser extends Phobject { $hunk->setNewOffset($matches[3]); // Cover for the cases where length wasn't present (implying one line). - $old_len = idx($matches, 2); + $old_len = idx($matches, 2, ''); if (!strlen($old_len)) { $old_len = 1; } - $new_len = idx($matches, 4); + $new_len = idx($matches, 4, ''); if (!strlen($new_len)) { $new_len = 1; } @@ -1041,7 +1044,7 @@ final class ArcanistDiffParser extends Phobject { $line = $this->nextNonemptyLine(); } - } while (preg_match('/^@@ /', $line)); + } while (($line !== null) && preg_match('/^@@ /', $line)); } protected function buildChange($path = null) { diff --git a/src/parser/PhutilBugtraqParser.php b/src/parser/PhutilBugtraqParser.php index 407dcb4c..708f3749 100644 --- a/src/parser/PhutilBugtraqParser.php +++ b/src/parser/PhutilBugtraqParser.php @@ -85,7 +85,7 @@ final class PhutilBugtraqParser extends Phobject { $captured_text = $capture['text']; $captured_offset = $capture['at']; - if (strlen($select_regexp)) { + if ($select_regexp !== null) { $selections = null; preg_match_all( $select_regexp, diff --git a/src/parser/PhutilEmailAddress.php b/src/parser/PhutilEmailAddress.php index 6acda756..d02c4bce 100644 --- a/src/parser/PhutilEmailAddress.php +++ b/src/parser/PhutilEmailAddress.php @@ -13,6 +13,7 @@ final class PhutilEmailAddress extends Phobject { private $domainName; public function __construct($email_address = null) { + $email_address = phutil_string_cast($email_address); $email_address = trim($email_address); $matches = null; @@ -89,7 +90,7 @@ final class PhutilEmailAddress extends Phobject { public function getAddress() { $address = $this->localPart; - if (strlen($this->domainName)) { + if ($this->domainName !== null && strlen($this->domainName)) { $address .= '@'.$this->domainName; } return $address; diff --git a/src/parser/PhutilURI.php b/src/parser/PhutilURI.php index 1902cd1f..d9701fb0 100644 --- a/src/parser/PhutilURI.php +++ b/src/parser/PhutilURI.php @@ -171,14 +171,19 @@ final class PhutilURI extends Phobject { } } - if (strlen($protocol) || strlen($auth) || strlen($domain)) { + $has_protocol = ($protocol !== null) && strlen($protocol); + $has_auth = ($auth !== null); + $has_domain = ($domain !== null) && strlen($domain); + $has_port = ($port !== null) && strlen($port); + + if ($has_protocol || $has_auth || $has_domain) { if ($this->isGitURI()) { $prefix = "{$auth}{$domain}"; } else { $prefix = "{$protocol}://{$auth}{$domain}"; } - if (strlen($port)) { + if ($has_port) { $prefix .= ':'.$port; } } diff --git a/src/parser/aast/api/AASTNodeList.php b/src/parser/aast/api/AASTNodeList.php index 92f76f2a..985fda31 100644 --- a/src/parser/aast/api/AASTNodeList.php +++ b/src/parser/aast/api/AASTNodeList.php @@ -80,6 +80,7 @@ final class AASTNodeList /* -( Countable )---------------------------------------------------------- */ + #[\ReturnTypeWillChange] public function count() { return count($this->ids); } @@ -87,22 +88,27 @@ final class AASTNodeList /* -( Iterator )----------------------------------------------------------- */ + #[\ReturnTypeWillChange] public function current() { return $this->list[$this->key()]; } + #[\ReturnTypeWillChange] public function key() { return $this->ids[$this->pos]; } + #[\ReturnTypeWillChange] public function next() { $this->pos++; } + #[\ReturnTypeWillChange] public function rewind() { $this->pos = 0; } + #[\ReturnTypeWillChange] public function valid() { return $this->pos < count($this->ids); } diff --git a/src/serviceprofiler/PhutilServiceProfiler.php b/src/serviceprofiler/PhutilServiceProfiler.php index 7ab8a12e..9ca9b9c1 100644 --- a/src/serviceprofiler/PhutilServiceProfiler.php +++ b/src/serviceprofiler/PhutilServiceProfiler.php @@ -137,7 +137,7 @@ final class PhutilServiceProfiler extends Phobject { $uri = phutil_censor_credentials($data['uri']); - if (strlen($proxy)) { + if ($proxy !== null) { $desc = "{$proxy} >> {$uri}"; } else { $desc = $uri; @@ -203,6 +203,10 @@ final class PhutilServiceProfiler extends Phobject { } private static function escapeProfilerStringForDisplay($string) { + if ($string === null) { + return ''; + } + // Convert tabs and newlines to spaces and collapse blocks of whitespace, // most often formatting in queries. $string = preg_replace('/\s{2,}/', ' ', $string); diff --git a/src/unit/renderer/ArcanistUnitConsoleRenderer.php b/src/unit/renderer/ArcanistUnitConsoleRenderer.php index 6294cdee..729c4b8d 100644 --- a/src/unit/renderer/ArcanistUnitConsoleRenderer.php +++ b/src/unit/renderer/ArcanistUnitConsoleRenderer.php @@ -65,9 +65,10 @@ final class ArcanistUnitConsoleRenderer extends ArcanistUnitRenderer { 50 => "%s{$star} ", 200 => '%s ', 500 => '%s ', - INF => '%s ', ); + $least_acceptable = '%s '; + $milliseconds = $seconds * 1000; $duration = $this->formatTime($seconds); foreach ($acceptableness as $upper_bound => $formatting) { @@ -75,7 +76,8 @@ final class ArcanistUnitConsoleRenderer extends ArcanistUnitRenderer { return phutil_console_format($formatting, $duration); } } - return phutil_console_format(end($acceptableness), $duration); + + return phutil_console_format($least_acceptable, $duration); } private function formatTime($seconds) { diff --git a/src/utils/PhutilArray.php b/src/utils/PhutilArray.php index 8656bab3..1ca02a4a 100644 --- a/src/utils/PhutilArray.php +++ b/src/utils/PhutilArray.php @@ -29,6 +29,7 @@ abstract class PhutilArray /* -( Countable Interface )------------------------------------------------ */ + #[\ReturnTypeWillChange] public function count() { return count($this->data); } @@ -37,22 +38,27 @@ abstract class PhutilArray /* -( Iterator Interface )------------------------------------------------- */ + #[\ReturnTypeWillChange] public function current() { return current($this->data); } + #[\ReturnTypeWillChange] public function key() { return key($this->data); } + #[\ReturnTypeWillChange] public function next() { return next($this->data); } + #[\ReturnTypeWillChange] public function rewind() { reset($this->data); } + #[\ReturnTypeWillChange] public function valid() { return (key($this->data) !== null); } @@ -61,18 +67,22 @@ abstract class PhutilArray /* -( ArrayAccess Interface )---------------------------------------------- */ + #[\ReturnTypeWillChange] public function offsetExists($key) { return array_key_exists($key, $this->data); } + #[\ReturnTypeWillChange] public function offsetGet($key) { return $this->data[$key]; } + #[\ReturnTypeWillChange] public function offsetSet($key, $value) { $this->data[$key] = $value; } + #[\ReturnTypeWillChange] public function offsetUnset($key) { unset($this->data[$key]); } diff --git a/src/utils/PhutilCallbackFilterIterator.php b/src/utils/PhutilCallbackFilterIterator.php index 2eab78df..c1a95a4d 100644 --- a/src/utils/PhutilCallbackFilterIterator.php +++ b/src/utils/PhutilCallbackFilterIterator.php @@ -18,6 +18,7 @@ final class PhutilCallbackFilterIterator extends FilterIterator { $this->callback = $callback; } + #[\ReturnTypeWillChange] public function accept() { return call_user_func($this->callback, $this->current()); } diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php index 38aa4b62..7201a003 100644 --- a/src/workflow/ArcanistDiffWorkflow.php +++ b/src/workflow/ArcanistDiffWorkflow.php @@ -701,7 +701,7 @@ EOTEXT $this->revisionID = $revision_id; $revision['message'] = $this->getArgument('message'); - if (!strlen($revision['message'])) { + if ($revision['message'] === null) { $update_messages = $this->readScratchJSONFile('update-messages.json'); $update_messages[$revision_id] = $this->getUpdateMessage( @@ -1740,9 +1740,12 @@ EOTEXT if ($template == '') { $comments = $this->getDefaultUpdateMessage(); + $comments = phutil_string_cast($comments); + $comments = rtrim($comments); + $template = sprintf( "%s\n\n# %s\n#\n# %s\n# %s\n#\n# %s\n# $ %s\n\n", - rtrim($comments), + $comments, pht( 'Updating %s: %s', "D{$fields['revisionID']}", @@ -2360,7 +2363,7 @@ EOTEXT if (strlen($branch)) { $upstream_path = $api->getPathToUpstream($branch); $remote_branch = $upstream_path->getRemoteBranchName(); - if (strlen($remote_branch)) { + if ($remote_branch !== null) { return array( array( 'type' => 'branch', @@ -2374,7 +2377,7 @@ EOTEXT // If "arc.land.onto.default" is configured, use that. $config_key = 'arc.land.onto.default'; $onto = $this->getConfigFromAnySource($config_key); - if (strlen($onto)) { + if ($onto !== null) { return array( array( 'type' => 'branch', diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index 43ed0c0e..85b917b7 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -154,7 +154,7 @@ abstract class ArcanistWorkflow extends Phobject { } $help = $information->getHelp(); - if (strlen($help)) { + if ($help !== null) { // Unwrap linebreaks in the help text so we don't get weird formatting. $help = preg_replace("/(?<=\S)\n(?=\S)/", ' ', $help); diff --git a/support/lib/extract-symbols.php b/support/lib/extract-symbols.php index dac4ca71..a1cd28fa 100755 --- a/support/lib/extract-symbols.php +++ b/support/lib/extract-symbols.php @@ -219,7 +219,13 @@ foreach ($calls as $call) { $symbol = array_shift($params); $type = 'function'; $symbol_value = $symbol->getStringLiteralValue(); - $pos = strpos($symbol_value, '::'); + + if ($symbol_value !== null) { + $pos = strpos($symbol_value, '::'); + } else { + $pos = false; + } + if ($pos) { $type = 'class'; $symbol_value = substr($symbol_value, 0, $pos);