Use static analysis to detect new update functions, to reduce false positives in StagedDBUpdateValidator

I noticed that 10.0.8 to 10.0.9 actually has a change to hook_requirements that would flag has a DB update when it is not

#18 happened at #3351895-24: Add command to allow running cron updates via console and by a separate user, for defense-in-depth → .

tedbow → credited FizCS3 → .

crediting @FizCS3 for finding in testing

I dug into this a bit and I think it actually wouldn't be too tricky.

It would require us to create a node visitor (as documented) that detects functions in certain files, matching certain naming patterns, and simply compares the lists of found functions. Obviously this would only be done if nikic/php-parser were installed; otherwise, we'd fall back to hashing.

But generally I think this would be relatively simple.

@phenaproxima mentioned token_get_all(). It looks REALLY promising: https://3v4l.org/SX0rT

Any sequence like this:

T_FUNCTION
T_WHITESPACE
T_STRING

we'd need the value of T_STRING for, and … we'd have a complete list! 🥳

re #18 we should manually test that this new method would not have the false positive for the DB updates that current method has

I few comments in the MR

For manual testing:

Drupal 10.0.1 changed some code in system.install, compared to 10.0.0. (The code in question is part of hook_requirements.) No update functions were added that I can see. So this is a perfect pairing of versions to prove that this static analysis is smarter than what we previously had.

To manually test the status quo, I did the following:

composer create-project drupal/recommended-project:10.0.0 au
cd au
composer config minimum-stability alpha
composer require drush/drush:^11
composer require drupal/automatic_updates:^3
./vendor/bin/drush si -y
./vendor/bin/drush en -y automatic_updates
./vendor/bin/drush cset -y automatic_updates.settings unattended.level patch
./vendor/bin/drush auto-update

As expected, this died with the error:

 [error]  The update cannot proceed because possible database updates have been detected in the following extensions.
System
User

composer create-project drupal/recommended-project:10.0.0 au
cd au
composer config minimum-stability alpha
composer require drush/drush:^11
composer config repo.au vcs https://git.drupalcode.org/issue/automatic_updates-3253828.git
composer require drupal/automatic_updates:dev-3253828-use-static-analysis
./vendor/bin/drush si -y
./vendor/bin/drush -y en automatic_updates
./vendor/bin/drush cset -y automatic_updates.settings unattended.level patch
./vendor/bin/drush auto-update

This time, it died with the following:

 [error]  The update cannot proceed because possible database updates have been detected in the following extensions.
User

However, this makes perfect sense: running git diff 10.0.0:core/modules/user/user.post_update.php 10.0.9:core/modules/user/user.post_update.php shows me that post-update functions were added to the User module!

This is already a lot more promising.

Thanks for testing!

That looks amazing! 🤩

I am a bit curious if this is sufficiently fast for large codebases, but considering how little code is typically in *.install and *.post_update.php files, I'm not very concerned. Still, this might come back to haunt us at some point in the future in some extreme cases…

Automatically closed - issue fixed for 2 weeks with no activity.