xazax.hun created this revision.
xazax.hun added a reviewer: alexfh.
xazax.hun added subscribers: dkrupp, cfe-commits.
This patch adds a checker to find semicolons that are probably unintended and
modify the semantics of the program. See the examples and the documentation for
more details.
http://reviews.llvm.org/D16535
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/SuspiciousSemicolonCheck.cpp
clang-tidy/misc/SuspiciousSemicolonCheck.h
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-suspicious-semicolon.rst
test/clang-tidy/misc-suspicious-semicolon.cpp
Index: test/clang-tidy/misc-suspicious-semicolon.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-suspicious-semicolon.cpp
@@ -0,0 +1,95 @@
+// 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 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;
+}
Index: docs/clang-tidy/checks/misc-suspicious-semicolon.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-suspicious-semicolon.rst
@@ -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 increased 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 identation 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).
+
+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 checker will assume that you know what you are doing, and will
+not raise a warning.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -58,6 +58,7 @@
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
Index: clang-tidy/misc/SuspiciousSemicolonCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/SuspiciousSemicolonCheck.h
@@ -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
Index: clang-tidy/misc/SuspiciousSemicolonCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/SuspiciousSemicolonCheck.cpp
@@ -0,0 +1,70 @@
+//===--- 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"))),
+ 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);
+
+ if (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;
+
+ const auto *Statement = Result.Nodes.getNodeAs<Stmt>("stmt");
+ unsigned BaseIndent = SM.getSpellingColumnNumber(Statement->getLocStart());
+ unsigned NewTokenIndent = SM.getSpellingColumnNumber(Token.getLocation());
+ unsigned NewTokenLine = SM.getSpellingLineNumber(Token.getLocation());
+
+ if (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
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -27,6 +27,7 @@
#include "SizeofContainerCheck.h"
#include "StaticAssertCheck.h"
#include "StringIntegerAssignmentCheck.h"
+#include "SuspiciousSemicolonCheck.h"
#include "SwappedArgumentsCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
#include "UndelegatedConstructor.h"
@@ -75,6 +76,8 @@
"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>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -19,6 +19,7 @@
SizeofContainerCheck.cpp
StaticAssertCheck.cpp
StringIntegerAssignmentCheck.cpp
+ SuspiciousSemicolonCheck.cpp
SwappedArgumentsCheck.cpp
ThrowByValueCatchByReferenceCheck.cpp
UndelegatedConstructor.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
