ProxyBuilder generates incorrect @see reference

Closed #3120332: Do not allow @see to reference a namespace as a duplicate. Changing to documentation component because this is API documentation and updating tags.

In that issue longwave suggested

Can we have a rule that @see beginning with a backslash must reference an existing class or method? That would also cover #3100251: Several code comments have incorrect namespaces for classes or interfaces they reference

Adding related issue from Coding Standards project.

Adding related issue.

It was simple task to correct the @see. The harder part was generating the proxy files. I happened to test with the book module and the script was not providing any feedback. I eventually found #2539494: Provide a nice error message when trying to run the generate proxy class script on uninstalled/missing modules which informed me that the book module needs to be enabled. With that fixed I then went to generate the files. I didn't see an option to generate all the files so I wrote my own script to do that.

Looking at the changed proxy files they all have the expected change except for ./core/modules/field/src/ProxyClass/FieldUninstallValidator.php. That has a method added that was added to ./core/modules/field/src/FieldUninstallValidator.php in #2392815: Module uninstall validators are not used to validate a config import. I suspect that is out of scope for this issue.

Could the issue summary be updated? Not super clear what's being updated.

Also will this require a test case?

This is a fix to documentation, specifically @see statements and we don't test documentation changes. As for generating the proxy classes, that is done via a script, so again no tests needed.

What about the change to core/modules/field/src/ProxyClass/FieldUninstallValidator.php

@smustgrave, as the reviewer your opinion is valuable. What do you think should happen?

More asking. This is a documentation change but the one file is adding a new method

Dropping my 0.02 EUR...

Changes in phpdoc are fine/excellent.
The new method that was missed in #2392815: Module uninstall validators are not used to validate a config import should be added, but the fact that it was missed and that @quietone needed to make a script to recreate all proxy classes shows IMHO that we're (at least) missing test-coverage.

For that reason I would say, it needs a follow-up to add it and make sure we have "something" that makes sure that this doesn't happen in the future. Especially the latter justifying the admin-overhead of a follow-up

Thanks @Spokje! @quietone are you in agreement?

@quietone can you please share the script to generate all the files?

@nikolay shapovalov, each file generated by core/scripts/generate-proxy-class.php has a comment with the full command used to generate this file. I simply gathered those into a bash script.

Followup is, #3503965: Add test to ensure generated proxy files are up to date

The latest run on the pull request doesn't succeed. I should be rebased. The follow-up in #24 is what resolves the questions in #21.

So I think this needs a rebase (and possibly a rerun of the script in #24), then it is good to go imho.

@borisson_ actually, that error is 'real'.

I don't know why there is a phpstan error on the file. I intended to leave a comment but clearly I forgot. I have added finding out why this is happening to the remaining steps.

The issue still needs fix however the fork has no main branch. The existing branch or new branch from 11.x are showing unwanted changes from other issues as well.

I don't understand the comment about "the fork has no main branch". I certainly see it and do on each of the branches.

I rebased the original branch and pushed.

Generated the proxy files and there is now a phpstan error.

Thank you @quietone. I've fixed the phpstan errors. The piepline seems okay now.

@sivaji_ganesh_jojodae, the file core/lib/Drupal/Core/ProxyClass/Routing/RouteBuilder.php is automatically generated and is not meant to be edited by hand. One way to find that is to read the beginning of the file. It shows how the file is generated.

I think that the last commit should be reverted. And then figure out why the ProxyBuilder is creating incorrect code.

Adding the following check prevents from settings return type to ?mixed in RouteBuilder::__get()

diff --git a/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php b/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php
index 013afe8078f..f594a28bf27 100644
--- a/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php
+++ b/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php
@@ -233,7 +233,7 @@ protected function buildMethod(\ReflectionMethod $reflection_method) {
     if ($reflection_method->hasReturnType()) {
       $signature_line .= ': ';
       $return_type = $reflection_method->getReturnType();
-      if ($return_type->allowsNull()) {
+      if ($return_type->allowsNull() && $return_type->getName() !== 'mixed') {
         $signature_line .= '?';
       }
       if (!$return_type->isBuiltin()) {

Now running php core/scripts/generate-proxy-class.php 'Drupal\Core\Routing\RouteBuilder' "core/lib/Drupal/Core" generates same code as in last comment.

May i commit the above change?

I've pushed the above change.

@sivaji_ganesh_jojodae I am reverting RouteBuilder.php to see if your code fix works.

To prove the change to PRoxyBuild is correct all the files generated by the proxy builder must be re-generated.

Not really related. The @see here are just wrong.

I have run the bash script supplied by @quietone on a local ddev site.

The script output was:

andrew@d11-core-new-web:/var/www/html$ bash proxyscript.sh 
Proxy of class Drupal\Core\Batch\BatchStorage written to core/lib/Drupal/Core/ProxyClass/Batch/BatchStorage.php
Proxy of class Drupal\Core\Config\ConfigInstaller written to core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php
Proxy of class Drupal\Core\Cron written to core/lib/Drupal/Core/ProxyClass/Cron.php
Proxy of class Drupal\Core\Extension\ModuleInstaller written to core/lib/Drupal/Core/ProxyClass/Extension/ModuleInstaller.php
Proxy of class Drupal\Core\File\MimeType\ExtensionMimeTypeGuesser written to core/lib/Drupal/Core/ProxyClass/File/MimeType/ExtensionMimeTypeGuesser.php
Proxy of class Drupal\Core\File\MimeType\MimeTypeGuesser written to core/lib/Drupal/Core/ProxyClass/File/MimeType/MimeTypeGuesser.php
Proxy of class Drupal\Core\Lock\DatabaseLockBackend written to core/lib/Drupal/Core/ProxyClass/Lock/DatabaseLockBackend.php
Proxy of class Drupal\Core\Lock\PersistentDatabaseLockBackend written to core/lib/Drupal/Core/ProxyClass/Lock/PersistentDatabaseLockBackend.php
Proxy of class Drupal\Core\Menu\MenuActiveTrail written to core/lib/Drupal/Core/ProxyClass/Menu/MenuActiveTrail.php
Proxy of class Drupal\Core\PageCache\ChainResponsePolicy written to core/lib/Drupal/Core/ProxyClass/PageCache/ChainResponsePolicy.php
Proxy of class Drupal\Core\ParamConverter\AdminPathConfigEntityConverter written to core/lib/Drupal/Core/ProxyClass/ParamConverter/AdminPathConfigEntityConverter.php
Proxy of class Drupal\Core\ParamConverter\MenuLinkPluginConverter written to core/lib/Drupal/Core/ProxyClass/ParamConverter/MenuLinkPluginConverter.php
Proxy of class Drupal\Core\Render\BareHtmlPageRenderer written to core/lib/Drupal/Core/ProxyClass/Render/BareHtmlPageRenderer.php
Proxy of class Drupal\Core\Routing\MatcherDumper written to core/lib/Drupal/Core/ProxyClass/Routing/MatcherDumper.php
Proxy of class Drupal\Core\Routing\RouteBuilder written to core/lib/Drupal/Core/ProxyClass/Routing/RouteBuilder.php

In ProxyBuilder.php line 56:
                                                            
  Class "Drupal\language\LanguageConverter" does not exist  
                                                            

generate-proxy-class <class_name> <namespace_root_path>

Proxy of class Drupal\node\ParamConverter\NodePreviewConverter written to core/modules/node/src/ProxyClass/ParamConverter/NodePreviewConverter.php
Proxy of class Drupal\views_ui\ParamConverter\ViewUIConverter written to core/modules/views_ui/src/ProxyClass/ParamConverter/ViewUIConverter.php

The only change to core files caused by running the script is to the RouteBuilder.php file as revealed by 'git status'. Running 'git diff core/lib/Drupal/Core/ProxyClass/Routing/RouteBuilder.php' output this:

index 399df911734..a26fe12e5a6 100644
--- a/core/lib/Drupal/Core/ProxyClass/Routing/RouteBuilder.php
+++ b/core/lib/Drupal/Core/ProxyClass/Routing/RouteBuilder.php
@@ -10,7 +10,7 @@
     /**
      * Provides a proxy class for \Drupal\Core\Routing\RouteBuilder.
      *
-     * @see \Drupal\Component\ProxyBuilder
+     * @see \Drupal\Component\ProxyBuilder\ProxyBuilder
      */
     class RouteBuilder implements \Drupal\Core\Routing\RouteBuilderInterface, \Drupal\Core\DestructableInterface
     {
@@ -99,6 +99,14 @@ public function destruct()
             return $this->lazyLoadItself()->destruct();
         }
 
+        /**
+         * {@inheritdoc}
+         */
+        public function __get(string $name): mixed
+        {
+            return $this->lazyLoadItself()->__get($name);
+        }

So it seems to have corrected the '@see' path. Also it has added 7 additional lines.

I switched to the 'main' branch and re-ran the script of @quietone. The regenerated RouteBuilder.php file contains the __get() function. The only difference is that the 'mixed' return type has the '?' nullable operator so is '?mixed'.

In fact there are a bunch of other files that get generated but that are untracked. For example, core/lib/Drupal/Core/ProxyClass/Config/ConfigInstaller.php.

Anyone who wants to run the script using a ddev installation of github version of drupal, a tip: remember to install the site so just make sure you can launch it and see the homepage. Then use 'ddev ssh' before running the script..

edited When running the script on the main branch it modifies two files, the RouteBuilder.php as already described but also the core/lib/Drupal/Core/ProxyClass/Menu/MenuActiveTrail.php file.

Not sure why that only happens on the main branch but not on the issue branch?