Skip to content

Commit 8310591

Browse files
committed
Fix DifferentialDiff getFieldValuesForConduit PHP 8.1 strlen(null) errors
Summary: Fix DifferentialDiff getFieldValuesForConduit PHP 8.1 strlen(null) errors by replacing the strlen() calls with phutil_nonempty_string(). Also needed to update DifferentialDiffTestCase.php to being a PhabricatorTestCase, as it was throwing an exception prior to any code changes - ``` EXCEPTION (Exception): Trying to read configuration "policy.locked" before configuration has been initialized. ``` Fixes T15529 Test Plan: arc diff Reviewers: O1 Blessed Committers, valerio.bozzolan, avivey Reviewed By: O1 Blessed Committers, valerio.bozzolan, avivey Subscribers: avivey, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15529 Differential Revision: https://we.phorge.it/D25333
1 parent 4b3c384 commit 8310591

File tree

3 files changed

+42
-6
lines changed

3 files changed

+42
-6
lines changed

src/__phutil_library_map__.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -6565,7 +6565,7 @@
65656565
'DifferentialDiffRepositoryProjectsHeraldField' => 'DifferentialDiffHeraldField',
65666566
'DifferentialDiffSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
65676567
'DifferentialDiffSearchEngine' => 'PhabricatorApplicationSearchEngine',
6568-
'DifferentialDiffTestCase' => 'PhutilTestCase',
6568+
'DifferentialDiffTestCase' => 'PhabricatorTestCase',
65696569
'DifferentialDiffTransaction' => 'PhabricatorApplicationTransaction',
65706570
'DifferentialDiffTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
65716571
'DifferentialDiffViewController' => 'DifferentialController',

src/applications/differential/storage/DifferentialDiff.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -780,31 +780,31 @@ public function getFieldValuesForConduit() {
780780
$refs = array();
781781

782782
$branch = $this->getBranch();
783-
if (strlen($branch)) {
783+
if (phutil_nonempty_string($branch)) {
784784
$refs[] = array(
785785
'type' => 'branch',
786786
'name' => $branch,
787787
);
788788
}
789789

790790
$onto = $this->loadTargetBranch();
791-
if (strlen($onto)) {
791+
if (phutil_nonempty_string($onto)) {
792792
$refs[] = array(
793793
'type' => 'onto',
794794
'name' => $onto,
795795
);
796796
}
797797

798798
$base = $this->getSourceControlBaseRevision();
799-
if (strlen($base)) {
799+
if ($base !== null && strlen($base)) {
800800
$refs[] = array(
801801
'type' => 'base',
802802
'identifier' => $base,
803803
);
804804
}
805805

806806
$bookmark = $this->getBookmark();
807-
if (strlen($bookmark)) {
807+
if (phutil_nonempty_string($bookmark)) {
808808
$refs[] = array(
809809
'type' => 'bookmark',
810810
'name' => $bookmark,

src/applications/differential/storage/__tests__/DifferentialDiffTestCase.php

+37-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
<?php
22

3-
final class DifferentialDiffTestCase extends PhutilTestCase {
3+
final class DifferentialDiffTestCase extends PhabricatorTestCase {
4+
5+
protected function getPhabricatorTestCaseConfiguration() {
6+
return array(
7+
self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => true,
8+
);
9+
}
410

511
public function testDetectCopiedCode() {
612
$copies = $this->detectCopiesIn('lint_engine.diff');
@@ -73,5 +79,35 @@ public function testDetectSlowCopiedCode() {
7379
$this->assertTrue(true);
7480
}
7581

82+
public function testGetFieldValuesForConduit() {
83+
84+
$parser = new ArcanistDiffParser();
85+
$raw_diff = <<<EODIFF
86+
diff --git a/src b/src
87+
index 123457..bb216b1 100644
88+
--- a/src
89+
+++ b/src
90+
@@ -1,5 +1,5 @@
91+
Line a
92+
-Line b
93+
+Line 2
94+
Line c
95+
Line d
96+
Line e
97+
EODIFF;
98+
99+
$diff = DifferentialDiff::newFromRawChanges(
100+
PhabricatorUser::getOmnipotentUser(),
101+
$parser->parseDiff($raw_diff));
102+
$this->assertTrue(true);
103+
104+
$field_values = $diff->getFieldValuesForConduit();
105+
$this->assertTrue(is_array($field_values));
106+
foreach (['revisionPHID', 'authorPHID', 'repositoryPHID', 'refs']
107+
as $key) {
108+
$this->assertTrue(array_key_exists($key, $field_values));
109+
}
110+
111+
}
76112

77113
}

0 commit comments

Comments
 (0)