query: re code coverage support

[SignpostMarv]

Is it feasible for this plugin to parse phpunit coverage files to prevent PossiblyUnusedMethod when calling psalm --find-dead-code from being flagged up if the method is present as being covered in the coverage file?

[weirdan]

Are you referring to methods in test case classes?

[SignpostMarv]

pretty much- it's quite common that I'll see data providers & test cases pop up, although it'll also crop up for fixtures & utility classes that're implemented specifically for testing.

I'm specifically suggesting using coverage because a test method can be skipped if the data provider ends up being empty & if that test method has any dependants with otherwise correct data providers then those would still need to be set as PossiblyUnusedMethod. unless you think it'd be "better"to suppress all PossiblyUnusedMethod for test & data provider methods & introduce a new issue type for test case classes.

[weirdan]

unless you think it'd be "better"to suppress all PossiblyUnusedMethod for test & data provider methods

Yep, that was my idea (as well as automatically suppressing MissingConstructor). The problem with using coverage data, as I see it, is that it's almost always stale. I tend to only ever run phpunit with coverage enabled on travis, and then only for a single environment, as it's extremely slow.

[SignpostMarv]

The problem with using coverage data, as I see it, is that it's almost always stale.

compare ~filemod time of baseline to~ filemod time of coverage file ~,~ w/ psalm.xml config option for overriding freshness ?

edit: comparing to filemod time of baseline is a terrible idea.

[weirdan]

Well, that looks feasible. But then, it sounds pretty generic, so why limit it to tests only? Could be a plugin of its own.

[SignpostMarv]

I tend to only ever run phpunit with coverage enabled on travis, and then only for a single environment, as it's extremely slow.

separate baseline & config files for the environment that runs phpunit ?

[SignpostMarv]

so why limit it to tests only

it depends on if you view this plugin as "stubs out phpunit only" or "makes psalm work nicely with phpunit".

[SignpostMarv]

"makes psalm work nicely with phpunit".

i.e. "psalm thinks this method isn't called but psalm/plugin-phpunit knows better"

[SignpostMarv]

Could be a plugin of its own.

ah, I get what you mean now- it's more than just phpunit generates coverage files :P

[weirdan]

ah, I get what you mean now- it's more than just phpunit generates coverage files :P

Not just that. It may be used to clean up some false-positives from the system-under-test code as well. In fact, it may actually be only useful to do that, as test code is likely to be excluded from code coverage (with xdebug_set_filter or otherwise by PHPUnit itself). I know it's the case for psalm code coverage runs.

[SignpostMarv]

I'm insufficiently familiar with how code coverage works to create such a plugin from scratch :s

[weirdan]

as well as automatically suppressing MissingConstructor

Implemented that in #14

[weirdan]

@SignpostMarv can you provide isolated, short and concise test showing false-positive *UnusedMethod? Ideally something I can add in a Gherkin test like here: https://github.com/psalm/phpunit-psalm-plugin/blob/master/tests/acceptance/TestCase.feature

[SignpostMarv]

@weirdan smallest testcase: https://github.com/SignpostMarv/psalm-phpunit-psalm-plugin

fails with

ERROR: PossiblyUnusedMethod - Tests\FooTest.php:11:5 - Cannot find public calls to method FooTest::testThing
public function testThing() : void

[SignpostMarv]

@weirdan the fork takes care of the simple test case- i'm just working over the daft-object fork & master branch to identify how many of these errors are resolved but it's looking good thus far :)

[SignpostMarv]

~@weirdan ran into an issue that doesn't occur with master branch code: https://github.com/SignpostMarv/daft-object/commit/3732d8ed12d1202ca6d259c4b1e5d49ec67eaf21~ @weirdan fixed it

[SignpostMarv]

practical example of code that could use coverage support regarding "unused methods": https://github.com/SignpostMarv/Daft-Schema.org

The manually created test cases were getting cumbersome to write, so I had a generator generate test cases for me. The coverage shows that all the setters & getters are being called, but this plugin can't flag the methods up as used because very few of them are manually invoked.

Not sure what the xpath would be to scan a clover file, but the css-equiv would be class[Foo\Bar] ~ line[type="method"][name="Baz"]:not([count="0"])

[weirdan]

I still think this would be better done as a separate plugin. You would need to hook into, say, AfterCodebasePopulated, read the coverage xml file and then mark methods as used with methodExists() call.