git-afsantos / haros

H(igh) A(ssurance) ROS - Static analysis of ROS application code.
MIT License
191 stars 37 forks source link

Wrong coding style rules #93

Closed StefanFabian closed 4 months ago

StefanFabian commented 4 years ago

I've noticed issues such as this one: Screenshot from 2019-11-23 11-25-55 which is not in line with the ROS C++ Style Guide.

I'm not sure if there is already an option for this but I have not modified anything (yet) and I think it would make the most sense that the ROS guidelines are the default.

git-afsantos commented 4 years ago

There is not really a default, per se. The plugin uses a combination of roslint and cpplint.

In this case, I assume your code is compliant with ROS C++, but it is one of those cases where ROS and Google disagree, and so the other standard (in this case Google's) complains.

There are a couple of ways to go about it in the current version.

If you care only about the visualization, use the filtering button at the top, to either show only ros, or to ignore curly-braces or google-cpp.

If you do not want those specific issues to be part of the report at all, edit your Haros configuration file (defaults to ~/.haros/configs.yaml, unless you are using the --home option). Add the following sections to this file:

analysis:
    ignore:
        rules:
            - "haros_plugin_cpplint:opening_curly_brace"

This is not yet documented because it is not particularly clean, and I would like it to be more user-friendly in a future version.

You can see all rules reported by that plugin here. To ignore specific rules add strings to that list of the form "plugin_name:rule_id".

The rule IDs are those IDs you see in the file linked above, just under the rules section.

Let me know if you find something unclear.

StefanFabian commented 4 years ago

What's the reason for using two linters? Is roslint missing some major features that would make using only that insufficient? Can I disable cpplint altogether?

git-afsantos commented 4 years ago

Actually, roslint is (or was, at the time I made the plugin) just a wrapper for cpplint that changed a few specific reports, since most of the rules are the same between the two style guides. So I just used cpplint and added (instead of changing) the rules of roslint.

Due to this, in this version it is not possible to disable cpplint without disabling roslint too. The only thing you can do as a user, right now, is to ignore the rules that are different, unfortunately.

If this is too inconvenient, I will make a note to separate the two in a future release.