Replace explicite type casting with dynamic casts
AcceptedPublic

Authored by abika on Sep 17 2019, 3:59 PM.

Details

Reviewers
gengisdave
Group Reviewers
Krusader
Summary

And replaced some static with dynamic casts.

Auto-correction using "clang-tidy -fix ..." plus manual correction of 'const'
qualifier where needed.

Test Plan

Tested application start and Disc Usage tool.

Diff Detail

Repository
R167 Krusader
Branch
my-clang-fixes-wrong-cast-fixes-arc
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16685
Build 16703: arc lint + arc unit
abika requested review of this revision.Sep 17 2019, 3:59 PM
abika created this revision.
abika added a subscriber: Krusader.
gengisdave accepted this revision as: gengisdave.Sat, Sep 21, 2:21 PM
gengisdave added a subscriber: gengisdave.

Tested Synchronizer, Konfigurator and DiskUsage, no problems found.

This revision is now accepted and ready to land.Sat, Sep 21, 2:21 PM
asensi added a subscriber: asensi.Mon, Oct 7, 9:17 PM

Hi, Alex, thanks for improving the source code, I would like to add that the krusader/GUI/krhistorycombobox.cpp code has been reviewed recently and replacing

auto keyEvent = static_cast<QKeyEvent *>(event);

with

auto keyEvent = dynamic_cast<QKeyEvent *>(event);

would not be what it's written in https://doc.qt.io/qt-5/qobject.html:

QKeyEvent *keyEvent = static_cast<QKeyEvent *>(event);

The same could be said about

krusader/Panel/krsearchbar.cpp

and its existing

auto *ke = static_cast<QKeyEvent *>(event);

While we are at it, what about using qobject_cast (instead of dynamic_cast) in the following places?

diff --git a/krusader/KViewer/lister.cpp b/krusader/KViewer/lister.cpp
    --- a/krusader/KViewer/lister.cpp
    +++ b/krusader/KViewer/lister.cpp
    @@ -1360,7 +1360,7 @@ bool Lister::openUrl(const QUrl &listerUrl)
            connect(downloadJob, &KIO::TransferJob::result, this, [=](KJob *job) {
                _tempFile->flush();
                if (job->error()) {   /* any error occurred? */
    -                auto *kioJob = (KIO::TransferJob *)job;
    +                auto *kioJob = qobject_cast<KIO::TransferJob *>(job);
                    KMessageBox::error(_textArea, i18n("Error reading file %1.", kioJob->url().toDisplayString(QUrl::PreferLocalFile)));
                }
                _downloading = false;

            
diff --git a/krusader/Konfigurator/kgdependencies.cpp b/krusader/Konfigurator/kgdependencies.cpp
    --- a/krusader/Konfigurator/kgdependencies.cpp
    +++ b/krusader/Konfigurator/kgdependencies.cpp
    @@ -145,7 +145,7 @@ void KgDependencies::addApplication(const QString& name, QGridLayout *grid, int
    
    void KgDependencies::slotApply(QObject *obj, const QString& configGroup, const QString& name)
    {
    -    auto *urlRequester = (KonfiguratorURLRequester *) obj;
    +    auto *urlRequester = qobject_cast<KonfiguratorURLRequester *>( obj);
    
        KConfigGroup group(krConfig, configGroup);
        group.writeEntry(name, urlRequester->url().toDisplayString(QUrl::PreferLocalFile));        


diff --git a/krusader/Konfigurator/kggeneral.cpp b/krusader/Konfigurator/kggeneral.cpp
    --- a/krusader/Konfigurator/kggeneral.cpp
    +++ b/krusader/Konfigurator/kggeneral.cpp
    @@ -287,7 +287,7 @@ void KgGeneral::createGeneralTab()
    
    void KgGeneral::applyTempDir(QObject *obj, const QString& configGroup, const QString& name)
    {
    -    auto *urlReq = (KonfiguratorURLRequester *)obj;
    +    auto *urlReq = qobject_cast<KonfiguratorURLRequester *>(obj);
        QString value = urlReq->url().toDisplayString(QUrl::PreferLocalFile);
    
        KConfigGroup(krConfig, configGroup).writeEntry(name, value);
    );        


diff --git a/krusader/Konfigurator/konfigurator.cpp b/krusader/Konfigurator/konfigurator.cpp
    --- a/krusader/Konfigurator/konfigurator.cpp
    +++ b/krusader/Konfigurator/konfigurator.cpp
    @@ -163,7 +163,7 @@ void Konfigurator::closeEvent(QCloseEvent *event)
    void Konfigurator::slotApplyEnable()
    {
        lastPage = currentPage();
    -    bool isChanged = ((KonfiguratorPage *)(lastPage->widget()))->isChanged();
    +    bool isChanged = (qobject_cast<KonfiguratorPage *>(lastPage->widget()))->isChanged();
        button(QDialogButtonBox::Apply)->setEnabled(isChanged);
        button(QDialogButtonBox::Reset)->setEnabled(isChanged);
    }
    @@ -173,7 +173,7 @@ bool Konfigurator::slotPageSwitch(KPageWidgetItem *current, KPageWidgetItem *bef
        if (before == nullptr)
            return true;
    
    -    auto *currentPg = (KonfiguratorPage *)(before->widget());
    +    auto *currentPg = qobject_cast<KonfiguratorPage *>(before->widget());
    
        if (internalCall) {
            internalCall = false;
    @@ -221,17 +221,17 @@ void Konfigurator::slotClose()
    
    void Konfigurator::slotApply()
    {
    -    emit configChanged(((KonfiguratorPage*)(currentPage()->widget()))->apply());
    +    emit configChanged((qobject_cast<KonfiguratorPage*>(currentPage()->widget()))->apply());
    }
    
    void Konfigurator::slotReset()
    {
    -    ((KonfiguratorPage *)(currentPage()->widget()))->loadInitialValues();
    +    (qobject_cast<KonfiguratorPage *>(currentPage()->widget()))->loadInitialValues();
    }
    
    void Konfigurator::slotRestoreDefaults()
    {
    -    ((KonfiguratorPage *)(currentPage()->widget()))->setDefaults();
    +    (qobject_cast<KonfiguratorPage *>(currentPage()->widget()))->setDefaults();
    }
    
    void Konfigurator::slotShowHelp()


diff --git a/krusader/Synchronizer/synchronizer.cpp b/krusader/Synchronizer/synchronizer.cpp
    --- a/krusader/Synchronizer/synchronizer.cpp
    +++ b/krusader/Synchronizer/synchronizer.cpp
    @@ -179,7 +179,7 @@ void Synchronizer::compareLoop()
    
                if (entry->inherits("CompareTask")) {
                    if (entry->state() == ST_STATE_READY) {
    -                    auto *ctentry = (CompareTask *) entry;
    +                    auto *ctentry = qobject_cast<CompareTask *>( entry);
                        if (ctentry->isDuplicate())
                            compareDirectory(ctentry->parent(), ctentry->leftDirList(), ctentry->rightDirList(),
                                            ctentry->leftDir(), ctentry->rightDir());

                
diff --git a/krusader/Synchronizer/synchronizertask.cpp b/krusader/Synchronizer/synchronizertask.cpp
    --- a/krusader/Synchronizer/synchronizertask.cpp
    +++ b/krusader/Synchronizer/synchronizertask.cpp
    @@ -272,8 +272,8 @@ void CompareContentTask::slotDataReceived(KIO::Job *job, const QByteArray &data)
            if (compareArray.size())
                abortContentComparing();
        } else {
    -        if (!((KIO::TransferJob *)job)->isSuspended()) {
    -            ((KIO::TransferJob *)job)->suspend();
    +        if (!(qobject_cast<KIO::TransferJob *>(job))->isSuspended()) {
    +            (qobject_cast<KIO::TransferJob *>(job))->suspend();
                otherJob->resume();
            }
        }


diff --git a/krusader/actionsbase.cpp b/krusader/actionsbase.cpp
    --- a/krusader/actionsbase.cpp
    +++ b/krusader/actionsbase.cpp
    @@ -98,13 +98,13 @@ QAction *ActionsBase::action(QString text, QString icon, const QKeySequence& sho
    KToggleAction *ActionsBase::toggleAction(QString text, QString icon, const QKeySequence& shortcut,
                                    QObject *recv, const char *slot, QString name)
    {
    -    return (KToggleAction *)(action(std::move(text), std::move(icon), shortcut, recv, slot, std::move(name), true));
    +    return qobject_cast<KToggleAction *>(action(std::move(text), std::move(icon), shortcut, recv, slot, std::move(name), true));
    }
    
    KToggleAction *ActionsBase::toggleAction(QString text, QString icon, const QKeySequence& shortcut,
                                            ActionGroup &group, const char *slot, QString name)
    {
    -    return (KToggleAction *)(action(std::move(text), std::move(icon), shortcut, group, slot, std::move(name), true));
    +    return qobject_cast<KToggleAction *>(action(std::move(text), std::move(icon), shortcut, group, slot, std::move(name), true));
    }
    
    QAction *ActionsBase::stdAction(KStandardAction::StandardAction id, QObject *recv, const char *slot)


diff --git a/krusader/krusader.cpp b/krusader/krusader.cpp
    --- a/krusader/krusader.cpp
    +++ b/krusader/krusader.cpp
    @@ -540,7 +540,7 @@ void Krusader::stopWait() {
    }
    
    void Krusader::updateUserActions() {
    -    auto *userActionMenu = (KActionMenu *) KrActions::actUserMenu;
    +    auto *userActionMenu = qobject_cast<KActionMenu *>( KrActions::actUserMenu);
        if (userActionMenu) {
            userActionMenu->menu()->clear();

Greetings!