Skip to content

Support checking method_exists for backward compatibility support #461

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

Closed
mglaman opened this issue Jul 20, 2022 · 10 comments
Closed

Support checking method_exists for backward compatibility support #461

mglaman opened this issue Jul 20, 2022 · 10 comments

Comments

@mglaman
Copy link
Owner

mglaman commented Jul 20, 2022

if (method_exists($this->moduleHandler, 'invokeAllWIth')) {
      $this->moduleHandler->invokeAllWith('mail', function (callable $hook, string $module) use (&$list) {
        $list[$module] = $this->moduleHandler->getName($module);
      });
    }
    else {
      foreach ($this->moduleHandler->getImplementations('mail') as $module) {
        $list[$module] = $this->moduleHandler->getName($module);
      }
    }

Let's see if the latter can not error. It currently does

@mglaman
Copy link
Owner Author

mglaman commented Jul 20, 2022

Actually, I'm not sure if this is possible since the error comes from phpstan-deprecation-rules

@mglaman
Copy link
Owner Author

mglaman commented Jul 20, 2022

Maybe there is some kind of hack that can be done with \PhpParser\NodeVisitorAbstract

@mglaman
Copy link
Owner Author

mglaman commented Jul 20, 2022

Can't be done. We need folks to use // @phpstan-ignore-next-line when providing backward compatibility support via if/else

@mglaman
Copy link
Owner Author

mglaman commented Jun 30, 2023

This is possible now that phpstan/phpstan-deprecation-rules#99 has been merged, but depends on PHPStan 11.0.

@mglaman mglaman pinned this issue Jun 30, 2023
@bbrala
Copy link
Contributor

bbrala commented Aug 5, 2023

The issue was merged and it was released into the 1.10 branch, we can continue this without waiting for 1.11 :)

@mglaman
Copy link
Owner Author

mglaman commented Aug 5, 2023

Oh it did get released? I'll queue this up

@bbrala
Copy link
Contributor

bbrala commented Sep 23, 2023

Since the pattern of the BC fixes in rector is pretty much done. Perhaps it is time to see if you can support that. Otherwise the Rector rule will need to be rewritten a little to support the ignore comment.

palantirnet/drupal-rector#250

@mglaman
Copy link
Owner Author

mglaman commented Sep 27, 2023

I'll need to find some time for this. I did an investigation and the scope provided by PHPStan is not in-depth enough to make this kind of scope decisions. We don't know about the being processed. We may need drupal-rector to add the comment :/

@bbrala
Copy link
Contributor

bbrala commented Sep 28, 2023

I feel like the the deprecated scope thingies should be able to do this. But only if the referred arguments on the deprecatedhelper class are anonimous functions. I understand that calls to other methods as arguments might not work. But anonimous functions should be doable?

@mglaman
Copy link
Owner Author

mglaman commented Feb 14, 2024

This isn't needed now that we have DeprecatedScopeResolver (#714)

@mglaman mglaman closed this as completed Feb 14, 2024
@mglaman mglaman unpinned this issue Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants