Problem/Motivation

The logic makes assumptions about the Drupal 8 to 9 process. Let's make it work fine on Drupal 9 so we can run it at least through Drush for meaningful results.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

Gábor Hojtsy created an issue. See original summary.

gábor hojtsy’s picture

Status: Active » Needs review
StatusFileSize
new4.84 KB

Starting with the detection code since this would make the code useful in CI environments at least to start find Drupal 9 to 10 deprecated API use.

gábor hojtsy’s picture

There are still lots of UI elements, the environment check, the table formatting, etc. that are Drupal 8 to 9 specific.

Status: Needs review » Needs work

The last submitted patch, 2: 3201156.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

gábor hojtsy’s picture

Title: Make Upgrade Status UI make sense on Drupal 9 » Make Upgrade Status logic make sense on Drupal 9
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new4.42 KB
new2.69 KB

Realized we can make an interim step and fix the logic first with tests and have useful Drush results. Fixing one problem in the logic I used and decided to move the major version logic to the project collector where the condition checking is also housed.

andypost’s picture

+++ b/src/DeprecationAnalyzer.php
@@ -574,7 +568,7 @@ final class DeprecationAnalyzer {
-    elseif (preg_match('!Deprecated (in|as of) [Dd]rupal[ :](8.\d)!', $error, $version_found)) {
+    elseif (preg_match('!Deprecated (in|as of) [Dd]rupal[ :](\d.\d)!', $error, $version_found)) {

I think it could use 10.x compatible preg (I mean \d+.\d)

gábor hojtsy’s picture

StatusFileSize
new10.57 KB
new13.79 KB

Good find @andypost. Resolving #6 here and adding tests for things deprecated in 8 for 9 (existing), 8 for 10 (new), 9 for 10 (new) and 9 for 11 (new). Fixing logic for detection of category for future removals and adding tests coverage for these.

I think the combination of these should allow to run the module through drush on Drupal 9 and get the right results. At least in checkstyle output :D

gábor hojtsy’s picture

StatusFileSize
new13.79 KB
new2.38 KB

Changing the 9 to 10 deprecation to 9.2.0 source so the 'later' category will kick in as intended (given 9.0 is still supported).

andypost’s picture

+++ b/src/DeprecationAnalyzer.php
@@ -589,7 +583,7 @@ final class DeprecationAnalyzer {
-        if (version_compare($version, self::CORE_MINOR_OLDEST_SUPPORTED) <= 0) {
+        if (version_compare($version, ProjectCollector::getOldestSupportedMinor()) <= 0) {

+++ b/src/ProjectCollector.php
@@ -512,4 +512,30 @@ class ProjectCollector {
+    switch ($major) {
+      case 8:
+        return '8.9';
+      case 9:
+        return '9.0';

I think here should be default because otherwise condition will use NULL and could lead to false positives

gábor hojtsy’s picture

When would the major version of Drupal not be known? That would lead to various other fails in the tests :D

  • Gábor Hojtsy committed 4a5e158 on 8.x-3.x
    Issue #3201156 by Gábor Hojtsy, andypost: Make Upgrade Status logic make...
andypost’s picture

+++ b/src/ProjectCollector.php
@@ -512,4 +512,30 @@ class ProjectCollector {
+   * @return string
+   *   Oldest supported core version number.

Then at least it should declare that for 10 core it will return NULL instead of string which expected as second argument for version_compare and will bring issues for php 8

andypost’s picture

StatusFileSize
new69.26 KB
new894 bytes

Here's a small fix for UI using new method and fix to return empty string instead of null

andypost’s picture

Used to run on custom project and one of modules showed warning but probably it's unrelated because it used to fail before

Internal error: Interface PHPUnit\Framework\Test does not exist Run PHPStan with --debug option and post the stack trace to: https://github.com/phpstan/phpstan/issues/new?template=Bug_report.md

Debug option brings
PHP Warning: file_get_contents(): Filename cannot be empty in /var/www/html/web/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php on line 43 Warning: file_get_contents(): Filename cannot be empty in /var/www/html/web/core/tests/Drupal/TestTools/PhpUnitCompatibility/PhpUnit8/ClassWriter.php on line 43 PHP Fatal error: Uncaught ReflectionException: Interface PHPUnit\Framework\Test does not exist in /var/www/html/vendor/mglaman/phpstan-drupal/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php:50 Stack trace: #0 /var/www/html/vendor/mglaman/phpstan-drupal/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php(50): ReflectionClass->implementsInterface() #1 phar:///var/www/html/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php(69): PHPStan\Rules\Drupal\GlobalDrupalDependencyInjectionRule->processNode() #2 phar:///var/www/html/vendor/phpstan/phpstan/phpstan.phar/src/Node/ClassStatementsGatherer.php(91): PHPStan\Analyser\FileAnalyser->PHPStan\Analyser\{closure}() #3 phar:///var/www/html/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(388): PHPStan\Node\ClassStatementsGatherer->__invoke() #4 phar:///var/www/html/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(1762): PHPStan\Analyser\NodeScopeResolver::PHPStan\Analyser\{closure}() #5 phar:///var/www/html/vendor/p in /var/www/html/vendor/mglaman/phpstan-drupal/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php on line 50 Fatal error: Uncaught ReflectionException: Interface PHPUnit\Framework\Test does not exist in /var/www/html/vendor/mglaman/phpstan-drupal/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php:50 Stack trace: #0 /var/www/html/vendor/mglaman/phpstan-drupal/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php(50): ReflectionClass->implementsInterface() #1 phar:///var/www/html/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php(69): PHPStan\Rules\Drupal\GlobalDrupalDependencyInjectionRule->processNode() #2 phar:///var/www/html/vendor/phpstan/phpstan/phpstan.phar/src/Node/ClassStatementsGatherer.php(91): PHPStan\Analyser\FileAnalyser->PHPStan\Analyser\{closure}() #3 phar:///var/www/html/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(388): PHPStan\Node\ClassStatementsGatherer->__invoke() #4 phar:///var/www/html/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php(1762): PHPStan\Analyser\NodeScopeResolver::PHPStan\Analyser\{closure}() #5 phar:///var/www/html/vendor/p in /var/www/html/vendor/mglaman/phpstan-drupal/src/Rules/Drupal/GlobalDrupalDependencyInjectionRule.php on line 50

After installing phpunit/phpunit I'm getting

PHPStan command failed:

/var/www/html/vendor/bin/phpstan analyse --debug --error-format=json -c /tmp/upgrade_status/deprecation_testing.neon /var/www/html/web/modules/custom/custom_menu 2> /tmp/upgrade_status/phpstan_error_output

Command output:

/var/www/html/web/modules/custom/custom_menu/custom_menu.install/var/www/html/web/modules/custom/custom_menu/src/Plugin/Menu/CurrentNodeMenuLink.php{"totals":{"errors":0,"file_errors":0},"files":[],"errors":[]}
gábor hojtsy’s picture

Status: Needs review » Fixed

@andypost: you did not post the error output in the last example, only the command output. What was in /tmp/upgrade_status/phpstan_error_output? That is also logged into dblog unless you have dblog disabled.

Re the UI updates, I opened #3201336: Update Upgrade Status UI for Drupal 9 to 10 support and going to continue there.

Status: Fixed » Closed (fixed)

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