Cpplint fix #342
Merged
Cpplint fix #342
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peterschrammel
force-pushed
the
cpplint-fix
branch
from
1ee3e7b to
22d94df
Compare
tautschnig
requested changes
Dec 10, 2016
|
|
||
| gitRoot="$(dirname $0)/../.." | ||
|
|
||
| tmpStaging=".tmp_staging" |
There was a problem hiding this comment.
cleanup ()
{
rm -rf $tmpStaging
}
trap EXIT cleanup
tmpStaging=$(mktemp -d)
| if [ -e $tmpStaging ]; then | ||
| rm -rf $tmpStaging | ||
| fi | ||
| mkdir $tmpStaging |
There was a problem hiding this comment.
Those lines become unnecessary with the above proposal.
| retval=$? | ||
|
|
||
| # delete temporary copy of staging area | ||
| rm -rf $tmpStaging |
There was a problem hiding this comment.
Unnecessary with the above cleanup suggestion.
Merged
tautschnig
force-pushed
the
cpplint-fix
branch
from
f4f06e5 to
a90662f
Compare
To install the hook cp .githooks/pre-ommit .git/hooks/pre-commit or use a symbolic link. Then, when running git commit, you should get the linter output (if any) before being prompted to enter a commit message. To bypass the check (e.g. if there was a false positive), add the option --no-verify.
The linter script has exceptions to exclude incorrect detection of C style casts. Added the keyword catch to this list. Added a regression test to verify this.
In class declarations, it was previously warnining that the class identifier didn't end with a t because there was a space between the end of the identifier and the : (for inheritance). Now provides a warning that this space shouldn't be present and doesn't warn about the identifier.
If the line being tested is a single line comment, we don't bother checking whether the previous line is an incomplete function declaration).
This has been fixed, but previously it would complain about whitespace before the +
The make file runs the linter script found in the scripts/ folder on the CPP file found in the test folder.
The linter was picking up that while(...); looks like an empty while statement. However, unlike what the linter is expecting, we put while on a separate line to the closing brace for a do...while statement. Instead the linter now searches previous lines for a matching do statement. If it finds a while before the do then it obviously isn't related to this while.
We check for lines that look like `} while` and see if there is a matching do statement. If there is, we advise the while be put on a separate line.
When checking for spacing around operators, we need to be careful with <, > since are also brackets around template types. In these cases, a space after the > but not before is acceptable. The approach I've taken is to find the matching opening angled bracket and inspect the contents between them.
The if and the prevline weren't commented out, but since the variable wasn't being used it was just making it hard to identify the scope of the if statement.
Tidied up the if/else if/else errors (needs more tests)
Removed most of the old lint if checking as they want braces on the same
line so was mostly the opposite of what we wanted (which is in fact much
simpler).
Removed the old check for if(...) { as this is already picked up by the
braces at the end of line checks.
Added additional tests for different ways the if statement can be wrong
or right.
To deal with splitting lines containing operators up, if the operator starts the new line, we shouldn't require white space before it be removed. Instead, we only check for white space on the left side of operators if we've encountered some non-white-space characters that line. There are about 500 cases in the code where the operator starts the new line and 250 where it ends the previous line so not practical to require one over the other.
This fixes previous bug with brackets in comments (in a cleaner and more consistent way) as well as addressing the bug to do with brackets in comments. Weirdly, it was claiming to be using lines with strings removed, but printing the line clearly shows that `lines_without_raw_strings` still contains at least some strings.
Since we can't in general (without type analysis I think) tell the difference between a bit shift operator << and a stream manipulator << we don't warn about it. In some instances we can tell if it is a stream manipulator so we check for spaces there. I've added a known bug operator-spacing3 that would be the perfect report of errors.
Each function with a body should have a function comment header directly above it. Modified the lint script to check for its presence and validate that it meeds certain requirements. Updated existing tests to include the funciton header so they don't generate warnings for its absence. Also updated the test description as the line numbers of the deliberate errors have changed. Added new tests for the different ways the function header can be wrong
added 8 commits
December 23, 2016 15:19
Removed specifying whitespace of the contents of the comment header. It is fairly inconsistent across the code base and we are hopefully moving to doxygen style comments in the near future so no point wasting a lot of time fixing up these comments.
When checking for function comments we don't want to include the * or & in the name. Instead we allow after the space of the return type an optional * or & before accepting the function name.
Since operator functions can have non-word characters as part of their name (specifically, can have an opening bracket) we deal with these seperately in the explicit case where the functions name is operator. Since the function name can now contain non-word characters (e.g. an opening bracket) we must escape it before using it in the regex.
Previously we would always checkf for function header comments if we
found a function. Now if we find a ; before a { then we assume it is a
function declaration, otherwise we assume it is a function
implementation and therefore should have the function header comment.
This does mean that functions with bodies in header files are going to
require a function header comment.
tautschnig
force-pushed
the
cpplint-fix
branch
from
a90662f to
d590db4
Compare
tautschnig
approved these changes
Dec 23, 2016
smowton
pushed a commit
to smowton/cbmc
that referenced
this pull request
May 9, 2018
Added script for computation of statistics of a Java application.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes also a git hook that you can (optionally) install to check your diff automatically, locally on git commit.
To install the hook
cp .githooks/prepare-commit-msg .git/hooks/prepare-commit-msg
Then, when running git commit, you should get the linter output
(if any) before being prompted to enter a commit message.
To bypass the check (e.g. if there was a false positive),
add the option --no-verify.