coding-standard
coding-standard copied to clipboard
doctrine
Add "dev" Composer keyword
As per https://getcomposer.org/doc/04-schema.md#keywords by including "static analysis" as a keyword in the composer.json file, Composer 2.4.0-RC1 and later will prompt users if the package is installed with composer require instead of composer require --dev. See https://github.com/composer/composer/pull/10960 for more info.
Should you send a PR to Composer to add the "coding standard" keyword to the list of keywords triggering that behavior? Static analysis and coding standards are not the same thing IMO
Should you send a PR to Composer to add the "coding standard" keyword to the list of keywords triggering that behavior? Static analysis and coding standards are not the same thing IMO
It's a fair observation, though one that the author of PHPCS (the GitHub repo has a "static-analysis" tag), and many PHPCS-related packages disagree with.
PHPCS does do static analysis (literally, analysis of the static code through the tokeniser), with the intent that it can then be used to fix up inconsistencies in code styles and implementations. Of course, Psalm, PHPStan and others also do static analysis with the intent to find inconsistencies in used types to prevent bugs. While the intent of these tools is different, the underlying first step of "read the raw code" is the same (albeit with different implementations), hence both being different types of "static analysis".
If you still feel uncomfortable, then adding "dev" keyword instead would achieve the same benefit, though that seems less specific to me.
These are:
dev,testing,static analysis.
Maybe we can use dev and skip semantics?
Should you send a PR to Composer to add the "coding standard" keyword to the list of keywords triggering that behavior?
I think that would be reasonable, no matter how we decide here.
PHPCS does do static analysis (literally, analysis of the static code through the tokeniser)
I agree. PHPCS belongs to the family of static analysis tools. Adding that tag would be justified.
adding "dev" keyword instead would achieve the same benefit
Also fine for me.
It's a fair observation, though one that the author of PHPCS (the GitHub repo has a "static-analysis" tag), and many PHPCS-related packages disagree
I cannot find a discussion about this in #3655 , so it's possible they merged that PR without thinking too much about this.
I agree. PHPCS belongs to the family of static analysis tools. Adding that tag would be justified.
According to https://en.wikipedia.org/wiki/Static_program_analysis , the human equivalent of a static analysis tool is ""program understanding", program comprehension, or code review". I don't think that's what phpcs rules do, or at least it's not what they should do.
There has been a similar discussion in https://github.com/doctrine/coding-standard/pull/289 and there seems a be a disagreement on this point exactly.
According to https://en.wikipedia.org/wiki/Static_program_analysis , the human equivalent of a static analysis tool is ""program understanding", program comprehension, or code review". I don't think that's what phpcs rules do, or at least it's not what they should do.
If you click on "code review", you see that one goal of code review is
improve internal code quality and maintainability (readability, uniformity, understandability, etc.)
And this is as far as I can tell what we use PHPCS for. It does not replace a code review, but it helps a human reviewer to focus on those aspects of a review that cannot be done by a robot.
That being said, I believe we're nit-picking here. The goal of this PR was to opt-in to a new composer feature. And I think we should try to achieve that. I personally don't mind whether we get there by tagging this package with "static analysis" or a different term supported by Composer, as long as we get there. 🙂
The "Allow edit by maintainers" checkbox is active, so I'll let you folks make the final decision and merge updates and the whole PR when you're ready :-)
@greg0ire The commit message Add "static analysis" Composer keyword is strange now :)
