loplugin:unnecessaryoverride look for more patterns

like
   bool Foo::bar() {
       b = Super::bar();
       return b;
   }

Change-Id: I5e4c8005a3da7d7487c9039c35dcbb1d17e65bd7
Reviewed-on: https://gerrit.libreoffice.org/68418
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Tested-by: Noel Grandin <noel.grandin@collabora.co.uk>
This commit is contained in:
Noel Grandin 2019-02-27 08:49:39 +02:00
parent 4ca1789e57
commit 48dc1e48d0
20 changed files with 115 additions and 156 deletions

View File

@ -90,12 +90,6 @@ AreaChart::~AreaChart()
{
}
double AreaChart::getMaximumX()
{
double fMax = VSeriesPlotter::getMaximumX();
return fMax;
}
bool AreaChart::isSeparateStackingForDifferentSigns( sal_Int32 /*nDimensionIndex*/ )
{
// no separate stacking in all types of line/area charts

View File

@ -45,7 +45,6 @@ public:
virtual css::drawing::Direction3D getPreferredDiagramAspectRatio() const override;
// MinimumAndMaximumSupplier
virtual double getMaximumX() override;
virtual bool isSeparateStackingForDifferentSigns( sal_Int32 nDimensionIndex ) override;
virtual LegendSymbolStyle getLegendSymbolStyle() override;

View File

@ -177,4 +177,17 @@ struct Derived5 : public Base5_1, public Base5_2
void f1() { Base5_1::f1(); } // no warning expected
};
struct Base6_1
{
bool f1();
};
struct Derived6 : public Base6_1
{
bool
f1() // expected-error {{public function just calls public parent [loplugin:unnecessaryoverride]}}
{
bool ret = Base6_1::f1();
return ret;
}
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

View File

@ -101,6 +101,7 @@ public:
private:
const CXXMethodDecl * findOverriddenOrSimilarMethodInSuperclasses(const CXXMethodDecl *);
bool BaseCheckCallback(const CXXRecordDecl *BaseDefinition);
CXXMemberCallExpr const * extractCallExpr(Expr const *);
};
bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
@ -280,66 +281,58 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
//TODO: check for identical exception specifications
const CompoundStmt* compoundStmt = dyn_cast<CompoundStmt>(methodDecl->getBody());
if (!compoundStmt || compoundStmt->size() != 1)
if (!compoundStmt || compoundStmt->size() > 2)
return true;
const CXXMemberCallExpr* callExpr = nullptr;
if (methodDecl->getReturnType().getCanonicalType()->isVoidType())
if (compoundStmt->size() == 1)
{
if (auto const e = dyn_cast<Expr>(*compoundStmt->body_begin())) {
callExpr = dyn_cast<CXXMemberCallExpr>(e->IgnoreImplicit()->IgnoreParens());
}
}
else
{
auto returnStmt = dyn_cast<ReturnStmt>(*compoundStmt->body_begin());
if (returnStmt == nullptr) {
return true;
}
auto returnExpr = returnStmt->getRetValue();
if (returnExpr == nullptr) {
return true;
}
returnExpr = returnExpr->IgnoreImplicit();
// In something like
//
// Reference< XResultSet > SAL_CALL OPreparedStatement::executeQuery(
// const rtl::OUString& sql)
// throw(SQLException, RuntimeException, std::exception)
// {
// return OCommonStatement::executeQuery( sql );
// }
//
// look down through all the
//
// ReturnStmt
// `-ExprWithCleanups
// `-CXXConstructExpr
// `-MaterializeTemporaryExpr
// `-ImplicitCastExpr
// `-CXXBindTemporaryExpr
// `-CXXMemberCallExpr
//
// where the fact that the overriding and overridden function have identical
// return types makes us confident that all we need to check here is whether
// there's an (arbitrary, one-argument) CXXConstructorExpr and
// CXXBindTemporaryExpr in between:
if (auto ctorExpr = dyn_cast<CXXConstructExpr>(returnExpr)) {
if (ctorExpr->getNumArgs() == 1) {
auto tempExpr1 = ctorExpr->getArg(0)->IgnoreImplicit();
if (auto tempExpr2 = dyn_cast<CXXBindTemporaryExpr>(tempExpr1))
{
returnExpr = tempExpr2->getSubExpr();
}
else if (auto tempExpr2 = dyn_cast<CXXMemberCallExpr>(tempExpr1))
{
returnExpr = tempExpr2;
}
if (methodDecl->getReturnType().getCanonicalType()->isVoidType())
{
if (auto const e = dyn_cast<Expr>(*compoundStmt->body_begin())) {
callExpr = dyn_cast<CXXMemberCallExpr>(e->IgnoreImplicit()->IgnoreParens());
}
}
callExpr = dyn_cast<CXXMemberCallExpr>(returnExpr->IgnoreParenImpCasts());
else
{
auto returnStmt = dyn_cast<ReturnStmt>(*compoundStmt->body_begin());
if (returnStmt == nullptr) {
return true;
}
auto returnExpr = returnStmt->getRetValue();
if (returnExpr == nullptr) {
return true;
}
callExpr = extractCallExpr(returnExpr);
}
}
else if (!methodDecl->getReturnType().getCanonicalType()->isVoidType())
{
/** handle constructions like
bool foo() {
bool ret = Base::foo();
return ret;
}
*/
auto bodyIt = compoundStmt->body_begin();
auto declStmt = dyn_cast<DeclStmt>(*bodyIt);
if (!declStmt || !declStmt->isSingleDecl())
return true;
auto varDecl = dyn_cast<VarDecl>(declStmt->getSingleDecl());
++bodyIt;
auto returnStmt = dyn_cast<ReturnStmt>(*bodyIt);
if (!varDecl || !returnStmt)
return true;
Expr const * retValue = returnStmt->getRetValue()->IgnoreParenImpCasts();
if (auto exprWithCleanups = dyn_cast<ExprWithCleanups>(retValue))
retValue = exprWithCleanups->getSubExpr()->IgnoreParenImpCasts();
if (auto constructExpr = dyn_cast<CXXConstructExpr>(retValue)) {
if (constructExpr->getNumArgs() == 1)
retValue = constructExpr->getArg(0)->IgnoreParenImpCasts();
}
if (!isa<DeclRefExpr>(retValue))
return true;
callExpr = extractCallExpr(varDecl->getInit());
}
if (!callExpr || callExpr->getMethodDecl() != overriddenMethodDecl)
@ -362,6 +355,18 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
return true;
}
const CXXMethodDecl* pOther = nullptr;
if (methodDecl->getCanonicalDecl()->getLocation() != methodDecl->getLocation())
pOther = methodDecl->getCanonicalDecl();
if (pOther) {
StringRef aFileName = getFileNameOfSpellingLoc(
compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(pOther)));
// SFX_DECL_CHILDWINDOW_WITHID macro
if (loplugin::isSamePathname(aFileName, SRCDIR "/include/sfx2/childwin.hxx"))
return true;
}
report(
DiagnosticsEngine::Warning, "%0%1 function just calls %2 parent",
methodDecl->getLocation())
@ -466,6 +471,49 @@ const CXXMethodDecl* UnnecessaryOverride::findOverriddenOrSimilarMethodInSupercl
return similarMethod;
}
CXXMemberCallExpr const * UnnecessaryOverride::extractCallExpr(Expr const *returnExpr)
{
returnExpr = returnExpr->IgnoreImplicit();
// In something like
//
// Reference< XResultSet > SAL_CALL OPreparedStatement::executeQuery(
// const rtl::OUString& sql)
// throw(SQLException, RuntimeException, std::exception)
// {
// return OCommonStatement::executeQuery( sql );
// }
//
// look down through all the
//
// ReturnStmt
// `-ExprWithCleanups
// `-CXXConstructExpr
// `-MaterializeTemporaryExpr
// `-ImplicitCastExpr
// `-CXXBindTemporaryExpr
// `-CXXMemberCallExpr
//
// where the fact that the overriding and overridden function have identical
// return types makes us confident that all we need to check here is whether
// there's an (arbitrary, one-argument) CXXConstructorExpr and
// CXXBindTemporaryExpr in between:
if (auto ctorExpr = dyn_cast<CXXConstructExpr>(returnExpr)) {
if (ctorExpr->getNumArgs() == 1) {
auto tempExpr1 = ctorExpr->getArg(0)->IgnoreImplicit();
if (auto tempExpr2 = dyn_cast<CXXBindTemporaryExpr>(tempExpr1))
{
returnExpr = tempExpr2->getSubExpr();
}
else if (auto tempExpr2 = dyn_cast<CXXMemberCallExpr>(tempExpr1))
{
returnExpr = tempExpr2;
}
}
}
return dyn_cast<CXXMemberCallExpr>(returnExpr->IgnoreParenImpCasts());
}
loplugin::Plugin::Registration< UnnecessaryOverride > X("unnecessaryoverride", true);

View File

@ -220,13 +220,6 @@ SvXMLStyleContext *OTableStylesContext::CreateStyleStyleChildContext(
return pStyle;
}
Reference < XNameContainer >
OTableStylesContext::GetStylesContainer( sal_uInt16 nFamily ) const
{
Reference < XNameContainer > xStyles = SvXMLStylesContext::GetStylesContainer(nFamily);
return xStyles;
}
OUString OTableStylesContext::GetServiceName( sal_uInt16 nFamily ) const
{
OUString sServiceName = SvXMLStylesContext::GetServiceName(nFamily);

View File

@ -103,9 +103,6 @@ namespace dbaxml
virtual rtl::Reference < SvXMLImportPropertyMapper > GetImportPropertyMapper(
sal_uInt16 nFamily ) const override;
virtual css::uno::Reference <
css::container::XNameContainer >
GetStylesContainer( sal_uInt16 nFamily ) const override;
virtual OUString GetServiceName( sal_uInt16 nFamily ) const override;
sal_Int32 GetIndex(const sal_Int16 nContextID);

View File

@ -40,9 +40,6 @@ public:
// css::awt::XView
void SAL_CALL draw( sal_Int32 nX, sal_Int32 nY ) override;
// css::awt::XDevice,
css::awt::DeviceInfo SAL_CALL getInfo() override;
// css::awt::grid::XTabPageContainer
virtual ::sal_Int16 SAL_CALL getActiveTabPageID() override;
virtual void SAL_CALL setActiveTabPageID( ::sal_Int16 _activetabpageid ) override;

View File

@ -344,9 +344,6 @@ public:
// css::awt::XView
void SAL_CALL draw( sal_Int32 nX, sal_Int32 nY ) override;
// css::awt::XDevice,
css::awt::DeviceInfo SAL_CALL getInfo() override;
// css::awt::XVclWindowPeer
void SAL_CALL setProperty( const OUString& PropertyName, const css::uno::Any& Value ) override;
@ -416,9 +413,6 @@ public:
// css::awt::XView
void SAL_CALL draw( sal_Int32 nX, sal_Int32 nY ) override;
// css::awt::XDevice,
css::awt::DeviceInfo SAL_CALL getInfo() override;
// css::awt::XVclWindowPeer
void SAL_CALL setProperty( const OUString& PropertyName, const css::uno::Any& Value ) override;
@ -453,9 +447,6 @@ public:
// css::awt::XView
void SAL_CALL draw( sal_Int32 nX, sal_Int32 nY ) override;
// css::awt::XDevice,
css::awt::DeviceInfo SAL_CALL getInfo() override;
// css::awt::XVclWindowPeer
void SAL_CALL setProperty( const OUString& PropertyName, const css::uno::Any& Value ) override;
css::uno::Any SAL_CALL getProperty( const OUString& PropertyName ) override;

View File

@ -406,12 +406,6 @@ sal_Int32 OReportStylesContext::GetIndex(const sal_Int16 nContextID)
}
sal_uInt16 OReportStylesContext::GetFamily( const OUString& rFamily ) const
{
sal_uInt16 nFamily = SvXMLStylesContext::GetFamily(rFamily);
return nFamily;
}
} // rptxml

