[clang-tidy] Add a check to find unintended semicolons that changes the semantics.
Reviewers: hokein, alexfh Differential Revision: http://reviews.llvm.org/D16535 llvm-svn: 260503
This commit is contained in:
parent
6e2edc4b84
commit
8b6434e56e
|
@ -21,6 +21,7 @@ add_clang_library(clangTidyMiscModule
|
|||
SizeofContainerCheck.cpp
|
||||
StaticAssertCheck.cpp
|
||||
StringIntegerAssignmentCheck.cpp
|
||||
SuspiciousSemicolonCheck.cpp
|
||||
SwappedArgumentsCheck.cpp
|
||||
ThrowByValueCatchByReferenceCheck.cpp
|
||||
UndelegatedConstructor.cpp
|
||||
|
|
|
@ -29,6 +29,7 @@
|
|||
#include "SizeofContainerCheck.h"
|
||||
#include "StaticAssertCheck.h"
|
||||
#include "StringIntegerAssignmentCheck.h"
|
||||
#include "SuspiciousSemicolonCheck.h"
|
||||
#include "SwappedArgumentsCheck.h"
|
||||
#include "ThrowByValueCatchByReferenceCheck.h"
|
||||
#include "UndelegatedConstructor.h"
|
||||
|
@ -81,6 +82,8 @@ public:
|
|||
"misc-static-assert");
|
||||
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
|
||||
"misc-string-integer-assignment");
|
||||
CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
|
||||
"misc-suspicious-semicolon");
|
||||
CheckFactories.registerCheck<SwappedArgumentsCheck>(
|
||||
"misc-swapped-arguments");
|
||||
CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
|
||||
|
|
|
@ -0,0 +1,74 @@
|
|||
//===--- SuspiciousSemicolonCheck.cpp - clang-tidy-------------------------===//
|
||||
//
|
||||
// The LLVM Compiler Infrastructure
|
||||
//
|
||||
// This file is distributed under the University of Illinois Open Source
|
||||
// License. See LICENSE.TXT for details.
|
||||
//
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#include "../utils/LexerUtils.h"
|
||||
#include "SuspiciousSemicolonCheck.h"
|
||||
#include "clang/AST/ASTContext.h"
|
||||
#include "clang/ASTMatchers/ASTMatchFinder.h"
|
||||
|
||||
using namespace clang::ast_matchers;
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace misc {
|
||||
|
||||
void SuspiciousSemicolonCheck::registerMatchers(MatchFinder *Finder) {
|
||||
Finder->addMatcher(
|
||||
stmt(anyOf(ifStmt(hasThen(nullStmt().bind("semi")),
|
||||
unless(hasElse(stmt()))),
|
||||
forStmt(hasBody(nullStmt().bind("semi"))),
|
||||
cxxForRangeStmt(hasBody(nullStmt().bind("semi"))),
|
||||
whileStmt(hasBody(nullStmt().bind("semi")))))
|
||||
.bind("stmt"),
|
||||
this);
|
||||
}
|
||||
|
||||
void SuspiciousSemicolonCheck::check(const MatchFinder::MatchResult &Result) {
|
||||
const auto *Semicolon = Result.Nodes.getNodeAs<NullStmt>("semi");
|
||||
SourceLocation LocStart = Semicolon->getLocStart();
|
||||
|
||||
if (LocStart.isMacroID())
|
||||
return;
|
||||
|
||||
ASTContext &Ctxt = *Result.Context;
|
||||
auto Token = lexer_utils::getPreviousNonCommentToken(Ctxt, LocStart);
|
||||
auto &SM = *Result.SourceManager;
|
||||
unsigned SemicolonLine = SM.getSpellingLineNumber(LocStart);
|
||||
|
||||
const auto *Statement = Result.Nodes.getNodeAs<Stmt>("stmt");
|
||||
const bool IsIfStmt = isa<IfStmt>(Statement);
|
||||
|
||||
if (!IsIfStmt &&
|
||||
SM.getSpellingLineNumber(Token.getLocation()) != SemicolonLine)
|
||||
return;
|
||||
|
||||
SourceLocation LocEnd = Semicolon->getLocEnd();
|
||||
FileID FID = SM.getFileID(LocEnd);
|
||||
llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, LocEnd);
|
||||
Lexer Lexer(SM.getLocForStartOfFile(FID), Ctxt.getLangOpts(),
|
||||
Buffer->getBufferStart(), SM.getCharacterData(LocEnd) + 1,
|
||||
Buffer->getBufferEnd());
|
||||
if (Lexer.LexFromRawLexer(Token))
|
||||
return;
|
||||
|
||||
unsigned BaseIndent = SM.getSpellingColumnNumber(Statement->getLocStart());
|
||||
unsigned NewTokenIndent = SM.getSpellingColumnNumber(Token.getLocation());
|
||||
unsigned NewTokenLine = SM.getSpellingLineNumber(Token.getLocation());
|
||||
|
||||
if (!IsIfStmt && NewTokenIndent <= BaseIndent &&
|
||||
Token.getKind() != tok::l_brace && NewTokenLine != SemicolonLine)
|
||||
return;
|
||||
|
||||
diag(LocStart, "potentially unintended semicolon")
|
||||
<< FixItHint::CreateRemoval(SourceRange(LocStart, LocEnd));
|
||||
}
|
||||
|
||||
} // namespace misc
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
|
@ -0,0 +1,36 @@
|
|||
//===--- SuspiciousSemicolonCheck.h - clang-tidy-----------------*- C++ -*-===//
|
||||
//
|
||||
// The LLVM Compiler Infrastructure
|
||||
//
|
||||
// This file is distributed under the University of Illinois Open Source
|
||||
// License. See LICENSE.TXT for details.
|
||||
//
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_SEMICOLON_H
|
||||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_SEMICOLON_H
|
||||
|
||||
#include "../ClangTidy.h"
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace misc {
|
||||
|
||||
/// This check finds semicolon that modifies the meaning of the program
|
||||
/// unintendedly.
|
||||
///
|
||||
/// For the user-facing documentation see:
|
||||
/// http://clang.llvm.org/extra/clang-tidy/checks/misc-suspicious-semicolon.html
|
||||
class SuspiciousSemicolonCheck : public ClangTidyCheck {
|
||||
public:
|
||||
SuspiciousSemicolonCheck(StringRef Name, ClangTidyContext *Context)
|
||||
: ClangTidyCheck(Name, Context) {}
|
||||
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
|
||||
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
|
||||
};
|
||||
|
||||
} // namespace misc
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
||||
|
||||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_SEMICOLON_H
|
|
@ -61,6 +61,7 @@ Clang-Tidy Checks
|
|||
misc-sizeof-container
|
||||
misc-static-assert
|
||||
misc-string-integer-assignment
|
||||
misc-suspicious-semicolon
|
||||
misc-swapped-arguments
|
||||
misc-throw-by-value-catch-by-reference
|
||||
misc-undelegated-constructor
|
||||
|
|
|
@ -0,0 +1,72 @@
|
|||
.. title:: clang-tidy - misc-suspicious-semicolon
|
||||
|
||||
misc-suspicious-semicolon
|
||||
=========================
|
||||
|
||||
Finds most instances of stray semicolons that unexpectedly alter the meaning of
|
||||
the code. More specifically, it looks for `if`, `while`, `for` and `for-range`
|
||||
statements whose body is a single semicolon, and then analyzes the context of
|
||||
the code (e.g. indentation) in an attempt to determine whether that is
|
||||
intentional.
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
if(x < y);
|
||||
{
|
||||
x++;
|
||||
}
|
||||
|
||||
Here the body of the `if` statement consists of only the semicolon at the end of
|
||||
the first line, and `x` will be incremented regardless of the condition.
|
||||
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
while((line = readLine(file)) != NULL);
|
||||
processLine(line);
|
||||
|
||||
As a result of this code, `processLine()` will only be called once, when the
|
||||
`while` loop with the empty body exits with `line == NULL`. The indentation of
|
||||
the code indicates the intention of the programmer.
|
||||
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
if(x >= y);
|
||||
x -= y;
|
||||
|
||||
While the indentation does not imply any nesting, there is simply no valid
|
||||
reason to have an `if` statement with an empty body (but it can make sense for
|
||||
a loop). So this check issues a warning for the code above.
|
||||
|
||||
To solve the issue remove the stray semicolon or in case the empty body is
|
||||
intentional, reflect this using code indentation or put the semicolon in a new
|
||||
line. For example:
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
while(readWhitespace());
|
||||
Token t = readNextToken();
|
||||
|
||||
Here the second line is indented in a way that suggests that it is meant to be
|
||||
the body of the `while` loop - whose body is in fact empty, becasue of the
|
||||
semicolon at the end of the first line.
|
||||
|
||||
Either remove the indentation from the second line:
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
while(readWhitespace());
|
||||
Token t = readNextToken();
|
||||
|
||||
... or move the semicolon from the end of the first line to a new line:
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
while(readWhitespace())
|
||||
;
|
||||
|
||||
Token t = readNextToken();
|
||||
|
||||
In this case the check will assume that you know what you are doing, and will
|
||||
not raise a warning.
|
|
@ -0,0 +1,117 @@
|
|||
// RUN: %check_clang_tidy %s misc-suspicious-semicolon %t
|
||||
|
||||
int x = 5;
|
||||
|
||||
void nop();
|
||||
|
||||
void correct1()
|
||||
{
|
||||
if(x < 5) nop();
|
||||
}
|
||||
|
||||
void correct2()
|
||||
{
|
||||
if(x == 5)
|
||||
nop();
|
||||
}
|
||||
|
||||
void correct3()
|
||||
{
|
||||
if(x > 5)
|
||||
{
|
||||
nop();
|
||||
}
|
||||
}
|
||||
|
||||
void fail1()
|
||||
{
|
||||
if(x > 5); nop();
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: potentially unintended semicolon [misc-suspicious-semicolon]
|
||||
// CHECK-FIXES: if(x > 5) nop();
|
||||
}
|
||||
|
||||
void fail2()
|
||||
{
|
||||
if(x == 5);
|
||||
nop();
|
||||
// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: potentially unintended semicolon [misc-suspicious-semicolon]
|
||||
// CHECK-FIXES: if(x == 5){{$}}
|
||||
}
|
||||
|
||||
void fail3()
|
||||
{
|
||||
if(x < 5);
|
||||
{
|
||||
nop();
|
||||
}
|
||||
// CHECK-MESSAGES: :[[@LINE-4]]:11: warning: potentially unintended semicolon
|
||||
// CHECK-FIXES: if(x < 5){{$}}
|
||||
}
|
||||
|
||||
void correct4()
|
||||
{
|
||||
while(x % 5 == 1);
|
||||
nop();
|
||||
}
|
||||
|
||||
void correct5()
|
||||
{
|
||||
for(int i = 0; i < x; ++i)
|
||||
;
|
||||
}
|
||||
|
||||
void fail4()
|
||||
{
|
||||
for(int i = 0; i < x; ++i);
|
||||
nop();
|
||||
// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: potentially unintended semicolon
|
||||
// CHECK-FIXES: for(int i = 0; i < x; ++i){{$}}
|
||||
}
|
||||
|
||||
void fail5()
|
||||
{
|
||||
if(x % 5 == 1);
|
||||
nop();
|
||||
// CHECK-MESSAGES: :[[@LINE-2]]:16: warning: potentially unintended semicolon
|
||||
// CHECK-FIXES: if(x % 5 == 1){{$}}
|
||||
}
|
||||
|
||||
void fail6() {
|
||||
int a = 0;
|
||||
if (a != 0) {
|
||||
} else if (a != 1);
|
||||
a = 2;
|
||||
// CHECK-MESSAGES: :[[@LINE-2]]:21: warning: potentially unintended semicolon
|
||||
// CHECK-FIXES: } else if (a != 1){{$}}
|
||||
}
|
||||
|
||||
void fail7() {
|
||||
if (true)
|
||||
;
|
||||
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: potentially unintended semicolon
|
||||
}
|
||||
|
||||
void correct6()
|
||||
{
|
||||
do; while(false);
|
||||
}
|
||||
|
||||
int correct7()
|
||||
{
|
||||
int t_num = 0;
|
||||
char c = 'b';
|
||||
char *s = "a";
|
||||
if (s == "(" || s != "'" || c == '"') {
|
||||
t_num += 3;
|
||||
return (c == ')' && c == '\'');
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
void correct8() {
|
||||
if (true)
|
||||
;
|
||||
else {
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue