Beautifier: Re-organize internal interface

- pimpl BeautifierPlugin
- apply "static" pattern
- remove use of global object pool

Change-Id: I7a4ab2493d5b29787aca258d1bc46ab00a697176
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@qt.io>

src/plugins/beautifier/artisticstyle/artisticstyle.cpp

@@ -54,10 +54,8 @@ namespace Beautifier {
 namespace Internal {
 namespace ArtisticStyle {
 
-ArtisticStyle::ArtisticStyle(BeautifierPlugin *parent) :
-    BeautifierAbstractTool(parent),
-    m_beautifierPlugin(parent),
-    m_settings(new ArtisticStyleSettings)
+ArtisticStyle::ArtisticStyle()
+    : m_settings(new ArtisticStyleSettings)
 {
 }
 
@@ -81,6 +79,8 @@ bool ArtisticStyle::initialize()
     connect(m_settings, &ArtisticStyleSettings::supportedMimeTypesChanged,
             [this] { updateActions(Core::EditorManager::currentEditor()); });
 
+    new ArtisticStyleOptionsPage(m_settings, this);
+
     return true;
 }
 
@@ -94,11 +94,6 @@ void ArtisticStyle::updateActions(Core::IEditor *editor)
     m_formatFile->setEnabled(editor && m_settings->isApplicable(editor->document()));
 }
 
-QList<QObject *> ArtisticStyle::autoReleaseObjects()
-{
-    return {new ArtisticStyleOptionsPage(m_settings, this)};
-}
-
 void ArtisticStyle::formatFile()
 {
     const QString cfgFileName = configurationFile();
@@ -106,7 +101,7 @@ void ArtisticStyle::formatFile()
         BeautifierPlugin::showError(BeautifierPlugin::msgCannotGetConfigurationFile(
                                         tr(Constants::ArtisticStyle::DISPLAY_NAME)));
     } else {
-        m_beautifierPlugin->formatCurrentFile(command(cfgFileName));
+        BeautifierPlugin::formatCurrentFile(command(cfgFileName));
     }
 }
 

src/plugins/beautifier/artisticstyle/artisticstyle.h

