Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt @@ -21,6 +21,7 @@ SizeofContainerCheck.cpp StaticAssertCheck.cpp StringIntegerAssignmentCheck.cpp + SuspiciousSemicolonCheck.cpp SwappedArgumentsCheck.cpp ThrowByValueCatchByReferenceCheck.cpp UndelegatedConstructor.cpp Index: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.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 @@ "misc-static-assert"); CheckFactories.registerCheck( "misc-string-integer-assignment"); + CheckFactories.registerCheck( + "misc-suspicious-semicolon"); CheckFactories.registerCheck( "misc-swapped-arguments"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.h +++ clang-tools-extra/trunk/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-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/misc/SuspiciousSemicolonCheck.cpp @@ -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("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"); + const bool IsIfStmt = isa(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 Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -61,6 +61,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-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-semicolon.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-suspicious-semicolon.rst +++ clang-tools-extra/trunk/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 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. Index: clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-semicolon.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-semicolon.cpp +++ clang-tools-extra/trunk/test/clang-tidy/misc-suspicious-semicolon.cpp @@ -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 { + } +}