View File

@ -123,7 +123,6 @@ namespace rptxml
virtual css::uno::Reference< css::container::XNameContainer >
GetStylesContainer( sal_uInt16 nFamily ) const override;
virtual OUString GetServiceName( sal_uInt16 nFamily ) const override;
virtual sal_uInt16 GetFamily( const OUString& rFamily ) const override;
sal_Int32 GetIndex(const sal_Int16 nContextID);
};

View File

@ -825,17 +825,6 @@ sal_uInt16 Clipboard::DetermineInsertPosition (const SdTransferable& )
return 0;
}
sal_uInt16 Clipboard::InsertSlides (
const SdTransferable& rTransferable,
sal_uInt16 nInsertPosition)
{
sal_uInt16 nInsertedPageCount = ViewClipboard::InsertSlides (
rTransferable,
nInsertPosition);
return nInsertedPageCount;
}
Clipboard::DropType Clipboard::IsDropAccepted() const
{
const SdTransferable* pDragTransferable = SD_MOD()->pTransferDrag;

View File

@ -101,10 +101,6 @@ protected:
virtual sal_uInt16 DetermineInsertPosition (
const SdTransferable& rTransferable) override;
virtual sal_uInt16 InsertSlides (
const SdTransferable& rTransferable,
sal_uInt16 nInsertPosition) override;
private:
SlideSorter& mrSlideSorter;
SlideSorterController& mrController;

View File

@ -93,12 +93,6 @@ SfxRecordingFloat_Impl::~SfxRecordingFloat_Impl()
disposeOnce();
}
bool SfxRecordingFloat_Impl::Close()
{
bool bRet = SfxFloatingWindow::Close();
return bRet;
}
void SfxRecordingFloat_Impl::FillInfo( SfxChildWinInfo& rInfo ) const
{
SfxFloatingWindow::FillInfo( rInfo );

View File

@ -44,7 +44,6 @@ public:
SfxChildWindow* pChildWin ,
vcl::Window* pParent );
virtual ~SfxRecordingFloat_Impl() override;
virtual bool Close() override;
virtual void FillInfo( SfxChildWinInfo& rInfo ) const override;
virtual void StateChanged( StateChangedType nStateChange ) override;
};

