Using editormode of phpstan#2936
Conversation
|
In the long term checking to see which version of a tool in is a prerequiste to any version-dependent feature. For now I'd be happy with an use_editor_mode option |
|
Added a toggle for it now. It's off by default? And I also added an error message when you try to disable temp files and enable editor mode (which is impossible). |
| /** | ||
| * @return Promise<array<Diagnostic>> | ||
| */ | ||
| public function editorModeAnalyse(string $projectFile, string $tempFile): Promise |
There was a problem hiding this comment.
s/projectFile/filename as with the other method?
| return []; | ||
| } | ||
|
|
||
| $this->logger->error($stdout); |
There was a problem hiding this comment.
why are we logging an error here?
| $container->register( | ||
| Linter::class, | ||
| function (Container $container) { | ||
| if ($container->parameter(self::PARAM_EDITOR_MODE)->value() |
There was a problem hiding this comment.
this can be ->bool() instead of ->value() ?
|
Fixed. I also added some quotes around the file path argument. I don't know if there is a better way to escape the file path as argument in this place. Do we even have tests for files with spaces in the filename? |
If I understand correctly quotes are not required as the process is not being invoked in a shell |
1 similar comment
If I understand correctly quotes are not required as the process is not being invoked in a shell |
|
Fair enough. Removed the quotes, maybe Symfony has a better error handling. |
|
The thing is that escaping is only necessary because the shell uses spaces to delimit arguments, we pass the arguments individually to the operating system so we don't need to escape them. |
|
thanks |
|
btw @mamazu (not sure how else to contact you) if you like stickers send me an email at my git address and I'll send you some a couple phpactor ones 🙂 |
What I've done:
Considerations
Phpactor is configured to use the project's phpstan if this does not meet the minimum requirements for the editor feature it might break. (I don't know if phpstan is forward compatible). Should we make this configurable or should we check the version of the installed phpstan to see if the option is available.