More exceptions when viewing diffs

I prematurely closed my previous report, and can no longer reply with a followup.

After the fix D21169, the original stack trace is no longer reproduced, but the problem has moved elsewhere for some diffs.

#0 <#2> PhutilErrorHandler::handleError called at [/src/applications/differential/parser/DifferentialChangesetParser.php:1906]
#1 <#2> DifferentialChangesetParser::getRawDocumentEngineData called at [/src/applications/differential/parser/DifferentialChangesetParser.php:1717]
#2 <#2> DifferentialChangesetParser::newDocumentEngine called at [/src/applications/differential/parser/DifferentialChangesetParser.php:843]
#3 <#2> DifferentialChangesetParser::render called at [/src/applications/differential/parser/DifferentialChangesetParser.php:75]


Reproduction Instructions
Per Exceptions when viewing diffs
Probably requires PHP 7.4, as per the previous findings.

Phabricator/Arcanist Version

phabricator 4c2da0260be2 Sat, Apr 25 d05d8f655896
arcanist acf38083f7ff Sat, Apr 25 68f050bd14e0
php-fpm 7.4.4 Mar 20 2020

Thanks! I think this should be fixed by https://secure.phabricator.com/D21171, which is now in master and stable.

(I’ll upgrade to PHP 7.4.x locally, this specific notice is new as of PHP 7.4 but I’m currently on PHP 7.3.1, so it’s easier for these problems to slip thorugh.)

There seem to be three more locations where the bug appears:

  • PhutilErrorHandler::handleError called at [/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php:596]
  • PhutilErrorHandler::handleError called at [/src/applications/differential/parser/DifferentialChangesetParser.php:1329]
  • PhutilErrorHandler::handleError called at [/src/applications/differential/parser/DifferentialChangesetParser.php:1332]

The following patch fixes all the issues I can find for now:

diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php
index 18217d7394..8cee8013a6 100644
--- a/src/applications/differential/parser/DifferentialChangesetParser.php
+++ b/src/applications/differential/parser/DifferentialChangesetParser.php
@@ -1326,9 +1326,15 @@ final class DifferentialChangesetParser extends Phobject {
     $old_back = array();
     $new_back = array();
     foreach ($this->old as $ii => $old) {
+      if ($old === null) {
+        continue;
+      }
       $old_back[$old['line']] = $old['line'];
     }
     foreach ($this->new as $ii => $new) {
+      if ($new === null) {
+        continue;
+      }
       $new_back[$new['line']] = $new['line'];
     }

diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
index a7af9333ff..84e3e8a78b 100644
--- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
@@ -593,7 +593,7 @@ final class DifferentialChangesetTwoUpRenderer

       $map = array();
       foreach ($new as $offset => $new_line) {
-        if ($new_line['line'] === null) {
+        if ($new_line === null || $new_line['line'] === null) {
           continue;
         }
         $map[$new_line['line']] = $offset;

I upgraded to PHP 7.4 locally (see also: https://secure.phabricator.com/T13232).

I reproduced the latest batch of three issues; they should be fixed by https://secure.phabricator.com/D21172. I also fixed one issue with the Asana integration in that change.

I identified and fixed one other issue in https://secure.phabricator.com/D21173 with preg_match_all() using a pattern in the form a|(b) and PREG_OFFSET_CAPTURE. See also https://secure.phabricator.com/T13518.

I’m not immediately hitting any more issues after poking around a few revisions, but let me know if anything else crops up.