ament / ament_lint

Apache License 2.0
38 stars 107 forks source link

[ament_clang_tidy] [space_ros] ament_clang_tidy/main.py xml->SARIF assumes single line output from clang_tidy #405

Open Ronoman opened 2 years ago

Ronoman commented 2 years ago

Issue

If a result produced by clang_tidy does not have a [rule-id], then ament_clang_tidy/main.py fails to parse the resulting XML into a SARIF file on the space_ros branch.

Working conversion (expected behavior)

Example clang_tidy output:

/some/path/to/some_header.h:1:0: warning: namespace 'some_namespace' ends with a comment that refers to a wrong namespace '' [google-readability-namespace-comments]
}; // end of namespace
 ^~~~~~~~~~~~~~~~~~~~~
...

This is parsed into the following XML:

<testcase
    name="/some/path/to/some_header.h:1:0"
    classname="some_package.clang_tidy"
  >
      <failure message="namespace 'some_namespace' ends with a comment that refers to a wrong namespace '' [google-readability-namespace-comments]"><![CDATA[/some/path/to/some_header.h:1:0]]></failure>
  </testcase>

Which is properly parsed by ament_clang_tidy/main.py into SARIF.

(paths and names changed, but this is otherwise real output from a run of clang_tidy on a package)

Broken conversion

Example clang_tidy output:

/some/path/to/some_header.h:1:0: warning: 'declare_parameter' is deprecated: declare_parameter() with only a name is deprecated and will be deleted in the future.
If you want to declare a parameter that won't change type without a default value use:
`node->declare_parameter<ParameterT>(name)`, where e.g. ParameterT=int64_t.

If you want to declare a parameter that can dynamically change type use:
'''
rcl_interfaces::msg::ParameterDescriptor descriptor;
descriptor.dynamic_typing = true;
node->declare_parameter(name, rclcpp::ParameterValue{}, descriptor);
''' [clang-diagnostic-deprecated-declarations]
        node_->declare_parameter("prefix");
               ^
/opt/ros/galactic/include/rclcpp/node.hpp:409:5: note: 'declare_parameter' has been explicitly marked deprecated here
  [[deprecated(
    ^

Note that [clang-diagnostic-deprecated-declarations] is partway down the output, instead of on the first line.

This is parsed into the following XML:

<testcase
    name="/some/path/to/some_header.h:1:0"
    classname="some_package.clang_tidy"
  >
      <failure message="declare_parameter() with only a name is deprecated and will be deleted in the future."><![CDATA[/some/path/to/some_header.h:1:0]]></failure>
  </testcase>

Note that there is no [clang-diagnostic-deprecated-declarations] in the failure's message field.

This causes ament_clang_tidy/main.py to fail on line 444 on filter(), as there is no [ or ] to match on in the message.

Summary

The XML generated by ament_clang_tidy does not include a violated rule, if the violated rule name is not present on the first line of clang_tidy's output. This causes a failure generating SARIF on the space_ros branch.