Cannot satisfy both cpplint and uncrustify

[ivanpauno]

I've seen one case, where I can't make both pass (maybe, it's just my error).

If you have an else if, where the condition have to be split in two lines because it's too long, e.g.:

if (...) {
  ...
} else if ( <long condition
  that uses more than one line>)
{
  ...
}

cpplint shows this failure:
If an else has a brace on one side, it should have it on both [readability/braces] [5]

For every other possibility I've tried, uncrustify don't pass.
For example:

if (...) {
  ...
} else if ( <long condition
  that uses more than one line>) {
    ...
}

if (...) {
  ...
}
else if ( <long condition
  that uses more than one line>)
{
  ...
}

The only way of working it around, is putting the if clause nested in the else. But that's only acceptable for one else if, not for multiple.
e.g.:

if (...) {
  ...
} else {
  if ( <long condition
    that uses more than one line>)
  {
    ...
  }
}

---

Triggered by ros2/rclcpp#778 (comment).
I've also used the last workaround somewhere else (I don't remember where).

[ivanpauno]

IMHO, the first snippet I posted should pass without failures.

[dirk-thomas]

Neither of the snippets complies with our text-form style guide. The long condition should be wrapped directory after the open parenthesis:

} else if (
  <long condition
  that uses more than one line>
) {

[ivanpauno]

Neither of the snippets complies with our text-form style guide. It should be:

} else if (
  <long condition
  that uses more than one line>
) {

I tried that, but uncrustify didn't like it:

40: --- include/rclcpp/intra_process_manager.hpp
40: +++ include/rclcpp/intra_process_manager.hpp.uncrustify
40: @@ -273,2 +273,2 @@
40: -      sub_ids.take_shared_subscriptions.size() <= 1
40: -    ) {
40: +      sub_ids.take_shared_subscriptions.size() <= 1)
40: +    {
40: @@ -290,2 +290,2 @@
40: -      sub_ids.take_shared_subscriptions.size() > 1
40: -    ) {
40: +      sub_ids.take_shared_subscriptions.size() > 1)
40: +    {
40: 
40: 1 files with code style divergence

The complete snippet:

    } else if (
      !sub_ids.take_ownership_subscriptions.empty() &&
      sub_ids.take_shared_subscriptions.size() <= 1
    ) {

    } else if (
      !sub_ids.take_ownership_subscriptions.empty() &&
      sub_ids.take_shared_subscriptions.size() > 1
    ) {

P.S.: If I follow uncrustify suggestion, cpplint doesn't pass.

[rotu]

fyi: uncrustify/uncrustify#2360

[asorbini]

I just ran into this same issue while working on rmw_connextdds#22

[clalancette]

Updating to a newer uncrustify may fix it, but that always has additional fallout. It's not something we are going to do right now, but we could consider updating uncrustify in H-Turtle.

[AndyZe]

You can get around this by skipping cpplint for the line: // NOLINT

cpplint/cpplint#35

[clalancette]

So we actually did update both cpplint and uncrustify for Humble. @ivanpauno Can you give this another try on Rolling and see if your original issue is solved?

[ivanpauno]

I still can reproduce this.
I think we will need to modify our uncrustify settings to make it work (or if possible, delete/modify this check from cpplint).

[padhupradheep]

any updates guys?