ExtensionSystem: Remove the IPlugin back pointer to pluginspec

The price of having to loop in two places seems small compared
to cleaner relations between the classes.

There's a new hack in the helpmanager to make sure we aren't
looping to often. The hack wouldn't be needed if the (odd(?))
check there weren't there.

Change-Id: Ifed50213b2de8feedfb45c185808d163c00c19ca
Reviewed-by: Eike Ziller <eike.ziller@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/libs/extensionsystem/CMakeLists.txt

@@ -5,7 +5,7 @@ add_qtc_library(ExtensionSystem
     extensionsystem_global.h
     extensionsystemtr.h
     invoker.cpp invoker.h
-    iplugin.cpp iplugin.h iplugin_p.h
+    iplugin.cpp iplugin.h
     optionsparser.cpp optionsparser.h
     plugindetailsview.cpp plugindetailsview.h
     pluginerroroverview.cpp pluginerroroverview.h

src/libs/extensionsystem/extensionsystem.qbs

@@ -20,7 +20,6 @@ Project {
             "invoker.h",
             "iplugin.cpp",
             "iplugin.h",
-            "iplugin_p.h",
             "optionsparser.cpp",
             "optionsparser.h",
             "plugindetailsview.cpp",

src/libs/extensionsystem/iplugin.cpp

@@ -2,8 +2,6 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
 
 #include "iplugin.h"
-#include "iplugin_p.h"
-#include "pluginspec.h"
 
 #include <utils/algorithm.h>
 
@@ -160,7 +158,16 @@
     \sa aboutToShutdown()
 */
 
-using namespace ExtensionSystem;
+namespace ExtensionSystem {
+namespace Internal {
+
+class IPluginPrivate
+{
+public:
+    QList<std::function<QObject *()>> testCreators;
+};
+
+} // Internal
 
 /*!
     \internal
@@ -218,11 +225,4 @@ QVector<QObject *> IPlugin::createTestObjects() const
     return Utils::transform(d->testCreators, &TestCreator::operator());
 }
 
-/*!
+} // ExtensionSystem
-    Returns the PluginSpec corresponding to this plugin.
-    This is not available in the constructor.
-*/
-PluginSpec *IPlugin::pluginSpec() const
-{
-    return d->pluginSpec;
-}

src/libs/extensionsystem/iplugin.h

@@ -11,13 +11,7 @@
 
 namespace ExtensionSystem {
 
-namespace Internal {
+namespace Internal { class IPluginPrivate; }
-    class IPluginPrivate;
-    class PluginSpecPrivate;
-}
-
-class PluginManager;
-class PluginSpec;
 
 class EXTENSIONSYSTEM_EXPORT IPlugin : public QObject
 {
@@ -41,8 +35,6 @@ public:
                                    const QStringList & /* arguments */) { return nullptr; }
     virtual QVector<QObject *> createTestObjects() const;
 
-    PluginSpec *pluginSpec() const;
-
 protected:
     virtual void initialize() {}
 
@@ -56,8 +48,6 @@ signals:
 
 private:
     Internal::IPluginPrivate *d;
-
-    friend class Internal::PluginSpecPrivate;
 };
 
 } // namespace ExtensionSystem

src/libs/extensionsystem/iplugin_p.h

@@ -1,22 +0,0 @@
-// Copyright (C) 2016 The Qt Company Ltd.
-// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
-
-#pragma once
-
-#include "iplugin.h"
-
-namespace ExtensionSystem {
-
-class PluginSpec;
-
-namespace Internal {
-
-class IPluginPrivate
-{
-public:
-    PluginSpec *pluginSpec;
-    QList<std::function<QObject *()>> testCreators;
-};
-
-} // namespace Internal
-} // namespace ExtensionSystem

src/libs/extensionsystem/pluginmanager.cpp

@@ -1556,6 +1556,15 @@ void PluginManager::checkForProblematicPlugins()
     d->checkForProblematicPlugins();
 }
 
+/*!
+    Returns the PluginSpec corresponding to \a plugin.
+*/
+
+PluginSpec *PluginManager::specForPlugin(IPlugin *plugin)
+{
+    return findOrDefault(d->pluginSpecs, equal(&PluginSpec::plugin, plugin));
+}
+
 /*!
     \internal
 */

src/libs/extensionsystem/pluginmanager.h

@@ -76,6 +76,7 @@ public:
     static const QSet<PluginSpec *> pluginsRequiringPlugin(PluginSpec *spec);
     static const QSet<PluginSpec *> pluginsRequiredByPlugin(PluginSpec *spec);
     static void checkForProblematicPlugins();
+    static PluginSpec *specForPlugin(IPlugin *plugin);
 
     // Settings
     static void setSettings(Utils::QtcSettings *settings);

src/libs/extensionsystem/pluginspec.cpp

@@ -5,7 +5,6 @@
 
 #include "extensionsystemtr.h"
 #include "iplugin.h"
-#include "iplugin_p.h"
 #include "pluginmanager.h"
 #include "pluginspec_p.h"
 
@@ -1090,7 +1089,6 @@ bool PluginSpecPrivate::loadLibrary()
     }
     state = PluginSpec::Loaded;
     plugin = pluginObject;
