Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-56214

PHP CodeSniffer errors treated as warnings

    Details

    • Similar Issues:
    • Released As:
      5.0.0 (analysis-model and warnings-ng)

      Description

      Given the following stage definition and the attached CodeSniffer output
      I was expecting to have errors reported.
      Instead I get warnings with severity HIGH

      Changing the tool from phpCodeSniffer() to checkStyle() makes no difference

      stage('CodeSniffer') {
        steps {
          sh '''
             vendor/bin/phpcs \\
             --ignore=*/vendor/*,*/node_modules/* \\
             --extensions=php \\
             --standard=phpcs.xml \\
             --bootstrap=vendor/autoload.php \\
             --report=checkstyle \\
             --report-file=phpcs.log.xml \\
             Modules/
          '''
        }
      
        post {
          always {
            recordIssues enabledForFailure: true,
            aggregatingResults: true,
            tools: [phpCodeSniffer(pattern: 'phpcs.log.xml', reportEncoding: 'UTF-8')]
          }
        }
      }
      

        Attachments

          Activity

          Hide
          drulli Ulli Hafner added a comment -

          Yes, the parser has been contributed before I added errors to the API. Interested in providing a PR?

          Show
          drulli Ulli Hafner added a comment - Yes, the parser has been contributed before I added errors to the API. Interested in providing a PR?
          Hide
          catalin_nicolescu Cătălin Nicolescu added a comment -

          Yes, if you could point me to where the culprit code is

          Show
          catalin_nicolescu Cătălin Nicolescu added a comment - Yes, if you could point me to where the culprit code is
          Hide
          drulli Ulli Hafner added a comment -

          The class CheckStyleParser is responsible. Currently it maps errors to severity high.

          You can check if the Severity class already provides the correct mapping.

          Show
          drulli Ulli Hafner added a comment - The class CheckStyleParser is responsible. Currently it maps errors to severity high. You can check if the Severity class already provides the correct mapping.
          Hide
          tanjaro Tanja Roithmeier added a comment -

          I've already investigated the source code and as Ulli Hafner said, changing the mapping at the class  CheckStyleParser 
          would solve the problem. 

          But are these changes not a mismatch with the specification of the plugin?

          According to the plugin documentation the severity is defined as following:

          • Error: Indicates an error that typically fails the build
          • Warning (High, Normal, Low): Indicates a warning of the given priority. Mapping to the priorities is up to the individual parsers.

          (cf.: https://github.com/jenkinsci/warnings-ng-plugin/blob/master/doc/Documentation.md)

          None of the errors in the file phpcs.log.xml leads to a failed build. Probably no error provided by a plugin for static code analysis will fail the build, so maybe the severity warning-high is more suitable than an error. 

          Show
          tanjaro Tanja Roithmeier added a comment - I've already investigated the source code and as Ulli Hafner said, changing the mapping at the class   CheckStyleParser   would solve the problem.  But are these changes not a mismatch with the specification of the plugin? According to the plugin documentation the severity is defined as following: Error : Indicates an error that typically fails the build Warning  (High, Normal, Low): Indicates a warning of the given priority. Mapping to the priorities is up to the individual parsers. (cf.:  https://github.com/jenkinsci/warnings-ng-plugin/blob/master/doc/Documentation.md ) None of the errors in the file phpcs.log.xml  leads to a failed build. Probably no error provided by a plugin for static code analysis will fail the build, so maybe the severity warning-high is more suitable than an error. 
          Hide
          drulli Ulli Hafner added a comment - - edited

          Maybe I need to improve the documentation. The plugin now creates issues of severity

          • Error
          • Warning (High, Normal, Low)

          Previous versions of the plugin only used Warning (High, Normal, Low). An additional severity Error was requested by several users. How these severities actually change the status of the build is not predefined in the plugin, this needs to be done by setting quality gates. A recommendation is to fail the build on errors and set the build to unstable on warnings. But this is totally up to the project.

          So I think it is ok, to map errors in the Checkstyle file to Error severity. You can use method `Severity.guessFromString` to map the severity.

          Show
          drulli Ulli Hafner added a comment - - edited Maybe I need to improve the documentation. The plugin now creates issues of severity Error Warning (High, Normal, Low) Previous versions of the plugin only used Warning (High, Normal, Low). An additional severity Error was requested by several users. How these severities actually change the status of the build is not predefined in the plugin, this needs to be done by setting quality gates. A recommendation is to fail the build on errors and set the build to unstable on warnings. But this is totally up to the project. So I think it is ok, to map errors in the Checkstyle file to Error severity. You can use method `Severity.guessFromString` to map the severity.
          Show
          drulli Ulli Hafner added a comment - See https://github.com/jenkinsci/analysis-model/pull/142

            People

            • Assignee:
              tanjaro Tanja Roithmeier
              Reporter:
              catalin_nicolescu Cătălin Nicolescu
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: