search

staabm / phpstan-todo-by

Todo comments with expiration
https://staabm.github.io/2023/12/17/phpstan-todo-by-published.html
MIT License
173 stars 4 forks source link

Expire comment by composer dependency package version #32

Closed staabm closed 8 months ago

staabm commented 8 months ago

closes https://github.com/staabm/phpstan-todo-by/issues/31

staabm commented 8 months ago

@nikophil wdyt? feedback welcome

staabm commented 8 months ago

@m29corey maybe you can run this against some real world projects to see possible perf impact?

nikophil commented 8 months ago

I've also added a way to test against php version. I've done my rule few month ago, but I think if I've done this because InstalledVersions::satisfies() or InstalledVersions::isInstalled() do not recognize php as a dependency:

if ('php' === $dependency) {
    if (\version_compare(\PHP_VERSION, $constraint, '>=')) {
$errors[] = \sprintf('dependency "%s" satisfies constraint "%s"%s', $dependency, $constraint, $message ? ": {$message}." : '.');
    }

    continue;
}
nikophil commented 8 months ago

@VincentLanglet I don't think this PR satisfies your need: it only looks on what is currently required by composer.lock but does not read composer.json which is what you'd need

staabm commented 8 months ago

I've also added a way to test against php version. I've done my rule few month ago, but I think if I've done this because InstalledVersions::satisfies() or InstalledVersions::isInstalled() do not recognize php as a dependency:

if ('php' === $dependency) {
    if (\version_compare(\PHP_VERSION, $constraint, '>=')) {
$errors[] = \sprintf('dependency "%s" satisfies constraint "%s"%s', $dependency, $constraint, $message ? ": {$message}." : '.');
    }

    continue;
}

I have similar stuff locally. what I don't like is, that this code depends on the runtime php version. I would want to compare the composer.json php requirement against the comment constraint.. I haven't found a way to achieve that though :)

staabm commented 8 months ago

Does this PR could satisfy this need ?

@VincentLanglet I think it kind of satiesfies it, but in an unexpected way. the result of the check depends on your composer.lock, which means it compares against the constraints which have been installed with composer install.

this means as long as you run the check against a composer.lock which was generated with the minimum supported php version of your package, it should work