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

Exit code is ignored from phpmd (parsing errors for new php syntax) #230

Closed
archfz opened this issue May 11, 2021 · 6 comments · Fixed by #237
Closed

Exit code is ignored from phpmd (parsing errors for new php syntax) #230

archfz opened this issue May 11, 2021 · 6 comments · Fixed by #237

Comments

@archfz
Copy link

archfz commented May 11, 2021

Phpmd (v2.10.0) fails for a particular reason in this case (issue with trailing commas in function arguments):

ERROR while parsing 
--------------------
Unexpected token: ), line: 56, col: 5, file

The exit code is printed correctly but phpqa considers this a success.

 73/73 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100% 
 [Edge\QA\Task\ParallelExec] Output for "/app/vendor/edgedesign/phpqa/../../../vendor/bin/phpmd" "src","tests" xml "/app/phpmd.xml"  --exclude /vendors/,/var/,/tests/_support/_generated/ --suffixes php --reportfile ".phpqa-report/phpmd.xml":


 [Edge\QA\Task\ParallelExec]  Exit code 3  Time 0.447s

[phpqa] 
+---------+----------------+--------------+--------+----------------------------+
| Tool    | Allowed Errors | Errors count | Is OK? | HTML report                |
+---------+----------------+--------------+--------+----------------------------+
| phpcs   | 0              | 0            | ✓      | .phpqa-report/phpcs.html   |
| phpmd   | 0              | 0            | ✓      | .phpqa-report/phpmd.html   |
| phpcpd  | 0              | 0            | ✓      | .phpqa-report/phpcpd.html  |
| phpstan | 0              | 0            | ✓      | .phpqa-report/phpstan.html |
+---------+----------------+--------------+--------+----------------------------+
| phpqa   |                | 0            | ✓      | .phpqa-report/phpqa.html   |
+---------+----------------+--------------+--------+----------------------------+

[phpqa] No failed tools
@zdenekdrahos
Copy link
Member

I'm not sure if exit code strategy should be changed. phpqa counts phpmd violations, not parsing errors. There was similar issue for some php7 syntax:

phpmd does not support new syntax. Different exit code is ignored, something is displayed in "Parsing errors" tab in html report. Let's say phpqa respects phpmd's exit code. You can't fix the build unless phpmd releases new version. You could ignore the file, but it would be ignored for phpcs, phpstan etc.

I'm open to proposals. It could be overriden in config, but I don't think it should be default behavior. If you need it, you can use analysis without html report. CLI report analyzes only exit code:

 phpqa --tools phpmd:0 --output cli

@zdenekdrahos zdenekdrahos changed the title Exit code is ignored from phpmd Exit code is ignored from phpmd (parsing errors for new php syntax) May 12, 2021
@archfz
Copy link
Author

archfz commented May 12, 2021

Well it would be important to find a solution for this as in cases like this we literally tought everything was working fine, pipeline was passing, while in fact we were introducing lal kinds of violations.

I will think about it.

@zdenekdrahos
Copy link
Member

Brainstorming notes...


A) Similar solution like for phpcs

        'errorsXPath' => [
            # ignoreWarnings => xpath
            false => '//checkstyle/file/error',
            true => '//checkstyle/file/error[@severity="error"]',
        ],
  • .phpqa.yml contains phpmd.countParsingErrors (default: false)
  • countParsingErrors: false -> count only //pmd/file/violation
  • countParsingErrors: true -> count //pmd/file/violation and //pmd/error

Big disadvantage, that you can't fix the build unless phpmd releases new version.


B) Display skipped errors somewhere

Exit code strategy is not changed. Just display number of phpmd's parsing errors, phpcs's warnings somewhere in CLI output.

+---------+----------------+--------------+--------+----------------------------+
| Tool    | Allowed Errors | Errors count | Is OK? | HTML report                |
+---------+----------------+--------------+--------+----------------------------+
| phpcs   | 0              | 0 (10)       |!     | .phpqa-report/phpcs.html   |
| phpmd   | 0              | 0 (2)        |!     | .phpqa-report/phpmd.html   |
+---------+----------------+--------------+--------+----------------------------+

@zdenekdrahos
Copy link
Member

Released in v1.25 (demo)

@archfz
Copy link
Author

archfz commented Jun 11, 2021

Well I also taught about this and you are right about this You can't fix the build unless phpmd releases new version.
I think for this reason the option B would be most valued. I have installed 1.25 but I cannot see option B, was that no implemented ?

@zdenekdrahos
Copy link
Member

It's described in #237:

  • you can "hotfix" the build by editing phpmd.countParsingErrors
  • warning is displayed in html report

image

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