Merge pull request #415 from thk123/feature/lint-check-multi-line-function-call

Lint checks nested function calls broken over multiple lines

Author: peterschrammel

CODING_STANDARD

@@ -11,6 +11,8 @@ Whitespaces:
     - For brackets, break after the bracket
       - In the case of function calls, put each argument on a separate line if
         they do not fit after one line break
+      - Nested function calls do not need to be broken up into separate lines even
+        if the outer function call does.
 - If a method is bigger than 50 lines, break it into parts.
 - Put matching { } into the same column.
 - No spaces around operators (=, +, ==, ...)

regression/cpp-linter/multi-line-function-call1/main.cpp

@@ -0,0 +1,88 @@
+/*******************************************************************\
+
+Module: Lint Examples
+
+Author: Thomas Kiley, [email protected]
+
+\*******************************************************************/
+
+/*******************************************************************\
+
+Function: fun
+
+ Inputs:
+
+ Outputs:
+
+ Purpose:
+
+\*******************************************************************/
+
+static void fun()
+{
+  // Correct
+  foo(
+    x,
+    y);
+
+  // Incorrect, x should be on the next line
+  foo(x,
+    y);
+
+  // Incorrect indent should be only 2
+  foo(
+      x,
+      y);
+
+  // Correct
+  int x=bar(
+    x,
+    y);
+
+  // Incorrect, x should be on the next line
+  int x=bar(x,
+    y);
+
+  // Incorrect indent should be only 2
+  int x=bar(
+      x,
+      y);
+
+  // Correct
+  *model=baz(
+    x,
+    y);
+
+  // Incorrect, x should be on the next line
+  *model=baz(x,
+    y);
+
+  // Incorrect indent should be only 2
+  *model=baz(
+      x,
+      y);
+
+  // Correct
+  var->fun(
+    x,
+    y);
+
+  // Incorrect, x should be on the next line
+  var->fun(x,
+    y);
+
+  // Incorrect indent should be only 2
+  var->fun(
+      x,
+      y);
+
+  // Incorrect
+  fun(
+    x, y);
+
+  // Incorrect
+  fun(
+    x, y,
+    z);
+}
+

regression/cpp-linter/multi-line-function-call1/test.desc

@@ -0,0 +1,17 @@
+CORE
+main.cpp
+
+^main\.cpp:29:  If parameters or arguments require a line break, each parameter should be put on its own line\.  \[whitespace/indent\] \[4\]
+^main\.cpp:34:  Indent of wrapped parenthesized expression or parameter or argument list should be 2  \[whitespace/indent\] \[4\]
+^main\.cpp:43:  If parameters or arguments require a line break, each parameter should be put on its own line\.  \[whitespace/indent\] \[4\]
+^main\.cpp:48:  Indent of wrapped parenthesized expression or parameter or argument list should be 2  \[whitespace/indent\] \[4\]
+^main\.cpp:57:  If parameters or arguments require a line break, each parameter should be put on its own line\.  \[whitespace/indent\] \[4\]
+^main\.cpp:62:  Indent of wrapped parenthesized expression or parameter or argument list should be 2  \[whitespace/indent\] \[4\]
+^main\.cpp:71:  If parameters or arguments require a line break, each parameter should be put on its own line\.  \[whitespace/indent\] \[4\]
+^main\.cpp:76:  Indent of wrapped parenthesized expression or parameter or argument list should be 2  \[whitespace/indent\] \[4\]
+^main\.cpp:81:  If parameters or arguments require a line break, the closing bracket should be on the same line as the final parameter  \[whitespace/indent\] \[4\]
+^main\.cpp:85:  If parameters or arguments require a line break, each parameter should be put on its own line.  \[whitespace/indent\] \[4\]
+^Total errors found: 10$
+^EXIT=1$
+^SIGNAL=0$
+--

regression/cpp-linter/multi-line-function-call2/main.cpp

@@ -0,0 +1,67 @@
+/*******************************************************************\
+
+Module: Lint Examples
+
+Author: Thomas Kiley, [email protected]
+
+\*******************************************************************/
+
+/*******************************************************************\
+
+Function: fun
+
+ Inputs:
+
+ Outputs:
+
+ Purpose:
+
+\*******************************************************************/
+
+static void fun()
+{
+  // Incorrect, call should be on a new line
+  nested(call(),
+    another_param);
+
+  // Incorrect - should be indented by 2
+  nested(
+         call(),
+         another_param);
+
+  // Correct
+  nested(
+    call(),
+    another_param);
+
+  // Correct
+  twice(
+    nested(
+      call(
+        param1),
+      param2),
+    param3)
+
+  // Correct
+  foo(
+    bar(x, y),
+    z);
+
+  // Correct
+  foo(
+    bar(
+      x,
+      y),
+    z);
+
+  // Incorrect, the bar arguments have been split up therefore
+  // they all should be split up
+  foo(
+    bar(x,
+      y),
+    z);
+
+  // Incorrect bar should be on the next line
+  foo(bar(x, y),
+    z);
+}

regression/cpp-linter/multi-line-function-call2/test.desc

@@ -0,0 +1,11 @@
+CORE
+main.cpp
+
+^main\.cpp:24:  If parameters or arguments require a line break, each parameter should be put on its own line\.  \[whitespace/indent\] \[4\]
+^main\.cpp:29:  Indent of wrapped parenthesized expression or parameter or argument list should be 2  \[whitespace/indent\] \[4\]
+^main\.cpp:60:  If parameters or arguments require a line break, each parameter should be put on its own line\.  \[whitespace/indent\] \[4\]
+^main\.cpp:65:  If parameters or arguments require a line break, each parameter should be put on its own line\.  \[whitespace/indent\] \[4\]
+^Total errors found: 4$
+^EXIT=1$
+^SIGNAL=0$
+--

regression/cpp-linter/multi-line-function-call3/main.cpp

@@ -0,0 +1,55 @@
+/*******************************************************************\
+
+Module: Lint Examples
+
+Author: Thomas Kiley, [email protected]
+
+\*******************************************************************/
+
+/*******************************************************************\
+
+Function: fun
+
+ Inputs:
+
+ Outputs:
+
+ Purpose:
+
+\*******************************************************************/
+
+static void fun()
+{
+  // Correct as for loop not function
+  for(int x=0;
+    x<10;
+    ++x)
+  {}
+
+  // Correct as an if statement not a function
+  if(a==b ||
+    c==d)
+  {
+    do_something();
+  }
+
+  // Correct as ranged based for loop not function
+  for(x:
+    list)
+  {}
+
+  // Correct since if statement
+  if(some_check(with, params)==
+    some_value)
+  {
+    do_something();
+  }
+
+  // Correct since if statement
+  if(condition_a&&
+    (condition_b||
+      condition_c))
+  {
+    do_something();
+  }
+}

regression/cpp-linter/multi-line-function-call3/test.desc

@@ -0,0 +1,7 @@
+CORE
+main.cpp
+
+^Total errors found: 0$
+^EXIT=0$
+^SIGNAL=0$
+--

scripts/cpplint.py

@@ -4687,9 +4687,79 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state,
   if linenum>0:
     while prev_initial_spaces < len(prev) and prev[prev_initial_spaces] == ' ':
       prev_initial_spaces += 1
-  if Search(r'\([^\)]*,$', elided_line) or Search(r'\(\[^\)]*, $', elided_line):
-    error(filename, linenum, 'whitespace/indent', 4,
-          'If parameters or arguments require a line break, each parameter should be put on its own line.')
+  # here a regex isn't sufficent we need a stack to match brackets
+  # because even an open bracket and a , at the end might just be function call
+  # as a parameter.
+  # Instead the rule we try to apply here is:
+  #  - if we find an opening bracket, we find the matching closing bracket
+  #  - if the bracket is on a different line we require all of the parameters to be on a separate line
+  #  - if there is another opening bracket Skip to the closing bracket as will be checked in a subsequent line check
+  #  - ignore the line if it is a for/if etc since rules are different
+
+  # Look for an opening bracket that doesn't have a semi-colon on the same line
+  bracket_search = Search(r'[\w_]+\s*(?P<bracket>\()[^;]*$', elided_line)
+
+  # Exclude the check if any of these keywords are present
+  # They could trip us up as they have different formatting rules to functions
+  keyword_search = Search(r'\b(if|for|while|catch|switch)\b', elided_line)
+
+  if bracket_search and not keyword_search:
+    open_bracket_pos = bracket_search.start('bracket')
+    close_line, close_linenum, close_pos = CloseExpression(clean_lines, linenum, open_bracket_pos)
+    if close_pos != -1:
+      # If the closing line is different from the opening line we need to
+      # verify that each of the parameters are on separate lines
+      if close_linenum != linenum:
+        # The first line needs to have no parameters on it
+        if(Search(r'\(+[^\(]+', elided_line)):
+          error(filename, linenum, 'whitespace/indent', 4,
+                'If parameters or arguments require a line break, each parameter should be put on its own line.')
+
+        # For each line afer we need to verify it consists of exactly one parameter
+        # Except if we find an opening bracket - in this case we ignore everything until the closing
+        # bracket (any errors within these brackets will be picked up when we check this line)
+        start_linenum = linenum + 1
+        while(start_linenum < close_linenum):
+          arg_line = clean_lines.elided[start_linenum]
+          nested_bracket_search = Search('\(', arg_line)
+          if nested_bracket_search:
+            nested_open_bracket_pos = nested_bracket_search.start()
+            # Just because we are calling a nested function doesn't mean
+            # we allow multiple parameters on the line
+            if(Search(',', arg_line[:nested_open_bracket_pos])):
+              error(filename, start_linenum, 'whitespace/indent', 4,
+                    'If parameters or arguments require a line break, each parameter should be put on its own line.')
+
+            nested_close_line, nested_close_linenum, _ = CloseExpression(clean_lines, start_linenum, nested_open_bracket_pos)
+
+            # If anything other closing statements or commas appear there is another parameter after the nested call
+            if not Search(r'\)(,|\)|;)*', nested_close_line):
+              error(filename, start_linenum, 'whitespace/indent', 4,
+                    'If parameters or arguments require a line break, each parameter should be put on its own line.')
+            # Skip to the end of the bracket
+            start_linenum = nested_close_linenum
+          else:
+            if(not Match('^\s*[^,]+,$', arg_line)):
+              error(filename, start_linenum, 'whitespace/indent', 4,
+                    'If parameters or arguments require a line break, each parameter should be put on its own line.')
+
+          start_linenum+=1
+        # For the final line we also need to check one parameter on it
+        # e.g. we require bracket on same line as last parameter
+        # foo(
+        #   x);
+        if not Search(r'^\s*[^,]+\)', close_line):
+          # If this is true, the we failed because we just had the close bracket
+          if Search(r'[^,]*\)', close_line):
+            error(filename, close_linenum, 'whitespace/indent', 4,
+                  'If parameters or arguments require a line break, the closing bracket should be on the same line as the final parameter')
+          else:
+            # In this case the problem is we had a bracket
+            # i.e. more than one parameter on the last line
+            error(filename, close_linenum, 'whitespace/indent', 4,
+                  'If parameters or arguments require a line break, each parameter should be put on its own line.')
+
+
   if (Search(r'\([^\)]*$', elided_prev) and initial_spaces-2 != prev_initial_spaces) and not Search(r'for|while|if|;', elided_prev):
     error(filename, linenum, 'whitespace/indent', 4,
           'Indent of wrapped parenthesized expression or parameter or argument list should be 2')