@@ -31,9 +31,6 @@ QT_FORWARD_DECLARE_CLASS(QAction)
 
 namespace Beautifier {
 namespace Internal {
-
-class BeautifierPlugin;
-
 namespace ArtisticStyle {
 
 class ArtisticStyleSettings;
@@ -43,18 +40,17 @@ class ArtisticStyle : public BeautifierAbstractTool
     Q_OBJECT
 
 public:
-    explicit ArtisticStyle(BeautifierPlugin *parent = nullptr);
-    virtual ~ArtisticStyle();
+    ArtisticStyle();
+    ~ArtisticStyle() override;
+
     bool initialize() override;
     QString id() const override;
     void updateActions(Core::IEditor *editor) override;
-    QList<QObject *> autoReleaseObjects() override;
     Command command() const override;
     bool isApplicable(const Core::IDocument *document) const override;
 
 private:
     void formatFile();
-    BeautifierPlugin *m_beautifierPlugin;
     QAction *m_formatFile = nullptr;
     ArtisticStyleSettings *m_settings;
     QString configurationFile() const;

src/plugins/beautifier/beautifierabstracttool.h

@@ -27,7 +27,6 @@
 
 #include "command.h"
 
-#include <QList>
 #include <QObject>
 
 namespace Core {
@@ -43,13 +42,11 @@ class BeautifierAbstractTool : public QObject
     Q_OBJECT
 
 public:
-    explicit BeautifierAbstractTool(QObject *parent = nullptr) : QObject(parent) {}
-    virtual ~BeautifierAbstractTool() {}
+    BeautifierAbstractTool() = default;
 
     virtual QString id() const = 0;
     virtual bool initialize() = 0;
     virtual void updateActions(Core::IEditor *editor) = 0;
-    virtual QList<QObject *> autoReleaseObjects() = 0;
 
     /**
      * Returns the tool's command to format an entire file.

src/plugins/beautifier/beautifierplugin.cpp

@@ -72,6 +72,27 @@ using namespace TextEditor;
 namespace Beautifier {
 namespace Internal {
 
+struct FormatTask
+{
+    FormatTask(QPlainTextEdit *_editor, const QString &_filePath, const QString &_sourceData,
+               const Command &_command, int _startPos = -1, int _endPos = 0) :
+        editor(_editor),
+        filePath(_filePath),
+        sourceData(_sourceData),
+        command(_command),
+        startPos(_startPos),
+        endPos(_endPos) {}
+
+    QPointer<QPlainTextEdit> editor;
+    QString filePath;
+    QString sourceData;
+    Command command;
+    int startPos = -1;
+    int endPos = 0;
+    QString formattedData;
+    QString error;
+};
+
 FormatTask format(FormatTask task)
 {
     task.error.clear();
@@ -187,6 +208,34 @@ bool isAutoFormatApplicable(const Core::IDocument *document,
     });
 }
 
+class BeautifierPluginPrivate : public QObject
+{
+public:
+    BeautifierPluginPrivate();
+
+    void updateActions(Core::IEditor *editor = nullptr);
+
+    void formatEditor(TextEditor::TextEditorWidget *editor, const Command &command,
+                      int startPos = -1, int endPos = 0);
+    void formatEditorAsync(TextEditor::TextEditorWidget *editor, const Command &command,
+                           int startPos = -1, int endPos = 0);
+    void checkAndApplyTask(const FormatTask &task);
+    void updateEditorText(QPlainTextEdit *editor, const QString &text);
+
+    void autoFormatOnSave(Core::IDocument *document);
+
+    QSharedPointer<GeneralSettings> m_generalSettings;
+    QHash<QObject*, QMetaObject::Connection> m_autoFormatConnections;
+
+    ArtisticStyle::ArtisticStyle artisticStyleBeautifier;
+    ClangFormat::ClangFormat clangFormatBeautifier;
+    Uncrustify::Uncrustify uncrustifyBeautifier;
+
+    QList<BeautifierAbstractTool *> m_tools;
+};
+
+static BeautifierPluginPrivate *dd = nullptr;
+
 bool BeautifierPlugin::initialize(const QStringList &arguments, QString *errorString)
 {
     Q_UNUSED(arguments)
@@ -201,42 +250,38 @@ bool BeautifierPlugin::initialize(const QStringList &arguments, QString *errorSt
 
 void BeautifierPlugin::extensionsInitialized()
 {
-    m_tools = {
-        new ArtisticStyle::ArtisticStyle(this),
-        new ClangFormat::ClangFormat(this),
-        new Uncrustify::Uncrustify(this)
-    };
+    dd = new BeautifierPluginPrivate;
+}
 
+BeautifierPluginPrivate::BeautifierPluginPrivate()
+    : m_tools({&artisticStyleBeautifier, &uncrustifyBeautifier, &clangFormatBeautifier})
+{
     QStringList toolIds;
     toolIds.reserve(m_tools.count());
     for (BeautifierAbstractTool *tool : m_tools) {
         toolIds << tool->id();
         tool->initialize();
-        const QList<QObject *> autoReleasedObjects = tool->autoReleaseObjects();
-        for (QObject *object : autoReleasedObjects)
-            addAutoReleasedObject(object);
     }
 
     m_generalSettings.reset(new GeneralSettings);
-    auto settingsPage = new GeneralOptionsPage(m_generalSettings, toolIds, this);
-    addAutoReleasedObject(settingsPage);
+    new GeneralOptionsPage(m_generalSettings, toolIds, this);
 
     updateActions();
 
     const Core::EditorManager *editorManager = Core::EditorManager::instance();
     connect(editorManager, &Core::EditorManager::currentEditorChanged,
-            this, &BeautifierPlugin::updateActions);
+            this, &BeautifierPluginPrivate::updateActions);
     connect(editorManager, &Core::EditorManager::aboutToSave,
-            this, &BeautifierPlugin::autoFormatOnSave);
+            this, &BeautifierPluginPrivate::autoFormatOnSave);
 }
 
-void BeautifierPlugin::updateActions(Core::IEditor *editor)
+void BeautifierPluginPrivate::updateActions(Core::IEditor *editor)
 {
     for (BeautifierAbstractTool *tool : m_tools)
         tool->updateActions(editor);
 }
 
-void BeautifierPlugin::autoFormatOnSave(Core::IDocument *document)
+void BeautifierPluginPrivate::autoFormatOnSave(Core::IDocument *document)
 {
     if (!m_generalSettings->autoFormatOnSave())
         return;
@@ -272,8 +317,9 @@ void BeautifierPlugin::autoFormatOnSave(Core::IDocument *document)
 
 void BeautifierPlugin::formatCurrentFile(const Command &command, int startPos, int endPos)
 {
+    QTC_ASSERT(dd, return);
     if (TextEditorWidget *editor = TextEditorWidget::currentTextEditorWidget())
-        formatEditorAsync(editor, command, startPos, endPos);
+        dd->formatEditorAsync(editor, command, startPos, endPos);
 }
 
 /**
@@ -283,8 +329,8 @@ void BeautifierPlugin::formatCurrentFile(const Command &command, int startPos, i
  *
  * @pre @a endPos must be greater than or equal to @a startPos
  */
-void BeautifierPlugin::formatEditor(TextEditorWidget *editor, const Command &command, int startPos,
-                                    int endPos)
+void BeautifierPluginPrivate::formatEditor(TextEditorWidget *editor, const Command &command,
+                                           int startPos, int endPos)
 {
     QTC_ASSERT(startPos <= endPos, return);
 
@@ -298,7 +344,7 @@ void BeautifierPlugin::formatEditor(TextEditorWidget *editor, const Command &com
 /**
  * Behaves like formatEditor except that the formatting is done asynchronously.
  */
-void BeautifierPlugin::formatEditorAsync(TextEditorWidget *editor, const Command &command,
+void BeautifierPluginPrivate::formatEditorAsync(TextEditorWidget *editor, const Command &command,
                                                 int startPos, int endPos)
 {
     QTC_ASSERT(startPos <= endPos, return);
@@ -312,7 +358,7 @@ void BeautifierPlugin::formatEditorAsync(TextEditorWidget *editor, const Command
     connect(doc, &TextDocument::contentsChanged, watcher, &QFutureWatcher<FormatTask>::cancel);
     connect(watcher, &QFutureWatcherBase::finished, [this, watcher] {
         if (watcher->isCanceled())
-            showError(tr("File was modified."));
+            BeautifierPlugin::showError(BeautifierPlugin::tr("File was modified."));
         else
             checkAndApplyTask(watcher->result());
         watcher->deleteLater();
@@ -325,21 +371,21 @@ void BeautifierPlugin::formatEditorAsync(TextEditorWidget *editor, const Command
  * Checks the state of @a task and if the formatting was successful calls updateEditorText() with
  * the respective members of @a task.
  */
-void BeautifierPlugin::checkAndApplyTask(const FormatTask &task)
+void BeautifierPluginPrivate::checkAndApplyTask(const FormatTask &task)
 {
     if (!task.error.isEmpty()) {
-        showError(task.error);
+        BeautifierPlugin::showError(task.error);
         return;
     }
 
     if (task.formattedData.isEmpty()) {
-        showError(tr("Could not format file %1.").arg(task.filePath));
+        BeautifierPlugin::showError(BeautifierPlugin::tr("Could not format file %1.").arg(task.filePath));
         return;
     }
 
     QPlainTextEdit *textEditor = task.editor;
     if (!textEditor) {
-        showError(tr("File %1 was closed.").arg(task.filePath));
+        BeautifierPlugin::showError(BeautifierPlugin::tr("File %1 was closed.").arg(task.filePath));
         return;
     }
 
@@ -356,7 +402,7 @@ void BeautifierPlugin::checkAndApplyTask(const FormatTask &task)
  * actually changed parts are updated while preserving the cursor position, the folded
  * blocks, and the scroll bar position.
  */
-void BeautifierPlugin::updateEditorText(QPlainTextEdit *editor, const QString &text)
+void BeautifierPluginPrivate::updateEditorText(QPlainTextEdit *editor, const QString &text)
 {
     const QString editorText = editor->toPlainText();
     if (editorText == text)

src/plugins/beautifier/beautifierplugin.h

@@ -29,53 +29,16 @@
 
 #include <extensionsystem/iplugin.h>
 
-#include <QPlainTextEdit>
-#include <QPointer>
-#include <QSharedPointer>
-
-namespace Core {
-class IDocument;
-class IEditor;
-}
-namespace TextEditor {class TextEditorWidget;}
-
 namespace Beautifier {
 namespace Internal {
 
-class BeautifierAbstractTool;
-class GeneralSettings;
-
-struct FormatTask
-{
-    FormatTask(QPlainTextEdit *_editor, const QString &_filePath, const QString &_sourceData,
-               const Command &_command, int _startPos = -1, int _endPos = 0) :
-        editor(_editor),
-        filePath(_filePath),
-        sourceData(_sourceData),
-        command(_command),
-        startPos(_startPos),
-        endPos(_endPos) {}
-
-    QPointer<QPlainTextEdit> editor;
-    QString filePath;
-    QString sourceData;
-    Command command;
-    int startPos = -1;
-    int endPos = 0;
-    QString formattedData;
-    QString error;
-};
-
 class BeautifierPlugin : public ExtensionSystem::IPlugin
 {
     Q_OBJECT
     Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QtCreatorPlugin" FILE "Beautifier.json")
 
 public:
-    bool initialize(const QStringList &arguments, QString *errorString) override;
-    void extensionsInitialized() override;
-
-    void formatCurrentFile(const Command &command, int startPos = -1, int endPos = 0);
+    static void formatCurrentFile(const Command &command, int startPos = -1, int endPos = 0);
 
     static QString msgCannotGetConfigurationFile(const QString &command);
     static QString msgFormatCurrentFile();
@@ -86,18 +49,8 @@ public:
     static void showError(const QString &error);
 
 private:
-    void updateActions(Core::IEditor *editor = nullptr);
-    QList<BeautifierAbstractTool *> m_tools;
-    QSharedPointer<GeneralSettings> m_generalSettings;
-    QHash<QObject*, QMetaObject::Connection> m_autoFormatConnections;
-    void formatEditor(TextEditor::TextEditorWidget *editor, const Command &command,
-                      int startPos = -1, int endPos = 0);
-    void formatEditorAsync(TextEditor::TextEditorWidget *editor, const Command &command,
-                           int startPos = -1, int endPos = 0);
-    void checkAndApplyTask(const FormatTask &task);
-    void updateEditorText(QPlainTextEdit *editor, const QString &text);
-
-    void autoFormatOnSave(Core::IDocument *document);
+    bool initialize(const QStringList &arguments, QString *errorString) override;
+    void extensionsInitialized() override;
 };
 
 } // namespace Internal

src/plugins/beautifier/clangformat/clangformat.cpp

@@ -54,10 +54,8 @@ namespace Beautifier {
 namespace Internal {
 namespace ClangFormat {
 
-ClangFormat::ClangFormat(BeautifierPlugin *parent) :
-    BeautifierAbstractTool(parent),
-    m_beautifierPlugin(parent),
-    m_settings(new ClangFormatSettings)
+ClangFormat::ClangFormat()
+    : m_settings(new ClangFormatSettings)
 {
 }
 
@@ -102,6 +100,8 @@ bool ClangFormat::initialize()
     connect(m_settings, &ClangFormatSettings::supportedMimeTypesChanged,
             [this] { updateActions(Core::EditorManager::currentEditor()); });
 
+    new ClangFormatOptionsPage(m_settings, this);
+
     return true;
 }
 
@@ -112,14 +112,9 @@ void ClangFormat::updateActions(Core::IEditor *editor)
     m_formatRange->setEnabled(enabled);
 }
 
-QList<QObject *> ClangFormat::autoReleaseObjects()
-{
-    return {new ClangFormatOptionsPage(m_settings, this)};
-}
-
 void ClangFormat::formatFile()
 {
-    m_beautifierPlugin->formatCurrentFile(command());
+    BeautifierPlugin::formatCurrentFile(command());
 }
 
 void ClangFormat::formatAtCursor()
@@ -133,7 +128,7 @@ void ClangFormat::formatAtCursor()
     if (tc.hasSelection()) {
         const int offset = tc.selectionStart();
         const int length = tc.selectionEnd() - offset;
-        m_beautifierPlugin->formatCurrentFile(command(offset, length));
+        BeautifierPlugin::formatCurrentFile(command(offset, length));
     } else {
         // Pretend that the current line was selected.
         // Note that clang-format will extend the range to the next bigger
@@ -141,7 +136,7 @@ void ClangFormat::formatAtCursor()
         const QTextBlock block = tc.block();
         const int offset = block.position();
         const int length = block.length();
-        m_beautifierPlugin->formatCurrentFile(command(offset, length));
+        BeautifierPlugin::formatCurrentFile(command(offset, length));
     }
 }
 
@@ -177,7 +172,7 @@ void ClangFormat::disableFormattingSelectedText()
     // The indentation of these markers might be undesired, so reformat.
     // This is not optimal because two undo steps will be needed to remove the markers.
     const int reformatTextLength = insertCursor.position() - selectionStartBlock.position();
-    m_beautifierPlugin->formatCurrentFile(command(selectionStartBlock.position(),
+    BeautifierPlugin::formatCurrentFile(command(selectionStartBlock.position(),
                                                   reformatTextLength));
 }
 

src/plugins/beautifier/clangformat/clangformat.h

@@ -31,9 +31,6 @@ QT_FORWARD_DECLARE_CLASS(QAction)
 
 namespace Beautifier {
 namespace Internal {
-
-class BeautifierPlugin;
-
 namespace ClangFormat {
 
 class ClangFormatSettings;
@@ -43,12 +40,12 @@ class ClangFormat : public BeautifierAbstractTool
     Q_OBJECT
 
 public:
-    explicit ClangFormat(BeautifierPlugin *parent = nullptr);
-    virtual ~ClangFormat();
+    ClangFormat();
+     ~ClangFormat() override;
+
     QString id() const override;
     bool initialize() override;
     void updateActions(Core::IEditor *editor) override;
-    QList<QObject *> autoReleaseObjects() override;
     Command command() const override;
     bool isApplicable(const Core::IDocument *document) const override;
 
@@ -56,7 +53,6 @@ private:
     void formatFile();
     void formatAtCursor();
     void disableFormattingSelectedText();
-    BeautifierPlugin *m_beautifierPlugin;
     QAction *m_formatFile = nullptr;
     QAction *m_formatRange = nullptr;
     QAction *m_disableFormattingSelectedText = nullptr;

src/plugins/beautifier/uncrustify/uncrustify.cpp

@@ -54,10 +54,8 @@ namespace Beautifier {
 namespace Internal {
 namespace Uncrustify {
 
-Uncrustify::Uncrustify(BeautifierPlugin *parent) :
-    BeautifierAbstractTool(parent),
-    m_beautifierPlugin(parent),
-    m_settings(new UncrustifySettings)
+Uncrustify::Uncrustify()
+    : m_settings(new UncrustifySettings)
 {
 }
 
@@ -104,11 +102,6 @@ void Uncrustify::updateActions(Core::IEditor *editor)
     m_formatRange->setEnabled(enabled);
 }
 
-QList<QObject *> Uncrustify::autoReleaseObjects()
-{
-    return {new UncrustifyOptionsPage(m_settings, this)};
-}
-
 void Uncrustify::formatFile()
 {
     const QString cfgFileName = configurationFile();
@@ -116,7 +109,7 @@ void Uncrustify::formatFile()
         BeautifierPlugin::showError(BeautifierPlugin::msgCannotGetConfigurationFile(
                                         tr(Constants::Uncrustify::DISPLAY_NAME)));
     } else {
-        m_beautifierPlugin->formatCurrentFile(command(cfgFileName));
+        BeautifierPlugin::formatCurrentFile(command(cfgFileName));
     }
 }
 
@@ -146,7 +139,7 @@ void Uncrustify::formatSelectedText()
         if (tc.positionInBlock() > 0)
             tc.movePosition(QTextCursor::EndOfLine);
         const int endPos = tc.position();
-        m_beautifierPlugin->formatCurrentFile(command(cfgFileName, true), startPos, endPos);
+        BeautifierPlugin::formatCurrentFile(command(cfgFileName, true), startPos, endPos);
     } else if (m_settings->formatEntireFileFallback()) {
         formatFile();
     }

src/plugins/beautifier/uncrustify/uncrustify.h

@@ -31,9 +31,6 @@ QT_FORWARD_DECLARE_CLASS(QAction)
 
 namespace Beautifier {
 namespace Internal {
-
-class BeautifierPlugin;
-
 namespace Uncrustify {
 
 class UncrustifySettings;
@@ -43,19 +40,17 @@ class Uncrustify : public BeautifierAbstractTool
     Q_OBJECT
 
 public:
-    explicit Uncrustify(BeautifierPlugin *parent = nullptr);
-    virtual ~Uncrustify();
+    Uncrustify();
+    ~Uncrustify() override;
     bool initialize() override;
     QString id() const override;
     void updateActions(Core::IEditor *editor) override;
-    QList<QObject *> autoReleaseObjects() override;
     Command command() const override;
     bool isApplicable(const Core::IDocument *document) const override;
 
 private:
     void formatFile();
     void formatSelectedText();
-    BeautifierPlugin *m_beautifierPlugin;
     QAction *m_formatFile = nullptr;
     QAction *m_formatRange = nullptr;
     UncrustifySettings *m_settings;