loplugin:salbool: Implicit conversions from non-Boolean fundamental types

Change-Id: I67eac95686678e6f5a2d60798535b2c65a9ba5d7

4 files changed, 81 insertions, 24 deletions

diff --git a/compilerplugins/clang/implicitboolconversion.cxx b/compilerplugins/clang/implicitboolconversion.cxx
index 8c472125939f..1e4c5e29c0cf 100644
--- a/compilerplugins/clang/implicitboolconversion.cxx
+++ b/compilerplugins/clang/implicitboolconversion.cxx
@@ -17,6 +17,7 @@
 
 #include "compat.hxx"
 #include "plugin.hxx"
+#include "typecheck.hxx"
 
 #if CLANG_VERSION < 30700
 
@@ -83,26 +84,10 @@ bool areSameTypedef(QualType type1, QualType type2) {
     return t1 != nullptr && t2 != nullptr && t1->getDecl() == t2->getDecl();
 }
 
-bool isBool(QualType type, bool allowTypedefs = true) {
-    if (type->isBooleanType()) {
-        return true;
-    }
-    if (!allowTypedefs) {
-        return false;
-    }
-    TypedefType const * t2 = type->getAs<TypedefType>();
-    if (t2 == nullptr) {
-        return false;
-    }
-    std::string name(t2->getDecl()->getNameAsString());
-    return name == "sal_Bool" || name == "BOOL" || name == "Boolean"
-        || name == "FT_Bool" || name == "FcBool" || name == "GLboolean"
-        || name == "NPBool" || name == "UBool" || name == "dbus_bool_t"
-        || name == "gboolean" || name == "hb_bool_t" || name == "jboolean";
-}
-
 bool isBool(Expr const * expr, bool allowTypedefs = true) {
-    return isBool(expr->getType(), allowTypedefs);
+    auto t = expr->getType();
+    return allowTypedefs
+        ? bool(loplugin::TypeCheck(t).AnyBoolean()) : t->isBooleanType();
 }
 
 bool isMatchingBool(Expr const * expr, Expr const * comparisonExpr) {
@@ -209,7 +194,7 @@ bool isBoolExpr(Expr const * expr) {
             }
             stack.pop();
             if (stack.empty()) {
-                if (isBool(ty)) {
+                if (loplugin::TypeCheck(ty).AnyBoolean()) {
                     return true;
                 }
                 break;
@@ -379,8 +364,10 @@ bool ImplicitBoolConversion::TraverseCallExpr(CallExpr * expr) {
                                 auto const ta = dr->getTemplateArgs();
                                 if ((ta[0].getArgument().getKind()
                                      == TemplateArgument::Type)
-                                    && isBool(
-                                        ta[0].getTypeSourceInfo()->getType()))
+                                    && (loplugin::TypeCheck(
+                                            ta[0].getTypeSourceInfo()
+                                            ->getType())
+                                        .AnyBoolean()))
                                 {
                                     continue;
                                 }
@@ -442,7 +429,8 @@ bool ImplicitBoolConversion::TraverseCXXMemberCallExpr(CXXMemberCallExpr * expr)
                         //TODO: fix this superficial nonsense check:
                         if (ct->getNumArgs() >= 1
                             && ct->getArg(0).getKind() == TemplateArgument::Type
-                            && isBool(ct->getArg(0).getAsType()))
+                            && (loplugin::TypeCheck(ct->getArg(0).getAsType())
+                                .AnyBoolean()))
                         {
                             continue;
                         }
@@ -994,7 +982,8 @@ void ImplicitBoolConversion::checkCXXConstructExpr(
                             TemplateArgument const & arg = t1->getArg(
                                 i - ps->begin());
                             if (arg.getKind() == TemplateArgument::Type
-                                && isBool(arg.getAsType()))
+                                && (loplugin::TypeCheck(arg.getAsType())
+                                    .AnyBoolean()))
                             {
                                 continue;
                             }
diff --git a/compilerplugins/clang/salbool.cxx b/compilerplugins/clang/salbool.cxx
index f70135805fa7..f08b1c0da0c0 100644
--- a/compilerplugins/clang/salbool.cxx
+++ b/compilerplugins/clang/salbool.cxx
@@ -16,6 +16,7 @@
 
 #include "compat.hxx"
 #include "plugin.hxx"
+#include "typecheck.hxx"
 
 namespace {
 
@@ -143,6 +144,8 @@ public:
 
     bool VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr * expr);
 
+    bool VisitImplicitCastExpr(ImplicitCastExpr * expr);
+
     bool VisitReturnStmt(ReturnStmt const * stmt);
 
     bool WalkUpFromParmVarDecl(ParmVarDecl const * decl);
@@ -382,6 +385,53 @@ bool SalBool::VisitCXXFunctionalCastExpr(CXXFunctionalCastExpr * expr) {
     return true;
 }
 
+bool SalBool::VisitImplicitCastExpr(ImplicitCastExpr * expr) {
+    if (ignoreLocation(expr)) {
+        return true;
+    }
+    if (!isSalBool(expr->getType())) {
+        return true;
+    }
+    auto l = expr->getLocStart();
+    while (compiler.getSourceManager().isMacroArgExpansion(l)) {
+        l = compiler.getSourceManager().getImmediateMacroCallerLoc(l);
+    }
+    if (compat::isMacroBodyExpansion(compiler, l)) {
+        auto n = Lexer::getImmediateMacroName(
+            l, compiler.getSourceManager(), compiler.getLangOpts());
+        if (n == "sal_False" || n == "sal_True") {
+            return true;
+        }
+    }
+    auto e1 = expr->getSubExprAsWritten();
+    auto t = e1->getType();
+    if (!t->isFundamentalType() || loplugin::TypeCheck(t).AnyBoolean()) {
+        return true;
+    }
+    auto e2 = dyn_cast<ConditionalOperator>(e1);
+    if (e2 != nullptr) {
+        auto ic1 = dyn_cast<ImplicitCastExpr>(
+            e2->getTrueExpr()->IgnoreParens());
+        auto ic2 = dyn_cast<ImplicitCastExpr>(
+            e2->getFalseExpr()->IgnoreParens());
+        if (ic1 != nullptr && ic2 != nullptr
+            && ic1->getType()->isSpecificBuiltinType(BuiltinType::Int)
+            && (loplugin::TypeCheck(ic1->getSubExprAsWritten()->getType())
+                .AnyBoolean())
+            && ic2->getType()->isSpecificBuiltinType(BuiltinType::Int)
+            && (loplugin::TypeCheck(ic2->getSubExprAsWritten()->getType())
+                .AnyBoolean()))
+        {
+            return true;
+        }
+    }
+    report(
+        DiagnosticsEngine::Warning, "conversion from %0 to sal_Bool",
+        expr->getLocStart())
+        << t << expr->getSourceRange();
+    return true;
+}
+
 bool SalBool::VisitReturnStmt(ReturnStmt const * stmt) {
     // Just enough to avoid warnings in rtl_getUriCharClass (sal/rtl/uri.cxx),
     // which has
diff --git a/compilerplugins/clang/typecheck.cxx b/compilerplugins/clang/typecheck.cxx
index 800a2d295bc7..71f9994b6bc4 100644
--- a/compilerplugins/clang/typecheck.cxx
+++ b/compilerplugins/clang/typecheck.cxx
@@ -29,6 +29,22 @@ TerminalCheck TypeCheck::Char() const {
             || type_->isSpecificBuiltinType(clang::BuiltinType::Char_U)));
 }
 
+TerminalCheck TypeCheck::AnyBoolean() const {
+    if (type_->isBooleanType()) {
+        return TerminalCheck(true);
+    }
+    auto t = type_->getAs<clang::TypedefType>();
+    if (t == nullptr) {
+        return TerminalCheck(false);
+    }
+    auto n =t->getDecl()->getName();
+    return TerminalCheck(
+        n == "sal_Bool" || n == "BOOL" || n == "Boolean" || n == "FT_Bool"
+        || n == "FcBool" || n == "GLboolean" || n == "NPBool" || n == "UBool"
+        || n == "dbus_bool_t" || n == "gboolean" || n == "hb_bool_t"
+        || n == "jboolean");
+}
+
 TypeCheck TypeCheck::LvalueReference() const {
     if (!type_.isNull()) {
         auto const t = type_->getAs<clang::LValueReferenceType>();
diff --git a/compilerplugins/clang/typecheck.hxx b/compilerplugins/clang/typecheck.hxx
index c49adccae3c3..9862890a2fc7 100644
--- a/compilerplugins/clang/typecheck.hxx
+++ b/compilerplugins/clang/typecheck.hxx
@@ -31,6 +31,8 @@ public:
 
     TerminalCheck Char() const;
 
+    TerminalCheck AnyBoolean() const;
+
     TypeCheck Pointer() const;
 
     TypeCheck LvalueReference() const;