Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect contructor change for ConfigFormBase #769

Open
bbrala opened this issue Jun 2, 2024 · 3 comments
Open

Detect contructor change for ConfigFormBase #769

bbrala opened this issue Jun 2, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@bbrala
Copy link
Contributor

bbrala commented Jun 2, 2024

Feature request

https://www.drupal.org/node/3404140
See https://drupal.slack.com/archives/C03L6441E1W/p1717267194080329?thread_ts=1717267194.080329&cid=C03L6441E1W

@bbrala bbrala added the enhancement New feature or request label Jun 2, 2024
@bbrala bbrala changed the title Detect contructor chnage for ConfigFormBase Detect contructor change for ConfigFormBase Jun 2, 2024
@Berdir
Copy link

Berdir commented Jun 2, 2024

Most constrructor changes rarely affect contrib, but this one very common as it's an base form for basically all settings forms.

A specific solution for this one is probably doable (detect parent class, check if it has a parent::__construct() without the second argument. A more generic implementation would be interesting, but I don't really see how that could be achieved without defning some kind of pattern that allows phpstan to understand that this currently-optional parameter will become required in 11.x.

@bbrala
Copy link
Contributor Author

bbrala commented Jun 2, 2024

Well there is a deprecation message in the parent contructor.

  public function __construct(
    ConfigFactoryInterface $config_factory,
    protected ?TypedConfigManagerInterface $typedConfigManager = NULL,
  ) {
    $this->setConfigFactory($config_factory);
    if ($this->typedConfigManager === NULL) {
      @trigger_error('Calling ConfigFormBase::__construct() without the $typedConfigManager argument is deprecated in drupal:10.2.0 and will be required in drupal:11.0.0. See https://www.drupal.org/node/3373502', E_USER_DEPRECATED);
      $this->typedConfigManager = \Drupal::service('config.typed');
    }

So wouldn't that be quite possible to find out? Even if not only for this one. Shouldn't that generally be possible to find our if a depraction message might be triggered if default value is used? That might be to much load to do for all constructors perhaps. But if we have this pattern and perhaps we could have a list of classes that might be targetted.

(i might say stupid things, haven't done much phpstan dev)

@ayalon
Copy link

ayalon commented Jan 22, 2025

This is important. As a maintainer I rely heavily on phpstan-drupal to find issues but this one slipped through. Would be a great feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants