Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Include identifier in gitlab error format #4421

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

Merged
ondrejmirtes merged 1 commit into phpstan:2.1.x from mll-lab:gitlab-error-format-identifier
Nov 3, 2025

Conversation

@spawnia
Copy link
Contributor

@spawnia spawnia commented Oct 8, 2025

As described in the example in https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format:

[
 {
 "description": "'unused' is assigned a value but never used.",
 "check_name": "no-unused-vars",
 "fingerprint": "7815696ecbf1c96e6894b779456d330e",
 "severity": "minor",
 "location": {
 "path": "lib/index.js",
 "lines": {
 "begin": 42
 }
 }
 }
]

Copy link
Contributor Author

spawnia commented Oct 8, 2025

Hm, this does not seem to have the desired effect for me.

I tried with a custom formatter in a project and got the following result where check_name is now included:

$ vendor/bin/phpstan analyse -vv --memory-limit=2G --configuration=$CONFIG_FILE --no-progress --error-format=gitLabWithIdentifiers > phpstan-report.json
Result cache not used because the metadata do not match: projectConfig, analysedPaths, composerLocks, composerInstalled, executedFilesHashes
Result cache is saved.
Elapsed time: 3 minutes 18 seconds
Used memory: 4.82 GB
Running after_script
00:01
Running after script...
$ cat phpstan-report.json
[
 {
 "description": "Method App\\Models\\Examination::isOrderEntryTestPatient() should return string but returns bool.",
 "check_name": "return.type",
 "fingerprint": "ace6a50ade5c69cc6067300f8fa486f9fd499184e1ff3f48a9aed96424ec3542",
 "severity": "major",
 "location": {
 "path": "app/Models/Examination.php",
 "lines": {
 "begin": 1085
 }
 }
 }
]

However, the display shown in GitLab did not change:

image

Perhaps we can include the identifier in the description? And maybe also the tip, if present.

Copy link
Member

I think you should ask GitLab support first.

Copy link

GitLab specifies this in the documentation (https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format) :

Name Type Description
check_name String A unique name representing the check, or rule, associated with this violation.

Is the code quality report handled correctly if there is a NULL value for check_name ?

Copy link
Contributor Author

spawnia commented Oct 8, 2025

@Koretech10 It seems to me that check_name is described in the docs, but is not used for anything. I could not spot a difference in what happens in the UI with it present or not. At least it did not break anything 😅 Maybe they will add something in the future? I can test what happens with null to make sure that also does not break stuff.

Copy link
Contributor Author

spawnia commented Oct 8, 2025
edited
Loading

I had some success with the following custom implementation:

<?php declare(strict_types=1);
// See https://github.com/phpstan/phpstan-src/blob/1.12.32/src/Command/ErrorFormatter/GitlabErrorFormatter.php
namespace App\PHPStan;
use Nette\Utils\Json;
use PHPStan\Command\AnalysisResult;
use PHPStan\Command\ErrorFormatter\ErrorFormatter;
use PHPStan\Command\Output;
use PHPStan\File\RelativePathHelper;
/**
 * @see https://docs.gitlab.com/ee/user/project/merge_requests/code_quality.html#implementing-a-custom-tool
 */
final readonly class GitLabWithIdentifiersErrorFormatter implements ErrorFormatter
{
 public function __construct(
 private RelativePathHelper $relativePathHelper,
 ) {}
 public function formatErrors(AnalysisResult $analysisResult, Output $output): int
 {
 $errorsArray = [];
 foreach ($analysisResult->getFileSpecificErrors() as $fileSpecificError) {
 $error = [
 'description' => "{$fileSpecificError->getIdentifier()}: {$fileSpecificError->getMessage()}",
...
services:
 errorFormatter.gitLabWithIdentifiers:
 class: App\PHPStan\GitLabWithIdentifiersErrorFormatter
image

@ondrejmirtes Given the check_name is ineffective, would you accept it if I were to change the pull request to include the identifier in description?

Copy link
Member

Please ask GitLab support first and then get back. Thanks.

Copy link
Contributor Author

spawnia commented Oct 10, 2025

I did submit a GitLab support request (for my own reference: https://support.gitlab.com/hc/en-us/requests/662951), and am now awaiting feedback.

Copy link
Contributor Author

spawnia commented Oct 13, 2025

GitLab support did answer and sent me a link to https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117402, which introduces both the documentation and usafe of check_name. According to that, it should be used in merge request widgets. While it may not have worked in my project for some reason, the intent is there and adding check_name does not hurt, so I think this pull request is good to go. I plan to contribute a follow up merge request to GitLab to ensure check_name is used wherever possible.

@spawnia spawnia force-pushed the gitlab-error-format-identifier branch from e1c705f to 9cea543 Compare October 13, 2025 06:26
Copy link
Member

So what's this code quality scan widget? How can PHPStan make one?

Copy link
Contributor Author

spawnia commented Oct 13, 2025

The code quality scan widget is just one of a couple of ways that GitLab has to show the results of a code quality report. The role of PHPStan is only to produce the code quality report file, and I believe it should deliver information that is as accurate (already the case) and complete (missing field added through this pull request) as possible. The role of GitLab is then to take that report and display the information within in a useful and hopefully also complete way - that part currently seems bugged, but I plan to fix it in GitLab itself.

Copy link
Contributor Author

spawnia commented Oct 13, 2025

In regards to the GitLab documentation - the widget is described here: https://docs.gitlab.com/ci/testing/code_quality/#merge-request-widget. The part that concerns PHPStan is described here: https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format. The check_name field is clearly documented, and as evidenced by the answer from GitLab support and the linked merged request from GitLab, it is supposed to be used that way.

Copy link
Member

So this code quality scan widget already appears for PHPStan? If not, what makes it do appear for eslint but not for PHPStan?

I'm asking because I don't want to merge PR that's essentially noop today, and I generally want to improve the experience for GitLab PHPStan users.

Copy link
Contributor Author

spawnia commented Oct 27, 2025

So this code quality scan widget already appears for PHPStan?

Yes it does, when using artifacts.reports.codequality in the GitLab CI configuration.

There is no difference between eslint, PHPStan or any other tool - the code quality report is just JSON and can be simulated by manually adding a file.

I recently got feedback from GitLab support, they opened https://gitlab.com/gitlab-org/gitlab/-/issues/578172. It appears that since the check_name feature has initially been added, a regression has now caused it to have no effect.

I totally get your stance on not wanting to implement something that has no effect today. May I suggest the following plan:

  • maintain the addition of check_name in this pull request, as it targets the intended behavior of GitLab and does not hurt
  • add a temporary workaround of prefixing the description with the identifier in this pull request, together with a reference to https://gitlab.com/gitlab-org/gitlab/-/issues/578172 and a short explainer that it should be reverted when GitLab fixes their bug

This solution would make error identifiers work now, and at the worst show them redundantly when the GitLab bug is fixed (better than nothing). What do you think?

Copy link
Member

I'd be fine with just a comment above the line with check_name that would lead to the URL with the GitLab issue. Then I'd merge it. Thank you!

As described in the example in https://docs.gitlab.com/ci/testing/code_quality/#code-quality-report-format:
```json
[
 {
 "description": "'unused' is assigned a value but never used.",
 "check_name": "no-unused-vars",
 "fingerprint": "7815696ecbf1c96e6894b779456d330e",
 "severity": "minor",
 "location": {
 "path": "lib/index.js",
 "lines": {
 "begin": 42
 }
 }
 }
]
```
GitLab currently has a bug that prevents `check_name` from showing, see https://gitlab.com/gitlab-org/gitlab/-/issues/578172.
Thus, an explanatory comment and a TODO for its eventual removal were added.
Copy link
Member

No merge commits please, only rebase

@spawnia spawnia force-pushed the gitlab-error-format-identifier branch from 18c3fbe to 6510581 Compare October 28, 2025 08:16
Copy link
Contributor Author

spawnia commented Oct 28, 2025
edited
Loading

It is so inviting to click the Update branch button that GitHub offers 😅 Thanks for your consideration of this change, I appreciate having the information in the JSON file now and plan to work around the GitLab bug through some jq post-processing to prepend check_name to description.

Copy link
Contributor Author

spawnia commented Nov 3, 2025

I did rebase and push a single commit, this should be good to go. The failing test cases seem unrelated to me.

@ondrejmirtes ondrejmirtes merged commit 99020f8 into phpstan:2.1.x Nov 3, 2025
540 of 555 checks passed
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /