Page MenuHomePhabricator

phan-taint-check-plugin: Undefined constant 'ast\AST_LIST'
Open, Stalled, MediumPublic

Description

This is breaking Parsoid's CI: (cf https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/628943)

> phan -p --allow-polyfill-parser --config-file=.phan/standalone.php --long-progress-bar
17:06:48 Parsing files...
[...]
17:06:57 Analyzing methods...
17:06:59 ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 722MB
17:06:59 Analyzing files...
17:06:59 ERROR: Error: Undefined constant 'ast\AST_LIST' in /workspace/src/services/parsoid/vendor/mediawiki/phan-taint-check-plugin/src/TaintednessBaseVisitor.php:972
17:06:59 Stack trace:
17:06:59 #0 /workspace/src/services/parsoid/vendor/mediawiki/phan-taint-check-plugin/src/TaintednessVisitor.php(527): TaintednessVisitor->getPhanObjsForNode(Object(ast\Node))
17:06:59 #1 /workspace/src/services/parsoid/vendor/mediawiki/phan-taint-check-plugin/src/MWVisitor.php(834): TaintednessVisitor->visitAssign(Object(ast\Node))
17:06:59 #2 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Plugin/ConfigPluginSet.php(1165): MWVisitor->visitAssign(Object(ast\Node))
17:06:59 #3 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Plugin/ConfigPluginSet.php(1093): MWVisitor::Phan\Plugin\{closure}(Object(Phan\CodeBase), Object(Phan\Language\Context), Object(ast\Node), Array)
17:06:59 #4 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Plugin/ConfigPluginSet.php(298): Phan\Plugin\ConfigPluginSet::Phan\Plugin\{closure}(Object(Phan\CodeBase), Object(Phan\Language\Context), Object(ast\Node), Array)
17:06:59 #5 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(2497): Phan\Plugin\ConfigPluginSet->postAnalyzeNode(Object(Phan\CodeBase), Object(Phan\Language\Context), Object(ast\Node), Array)
17:06:59 #6 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(544): Phan\BlockAnalysisVisitor->postOrderAnalyze(Object(Phan\Language\Context), Object(ast\Node))
17:06:59 #7 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/AST/Visitor/KindVisitorImplementation.php(69): Phan\BlockAnalysisVisitor->visit(Object(ast\Node))
17:06:59 #8 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\AST\Visitor\KindVisitorImplementation->visitAssign(Object(ast\Node))
17:06:59 #9 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(233): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #10 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\BlockAnalysisVisitor->visitStmtList(Object(ast\Node))
17:06:59 #11 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(2378): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #12 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\BlockAnalysisVisitor->visitMethod(Object(ast\Node))
17:06:59 #13 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(233): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #14 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\BlockAnalysisVisitor->visitStmtList(Object(ast\Node))
17:06:59 #15 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(1337): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #16 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(2346): Phan\BlockAnalysisVisitor->visitClosedContext(Object(ast\Node))
17:06:59 #17 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\BlockAnalysisVisitor->visitClass(Object(ast\Node))
17:06:59 #18 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(233): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #19 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/AST/Visitor/KindVisitorImplementation.php(35): Phan\BlockAnalysisVisitor->visitStmtList(Object(ast\Node))
17:06:59 #20 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Analysis.php(560): Phan\AST\Visitor\KindVisitorImplementation->__invoke(Object(ast\Node))
17:06:59 #21 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Phan.php(526): Phan\Analysis::analyzeFile(Object(Phan\CodeBase), 'src/Config/Api/...', NULL, NULL)
17:06:59 #22 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Phan.php(583): Phan\Phan::Phan\{closure}(0, 'src/Config/Api/...', 232)
17:06:59 #23 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Phan.php(376): Phan\Phan::finishAnalyzingRemainingStatements(Object(Phan\CodeBase), NULL, Array, Array)
17:06:59 #24 /workspace/src/services/parsoid/vendor/phan/phan/src/phan.php(36): Phan\Phan::analyzeFileList(Object(Phan\CodeBase), Object(Closure))
17:06:59 #25 /workspace/src/services/parsoid/vendor/phan/phan/phan(9): require_once('/workspace/src/...')
17:06:59 #26 {main}
17:06:59 (Phan 2.6.1 crashed due to an uncaught Throwable when parsing/analyzing 'src/Config/Api/ApiHelper.php')
17:06:59 Script phan -p --allow-polyfill-parser --config-file=.phan/standalone.php --long-progress-bar handling the phan event returned with error code 1
17:06:59 Script @phan was called via test
[...]

We keep running into variants of this bug, but this started appearing just today (a new version of phan-taint-check-plugin?). There have been no changes to src/Config/Api/ApiHelper.php since May 21, 2020 and no changes to composer.json since July 23, 2020.

Event Timeline

cscott created this task.Sep 21 2020, 9:45 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 21 2020, 9:45 PM

Sounds like the base image was changed and no longer has ext-ast?

First seen as T251346, perhaps the comeback is related to T263313. Using that job would fix this, as it only happens when ext-ast is not installed (not just avoided with --force-polyfill-parser)

cscott reopened this task as Open.Sep 21 2020, 9:47 PM
cscott updated the task description. (Show Details)
Jdforrester-WMF triaged this task as Unbreak Now! priority.Sep 21 2020, 9:52 PM

Unable to merge production code -> UBN.

[17:42:21] <subbu> cscott, yes .. i saw a phab email over the weekend .. hashar iirc.
[17:43:50] <subbu> https://phabricator.wikimedia.org/T227352#6474369
[17:45:30] <James_F> cscott: Ah, yes.
[17:45:44] <cscott> https://phabricator.wikimedia.org/T263500 <- parsoid CI breaking
[17:46:26] <wikibugs> (CR) jerkins-bot: [V: -1] WIP: phan: Remove --allow-polyfill-parser [services/parsoid] - https://gerrit.wikimedia.org/r/628952 (owner: C. Scott Ananian)
[17:46:39] <cscott> i removed the --allow-polyfill-parser option in https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/628952 we'll see if that's enough to fix it
[17:47:54] <James_F> It won't, it'll just fail earlier.
[17:49:03] <cscott> James_F: gr8
[17:49:42] <James_F> The polyfill isn't complete, for some reason.
[17:49:46] -*- James_F rolls his eyes.
[17:50:09] <cscott> it's actually failing when running `composer test` AFAICT, but only in the quibble-noselenium case, not in the parsoidsvv-composer-package jobs, etc.
[17:50:30] <James_F> Yeah, the new quibble job is probably mis-built by hashar.
[17:50:43] <James_F> Mark the task as UBN and make it his problem when he wakes up tomorrow?
[17:51:40] <cscott> James_F: Daimona already closed this as a dup of T262451
[17:51:41] <stashbot> T262451: Release bugfix for ast\AST_LIST in phan-taint-check-plugin to unstuck libup updates on some repos - https://phabricator.wikimedia.org/T262451
[17:51:51] <James_F> cscott: Tough.
[17:52:04] <cscott> which is two weeks old
[17:52:38] <James_F> Yeah, that's a minor "nice to have". This is UBN.
cscott assigned this task to hashar.Sep 21 2020, 9:53 PM

Unable to merge production code -> UBN.

Ah, if that's the case yes, and sorry for closing as dupe. I'd still like to investigate the feasibility of T262451 so we won't have this bug return in another couple of months. However, yes, the quick fix is to add php-ast to the image in the meanwhile.

There are two root causes:


On Monday I have upgraded Quibble on CI with a logic change has to when we run composer test for the repository that triggered the job. Previously we ran composer test when the repository was either an extension or a skin (as determined by its name). Given Parsoid is not under mediawiki/extensions nor under mediawiki/skins, we never ran composer test for it!

In Quibble 0.0.45 the logic has been changed to anything NOT being mediawiki/core bd7c3c94a93a52a7840bbee5cb88bfde90373ab2:

quibble.cmd.py
- if quibble.util.isExtOrSkin(zuul_project):
+ if not is_core:
    run_composer = 'composer-test' in stages

Parsoid comes with mediawiki/phan-taint-check-plugin3.0.2, the next minor version includes a commit named Remove reference to AST_LIST ( 4364065f2be876c69515f195168cd4fb8c11f698 ).

@Daimona wrote a very nice commit message with all the references pointing out \ast\AST_LIST has been removed from PHP and from Phan as of of version 0.8.4 (Parsoid has 2.6.1). That got dropped in phan-taint-check-plugin3.0.3 T251346

The plugin is installed from mediawiki/mediawiki-phan-config. It has an unreleased commit to bump phan-taint-check-plugin 918be7aee617e5b3223548343a287395a3066c70 and refers again to T251346: ERROR: Error: Undefined constant 'ast\AST_LIST' .


What is needed?

  • cut a new version of mediawiki/mediawiki-phan-config
  • upgrade it in Parsoid composer.json
  • Determine Quibble behavior related to extra repositories such as mediawiki/services/parsoid or mediawiki/tools/api-testing. I guess it should never run composer test or npm test which for those repositories should be handled by standalone jobs instead of by Quibble. For Parsoid that is parsoidsvc-composer-package-php7*-docker jobs.

Determine what the logic should be in Quibble when handling mediawiki/services/parsoid, probably it should just be determined as not core.

Change 629057 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/quibble@master] tests: plan for mediawiki/services/parsoid

https://gerrit.wikimedia.org/r/629057

TLDR, the new Quibble 0.0.45 now runs those extra steps:

  • Extension and skin tests: composer, npm
  • PHPUnit default standalone suite on services/parsoid

Change 629060 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/tools/phan@master] Release 0.10.3

https://gerrit.wikimedia.org/r/629060

Change 629135 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/services/parsoid@master] build: update mediawiki-phan-config to 0.10.3

https://gerrit.wikimedia.org/r/629135

status

The workaround is to upgrade mediawiki-phan-config to 0.10.3 which is https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/629135
That is pending a git tag in the mediawiki/tools/phan repository and packagist.org to catch up on the tag to make it available.

Thanks @Daimona for the assistance!

I have send a patch for Quibble in order to no more run composer test and npm test for repositories that are neither an extension or a skin. That will definitely address the issue but it needs review, a new Quibble release and an update of all jobs. So that is a bit longer to achieve.

Change 629135 merged by C. Scott Ananian:
[mediawiki/services/parsoid@master] build: update mediawiki-phan-config to 0.10.3 and fix TMPDIR use

https://gerrit.wikimedia.org/r/629135

hashar changed the task status from Open to Stalled.Sep 22 2020, 9:08 PM
hashar lowered the priority of this task from Unbreak Now! to Medium.

No more an UBN since CI lets patches pass now. It is essentially fixed but I am keeping this task around to not forget to get Quibble updated. Marking as stalled meanwhile.

Change 629057 merged by jenkins-bot:
[integration/quibble@master] Only run linters for extensions/skins

https://gerrit.wikimedia.org/r/629057

Change 643953 had a related patch set uploaded (by Reedy; owner: Hashar):
[mediawiki/services/parsoid@REL1_35] build: update mediawiki-phan-config to 0.10.3 and fix TMPDIR use

https://gerrit.wikimedia.org/r/643953

Change 643953 merged by Legoktm:
[mediawiki/services/parsoid@REL1_35] build: update mediawiki-phan-config to 0.10.3 and fix TMPDIR use

https://gerrit.wikimedia.org/r/643953

Change 654804 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/quibble@master] Release Quibble 0.0.46

https://gerrit.wikimedia.org/r/654804

Change 654804 merged by jenkins-bot:
[integration/quibble@master] Release Quibble 0.0.46

https://gerrit.wikimedia.org/r/654804