squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

Add GitLab Code Climate report #3749

Open KamiYang opened 1 year ago

KamiYang commented 1 year ago

Adds a new reporter that complies with the GitLab subset of the CodeClimate spec for GitLab CI. Using the report in the specific format enables the use of GitLabs Code Quality widget. This PR is not actually related to #2813 because this PR only adds the GitLab subset and GitLab does not require the use of CodeClimate itself

herndlm commented 1 year ago

Nice, perfect timing! I was just looking for something like that.

jrfnl commented 1 year ago

Without an opinion on the report itself: why do you think this report should be hosted in this repo ?

You may not be aware of this, but Reports can be hosted in external packages. Also see: https://github.com/squizlabs/PHP_CodeSniffer/pull/1948

Chi-teck commented 9 months ago

why do you think this report should be hosted in this repo ?

What repo it should belong then? I think coding standards and reports should not be coupled.

PHPStan offers GitLab reporter out of box.

jrfnl commented 9 months ago

why do you think this report should be hosted in this repo ?

What repo it should belong then? I think coding standards and reports should not be coupled.

I'd suggest maybe a dedicated package for this report only ? Or possibly for a variation of CI specific reports.

Chi-teck commented 9 months ago

Just came across this package. Didn't test it myself yet. https://github.com/micheh/phpcs-gitlab

soullivaneuh commented 9 months ago

Without an opinion on the report itself: why do you think this report should be hosted in this repo ?

I maybe have an answer to convince you to accept that kind of proposition.

First, the tool already support another kind of report, such as junit. So why junit and not CodeClimate or GitLab code climate ? This is more an interrogation here. :-)

Then, I have a concrete case to expose. I have a GitLab CI project gathering recipes to implement different linters/fixers tools onto standalone CI jobs.

The end user, so the developer, MIGHT want to integrate external standard with composer. To have my CI job working on such a context, I have to require him/her to declare your tool as a project dependency alongside the standard he/she needs.

Currently, I am using the "provided out of the box" junit report, which is working flawlessly but create an integration widget for automated test report, which is not really appropriate.

![image](https://github.com/squizlabs/PHP_CodeSniffer/assets/1698357/f93f3c42-ebca-4cf4-b2d5-3dcdfbde5e55)

If I want to move the report to codequality, then I need to create/use an external package by the following methods:

  1. Adding it to the project dependencies, using composer require during the CI job execution.
  2. Ask the end user to add this package to the project dependencies and fail the job otherwise.

In both case, it is not a great solution to me because it make the CI job configuration too intrusive, regardless the potential additional dependencies conflicts it may introduce. Currently, I preferred to stick to the junit report because it does not worth to force that kind of setup to just moving to a more appropriate report widget. Furthermore, the widget is a paid feature of GitLab. So I MIGHT even force a user to install an additional package for... nothing. :upside_down_face:

By the way, I tested the micheh/phpcs-gitlab, reported by @Chi-teck, but it does not look to produce a correct report for GitLab. The widget does not display anything, but I didn't go deeper.

However, if you accept an integration inside the project, it will be simple as "Bonjour !", meaning changing the --report-junit option to --report-gitlab or --report-codeclimate without touching anything to the user's project.

If you want to learn more about my problematic and how I did the GitLab CI integration of this tool, please see the following configuration and documentation on that link: https://gitlab.com/ss-open/ci/recipes/-/tree/v3.2.1/stages/lint/php-code-sniffer

Finally, even if I know it is not really an argument, I would like to notify you that some tools are already implementing a gitlab/github report formats. Some sample I am currently using:

Note: If the debate is still actual, it may be preferable to re-open #2813 or create a new issue for that. What do you think?

Thanks for the reading! :+1:

jrfnl commented 9 months ago

@soullivaneuh None of those are arguments for putting the maintenance burden of this report on PHPCS. I, for one, don't use GitLab, so there would be no way for me to even test the validity of the report or the validity of future changes.

Saying that requiring an extra package is a burden for the end-user is something which I don't consider a valid argument nowadays with Composer making that as easy as can be.

To have my CI job working on such a context, I have to require him/her to declare your tool as a project dependency alongside the standard he/she needs.

That's just not true. Your reporting package would need a (non-dev) dependency on PHPCS, the "end-user", which also uses external standards, shouldn't need to have that dependency.

You could even offer a Docker container/action runner with your package already installed if needs be.

All in all, unless there is significantly more interest in this feature and a willingness from GitLab users to maintain the report, I don't think it has a place in PHPCS itself.

Veraxus commented 2 months ago

FYI, I just worked around this by having PHPCS create a JSON report --report-json=phpcs-report.json and then converting it to Gitlab CI's "Code Quality" (i.e. CodeClimate-lite) format using JQ.

jq '[
    .files | to_entries[] |
    .key as $file |
    .value.messages[] |
    {
        description: .message + " (" + .source + ")",
        check_name: .source,
        fingerprint: ($file + ":" + (.line|tostring) + ":" + (.column|tostring) + ":" + .message + ":" + .source | gsub("[^a-zA-Z0-9]"; "")),
        severity: (if .type == "WARNING" then "major" elif .type == "ERROR" then "critical" else "info" end),
        location: {
            path: ($file | sub("^/builds/DDM/project-templates/template-wp/"; "")),
            lines: {
                begin: .line
            }
        }
    }
]' phpcs-report.json > gitlab-report.json
soullivaneuh commented 2 weeks ago

That's just not true. Your reporting package would need a (non-dev) dependency on PHPCS, the "end-user", which also uses external standards, shouldn't need to have that dependency.

You could even offer a Docker container/action runner with your package already installed if needs be.

@jrfnl I might indeed create a docker container containing all the external library, but it does not fit my use case.

I wrote a GitLab CI recipes library that provide tons of pre-configured lint jobs.

For the PHP CodeSniffer tool, this library is not aware at all of which standard extension the end-user need. I may provide the most popular one, but it will never be exhaustive.

To use this recipe library, the user has to list the tool to his project dependencies and the CI job fetch them using composer install.

So no, global installation may not be the solution for all.