Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint checks nested function calls broken over multiple lines #415

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CODING_STANDARD
Original file line number Diff line number Diff line change
Expand Up @@ -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 (=, +, ==, ...)
Expand Down
88 changes: 88 additions & 0 deletions regression/cpp-linter/multi-line-function-call1/main.cpp
Original file line number Diff line number Diff line change
@@ -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);
}

17 changes: 17 additions & 0 deletions regression/cpp-linter/multi-line-function-call1/test.desc
Original file line number Diff line number Diff line change
@@ -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$
--
67 changes: 67 additions & 0 deletions regression/cpp-linter/multi-line-function-call2/main.cpp
Original file line number Diff line number Diff line change
@@ -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);
}
11 changes: 11 additions & 0 deletions regression/cpp-linter/multi-line-function-call2/test.desc
Original file line number Diff line number Diff line change
@@ -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$
--
55 changes: 55 additions & 0 deletions regression/cpp-linter/multi-line-function-call3/main.cpp
Original file line number Diff line number Diff line change
@@ -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();
}
}
7 changes: 7 additions & 0 deletions regression/cpp-linter/multi-line-function-call3/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CORE
main.cpp

^Total errors found: 0$
^EXIT=0$
^SIGNAL=0$
--
76 changes: 73 additions & 3 deletions scripts/cpplint.py
Original file line number Diff line number Diff line change
Expand Up @@ -4686,9 +4686,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')
Expand Down