Add support for SARIF output format

[jbelien]

Note
This is based on the work of @llaville in #765 and closes phpstan/phpstan#5973

This PR adds a new output format: Static Analysis Results Interchange Format (SARIF) Version 2.1.0.
This format is for instance used by GitHub Code Scanning features ; implementing this should allow us to "export" all alerts reported by PHPStan in Code Scanning alerts using GitHub Actions.

This is still a work in progress, I'll add the missing tests (based on the test made for JsonErrorFormatter) but I'm already looking forward for input.

[jbelien]

@ondrejmirtes In your #765 review, you mentioned using PrettyVersions::getVersion('phpstan/phpstan')->getPrettyVersion() to get PHPStan version (see #765 (comment))

I've tried to implement it but I have the following error:

Jean85\Exception\ReplacedPackageException: Cannot retrieve a version for package phpstan/phpstan since it is replaced by some other package

[herndlm]

Maybe you can use https://github.com/phpstan/phpstan-src/blob/1.9.11/src/Internal/ComposerHelper.php#L69 now? This has been simplified a bit in the meantime I believe

[jbelien]

Maybe you can use https://github.com/phpstan/phpstan-src/blob/1.9.11/src/Internal/ComposerHelper.php#L69 now? This has been simplified a bit in the meantime I believe

Awesome! Thanks a lot @herndlm, it works like a charm. 👍

[ondrejmirtes]

Hi, since this is expected to be consumed by an existing tool, it'd be great if you first tried your code on your own project (anyone can implement custom error formatters, they don't have to be in the core), tried it out on some error output, see if GitHub interprets it correctly, posted some screenshots, and then we'd be able to review this and merge it :) Thanks.

[jbelien]

@ondrejmirtes Yes, that was exactly my plan (and the reason it's still marked as draft).
I've added the missing tests, now I'll check with GitHub Code Scanning.

That being said I've 2 questions:

• How to get the rule class name that triggers an error/warning ? Is that possible ?
• How to easily use my "custom" formatter in another project while developing (without using phpstan.phar) ?

Thanks!

[herndlm]

Regarding the testing in another project: you can pull the phar from the compile action where it normally ends up as artifact. E.g. https://github.com/phpstan/phpstan-src/actions/runs/3923649573

[ondrejmirtes]

That's what I was trying to say - you don't need to do anything crazy, just use last stable PHPStan ans implement the formatter in your own project.

[jbelien]

Tests on Windows are currently failing (malformed JSON) but I don't really know why, I'll investigate later.

Other question:
I'm now using RelativeHelper to get the relative path ; but how can I get the working directory (it's set as private in RelativeHelper). I'm currently using getcwd() but that's not what I should use because in the tests it should be /data/folder/with space/and unicode 😃/project as defined by https://github.com/phpstan/phpstan-src/blob/1.10.x/src/Testing/ErrorFormatterTestCase.php#L26) ?

Thanks.

[jbelien]

I've created a repository to test the SarifErrorFormatter: https://github.com/jbelien/phpstan-sarif-test

Here is the result (using GitHub Actions):

[jbelien]

As you can see "Rule ID" is defined as "[unknown-rule]".

Is there a way to get the class name of the rule that triggered an error ?

[ondrejmirtes]

1. Can you please explain how is "code scanning" different from running PHPStan normally in CI and seeing normal PHPStan output in status checks? When I open https://github.com/jbelien/phpstan-sarif-test I don't know where to browse the results.
2. I don't want to use rule class as a rule ID. Just use "PHPStan" until we have a better identifier. Related issues: missing source attribute in checkstyle errorFormat phpstan#1205 Add Error identifiers phpstan#3065

[jbelien]

Can you please explain how is "code scanning" different from running PHPStan normally in CI and seeing normal PHPStan output in status checks? When I open https://github.com/jbelien/phpstan-sarif-test I don't know where to browse the results.

SARIF format allow you to upload those warnings and alerts to code GitHub Code Scanning report:

More information about GitHub Code Scanning: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors

The PHPStan report in SARIF format can be uploaded using https://docs.github.com/code-security/code-scanning/integrating-with-code-scanning/uploading-a-sarif-file-to-github

When I open https://github.com/jbelien/phpstan-sarif-test I don't know where to browse the results.

The "security report" is indeed only available to contributors and not public (for obvious reason).
I've invited you to https://github.com/jbelien/phpstan-sarif-test so can should be able to see it (once you've accepted the invite).

[jbelien]

I don't want to use rule class as a rule ID. Just use "PHPStan" until we have a better identifier. Related issues: missing source attribute in checkstyle errorFormat phpstan#1205 Add Error identifiers phpstan#3065

No worries, I'll update SARIF formatter when/if that feature is implemented. 👍
I just wanted to be sure I didn't miss that feature.

[ondrejmirtes]

SARIF format allow you to upload those warnings and alerts to code GitHub Code Scanning report:

What's the difference/benefit seeing the results in the "Security" tab versus status checks? Also, what's making me a bit sceptical about this is that most errors reported by PHPStan is not of security nature, so it might not be a great fit for this feature.

[jbelien]

What's the difference/benefit seeing the results in the "Security" tab versus status checks?

In the end the result is indeed the same for a single repository.
But at organization level, you can have a "risk" report and coverage report for ALL your repositories.

Also, what's making me a bit sceptical about this is that most errors reported by PHPStan is not of security nature, so it might not be a great fit for this feature.

Fully agree with you!
I said "Security" because this feature is under "Security" tab in GitHub but the feature itself is "Code scanning" which is what PHPStan does.
I thought about implementing this in PHPStan when I saw that ESLint (tool that I use for my TypeScript code) was doing it.
With SARIF support, PHPStan could be integrated as a scanning tool available here: https://github.com/jbelien/phpstan-sarif-test/actions/new?category=security

The reason I started working on this is that I would like to have 100% of my repositories covered by code scanning in the coverage report (see screenshot above).

[phpstan-bot]

This pull request has been marked as ready for review.

[jbelien]

Any update ? Thanks already.

[jbelien]

I've released jbelien/phpstan-sarif-formatter and created a new repository with some documentation and some tests: https://github.com/jbelien/phpstan-sarif-formatter

I still think SARIF is a good a fit for an "official" formatter since SARIF stands for Static Analysis Results Interchange Format and is now a standard for static analysis report.

In case you decide to integrate SARIF formatter in PHPStan itself, I'll archive my repository and keep the maintenance directly here ; otherwise, I'll just keep maintaining jbelien/phpstan-sarif-formatter (absolutely no worries).

[ondrejmirtes]

It still seems a bit weird to me. You're running PHPStan as part of your CI pipeline. The general expectation is that it has to be green, either by solving all errors, or by ignoring the remaining ones. Which means that the output formatter has nothing to output.

But in #2182 (comment) it looks like you want this formatter to achieve some kind of dashboard of PHPStan errors across projects in your organization. Which means that your projects would have red CI pipelines...

[jbelien]

The general expectation is that it has to be green, either by solving all errors, or by ignoring the remaining ones.

Yes, that's exactly what I'm trying to achieve here.

My comment was just an example of the use of SARIF in GitHub.
I would indeed like to have full code scanning coverage on all my repositories.

Which means that your projects would have red CI pipelines...

Maybe my explanations were not clear but that's of course not what I'm trying to get.
All CI pipelines should of course be green.

[ondrejmirtes]

When your pipeline is green, you're not gonna see anything in the Code scanning section. When your pipeline is red, you already have a nice output right inside the workflow that's running PHPStan. Thanks to automatically detected CI and the github formatter.

I don't really see any place for SARIF format in this usual workflow.

[jbelien]

No worries then, I'll just use the SARIF formatter separately then. 👍

I still think it would make sense to include it in PHPStan itself but the only thing that really matters is that it's working. 😄

[jbelien]

📢 For people looking for the SARIF formatter, here it is: https://github.com/jbelien/phpstan-sarif-formatter