PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

Add GitHub Actions annotations report

Open BrianHenryIE opened this issue 5 years ago • 6 comments

Adds a report type for use in GitHub Actions which annotates PRs etc with PHPCS errors.

~~Nomenclature is changed to use "warning" for errors that can be automatically fixed in CI and "error" for those that cannot.~~

example

BrianHenryIE avatar Mar 28 '20 20:03 BrianHenryIE

Error/Warning Label

I don't know what actually constitutes an error as distinct from a warning in PHP CodeSniffer.

e.g., from wiki:

 47 | ERROR | Line not indented correctly; expected 4 spaces but found 1

and

 21 | WARNING | Equals sign not aligned with surrounding assignments

both are automatically fixable indentation problems but designated different. I searched the documentation and code but didn't find a definition.

From a CI perspective, the real question is "does this require manual intervention?" which I think is communicated correctly here. So what I'm really trying to do is a PHPCBF report that shows what has been fixed and what needs to be fixed. But when I use phpcbf --report=GitHubActionsAnnotations the report used is always Cbf.php and https://github.com/squizlabs/PHP_CodeSniffer/issues/1818 suggests there is no way to override PHPCBF's report.

A compromise might be to continue fixable/not-fixable as warning/error for GitHub Actions, but prefix the message with the PHPCS warning/error label.

Docblock

I think this would be more appropriate in the wiki. Here's an example GitHub Actions workflow, to be saved in .github/workflows/phpcbf.yml

name: Run PHP CodeSniffer

# NB: Pull requests from forks do not have access to repository secrets so cannot commit changes.

on:
  push:
    branches:
      - master

jobs:
  php-codesniffer:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v2

      - name: Run Composer install
        uses: php-actions/composer@v1

      - name: Run PHPCS
        continue-on-error: true
        run: vendor/bin/phpcs --report=GitHubActionsAnnotations

      - name: Run PHPCBF
        continue-on-error: true
        run: vendor/bin/phpcbf

      - name: Commit PHPCBF changes
        uses: stefanzweifel/[email protected]
        with:
          commit_message: "PHPCBF"

Package.xml

I've added this and it seems the build is passing!

BrianHenryIE avatar Mar 29 '20 02:03 BrianHenryIE

Related: https://github.com/chekalsky/phpcs-action

jrfnl avatar Mar 31 '20 14:03 jrfnl

As I thought about this over the weekend, I think it does make sense to stick with the true error/warning output from PHPCS. I was thinking of it as running the report before PHPCBF so that the errors that CI was about to fix would be reported. When thinking of it as running PHPCBF then PHPCS, the traditional output makes more sense. So what I was really trying to do was a PHPCBF report.

I noticed there were projects that were full actions, but thought it's more appropriate as part of the main PHPCS project.

BrianHenryIE avatar Mar 31 '20 18:03 BrianHenryIE

Hi. I want this report type.

Is there any insufficient in this pull request?

muno92 avatar May 05 '22 13:05 muno92

Just as a heads-up, while it may still be nice to have a PHPCS native report for this, it isn't actually needed to have annotations show in GitHub.

The CS2PR tool, which can be automatically installed via the setup-php action runner, can handle the PHPCS checkstyle report perfectly and will show annotations inline in PRs and code views.

      - name: Install PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: '8.1'
          coverage: none
          tools: cs2pr
      - name: Install Composer dependencies
        uses: "ramsey/composer-install@v2"
      - name: Run PHPCS
        run: vendor/bin/phpcs --report=checkstyle -q /path/to/code | cs2pr

jrfnl avatar May 06 '22 22:05 jrfnl

I knew cs2pr, and thought it is nice to annotate PR without any other tool for less dependencies. But as you wrote, it is not necessary.

I use cs2pr.

Thanks.

muno92 avatar May 07 '22 08:05 muno92