#[DisableReturnValueGenerationForTestDoubles] attribute for disabling return value generation for test doubles created using createMock(), createMockForIntersectionOfInterfaces(), createPartialMock(), createStub(), and createStubForIntersectionOfInterfaces()

[sebastianbergmann]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (97fd4b9) 89.92% compared to head (6c53031) 89.93%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5696      +/-   ##
============================================
+ Coverage     89.92%   89.93%   +0.01%     
- Complexity     6450     6457       +7     
============================================
  Files           683      684       +1     
  Lines         19580    19610      +30     
============================================
+ Hits          17607    17637      +30     
  Misses         1973     1973              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ben-challis]

Thanks for this, looks great. Am I correct that this would not work for an abstract parent class? E.g.

#[DisableReturnValueGenerationForTestDoubles]
abstract class DisabledReturnVallueGenerationForTestDoublesTestCase extends TestCase {}

class MyTestCase extends DisabledReturnVallueGenerationForTestDoublesTestCase {}

From looking at how PHPUnit\Metadata\Parser\AttributeParser::forClass works I don't believe it will (https://3v4l.org/7uD4b being a cut down simplified proof of that I believe).

It makes sense why you aren't iterating through parent classes to parse attributes from them due to how complicated it would be to have say a parent class setting CoversClass and a child class also defining it, do you override? Do you merge them all together? An absolute messy time.

In the simplest sense it would be great if:

    private static function generateReturnValuesForTestDoubles(): bool
    {
        return MetadataRegistry::parser()->forClass(static::class)->isDisableReturnValueGenerationForTestDoubles()->isEmpty();
    }

Would also source from the phpunit config file or allow it to be overridden to just return true so a parent class can just enforce? Or any similar approach to that which could enable a wider enabling than on the concrete test case level.

Thanks again!

[sebastianbergmann]

Am I correct that this would not work for an abstract parent class?

Correct: attributes are not inherited, meaning only class-level attributes on a test class have an effect on the test methods of that test class. While this may be considered inconvenient in some cases, this approach allows for a consistent use of attributes and saves us from messy logic which attributes from a parent class may or may not be overwritten (and how) in a child class.

Would also source from the phpunit config file or allow it to be overridden to just return true so a parent class can just enforce?

Strong no: I neither want a global configuration option for this nor do I want generateReturnValuesForTestDoubles() to be part of the public API and/or overwritable.

[ben-challis]

Ok, we'll go the route of a Rector rule to add/ensure this is on all concrete test cases, I get why you want to keep complexity down and your API surface narrow. Again, thanks for implementing this!