Problem/Motivation

Detected in #3178534: Start running PHPStan on Drupal core (level 0) with PHPStan:

 ------ ------------------------------------------------------- 
  Line   ProxyBuilder/ProxyBuilder.php                          
 ------ ------------------------------------------------------- 
  169    Variable $proxy_class_shortname might not be defined.  
 ------ ------------------------------------------------------- 

It turns our that the second parameter $proxy_class_name is never used and buggy. If you pass a proxy class name "Foo" then you get the exception "ReflectionException: Class Foo does not exist". We cannot use reflection on a class that is about to be generated.

Steps to reproduce

Run PHPStan level 1 on Drupal core.

Proposed resolution

Remove the second parameter of ProxyBuilder::build() completely because it is unused and broken. This is a small API change, but should not affect callers (you can pass more parameters than necessary in PHP).

Remaining tasks

Confirm approach.

API changes

A method parameter is removed. ProxyBuilder is a utility class, not sure if anybody would extend that class and would have to update their method definition?

Issue fork drupal-3186626

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

klausi created an issue. See original summary.

klausi’s picture

Status: Active » Needs review

Merge request created.

andypost’s picture

Status: Needs review » Needs work
Issue tags: +Needs change record

It needs to deprecate this argument to be able to catch its usage in contrib/custom code

longwave’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs change record
Related issues: +#2408371: Proxies of module interfaces don't work

This was added in #2408371: Proxies of module interfaces don't work but I don't see any code that actually uses the second argument.

Even if you do pass a valid class name in as the second argument then there is still an undefined variable $proxy_class_shortname that is required to generate valid code, as PHPstan has picked up on. Therefore I don't see how contrib/custom code can be using this without reporting this bug. As I don't see how this can ever have worked I don't think we need a change record and can just fix this in place by removing this unused and broken feature.

  • catch committed 221ba65 on 9.2.x
    Issue #3186626 by klausi, longwave: Second parameter of ProxyBuilder::...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Agreed that we can remove this without issuing a deprecation notice since there's already a PHP error showing it'll be broken, it's also not runtime code so can only effect module development, not any live sites.

Committed 221ba65 and pushed to 9.2.x. Thanks!

Status: Fixed » Closed (fixed)

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