Skip to content

Quality of life improvement, PHPStorm error identifiers #4416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
+6 −3
Closed

Quality of life improvement, PHPStorm error identifiers #4416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9402bd
refactor(formatters): added error identifier to actual checkstyle the…
NickSdot Oct 7, 2025
ee972b4
refactor(formatters): code style
NickSdot Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/Command/ErrorFormatter/CheckstyleErrorFormatter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,14 @@ public function formatErrors(
$output->writeLineFormatted('');

foreach ($errors as $error) {

$identifier = $error->getIdentifier();

$output->writeRaw(sprintf(
' <error line="%d" column="1" severity="error" message="%s"%s />',
$this->escape((string) $error->getLine()),
$this->escape($error->getMessage()),
$error->getIdentifier() !== null ? sprintf(' source="%s"', $this->escape($error->getIdentifier())) : '',
$this->escape($error->getMessage() . ($identifier !== null ? sprintf(' // @phpstan-ignore %s', $identifier) : '')),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I don't want the error message to contain phpstan-ignore, just the identifier
  2. I don't want the error message to change for everyone, that's creating unnecessary noise. Can we detect we're running inside the PhpStorm plugin?

Copy link
Author

@NickSdot NickSdot Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Sure, I can do that. Allow me to add that it is super convinient to be able to just copy&paste the full comment. Your call.
image
  1. Not really. I was thinking about that. There is no PHPStorm native env var available. One hacky way to achieve an opt-in like behaviour is the following:
  • create phpstorm.php somewhere with content
<?php

putenv('PHPSTAN_IDE=PHP_STORM');
  • load it as bootstrap file
parameters:
	bootstrapFiles:
	- ./Foo/phpstorm.php

Then I have it available in the CheckstyleErrorFormatter.

But honestly, this feels too hacky. At this point it is probably better to just introduce CheckstyleErrorFormatterPhpStorm and then people can opt-in the usual way?

errorFormatter.checkstyle:
class: Foo\CheckstyleErrorFormatterPhpStorm

Copy link
Member

@ondrejmirtes ondrejmirtes Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I'm not too keen on fixing problems of a different commercial product on my side because they're not giving priority to work on their plugin, creating more work for me.

I'd rather have my own plugin that offers the best experience for PhpStorm users, probably offered as part of PHPStan Pro subscription.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am with you. Totally understandable. Also quite disappointing that @JetBrains leaves small issues like this open for such long time. Maybe @pronskiy or @brendt can get something rolling on their end.

Closing here. When official plugin? 🚀

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'll vibecode it 🪄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phpstorm can be detected, see https://github.com/phpstan/phpstan-src/pull/4369/files

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @staabm but that apparenly doesn't work in the inspection; terminal only. Tried, not available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that only for the integrated terminal? PHPStan in PHPStorm works as a plugin.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrapped it up as a composer package real quick composer require nicksdot/phpstan-phpstorm-error-identifiers

$identifier !== null ? sprintf(' source="%s"', $this->escape($identifier)) : '',
));
$output->writeLineFormatted('');
}
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Command/ErrorFormatter/CheckstyleErrorFormatterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ public function testIdentifier(): void
), $this->getOutput());
$this->assertXmlStringEqualsXmlString('<checkstyle>
<file name="Foo.php">
<error column="1" line="5" message="Foo" severity="error" source="argument.type" />
<error column="1" line="5" message="Foo // @phpstan-ignore argument.type" severity="error" source="argument.type" />
</file>
</checkstyle>', $this->getOutputContent());
}
Expand Down
Loading