ament_uncrustify formats messages with default values incorrenctly about ament_lint

[wjwwood]

@esteve what lead you to notice this? @mikaelarguedas or @dirk-thomas are we running uncrustify on our generated code by default?

[dirk-thomas]

Yes, when being explicitly requested (https://github.com/ros2/rosidl/blob/69400938eef1f6886fd65951d18fae7d3780e47d/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake#L36-L38) the generators add linters e.g. (https://github.com/ros2/rosidl/blob/master/rosidl_generator_cpp/cmake/rosidl_generator_cpp_generate_interfaces.cmake#L121-L144).

[esteve]

@wjwwood this branch (https://github.com/esteve/rosidl/tree/linter-errors) adds a message like the one in the issue description and activates linters to show that the generated message does not pass ament_uncrustify

[dirk-thomas]

The current configuration file for uncrustify actually configures the style checked exactly in the way the code generator generates the code. It seems uncrustify has a problem with this specific example:

Various_()
: value_a({{1, 2}}),
  value_b({{3, 4}})
{
}

Maybe you can try a similar example but with a different initialization / types. If that passes this may be a bug in uncrustify which gets confused by the {{ and }} from the initialization arguments. You might want to report this upstream with an example.

[esteve]

@dirk-thomas yeah, the uncrustify configuration seems to be fine, but it doesn't seem to work, at least on my workspace. Have you tried the branch I pushed? https://github.com/esteve/rosidl/tree/linter-errors

[dirk-thomas]

Have you tried the branch I pushed?

No, I haven't.

[esteve]

No, I haven't.

Will you then?

[esteve]

@dirk-thomas BTW, I took the time to back the claims of this issue with proof, so your statement doesn't seem to be entirely factual:

The current configuration file for uncrustify actually configures the style checked exactly in the way the code generator generates the code

[dirk-thomas]

Will you then?

I do believe your posted result. That uncrustify complains about the generated code. But since we pass the "correct" configuration to uncrusitfy (which works in similar examples as intended, please see plenty of existing code which uses the same style and passes the linter) there is not much we can do about it (except marking the lines in the code generation step to be ignored by uncustify or fixing the problem upstream in uncrustify).

Since I don't see what I would gain by testing your branch (I would expect to see exactly the result you posted) I am not sure why I should do that?

[esteve]

I do believe your posted result.

From your previous comment, where you pasted a snippet formatted according the style configuration, one could assume that it was generated by actually running uncrustify. So I wonder what was the point of posting that handcrafted snippet at all if I already had showed that it wasn't the expected result.

Anyway, we seem to agree that there may be an issue in uncrustify, so I filed a ticket detailing the issue: uncrustify/uncrustify#1142

[mikaelarguedas]

waiting for the bug to be addressed upstream

[dirk-thomas]

I will close this since the issue is tracked upstream. Keeping this open doesn't seem to provide a significant "visibility" advantage.