Skip to content
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.

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

Implement feature 5973 - add SARIF output format #765

Closed
wants to merge 3 commits into from

Conversation

llaville
Copy link

As it is my first PR on this project, hope I don't forget any steps.

  • make cs OK
  • make tests OK

My first remark:

  • As I don't found any API / constant that give current PHPStan version, I've hardcoded it in SarifErrorFormatter and unit tests.

Waiting for your feedbacks

@llaville
Copy link
Author

make phpstan has an error (even on master branch)

php bin/phpstan clear-result-cache -q && php -d memory_limit=768M bin/phpstan
Note: Using configuration file /shared/backups/github/my-phpstan-src/phpstan.neon.dist.
 1263/1263 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ----------------------------------------------------------------------------------------------------------
  Line   src/Rules/RuleErrorBuilder.php
 ------ ----------------------------------------------------------------------------------------------------------
         Ignored error pattern #^Variable property access on PHPStan\\Rules\\RuleError\.$# in path
         /shared/backups/github/my-phpstan-src/src/Rules/RuleErrorBuilder.php was not matched in reported errors.
  149    Variable property access on object.
  152    Method PHPStan\Rules\RuleErrorBuilder::build() should return PHPStan\Rules\RuleError but returns object.
 ------ ----------------------------------------------------------------------------------------------------------


 [ERROR] Found 3 errors


make: *** [Makefile:59: phpstan] Error 1

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

And please fix the Windows failures :)


services:
errorFormatter.sarif:
class: PHPStan\Command\ErrorFormatter\SarifErrorFormatter
Copy link
Member

Choose a reason for hiding this comment

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

This should be in config.neon.

@@ -6,6 +6,7 @@
],
"require": {
"php": "^7.4 || ^8.0",
"bartlett/sarif-php-sdk": "^1.0",
Copy link
Member

Choose a reason for hiding this comment

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

Please try to do this without a dependency. It's just encoding a simple JSON after all.

{
$driver = new ToolComponent('PHPStan');
$driver->setInformationUri('https://phpstan.org');
$driver->setVersion('1.1.2');
Copy link
Member

Choose a reason for hiding this comment

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

The version can be obtained like this:

phpstan-src/bin/phpstan

Lines 109 to 114 in 1789915

$version = 'Version unknown';
try {
$version = \Jean85\PrettyVersions::getVersion('phpstan/phpstan')->getPrettyVersion() ?: $version;
} catch (\OutOfBoundsException $e) {
}

@ondrejmirtes
Copy link
Member

Feel free to open a new PR if you decide to finish this, thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants