rectorphp / rector-phpunit

Rector upgrade rules for PHPUnit
http://getrector.com
MIT License
61 stars 45 forks source link

RemoveDataProviderTestPrefixRector does not take class inheritance into account #85

Closed danflanagan8 closed 2 years ago

danflanagan8 commented 2 years ago

I noticed a problem with how RemoveDataProviderTestPrefixRector that I think is related to how I'm using inheritance in tests for a project of mine.

I have a base class for tests with a test method that uses an illegally named data provider:

 /**
   * blah blah blah
   *
   * @dataProvider testExceptionsProvider
   */
  public function testExceptions($filename, $message) {

The rector rule correctly updates the annotation to read:

  /**
   * blah blah blah
   *
   * @dataProvider exceptionsProvider
   */
  public function testExceptions($filename, $message) {

However, the annotation in the base class is the only place that the data provider name is changed.

The rector rule seems to be getting "confused" because of how I'm relying on inheritance in these test classes. Specifically:

  1. testExceptions() is only declared in the base class.
  2. testExceptionsProvider() is not declared in the base test class. It is declared in each of the test classes that extend the base class.

For the record, I am seeing this on a Drupal project that I maintain: https://www.drupal.org/project/crossword/issues/3286830

Thanks!

TomasVotruba commented 2 years ago

Thank you for your report!

We'll need an isolated failing demo link from: http://getrector.org/demo, that way we can reproduce the bug.

danflanagan8 commented 2 years ago

Hi @TomasVotruba! Thanks for the quick reply.

Here's a failing demo link: https://getrector.org/demo/637e1142-1cb1-4f9d-9a21-67776e0219ff

That tool is super cool, by the way. Cheers!

TomasVotruba commented 2 years ago

Thank you for demo link and nice words 🙏

Could you send a failing test case in a pull-request, so we have it covered in Rector? You can click "Create a Test" button at demo page.

danflanagan8 commented 2 years ago

I'm closing this issue which appears to be considered "cannot fix" or "will not fix". Thanks!

TomasVotruba commented 2 years ago

Yes, that's correct. Thank you 👍