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

clang_tidy XML to SARIF conversion doesn't match the SARIF spec #418

Open
Ronoman opened this issue Oct 24, 2022 · 0 comments
Open

clang_tidy XML to SARIF conversion doesn't match the SARIF spec #418

Ronoman opened this issue Oct 24, 2022 · 0 comments

Comments

@Ronoman
Copy link

Ronoman commented Oct 24, 2022

I ran colcon test --packages-select rclcpp and examined the clang_tidy output. Below is the XML reported by static analysis, as well as the SARIF generated by ament_clang_tidy on the spaceros branch.

XML output:

<testcase
    name="/home/spaceros-user/src/spaceros/build/rclcpp/include/rclcpp/node_interfaces/get_node_base_interface.hpp:97:65"
    classname="rclcpp.clang_tidy">
      <failure message="non-const reference parameter 'node_interface', make it const or use a pointer [google-runtime-references]"><![CDATA[/home/spaceros-user/src/spaceros/build/rclcpp/include/rclcpp/node_interfaces/get_node_base_interface.hpp:97:65]]></failure>
</testcase>

SARIF output:

...
"rules": [
    {
        "id": "google-runtime-references",
        "shortDescription": {
            "text": "non-const reference parameter 'node_interface', make it const or use a pointer"
        },
        "helpUri": "https://clang.llvm.org/extra/clang-tidy/checks/list.html"
    },
...
"results": [
    {
        "ruleId": "google-runtime-references",
        "level": "warning",
        "kind": "review",
        "message": {
            "text": "  std::shared_ptr<rclcpp::node_interfaces::NodeBaseInterface> & node_interface)\n                                                                ^\n"
        },
        "locations": [
        {
            "physicalLocation": {
            "artifactLocation": {
                "uri": "/home/spaceros-user/src/spaceros/build/rclcpp/include/rclcpp/node_interfaces/get_node_base_interface.hpp",
                "index": 0
            },
            "region": {
                "startLine": "97",
                "startColumn": "65"
            }
            }
        }
        ]
    },
...

What isn't right:

  1. The rules.shortDescription.text references code-specific issues, instead of the general issue being reported. This ends up duplicating rules when converting XML to SARIF.
  2. results.message.text is not sufficient to describe the issue (related to 1 above).
  3. startLine and startColumn should be integers, not strings.
  4. results.message.text has extraneous characters that could throw off the SARIF dashboard. Not high priority, and may be difficult to intelligently strip from the result.
  5. google-runtime-references is deprecated, it was removed from Google's style guide in May 2020: https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg203119.html
@mjeronimo mjeronimo added the help wanted Extra attention is needed label Mar 29, 2023
@mjeronimo mjeronimo removed their assignment Mar 29, 2023
@mjeronimo mjeronimo removed the help wanted Extra attention is needed label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants