diff --git a/CODING_STANDARD b/CODING_STANDARD index 3466cd05c99..41b9bdd8679 100644 --- a/CODING_STANDARD +++ b/CODING_STANDARD @@ -17,6 +17,9 @@ Whitespaces: Exceptions: Spaces around &&, || and << - Space after comma (parameter lists, argument lists, ...) - Space after colon inside 'for' +- For pointers and references, the */& should be attached to the variable name + as oppposed to the tyep. E.g. for a pointer to an int the syntax would be: + `int *x;` - No whitespaces at end of line - No whitespaces in blank lines - Put argument lists on next line (and ident 2 spaces) if too long diff --git a/regression/cpp-linter/class-decl-space/main.cpp b/regression/cpp-linter/class-decl-space/main.cpp index 97c7db3a835..6f0ce90641e 100644 --- a/regression/cpp-linter/class-decl-space/main.cpp +++ b/regression/cpp-linter/class-decl-space/main.cpp @@ -8,3 +8,36 @@ Author: Thomas Kiley, thomas@diffblue.com class temp_classt : public base_classt {} + +class another_class : public base_classt +{} + +class more_class:public base_classt +{} + +class nonderived +{} + +class nonderivedt +{} + +#define ID_property_class dstring(23, 0) + +int foo(class define); + +int foo(class definet); + +template +void bar(U t); + +template +void bar(U t); + +template < class U > +void bar(U t); + +class testt +{ + template + void bar(U t); +} diff --git a/regression/cpp-linter/class-decl-space/test.desc b/regression/cpp-linter/class-decl-space/test.desc index 87b8d462144..c3e5fd574b4 100644 --- a/regression/cpp-linter/class-decl-space/test.desc +++ b/regression/cpp-linter/class-decl-space/test.desc @@ -2,6 +2,13 @@ CORE main.cpp ^main\.cpp:9: There shouldn.t be a space between class identifier and : \[readability/identifiers\] \[4\]$ -^Total errors found: 1$ +^main\.cpp:12: There shouldn.t be a space between class identifier and : \[readability/identifiers\] \[4\]$ +^main\.cpp:12: Class or struct identifier should end with t \[readability/identifiers\] \[4\]$ +^main\.cpp:15: Class or struct identifier should end with t \[readability/identifiers\] \[4\]$ +^main\.cpp:18: Class or struct identifier should end with t \[readability/identifiers\] \[4\]$ +^main\.cpp:26: Class or struct identifier should end with t \[readability/identifiers\] \[4\]$ +^main\.cpp:30: Remove spaces around < \[whitespace/operators\] \[4\]$ +^main\.cpp:36: Remove spaces around < \[whitespace/operators\] \[4\]$ +^Total errors found: 8$ ^SIGNAL=0$ -- diff --git a/regression/cpp-linter/operator-spacing2/main.cpp b/regression/cpp-linter/operator-spacing2/main.cpp index 90db55a167e..66567aba343 100644 --- a/regression/cpp-linter/operator-spacing2/main.cpp +++ b/regression/cpp-linter/operator-spacing2/main.cpp @@ -24,8 +24,11 @@ static void fun() int x = 1<<4; + // Ideally this should produce an error, see operator-spacing3 status()<<"Adding CPROVER library ("<hash())-result; } diff --git a/regression/cpp-linter/pointer-type1/main.cpp b/regression/cpp-linter/pointer-type1/main.cpp new file mode 100644 index 00000000000..dabe24e7dc6 --- /dev/null +++ b/regression/cpp-linter/pointer-type1/main.cpp @@ -0,0 +1,37 @@ +/*******************************************************************\ + +Module: Lint Examples + +Author: Thomas Kiley, thomas@diffblue.com + +\*******************************************************************/ + +/*******************************************************************\ + +Function: fun + + Inputs: + + Outputs: + + Purpose: + +\*******************************************************************/ + +static void fun() +{ + bool *x=nullptr; // Valid + bool* x=nullptr; // Invalid + + int &x=nullptr; // Valid + int& x=nullptr; // Invalid + + int y=at*bt; // Valid + + // Probably valid - could be a pointer to type yt or a + // variable called yt multilied by z. Would have to know + // it is a function call rather than a function declaration + foo( + x, + yt*z); +} diff --git a/regression/cpp-linter/pointer-type1/test.desc b/regression/cpp-linter/pointer-type1/test.desc new file mode 100644 index 00000000000..548cc52eee1 --- /dev/null +++ b/regression/cpp-linter/pointer-type1/test.desc @@ -0,0 +1,8 @@ +CORE +main.cpp + +^main\.cpp:24: Pointer type name must have \* attached to the type name \[whitespace/operators\] \[4\]$ +^main\.cpp:27: Reference type name must have & attached to the type name \[whitespace/operators\] \[4\]$ +^Total errors found: 2$ +^SIGNAL=0$ +-- diff --git a/regression/cpp-linter/struct-inline-decl/main.cpp b/regression/cpp-linter/struct-inline-decl/main.cpp new file mode 100644 index 00000000000..7676ae04912 --- /dev/null +++ b/regression/cpp-linter/struct-inline-decl/main.cpp @@ -0,0 +1,24 @@ +/*******************************************************************\ + +Module: Lint Examples + +Author: Thomas Kiley, thomas@diffblue.com + +\*******************************************************************/ + +/*******************************************************************\ + +Function: fun + + Inputs: + + Outputs: + + Purpose: + +\*******************************************************************/ + +static void fun() +{ + bar(struct mystructt& struct_var); +} diff --git a/regression/cpp-linter/struct-inline-decl/test.desc b/regression/cpp-linter/struct-inline-decl/test.desc new file mode 100644 index 00000000000..4a53c26c870 --- /dev/null +++ b/regression/cpp-linter/struct-inline-decl/test.desc @@ -0,0 +1,8 @@ +CORE +main.cpp + + +^Total errors found: 0$ +^EXIT=0$ +^SIGNAL=0$ +-- diff --git a/scripts/cpplint.py b/scripts/cpplint.py index 20f4b4b3fce..e4d7916f6e3 100644 --- a/scripts/cpplint.py +++ b/scripts/cpplint.py @@ -1547,20 +1547,17 @@ def IsTemplateArgumentList_DB(clean_lines, linenum, pos): return True -def OpenExpression(clean_lines, linenum, pos): - """If input points to ) or } or ] or >, finds the position that opens it. - - If lines[linenum][pos] points to a ')' or '}' or ']' or '>', finds the - linenum/pos that correspond to the closing of the expression. - - Essentially a mirror of what CloseExpression does +def ForceOpenExpression(clean_lines, linenum, pos, bracket): + """Find an opening bracket matching the specified closing bracket - TODO(tkiley): could probably be merged with CloseExpression + Search starting at the position for a matching closing bracket of the + same type as bracket. Args: clean_lines: A CleansedLines instance containing the file. linenum: The number of the line to check. pos: A position on the line. + bracket: The style of bracket to match Returns: A tuple (line, linenum, pos) pointer *to* the closing brace, or @@ -1568,13 +1565,10 @@ def OpenExpression(clean_lines, linenum, pos): strings and comments when matching; and the line we return is the 'cleansed' line at linenum. """ - line = clean_lines.elided[linenum] - if (line[pos] not in ')}]>'): - return (line, clean_lines.NumLines(), -1) # Check first line - (end_pos, stack) = FindStartOfExpressionInLine(line, pos , []) + (end_pos, stack) = FindStartOfExpressionInLine(line, pos , [bracket]) if end_pos > -1: return (line, linenum, end_pos) @@ -1589,6 +1583,37 @@ def OpenExpression(clean_lines, linenum, pos): # Did not find end of expression before end of file, give up return (line, clean_lines.NumLines(), -1) +def OpenExpression(clean_lines, linenum, pos): + """If input points to ) or } or ] or >, finds the position that opens it. + + If lines[linenum][pos] points to a ')' or '}' or ']' or '>', finds the + linenum/pos that correspond to the closing of the expression. + + Essentially a mirror of what CloseExpression does + + Calls ForceOpenExpression with the bracket type pointed at + + TODO(tkiley): could probably be merged with CloseExpression + + Args: + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + pos: A position on the line. + + Returns: + A tuple (line, linenum, pos) pointer *to* the closing brace, or + (line, len(lines), -1) if we never find a close. Note we ignore + strings and comments when matching; and the line we return is the + 'cleansed' line at linenum. + """ + + line = clean_lines.elided[linenum] + if (line[pos] not in ')}]>'): + return (line, clean_lines.NumLines(), -1) + else: + return ForceOpenExpression(clean_lines, linenum, pos-1, line[pos]) + + def FindEndOfExpressionInLine(line, startpos, stack): """Find the position just after the end of current parenthesized expression. @@ -3525,15 +3550,12 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # Otherwise not. Note we only check for non-spaces on *both* sides; # sometimes people put non-spaces on one side when aligning ='s among # many lines (not that this is behavior that I approve of...) - match1 = Search(r'[^\s]+[\s](=|>=|<=|==|!=|&=|\^=|\|=|\+=|\*=|\/=|\%=|>|<|\^|\+[^\+]|\/)', line) - match2 = Search(r'(=|>=|<=|==|!=|&=|\^=|\|=|\+=|\*=|\/=|\%=|>|<|!|\^|\+|\/)[\s]', line) - operator_pos, op_end = match1.span(1) if match1 else (-1, -1) - operator_pos2, op_end2 = match2.span(1) if match2 else (-1, -1) + left_hand_space_match = Search(r'[^\s]+[\s](=|>=|<=|==|!=|&=|\^=|\|=|\+=|\*=|\/=|\%=|>|<|\^|\+[^\+]|\/)', line) + right_hand_space_match = Search(r'(=|>=|<=|==|!=|&=|\^=|\|=|\+=|\*=|\/=|\%=|>|<|!|\^|\+|\/)[\s]', line) + operator_pos, op_end = left_hand_space_match.span(1) if left_hand_space_match else (-1, -1) + operator_pos2, op_end2 = right_hand_space_match.span(1) if right_hand_space_match else (-1, -1) - if match2 and match2.group(1) == '>': - res = IsTemplateArgumentList_DB(clean_lines, linenum, operator_pos2) - - if (match1 and + if (left_hand_space_match and not Search(r'<<', line) and # We ignore the left shift operator since might be a stream and then formatting rules go out of the window not Search(r'char \*', line) and # I don't know why this exception exists? not Search(r'\#include', line) and # I suppose file names could contains operators?? @@ -3545,12 +3567,12 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # and not Search(r'operator=', line)): error(filename, linenum, 'whitespace/operators', 4, # 'Missing spaces around =') - 'Remove spaces around %s' % match1.group(1)) - elif (match2 and + 'Remove spaces around %s' % left_hand_space_match.group(1)) + elif (right_hand_space_match and not Search(r'<<', line) and not IsTemplateArgumentList_DB(clean_lines, linenum, operator_pos2)): error(filename, linenum, 'whitespace/operators', 4, - 'Remove spaces around %s' % match2.group(0)) + 'Remove spaces around %s' % right_hand_space_match.group(0)) # these would cause too many false alarms if we checked for one-sided spaces only match = Search(r'\s(-|\*)(\s|$)', line) @@ -3562,10 +3584,16 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): if Search(r'(struct|class)\s[\w_]*\s+:', line): error(filename, linenum, 'readability/identifiers', 4, 'There shouldn\'t be a space between class identifier and :') -#check type definitions end with t - if Search(r'(struct|class)\s[\w_]*[^t^;^:^\s](;$|\s|:|$)', line) and not Search(r'^template <', line) and not Search(r'^template<', line): - error(filename, linenum, 'readability/identifiers', 4, - 'Class or struct identifier should end with t') + #check type definitions end with t + # Look for class declarations and check the final character is a t + # Exclude classes in side template argument lists (why?) + class_name_match = Search(r'\b(struct|class)\s(?P[\w_]+)', line) + if class_name_match: + class_name = class_name_match.group('class_name') + if not class_name.endswith('t'): + if not Search(r'\btemplate <', line) and not Search(r'\btemplate<', line): + error(filename, linenum, 'readability/identifiers', 4, + 'Class or struct identifier should end with t') if Search(r'(struct|class)\s[\w_]*_t(;$|\s|:|$)', line): error(filename, linenum, 'readability/identifiers', 4, @@ -3630,17 +3658,6 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): # error(filename, linenum, 'whitespace/operators', 3, # 'Missing spaces around >') - # We allow no-spaces around << when used like this: 10<<20, but - # not otherwise (particularly, not when used as streams) - # - # We also allow operators following an opening parenthesis, since - # those tend to be macros that deal with operators. - match = Search(r'(operator|[^\s(<])(?:L|UL|LL|ULL|l|ul|ll|ull)?<<([^\s,=<])', line) - if (match and not (match.group(1).isdigit() and match.group(2).isdigit()) and - not (match.group(1) == 'operator' and match.group(2) == ';')): - error(filename, linenum, 'whitespace/operators', 3, - 'Missing spaces around <<') - # We allow no-spaces around >> for almost anything. This is because # C++11 allows ">>" to close nested templates, which accounts for # most cases when ">>" is not followed by a space. @@ -3664,6 +3681,48 @@ def CheckOperatorSpacing(filename, clean_lines, linenum, error): error(filename, linenum, 'whitespace/operators', 4, 'Extra space for operator %s' % match.group(1)) +def CheckPointerReferenceSpacing(filename, clean_lines, linenum, error): + """Checks the */& are attached to variable names rather than types + + A pointer or reference type should have the */& attached to the variable + name rather than the type. I.e. a pointer to an int should be: + + int *var_name; + + Args: + filename: The name of the current file. + clean_lines: A CleansedLines instance containing the file. + linenum: The number of the line to check. + error: The function to call with any errors found. + """ + line = clean_lines.elided[linenum] + + # Find types by looking for word_names that are at the start of the line + # If there are followed by a * or & (after an optional ' const') + # then they appear to be a reference/pointer type with it attached to + # the type rather than the variable + wrong_type_match = Search(r'^\s*([\w_])+( const)?(?P[&\*])\s*\w', line) + + if wrong_type_match: + # This still could be a false positive, we must + # check that we are not inside brackets as then could be just be + # operators (multiply and logical AND) + pos = wrong_type_match.start(1) + _, _, opening_pos = ForceOpenExpression(clean_lines, linenum, pos, ')') + + # If we don't find a matching close brace then we aren't in brackets + # so can assume this is a type with the * or & attached to it + if opening_pos == -1: + op_type = wrong_type_match.group('type') + op_word = "" + if op_type == '*': + op_word = 'Pointer' + else: + op_word = 'Reference' + error(filename, linenum, 'whitespace/operators', 4, + op_word + ' type name must have ' + op_type + ' attached to the type name') + + def CheckParenthesisSpacing(filename, clean_lines, linenum, error): """Checks for horizontal spacing around parentheses. @@ -4679,6 +4738,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state, CheckAccess(filename, clean_lines, linenum, nesting_state, error) CheckSpacing(filename, clean_lines, linenum, nesting_state, error) CheckOperatorSpacing(filename, clean_lines, linenum, error) + CheckPointerReferenceSpacing(filename, clean_lines, linenum, error) CheckParenthesisSpacing(filename, clean_lines, linenum, error) CheckCommaSpacing(filename, clean_lines, linenum, error) CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error) @@ -5088,8 +5148,7 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension, break if not is_const: error(filename, linenum, 'runtime/arrays', 1, - 'Do not use variable-length arrays. Use an appropriately named ' - "('k' followed by CamelCase) compile-time constant for the size.") + 'Do not use variable-length arrays.') # Check for use of unnamed namespaces in header files. Registration # macros are typically OK, so we allow use of "namespace {" on lines