compilerplugins/clang/store/shouldreturnbool.cxx

   1 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
   2 /*
   3  * This file is part of the LibreOffice project.
   4  *
   5  * This Source Code Form is subject to the terms of the Mozilla Public
   6  * License, v. 2.0. If a copy of the MPL was not distributed with this
   7  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
   8  */
   9
  10 #include <string>
  11 #include <set>
  12 #include <iostream>
  13
  14 #include "config_clang.h"
  15
  16 #include "check.hxx"
  17 #include "compat.hxx"
  18 #include "plugin.hxx"
  19 #include "functionaddress.hxx"
  20
  21 /*
  22  Look for functions that only return 1 and/or 0, which sometimes indicates that the
  23  function should be returning bool.
  24
  25  Note that is partly a question of taste and code style, which is why this plugin is off by default.
  26 */
  27
  28 namespace
  29 {
  30 class ShouldReturnBool
  31     : public loplugin::FunctionAddress<loplugin::FilteringPlugin<ShouldReturnBool>>
  32 {
  33 public:
  34     explicit ShouldReturnBool(loplugin::InstantiationData const& data)
  35         : FunctionAddress(data)
  36     {
  37     }
  38
  39     virtual void run() override
  40     {
  41         if (!compiler.getLangOpts().CPlusPlus)
  42             return;
  43         StringRef fn(handler.getMainFileName());
  44         // functions used as function pointers
  45         if (loplugin::isSamePathname(fn, SRCDIR "/sal/rtl/alloc_cache.cxx"))
  46             return;
  47         // false +, slightly odd usage, but not wrong
  48         if (loplugin::isSamePathname(fn, SRCDIR "/libreofficekit/qa/tilebench/tilebench.cxx"))
  49             return;
  50         // template magic
  51         if (loplugin::isSamePathname(fn, SRCDIR "/vcl/source/gdi/bmpfast.cxx"))
  52             return;
  53         // fine
  54         if (loplugin::isSamePathname(fn, SRCDIR "/svl/unx/source/svdde/ddedummy.cxx"))
  55             return;
  56         if (loplugin::isSamePathname(fn, SRCDIR "/vcl/source/opengl/OpenGLHelper.cxx"))
  57             return;
  58         if (loplugin::isSamePathname(fn, SRCDIR "/svtools/source/misc/imap2.cxx"))
  59             return;
  60         if (loplugin::isSamePathname(fn, SRCDIR "/svx/source/dialog/docrecovery.cxx"))
  61             return;
  62         if (loplugin::isSamePathname(fn, SRCDIR "/hwpfilter/source/lexer.cxx"))
  63             return;
  64         if (loplugin::isSamePathname(fn, SRCDIR "/hwpfilter/source/grammar.cxx"))
  65             return;
  66         if (loplugin::isSamePathname(
  67                 fn, SRCDIR "/connectivity/source/drivers/odbc/ODatabaseMetaDataResultSet.cxx"))
  68             return;
  69         if (loplugin::isSamePathname(fn, SRCDIR "/dbaccess/source/ui/browser/dsEntriesNoExp.cxx"))
  70             return;
  71         if (loplugin::isSamePathname(fn, SRCDIR "/lotuswordpro/source/filter/explode.cxx"))
  72             return;
  73         if (loplugin::isSamePathname(fn, SRCDIR "/filter/source/graphicfilter/ipict/ipict.cxx"))
  74             return;
  75         if (loplugin::isSamePathname(fn, SRCDIR "/sc/source/core/data/dptabsrc.cxx"))
  76             return;
  77         if (loplugin::isSamePathname(fn, SRCDIR "/sc/source/ui/docshell/docsh3.cxx"))
  78             return;
  79         if (loplugin::isSamePathname(fn, SRCDIR "/sd/source/ui/dlg/masterlayoutdlg.cxx"))
  80             return;
  81         if (loplugin::isSamePathname(fn, SRCDIR "/sd/source/filter/ppt/pptinanimations.cxx"))
  82             return;
  83         if (loplugin::isSamePathname(fn, SRCDIR "/vcl/unx/generic/app/i18n_im.cxx"))
  84             return;
  85
  86         // callback
  87         if (loplugin::isSamePathname(fn, SRCDIR "/sax/source/expatwrap/sax_expat.cxx"))
  88             return;
  89         if (loplugin::isSamePathname(fn, SRCDIR "/xmlsecurity/source/xmlsec/xmlstreamio.cxx"))
  90             return;
  91         if (loplugin::isSamePathname(fn, SRCDIR "/sw/source/filter/ww8/ww8par.cxx"))
  92             return;
  93         if (loplugin::isSamePathname(fn, SRCDIR "/sw/source/filter/ww8/ww8par2.cxx"))
  94             return;
  95         if (loplugin::isSamePathname(fn, SRCDIR "/sw/source/filter/ww8/ww8par5.cxx"))
  96             return;
  97         // SaxWriterHelper::writeSequence a little weird
  98         if (loplugin::isSamePathname(fn, SRCDIR "/sax/source/expatwrap/saxwriter.cxx"))
  99             return;
 100         // main function
 101         if (loplugin::isSamePathname(fn, SRCDIR "/xmlsecurity/workben/pdfverify.cxx"))
 102             return;
 103
 104         TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
 105
 106         for (auto functionDecl : problemFunctions)
 107         {
 108             auto canonicalDecl = functionDecl->getCanonicalDecl();
 109             if (getFunctionsWithAddressTaken().find(canonicalDecl)
 110                 != getFunctionsWithAddressTaken().end())
 111                 continue;
 112             report(DiagnosticsEngine::Warning,
 113                    "only returning one or zero is an indication you want to return bool",
 114                    functionDecl->getBeginLoc())
 115                 << functionDecl->getSourceRange();
 116             if (canonicalDecl->getLocation() != functionDecl->getLocation())
 117             {
 118                 report(DiagnosticsEngine::Note, "canonical function declaration here",
 119                        canonicalDecl->getBeginLoc())
 120                     << canonicalDecl->getSourceRange();
 121             }
 122         }
 123     }
 124
 125     bool TraverseFunctionDecl(FunctionDecl*);
 126     bool TraverseCXXMethodDecl(CXXMethodDecl*);
 127     bool VisitReturnStmt(ReturnStmt const*);
 128
 129 private:
 130     bool mbInsideFunction = false;
 131     bool mbFunctionOnlyReturningOneOrZero = false;
 132     std::unordered_set<FunctionDecl const*> problemFunctions;
 133
 134     bool IsInteresting(FunctionDecl const*);
 135     void Report(FunctionDecl const*) const;
 136     bool isExprOneOrZero(Expr const*) const;
 137 };
 138
 139 bool ShouldReturnBool::TraverseFunctionDecl(FunctionDecl* functionDecl)
 140 {
 141     bool ret;
 142     if (IsInteresting(functionDecl))
 143     {
 144         mbInsideFunction = true;
 145         mbFunctionOnlyReturningOneOrZero = true;
 146         ret = FunctionAddress::TraverseFunctionDecl(functionDecl);
 147         mbInsideFunction = false;
 148         if (mbFunctionOnlyReturningOneOrZero)
 149             problemFunctions.insert(functionDecl);
 150     }
 151     else
 152         ret = FunctionAddress::TraverseFunctionDecl(functionDecl);
 153     return ret;
 154 }
 155
 156 bool ShouldReturnBool::TraverseCXXMethodDecl(CXXMethodDecl* methodDecl)
 157 {
 158     bool ret;
 159     if (IsInteresting(methodDecl))
 160     {
 161         mbInsideFunction = true;
 162         mbFunctionOnlyReturningOneOrZero = true;
 163         ret = FunctionAddress::TraverseCXXMethodDecl(methodDecl);
 164         mbInsideFunction = false;
 165         if (mbFunctionOnlyReturningOneOrZero)
 166             problemFunctions.insert(methodDecl);
 167     }
 168     else
 169         ret = FunctionAddress::TraverseCXXMethodDecl(methodDecl);
 170     return ret;
 171 }
 172
 173 bool ShouldReturnBool::IsInteresting(FunctionDecl const* functionDecl)
 174 {
 175     if (ignoreLocation(functionDecl))
 176         return false;
 177     // ignore stuff that forms part of the stable URE interface
 178     if (isInUnoIncludeFile(functionDecl))
 179         return false;
 180     if (functionDecl->getTemplatedKind() != FunctionDecl::TK_NonTemplate)
 181         return false;
 182     if (!functionDecl->isThisDeclarationADefinition())
 183         return false;
 184     if (functionDecl->isMain())
 185         return false;
 186     if (functionDecl->isExternC() || functionDecl->isInExternCContext())
 187         return false;
 188     auto methodDecl = dyn_cast<CXXMethodDecl>(functionDecl);
 189     if (methodDecl && methodDecl->isVirtual())
 190         return false;
 191     auto tc = loplugin::TypeCheck(functionDecl->getReturnType());
 192     if (tc.AnyBoolean() || tc.Void())
 193         return false;
 194     auto returnType = functionDecl->getReturnType();
 195     if (returnType->isEnumeralType() || !returnType->getUnqualifiedDesugaredType()->isIntegerType())
 196         return false;
 197     // Ignore functions that contains #ifdef-ery
 198     if (containsPreprocessingConditionalInclusion(functionDecl->getSourceRange()))
 199         return false;
 200
 201     // not sure what basegfx is doing here
 202     StringRef fileName{ getFilenameOfLocation(functionDecl->getLocation()) };
 203     if (loplugin::isSamePathname(fileName, SRCDIR "/include/basegfx/range/basicrange.hxx"))
 204         return false;
 205     // false +
 206     if (loplugin::isSamePathname(fileName, SRCDIR "/include/svl/macitem.hxx"))
 207         return false;
 208     if (loplugin::isSamePathname(fileName, SRCDIR "/lotuswordpro/source/filter/lwpcharsetmgr.hxx"))
 209         return false;
 210     if (loplugin::isSamePathname(fileName, SRCDIR "/sc/inc/dptabsrc.hxx"))
 211         return false;
 212
 213     return true;
 214 }
 215
 216 bool ShouldReturnBool::VisitReturnStmt(const ReturnStmt* returnStmt)
 217 {
 218     if (!mbInsideFunction)
 219         return true;
 220     if (!returnStmt->getRetValue())
 221         return true;
 222     if (loplugin::TypeCheck(returnStmt->getRetValue()->getType()).AnyBoolean())
 223         return true;
 224     if (!isExprOneOrZero(returnStmt->getRetValue()))
 225         mbFunctionOnlyReturningOneOrZero = false;
 226     return true;
 227 }
 228
 229 bool ShouldReturnBool::isExprOneOrZero(const Expr* arg) const
 230 {
 231     arg = arg->IgnoreParenCasts();
 232     // ignore this, it seems to trigger an infinite recursion
 233     if (isa<UnaryExprOrTypeTraitExpr>(arg))
 234     {
 235         return false;
 236     }
 237     APSInt x1;
 238     if (compat::EvaluateAsInt(arg, x1, compiler.getASTContext()))
 239     {
 240         return x1 == 1 || x1 == 0;
 241     }
 242     return false;
 243 }
 244
 245 loplugin::Plugin::Registration<ShouldReturnBool> X("shouldreturnbool", false);
 246 }
 247
 248 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */