From 6e226f5ed86efe942d5c49112e2115a70203edce Mon Sep 17 00:00:00 2001 From: Bjoern Michaelsen Date: Wed, 19 Nov 2014 19:26:11 +0100 Subject: [PATCH] also look for cascading assignments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I7fa18403698530eec379589c76631f0aa367b215 Reviewed-on: https://gerrit.libreoffice.org/12991 Reviewed-by: Björn Michaelsen Tested-by: Björn Michaelsen --- .../clang/store/cascadingassignop.cxx | 94 +++++++++++++++++++ .../clang/store/cascadingassignop.hxx | 40 ++++++++ 2 files changed, 134 insertions(+) create mode 100644 compilerplugins/clang/store/cascadingassignop.cxx create mode 100644 compilerplugins/clang/store/cascadingassignop.hxx diff --git a/compilerplugins/clang/store/cascadingassignop.cxx b/compilerplugins/clang/store/cascadingassignop.cxx new file mode 100644 index 000000000000..0a88b0ca6402 --- /dev/null +++ b/compilerplugins/clang/store/cascadingassignop.cxx @@ -0,0 +1,94 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * Based on LLVM/Clang. + * + * This file is distributed under the University of Illinois Open Source + * License. See LICENSE.TXT for details. + * + */ + +#include "cascadingassignop.hxx" + +/* +This is a compile check. + +It checks for complex statements with conditional operators in conditional +operators, which are errorprone, e.g. + Thing foo = IsBar() ? ( IsBaz() ? b1 : b2 ) : b3; + +However, it finds 556 cases in sw/source alone, thus likely needs some more +restricting, e.g. by checking for multiline conditional operator statements or +a certain length in characters (but that needs the Context/SourceManager, which +I havent played with yet). +*/ + +// the value is rather arbitrary, but code above this number of stmts begins to +// be smelly +static const int stmtlimit = 20; + +namespace loplugin +{ + +struct WalkCounter +{ + int stmtcount; + bool cascading; + bool conditionals; +}; + +// Ctor, nothing special, pass the argument(s). +CascadingAssignOp::CascadingAssignOp( const InstantiationData& data ) + : Plugin( data ) +{ +} + +// Perform the actual action. +void CascadingAssignOp::run() +{ + // Traverse the whole AST of the translation unit (i.e. examine the whole source file). + // The Clang AST helper class will call VisitReturnStmt for every return statement. + TraverseDecl( compiler.getASTContext().getTranslationUnitDecl()); +} + +void CascadingAssignOp::Walk( const Stmt* stmt, WalkCounter& c ) +{ + for(Stmt::const_child_iterator it = stmt->child_begin(); it != stmt->child_end(); ++it) + { + ++c.stmtcount; + const BinaryOperator* binop = dyn_cast< BinaryOperator >( *it ); + if ( binop ) + { + if ( (binop->isAssignmentOp() || binop->isCompoundAssignmentOp())) + c.cascading = true; + if ( dyn_cast< AbstractConditionalOperator >( binop ) || binop->isLogicalOp()) + c.conditionals = true; + } + Walk(*it, c); + } +} + +bool CascadingAssignOp::VisitStmt( const Stmt* stmt ) +{ + const BinaryOperator* binop = dyn_cast< BinaryOperator >( stmt ); + if ( binop && (binop->isAssignmentOp() || binop->isCompoundAssignmentOp())) + { + WalkCounter c = { 0, false, false }; + Walk(binop, c); + if(c.cascading && c.conditionals && c.stmtcount >= stmtlimit) + { + std::string msg("cascading assign operator mixing in conditionals, complexity: "); + msg.append(std::to_string(c.stmtcount)); + report( DiagnosticsEngine::Warning, msg, binop->getLocStart()); + } + } + return true; +} + +// Register the plugin action with the LO plugin handling. +static Plugin::Registration< CascadingAssignOp > X( "cascadingassignop" ); + +} // namespace loplugin + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/store/cascadingassignop.hxx b/compilerplugins/clang/store/cascadingassignop.hxx new file mode 100644 index 000000000000..fa776c7a3ba4 --- /dev/null +++ b/compilerplugins/clang/store/cascadingassignop.hxx @@ -0,0 +1,40 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* + * This file is part of the LibreOffice project. + * + * Based on LLVM/Clang. + * + * This file is distributed under the University of Illinois Open Source + * License. See LICENSE.TXT for details. + * + */ + +#ifndef CASCADINGCONDOP_H +#define CASCADINGCONDOP_H + +#include "plugin.hxx" + +namespace loplugin +{ + +struct WalkCounter; + +// The class implementing the plugin action. +class CascadingAssignOp + // Inherits from the Clang class that will allow examing the Clang AST tree (i.e. syntax tree). + : public RecursiveASTVisitor< CascadingAssignOp > + // And the base class for LO Clang plugins. + , public Plugin + { + public: + CascadingAssignOp( const InstantiationData& data ); + virtual void run() override; + void Walk( const Stmt* stmt, WalkCounter& c ); + bool VisitStmt( const Stmt* stmt ); + }; + +} // namespace loplugin + +#endif // CASCADINGCONDOP_H + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */