You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I made a couple of Psalm plugins these last weeks including some who fix code automatically and I noticed a handful of hindrances that I would like to remove.
The syntax to fix issues with a plugin is weird. vendor/bin/psalm --alter --plugin=vendor/vendorname/packagename/plugin.php This is weird in a lot of ways:
it involve a path (not practical)
it's not the standard way(--issues) of calling psalter so it's confusing at first
it ignores the fact that the plugin may already be enabled in xml file (and in fact, it will register and call the plugin twice if it is)
every other enabled plugin is called anyway, this means that modifications from other plugins will be applied too (I don't think there is a way for a plugin to know not to run when it wasn't called directly)
when a psalter plugin is enabled, it has to check manually that alter mode is enabled, otherwise it will perform it's analysis for nothing
There is no easy way for a user to choose what issues he wants to fix from a plugin. If the plugin want to do something about that, I guess the easier way would be to have the user tweak with issuesHandler and suppress issues it doesn't want to fix and the plugin would have to check whether the issue would be accepted before fixing it
There is now way to know in advance what the user wants to fix. This means a psalter plugin will have to run its full analysis, even if the user doesn't want to fix anything from the plugin for this run
Here are a few suggestions to improve the situation:
a) introduce a PsalterPlugin interface, It would declare that this plugin can be used in --alter mode and can fix things
b) the interface would declare a getSupportedIssuesToFix method which would be called by ProjectAnalyzer::getSupportedIssuesToFix, thus adding Plugin issues to standard way of displaying issues (for example in the "Psalm can fix the following issues..." or in --list-supported-issues.) It would also mean we could start using those in --issues=...
c) the interface would also declare a runOnStandardAnalysis method which would disable calling the plugin on non --alter mode for performance reasons (if it doesn't have issues to emit)
d) the plugin could call a method to retrieve every issue it needs to fix to avoid unnecessary analysis for issues that were not needed
e) drop the --plugin notation to avoid issues mentioned earlier (like double calls for already enabled plugins)
Additional improvements
a) we could disable calling a plugin implementing PsalterPlugin if none of the issue choosed by the user are from the plugin for performance reasons
This suggestion would be added on top of #5065 which is very related because --issues= could then be used for plugins and standard issues alike.
What do you think? Is there some adaptations to do?
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Hi,
I made a couple of Psalm plugins these last weeks including some who fix code automatically and I noticed a handful of hindrances that I would like to remove.
The syntax to fix issues with a plugin is weird.
vendor/bin/psalm --alter --plugin=vendor/vendorname/packagename/plugin.php
This is weird in a lot of ways:There is no easy way for a user to choose what issues he wants to fix from a plugin. If the plugin want to do something about that, I guess the easier way would be to have the user tweak with issuesHandler and suppress issues it doesn't want to fix and the plugin would have to check whether the issue would be accepted before fixing it
There is now way to know in advance what the user wants to fix. This means a psalter plugin will have to run its full analysis, even if the user doesn't want to fix anything from the plugin for this run
Here are a few suggestions to improve the situation:
a) introduce a PsalterPlugin interface, It would declare that this plugin can be used in --alter mode and can fix things
b) the interface would declare a
getSupportedIssuesToFix
method which would be called byProjectAnalyzer::getSupportedIssuesToFix
, thus adding Plugin issues to standard way of displaying issues (for example in the "Psalm can fix the following issues..." or in --list-supported-issues.) It would also mean we could start using those in --issues=...c) the interface would also declare a runOnStandardAnalysis method which would disable calling the plugin on non --alter mode for performance reasons (if it doesn't have issues to emit)
d) the plugin could call a method to retrieve every issue it needs to fix to avoid unnecessary analysis for issues that were not needed
e) drop the --plugin notation to avoid issues mentioned earlier (like double calls for already enabled plugins)
Additional improvements
a) we could disable calling a plugin implementing PsalterPlugin if none of the issue choosed by the user are from the plugin for performance reasons
This suggestion would be added on top of #5065 which is very related because --issues= could then be used for plugins and standard issues alike.
What do you think? Is there some adaptations to do?
Beta Was this translation helpful? Give feedback.
All reactions