View File

@ -22,8 +22,6 @@ class SvxShowCharSetUIObject : public WindowUIObject
public:
SvxShowCharSetUIObject(const VclPtr<vcl::Window>& xCharSetWin, SvxShowCharSet* pCharSet);
virtual StringMap get_state() override;
virtual void execute(const OUString& rAction,
const StringMap& rParameters) override;

View File

@ -18,13 +18,6 @@ SvxShowCharSetUIObject::SvxShowCharSetUIObject(const VclPtr<vcl::Window>& xCharS
{
}
StringMap SvxShowCharSetUIObject::get_state()
{
StringMap aMap = WindowUIObject::get_state();
return aMap;
}
void SvxShowCharSetUIObject::execute(const OUString& rAction,
const StringMap& rParameters)
{

View File

@ -28,7 +28,6 @@ public:
ConstPolygon(SwWrtShell* pSh, SwEditWin* pWin, SwView* pView);
// Mouse- & Key-Events
virtual bool MouseMove(const MouseEvent& rMEvt) override;
virtual bool MouseButtonUp(const MouseEvent& rMEvt) override;
virtual void Activate(const sal_uInt16 nSlotId) override; // activate function

View File

@ -33,13 +33,6 @@ ConstPolygon::ConstPolygon(SwWrtShell* pWrtShell, SwEditWin* pEditWin, SwView* p
{
}
bool ConstPolygon::MouseMove(const MouseEvent& rMEvt)
{
bool bReturn = SwDrawBase::MouseMove(rMEvt);
return bReturn;
}
bool ConstPolygon::MouseButtonUp(const MouseEvent& rMEvt)
{
bool bReturn = false;

View File

@ -78,12 +78,6 @@ void SAL_CALL VCLXTabPageContainer::draw( sal_Int32 nX, sal_Int32 nY )
VCLXWindow::draw( nX, nY );
}
css::awt::DeviceInfo VCLXTabPageContainer::getInfo()
{
css::awt::DeviceInfo aInfo = VCLXDevice::getInfo();
return aInfo;
}
void SAL_CALL VCLXTabPageContainer::setProperty(const OUString& PropertyName, const Any& Value )
{
SolarMutexGuard aGuard;

View File

@ -2486,13 +2486,6 @@ void SAL_CALL VCLXMultiPage::draw( sal_Int32 nX, sal_Int32 nY )
}
}
// css::awt::XDevice,
css::awt::DeviceInfo SAL_CALL VCLXMultiPage::getInfo()
{
css::awt::DeviceInfo aInfo = VCLXDevice::getInfo();
return aInfo;
}
uno::Any SAL_CALL VCLXMultiPage::getProperty( const OUString& PropertyName )
{
SAL_INFO("toolkit", " **** VCLXMultiPage::getProperty " << PropertyName );
@ -2744,13 +2737,6 @@ void SAL_CALL VCLXTabPage::draw( sal_Int32 nX, sal_Int32 nY )
}
}
// css::awt::XDevice,
css::awt::DeviceInfo SAL_CALL VCLXTabPage::getInfo()
{
css::awt::DeviceInfo aInfo = VCLXDevice::getInfo();
return aInfo;
}
void SAL_CALL VCLXTabPage::setProperty(
const OUString& PropertyName,
const css::uno::Any& Value )
@ -6555,13 +6541,6 @@ void SAL_CALL VCLXFrame::draw( sal_Int32 nX, sal_Int32 nY )
}
}
// css::awt::XDevice,
css::awt::DeviceInfo SAL_CALL VCLXFrame::getInfo()
{
css::awt::DeviceInfo aInfo = VCLXDevice::getInfo();
return aInfo;
}
void SAL_CALL VCLXFrame::setProperty(
const OUString& PropertyName,
const css::uno::Any& Value )