diff options
| -rw-r--r-- | compilerplugins/clang/cppunitassertequals.cxx | 142 | ||||
| -rw-r--r-- | compilerplugins/clang/test/cppunitassertequals.cxx | 56 | ||||
| -rw-r--r-- | compilerplugins/clang/test/cppunitassertequals.hxx | 23 | ||||
| -rw-r--r-- | solenv/CompilerTest_compilerplugins_clang.mk | 5 | ||||
| -rw-r--r-- | solenv/gbuild/CompilerTest.mk | 1 |
5 files changed, 183 insertions, 44 deletions
diff --git a/compilerplugins/clang/cppunitassertequals.cxx b/compilerplugins/clang/cppunitassertequals.cxx index 858ad9615cbe..37dff6acca97 100644 --- a/compilerplugins/clang/cppunitassertequals.cxx +++ b/compilerplugins/clang/cppunitassertequals.cxx @@ -7,11 +7,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include <cassert> -#include <string> -#include <iostream> -#include <fstream> -#include <set> #include "plugin.hxx" #include "check.hxx" @@ -29,74 +24,133 @@ public: virtual void run() override { - TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + if (compiler.getLangOpts().CPlusPlus) { + TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); + } } bool VisitCallExpr(const CallExpr*); - bool VisitBinaryOperator(const BinaryOperator*); + private: - void checkExpr(const Stmt* stmt); + void checkExpr( + SourceRange range, StringRef name, Expr const * expr, bool negated); + + void reportEquals(SourceRange range, StringRef name, bool negative); }; bool CppunitAssertEquals::VisitCallExpr(const CallExpr* callExpr) { - if (ignoreLocation(callExpr)) { + auto const decl = callExpr->getDirectCallee(); + if (decl == nullptr + || !(loplugin::DeclCheck(decl).Function("failIf").Struct("Asserter") + .Namespace("CppUnit").GlobalNamespace())) + { return true; } - if (callExpr->getDirectCallee() == nullptr) { + // Don't use callExpr->getLocStart() or callExpr->getExprLoc(), as those + // fall into a nested use of the CPPUNIT_NS macro; CallExpr::getRParenLoc + // happens to be readily available and cause good results: + auto loc = callExpr->getRParenLoc(); + while (compiler.getSourceManager().isMacroArgExpansion(loc)) { + loc = compiler.getSourceManager().getImmediateMacroCallerLoc(loc); + } + if (!compiler.getSourceManager().isMacroBodyExpansion(loc) + || ignoreLocation( + compiler.getSourceManager().getImmediateMacroCallerLoc(loc))) + { return true; } - const FunctionDecl* functionDecl = callExpr->getDirectCallee()->getCanonicalDecl(); - if (functionDecl->getOverloadedOperator() != OO_EqualEqual) { + auto name = Lexer::getImmediateMacroName( + loc, compiler.getSourceManager(), compiler.getLangOpts()); + if (name != "CPPUNIT_ASSERT" && name != "CPPUNIT_ASSERT_MESSAGE") { return true; } - checkExpr(callExpr); - return true; -} - -bool CppunitAssertEquals::VisitBinaryOperator(const BinaryOperator* binaryOp) -{ - if (ignoreLocation(binaryOp)) { + if (decl->getNumParams() != 3) { + report( + DiagnosticsEngine::Warning, + ("TODO: suspicious CppUnit::Asserter::failIf call with %0" + " parameters"), + callExpr->getExprLoc()) + << decl->getNumParams() << callExpr->getSourceRange(); return true; } - if (binaryOp->getOpcode() != BO_EQ) { + auto const e1 = callExpr->getArg(0)->IgnoreParenImpCasts(); + Expr const * e2 = nullptr; + if (auto const e3 = dyn_cast<UnaryOperator>(e1)) { + if (e3->getOpcode() == UO_LNot) { + e2 = e3->getSubExpr(); + } + } else if (auto const e4 = dyn_cast<CXXOperatorCallExpr>(e1)) { + if (e4->getOperator() == OO_Exclaim) { + e2 = e4->getArg(0); + } + } + if (e2 == nullptr) { + report( + DiagnosticsEngine::Warning, + ("TODO: suspicious CppUnit::Asserter::failIf call not wrapping" + " !(...)"), + callExpr->getExprLoc()) + << callExpr->getSourceRange(); return true; } - checkExpr(binaryOp); + auto range = compiler.getSourceManager().getImmediateExpansionRange(loc); + checkExpr( + SourceRange(range.first, range.second), name, + e2->IgnoreParenImpCasts(), false); return true; } -/* - * check that we are not a compound expression - */ -void CppunitAssertEquals::checkExpr(const Stmt* stmt) +void CppunitAssertEquals::checkExpr( + SourceRange range, StringRef name, Expr const * expr, bool negated) { - const Stmt* parent = parentStmt(stmt); - if (!parent || !isa<ParenExpr>(parent)) { - return; - } - parent = parentStmt(parent); - if (!parent || !isa<UnaryOperator>(parent)) { - return; - } - parent = parentStmt(parent); - if (!parent || !isa<CallExpr>(parent)) { + if (auto const e = dyn_cast<UnaryOperator>(expr)) { + if (e->getOpcode() == UO_LNot) { + checkExpr( + range, name, e->getSubExpr()->IgnoreParenImpCasts(), !negated); + } return; } - const CallExpr* callExpr = dyn_cast<CallExpr>(parent); - if (!callExpr || callExpr->getDirectCallee() == nullptr) { + if (auto const e = dyn_cast<BinaryOperator>(expr)) { + auto const op = e->getOpcode(); + if ((!negated && op == BO_EQ) || (negated && op == BO_NE)) { + reportEquals(range, name, op == BO_NE); + return; + } +#if 0 // TODO: enable later + if ((!negated && op == BO_LAnd) || (negated && op == BO_LOr)) { + report( + DiagnosticsEngine::Warning, + "rather split into two %0", e->getExprLoc()) + << name << range; + return; + } +#endif return; } - const FunctionDecl* functionDecl = callExpr->getDirectCallee()->getCanonicalDecl(); - if (!functionDecl || - !loplugin::DeclCheck(functionDecl).Function("failIf").Namespace("Asserter").Namespace("CppUnit").GlobalNamespace()) - { + if (auto const e = dyn_cast<CXXOperatorCallExpr>(expr)) { + auto const op = e->getOperator(); + if ((!negated && op == OO_EqualEqual) + || (negated && op == OO_ExclaimEqual)) + { + reportEquals(range, name, op == OO_ExclaimEqual); + return; + } return; } +} + +void CppunitAssertEquals::reportEquals( + SourceRange range, StringRef name, bool negative) +{ report( - DiagnosticsEngine::Warning, "rather call CPPUNIT_ASSERT_EQUALS", - stmt->getSourceRange().getBegin()) - << stmt->getSourceRange(); + DiagnosticsEngine::Warning, + ("rather call" + " %select{CPPUNIT_ASSERT_EQUAL|CPPUNIT_ASSERT_EQUAL_MESSAGE}0 (or" + " rewrite as an explicit operator %select{==|!=}1 call when the" + " operator itself is the topic)"), + range.getBegin()) + << (name == "CPPUNIT_ASSERT_MESSAGE") << negative << range; } loplugin::Plugin::Registration< CppunitAssertEquals > X("cppunitassertequals"); diff --git a/compilerplugins/clang/test/cppunitassertequals.cxx b/compilerplugins/clang/test/cppunitassertequals.cxx new file mode 100644 index 000000000000..239de58853b3 --- /dev/null +++ b/compilerplugins/clang/test/cppunitassertequals.cxx @@ -0,0 +1,56 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#include "sal/config.h" + +#include "cppunit/TestAssert.h" + +#include "cppunitassertequals.hxx" + +#define TEST1 CPPUNIT_ASSERT(b1 == b2) +#define TEST2(x) x + +void test(bool b1, bool b2, OUString const & s1, OUString const & s2, T t) { + CppUnit::Asserter::failIf(b1,""); +#if 0 // TODO: enable later + CPPUNIT_ASSERT(b1 && b2); // expected-error {{rather split into two CPPUNIT_ASSERT [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT((b1 && b2)); // expected-error {{rather split into two CPPUNIT_ASSERT [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(!(b1 || b2)); // expected-error {{rather split into two CPPUNIT_ASSERT [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(!(b1 && b2)); + CPPUNIT_ASSERT(!!(b1 && b2)); // expected-error {{rather split into two CPPUNIT_ASSERT [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT_MESSAGE("", b1 && b2); // expected-error {{rather split into two CPPUNIT_ASSERT_MESSAGE [loplugin:cppunitassertequals]}} +#endif + CPPUNIT_ASSERT(b1 == b2); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(b1 != b2); + CPPUNIT_ASSERT((b1 == b2)); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(!(b1 != b2)); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator != call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(!(b1 == b2)); + CPPUNIT_ASSERT(!!(b1 == b2)); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT_MESSAGE("", b1 == b2); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL_MESSAGE (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(s1 == s2); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(s1 != s2); + CPPUNIT_ASSERT((s1 == s2)); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(!(s1 != s2)); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator != call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + CPPUNIT_ASSERT(!(s1 == s2)); + CPPUNIT_ASSERT(!!(s1 == s2)); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + TEST1; // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + TEST2(CPPUNIT_ASSERT(b1 == b2)); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + TEST2(TEST1); // expected-error {{rather call CPPUNIT_ASSERT_EQUAL (or rewrite as an explicit operator == call when the operator itself is the topic) [loplugin:cppunitassertequals]}} + + // Useful when testing an equality iterator itself: + CPPUNIT_ASSERT(operator ==(s1, s1)); + CPPUNIT_ASSERT(t.operator ==(t)); + + // There might even be good reasons(?) not to warn inside explicit casts: + CPPUNIT_ASSERT(bool(b1 && b2)); + CPPUNIT_ASSERT(bool(b1 == b2)); + CPPUNIT_ASSERT(bool(s1 == s2)); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/compilerplugins/clang/test/cppunitassertequals.hxx b/compilerplugins/clang/test/cppunitassertequals.hxx new file mode 100644 index 000000000000..b844c8387e67 --- /dev/null +++ b/compilerplugins/clang/test/cppunitassertequals.hxx @@ -0,0 +1,23 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * This file is part of the LibreOffice project. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#ifndef INCLUDED_COMPILERPLUGINS_CLANG_TEST_CPPUNITASSERTEQUALS_HXX +#define INCLUDED_COMPILERPLUGINS_CLANG_TEST_CPPUNITASSERTEQUALS_HXX + +#include "sal/config.h" + +#include "rtl/ustring.hxx" + +struct T { bool operator ==(T); }; + +void test(bool b1, bool b2, OUString const & s1, OUString const & s2, T t); + +#endif + +/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */ diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk index 724a9ac07046..9f8810eb4057 100644 --- a/solenv/CompilerTest_compilerplugins_clang.mk +++ b/solenv/CompilerTest_compilerplugins_clang.mk @@ -10,6 +10,7 @@ $(eval $(call gb_CompilerTest_CompilerTest,compilerplugins_clang)) $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ + compilerplugins/clang/test/cppunitassertequals \ compilerplugins/clang/test/datamembershadow \ compilerplugins/clang/test/externvar \ compilerplugins/clang/test/finalprotected \ @@ -27,6 +28,10 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \ compilerplugins/clang/test/vclwidgets \ )) +$(eval $(call gb_CompilerTest_use_externals,compilerplugins_clang, \ + cppunit \ +)) + $(eval $(call gb_CompilerTest_use_udk_api,compilerplugins_clang)) # vim: set noet sw=4 ts=4: diff --git a/solenv/gbuild/CompilerTest.mk b/solenv/gbuild/CompilerTest.mk index a495cf368dda..e7f798ba0eae 100644 --- a/solenv/gbuild/CompilerTest.mk +++ b/solenv/gbuild/CompilerTest.mk @@ -42,6 +42,7 @@ $(eval $(foreach method, \ add_objcxxobjects \ add_cxxclrobject \ add_cxxclrobjects \ + use_externals \ use_udk_api \ , \ $(call gb_CompilerTest__forward_to_Linktarget,$(method)) \ |