From baf002304129fcfd8af8d6d1986310bd93b167ce Mon Sep 17 00:00:00 2001
From: Jessica Clarke <jrtc27@jrtc27.com>
Date: Sat, 9 Jan 2021 03:57:56 +0000
Subject: [PATCH 0/3] Fixes for basic PHP 8 functionality
This patch series is enough to get basic arc functionality to work with
PHP 8 (arc diff works, at least up until writing the review metadata, I
have yet to have another review to actually submit somewhere so it could
still be broken after that point). More may follow if I discover other
broken things.
Since I do not have an account on secure.phabricator.com this about the
only vaguely-sensible place for me to post these patches. I was going to
upload them but apparently only images are allowed so instead the best
you'll get is them concatenated inline below. If given an account I am
happy to post them for proper code review.
I have signed the CLA.
Jessica Clarke (3):
Remove final from private functions for PHP 8 compatibility
Fix PHP 8 deprecation warning in __arcanist_init_script__
Fix error handler on PHP 8
src/error/PhutilErrorHandler.php | 5 ++---
src/error/PhutilErrorTrap.php | 3 +--
src/lint/engine/ArcanistLintEngine.php | 4 ++--
src/lint/linter/ArcanistLinter.php | 2 +-
src/unit/engine/phutil/PhutilTestCase.php | 16 ++++++++--------
src/utils/AbstractDirectedGraph.php | 2 +-
src/workflow/ArcanistWorkflow.php | 4 ++--
support/init/init-script.php | 3 ++-
8 files changed, 19 insertions(+), 20 deletions(-)
--
2.29.2
From 9a3a21bfd7a88fd00a568b0a93f0cbae51d13ec5 Mon Sep 17 00:00:00 2001
From: Jessica Clarke <jrtc27@jrtc27.com>
Date: Sat, 9 Jan 2021 03:42:05 +0000
Subject: [PATCH 1/3] Remove final from private functions for PHP 8
compatibility
This combination does not make sense and PHP 8 errors with:
```
Private methods cannot be final as they are never overridden by other classes
```
Thus remove the redundant final from all such functions.
---
src/lint/engine/ArcanistLintEngine.php | 4 ++--
src/lint/linter/ArcanistLinter.php | 2 +-
src/unit/engine/phutil/PhutilTestCase.php | 16 ++++++++--------
src/utils/AbstractDirectedGraph.php | 2 +-
src/workflow/ArcanistWorkflow.php | 4 ++--
5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php
index f737fcd4..add11d15 100644
--- a/src/lint/engine/ArcanistLintEngine.php
+++ b/src/lint/engine/ArcanistLintEngine.php
@@ -274,7 +274,7 @@ abstract class ArcanistLintEngine extends Phobject {
return ArcanistLintSeverity::isAtLeastAsSevere($severity, $minimum);
}
- final private function shouldUseCache(
+ private function shouldUseCache(
$cache_granularity,
$repository_version) {
@@ -313,7 +313,7 @@ abstract class ArcanistLintEngine extends Phobject {
return $this;
}
- final private function isRelevantMessage(ArcanistLintMessage $message) {
+ private function isRelevantMessage(ArcanistLintMessage $message) {
// When a user runs "arc lint", we default to raising only warnings on
// lines they have changed (errors are still raised anywhere in the
// file). The list of $changed lines may be null, to indicate that the
diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php
index afb8cbe3..85a94453 100644
--- a/src/lint/linter/ArcanistLinter.php
+++ b/src/lint/linter/ArcanistLinter.php
@@ -291,7 +291,7 @@ abstract class ArcanistLinter extends Phobject {
* @param list<string>
* @return list<string>
*/
- final private function filterPaths(array $paths) {
+ private function filterPaths(array $paths) {
$engine = $this->getEngine();
$keep = array();
diff --git a/src/unit/engine/phutil/PhutilTestCase.php b/src/unit/engine/phutil/PhutilTestCase.php
index fddb0681..efa946d9 100644
--- a/src/unit/engine/phutil/PhutilTestCase.php
+++ b/src/unit/engine/phutil/PhutilTestCase.php
@@ -408,7 +408,7 @@ abstract class PhutilTestCase extends Phobject {
*
* @task internal
*/
- final private function failTest($reason) {
+ private function failTest($reason) {
$this->resultTest(ArcanistUnitTestResult::RESULT_FAIL, $reason);
}
@@ -421,7 +421,7 @@ abstract class PhutilTestCase extends Phobject {
*
* @task internal
*/
- final private function passTest($reason) {
+ private function passTest($reason) {
$this->resultTest(ArcanistUnitTestResult::RESULT_PASS, $reason);
}
@@ -433,12 +433,12 @@ abstract class PhutilTestCase extends Phobject {
* @return void
* @task internal
*/
- final private function skipTest($reason) {
+ private function skipTest($reason) {
$this->resultTest(ArcanistUnitTestResult::RESULT_SKIP, $reason);
}
- final private function resultTest($test_result, $reason) {
+ private function resultTest($test_result, $reason) {
$coverage = $this->endCoverage();
$result = new ArcanistUnitTestResult();
@@ -553,7 +553,7 @@ abstract class PhutilTestCase extends Phobject {
/**
* @phutil-external-symbol function xdebug_start_code_coverage
*/
- final private function beginCoverage() {
+ private function beginCoverage() {
if (!$this->enableCoverage) {
return;
}
@@ -566,7 +566,7 @@ abstract class PhutilTestCase extends Phobject {
* @phutil-external-symbol function xdebug_get_code_coverage
* @phutil-external-symbol function xdebug_stop_code_coverage
*/
- final private function endCoverage() {
+ private function endCoverage() {
if (!$this->enableCoverage) {
return;
}
@@ -618,7 +618,7 @@ abstract class PhutilTestCase extends Phobject {
return $coverage;
}
- final private function assertCoverageAvailable() {
+ private function assertCoverageAvailable() {
if (!function_exists('xdebug_start_code_coverage')) {
throw new Exception(
pht("You've enabled code coverage but XDebug is not installed."));
@@ -675,7 +675,7 @@ abstract class PhutilTestCase extends Phobject {
*
* @return map
*/
- final private static function getCallerInfo() {
+ private static function getCallerInfo() {
$callee = array();
$caller = array();
$seen = false;
diff --git a/src/utils/AbstractDirectedGraph.php b/src/utils/AbstractDirectedGraph.php
index 509f0f5f..24611c14 100644
--- a/src/utils/AbstractDirectedGraph.php
+++ b/src/utils/AbstractDirectedGraph.php
@@ -318,7 +318,7 @@ abstract class AbstractDirectedGraph extends Phobject {
* which cycle.
* @task cycle
*/
- final private function performCycleDetection($node, array $visited) {
+ private function performCycleDetection($node, array $visited) {
$visited[$node] = true;
foreach ($this->knownNodes[$node] as $edge) {
if (isset($visited[$edge])) {
diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php
index edad2d8d..9d6adb91 100644
--- a/src/workflow/ArcanistWorkflow.php
+++ b/src/workflow/ArcanistWorkflow.php
@@ -728,7 +728,7 @@ abstract class ArcanistWorkflow extends Phobject {
return $this->workingDirectory;
}
- final private function setParentWorkflow($parent_workflow) {
+ private function setParentWorkflow($parent_workflow) {
$this->parentWorkflow = $parent_workflow;
return $this;
}
@@ -1377,7 +1377,7 @@ abstract class ArcanistWorkflow extends Phobject {
));
}
- final private function loadBundleFromConduit(
+ private function loadBundleFromConduit(
ConduitClient $conduit,
$params) {
--
2.29.2
From f78c184a6025c05de4c3a0e2603a834d074e8d09 Mon Sep 17 00:00:00 2001
From: Jessica Clarke <jrtc27@jrtc27.com>
Date: Sat, 9 Jan 2021 03:44:21 +0000
Subject: [PATCH 2/3] Fix PHP 8 deprecation warning in __arcanist_init_script__
As of PHP 8, the XML entity loader is disabled by default and the
libxml_disable_entity_loader function is deprecated. Thus only call it
for versions of PHP lower than 8.
---
support/init/init-script.php | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/support/init/init-script.php b/support/init/init-script.php
index 2b742074..fdbab39a 100644
--- a/support/init/init-script.php
+++ b/support/init/init-script.php
@@ -94,7 +94,8 @@ function __arcanist_init_script__() {
)));
// Disable the insanely dangerous XML entity loader by default.
- if (function_exists('libxml_disable_entity_loader')) {
+ // PHP 8 deprecates this function and disables this by default.
+ if (function_exists('libxml_disable_entity_loader') && \PHP_VERSION_ID < 80000) {
libxml_disable_entity_loader(true);
}
--
2.29.2
From baf002304129fcfd8af8d6d1986310bd93b167ce Mon Sep 17 00:00:00 2001
From: Jessica Clarke <jrtc27@jrtc27.com>
Date: Sat, 9 Jan 2021 03:54:38 +0000
Subject: [PATCH 3/3] Fix error handler on PHP 8
PHP 7.2.0 deprecated the 5th parameter and PHP 8 removed it, so stop
using it and provide a default value to avoid erroring with:
```
Too few arguments to function PhutilErrorHandler::handleError(), 4 passed and exactly 5 expected
```
---
src/error/PhutilErrorHandler.php | 5 ++---
src/error/PhutilErrorTrap.php | 3 +--
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/error/PhutilErrorHandler.php b/src/error/PhutilErrorHandler.php
index deb6ee16..95af88b6 100644
--- a/src/error/PhutilErrorHandler.php
+++ b/src/error/PhutilErrorHandler.php
@@ -180,10 +180,10 @@ final class PhutilErrorHandler extends Phobject {
* @return void
* @task internal
*/
- public static function handleError($num, $str, $file, $line, $ctx) {
+ public static function handleError($num, $str, $file, $line, $ctx = null) {
foreach (self::$traps as $trap) {
- $trap->addError($num, $str, $file, $line, $ctx);
+ $trap->addError($num, $str, $file, $line);
}
if ((error_reporting() & $num) == 0) {
@@ -214,7 +214,6 @@ final class PhutilErrorHandler extends Phobject {
array(
'file' => $file,
'line' => $line,
- 'context' => $ctx,
'error_code' => $num,
'trace' => $trace,
));
diff --git a/src/error/PhutilErrorTrap.php b/src/error/PhutilErrorTrap.php
index 94d291d0..252a37f9 100644
--- a/src/error/PhutilErrorTrap.php
+++ b/src/error/PhutilErrorTrap.php
@@ -41,13 +41,12 @@ final class PhutilErrorTrap extends Phobject {
private $destroyed;
private $errors = array();
- public function addError($num, $str, $file, $line, $ctx) {
+ public function addError($num, $str, $file, $line) {
$this->errors[] = array(
'num' => $num,
'str' => $str,
'file' => $file,
'line' => $line,
- 'ctx' => $ctx,
);
return $this;
}
--
2.29.2