-    plugin->d->pluginSpec = q;
     return true;
 }
 

src/libs/extensionsystem/pluginspec_p.h

@@ -19,7 +19,6 @@
 namespace ExtensionSystem {
 
 class IPlugin;
-class PluginManager;
 
 namespace Internal {
 

src/plugins/coreplugin/helpmanager.cpp

@@ -5,7 +5,9 @@
 
 #include "coreplugin.h"
 
+#include <extensionsystem/pluginmanager.h>
 #include <extensionsystem/pluginspec.h>
+
 #include <utils/qtcassert.h>
 
 #include <QCoreApplication>
@@ -22,12 +24,17 @@ static Implementation *m_instance = nullptr;
 
 static bool checkInstance()
 {
-    auto plugin = Internal::CorePlugin::instance();
+    static bool afterPluginCreation = false;
-    // HelpManager API can only be used after the actual implementation has been created by the
+    if (!afterPluginCreation) {
-    // Help plugin, so check that the plugins have all been created. That is the case
+        using namespace ExtensionSystem;
-    // when the Core plugin is initialized.
+        auto plugin = Internal::CorePlugin::instance();
-    QTC_CHECK(plugin && plugin->pluginSpec()
+        // HelpManager API can only be used after the actual implementation has been created by the
-              && plugin->pluginSpec()->state() >= ExtensionSystem::PluginSpec::Initialized);
+        // Help plugin, so check that the plugins have all been created. That is the case
+        // when the Core plugin is initialized.
+        PluginSpec *pluginSpec = PluginManager::specForPlugin(plugin);
+        afterPluginCreation = (plugin && pluginSpec && pluginSpec->state() >= PluginSpec::Initialized);
+        QTC_CHECK(afterPluginCreation);
+    }
     return m_instance != nullptr;
 }
 

src/plugins/coreplugin/plugininstallwizard.cpp

@@ -7,6 +7,7 @@
 #include "coreplugintr.h"
 #include "icore.h"
 
+#include <extensionsystem/pluginmanager.h>
 #include <extensionsystem/pluginspec.h>
 
 #include <utils/archive.h>
@@ -251,7 +252,7 @@ public:
     {
         QTC_ASSERT(m_tempDir.get(), return );
 
-        PluginSpec *coreplugin = CorePlugin::instance()->pluginSpec();
+        PluginSpec *coreplugin = PluginManager::specForPlugin(CorePlugin::instance());
 
         // look for plugin
         QDirIterator it(m_tempDir->path().path(),

src/plugins/projectexplorer/projectexplorer.cpp

@@ -2500,7 +2500,8 @@ void ProjectExplorerPluginPrivate::currentModeChanged(Id mode, Id oldMode)
 void ProjectExplorerPluginPrivate::determineSessionToRestoreAtStartup()
 {
     // Process command line arguments first:
-    const bool lastSessionArg = m_instance->pluginSpec()->arguments().contains("-lastsession");
+    const bool lastSessionArg =
+        ExtensionSystem::PluginManager::specForPlugin(m_instance)->arguments().contains("-lastsession");
     m_sessionToRestoreAtStartup = lastSessionArg ? SessionManager::startupSession() : QString();
     const QStringList arguments = ExtensionSystem::PluginManager::arguments();
     if (!lastSessionArg) {

tests/auto/extensionsystem/pluginspec/tst_pluginspec.cpp

@@ -279,7 +279,7 @@ void tst_PluginSpec::loadLibrary()
     QVERIFY(QLatin1String(spec->plugin->metaObject()->className()) == QLatin1String("MyPlugin::MyPluginImpl"));
     QCOMPARE(spec->state, PluginSpec::Loaded);
     QVERIFY(!spec->hasError);
-    QCOMPARE(spec->plugin->pluginSpec(), ps);
+    QCOMPARE(spec->plugin, ps->plugin());
     delete ps;
 }