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

cppcheck issues with missingOverride: multiple locations not shown

      When cppcheck finds a missingOverride, it puts 2 <location>s in the xml. like htis:

       

          <error id="missingOverride" severity="style" msg="The function 'reset' overrides a function in a base class but is not marked with a 'override' specifier." verbose="The function 'reset' overrides a function in a base class but is not marked with a 'override' specifier.">
                  <location file="derived.hpp" line="115" column="7" info="Function in derived class"/>
                  <location file="base.hpp" line="117" column="15" info="Virtual function in base class"/>
                  <symbol>reset</symbol>
              </error>

      Clearly, the error is in the derived.hpp file. However, the result with the cppcheck parser is that it lists this as an issue in the base.hpp file, and the derived.hpp is not even mentioned, so you are left in the dark as to where to the problem is, even though the information is available in the xml.

      Proposed fix: if multiple locations, take the first, not the last. And maybe add the last as text to the detail of the issue, just like the 'info' part is added already.

          [JENKINS-75217] cppcheck issues with missingOverride: multiple locations not shown

          Ulli Hafner added a comment -

          Ulli Hafner added a comment - I'm not sure if this information is already available in the https://github.com/tomasbjerre/violations-lib/blob/b78d51060e05717d2e733f969ef77eea56ce08ca/src/main/java/se/bjurr/violations/lib/parsers/CPPCheckParser.java of the violations library. If it is available, then the adapter https://github.com/jenkinsci/analysis-model/blob/main/src/main/java/edu/hm/hafner/analysis/parser/violations/CppCheckAdapter.java needs to be improved.

          a added a comment -

          Looking at the adapter, and the Violation(Builder) it uses, that Violation class does not have a way to refer to multiple files.
          So I would add a way to know if we already set the message (=we already encountered one location) to the loop, and if we did, extend the existing message (and not overwrite the line/column/file) with the data in the 2nd location (file, line, column and info).

          So I think this can be done for all cppCheck cases that log multiple locations: I've checked some of our warning files, and it will also be better for "knownConditionTrueFalse", "duplInheritedMember" while it doesn't seem to matter which location we take first for "funcArgOrderDifferent", "constParameterCallback", "redundantAssignment" but having the other location and info will also be very helpful there. (And there are almost certainly more errors/warnings with several locations.)

          a added a comment - Looking at the adapter, and the Violation(Builder) it uses, that Violation class does not have a way to refer to multiple files. So I would add a way to know if we already set the message (=we already encountered one location) to the loop, and if we did, extend the existing message (and not overwrite the line/column/file) with the data in the 2nd location (file, line, column and info). So I think this can be done for all cppCheck cases that log multiple locations: I've checked some of our warning files, and it will also be better for "knownConditionTrueFalse", "duplInheritedMember" while it doesn't seem to matter which location we take first for "funcArgOrderDifferent", "constParameterCallback", "redundantAssignment" but having the other location and info will also be very helpful there. (And there are almost certainly more errors/warnings with several locations.)

            Unassigned Unassigned
            legolas1 a
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated: