LocatorFilterEntry: Discourage the use of internalData

Drop internalData from c'tor. The internalData is going to be
removed, soon. Drop also the icon arg from c'tor since
LocatorFilterEntry instances are usually created in non-main
thread, while operating on QIcon instances isn't really safe
in non-main thread. The use of QIcon inside this struct is
a subject to change in the future, in a way like it was done
in other parts of code that generated icons from non-main thread.

Change-Id: Ic6aa719a64e5fbd65883c54149796057c632780e
Reviewed-by: David Schulz <david.schulz@qt.io>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

src/plugins/bookmarks/bookmarkfilter.cpp

@@ -59,8 +59,8 @@ void BookmarkFilter::prepareSearch(const QString &entry)
         const Bookmark *bookmark = m_manager->bookmarkForIndex(idx);
         const QString filename = bookmark->filePath().fileName();
         LocatorFilterEntry filterEntry(this,
-                                       QString("%1:%2").arg(filename).arg(bookmark->lineNumber()),
+                                       QString("%1:%2").arg(filename).arg(bookmark->lineNumber()));
-                                       QVariant::fromValue(idx));
+        filterEntry.internalData = QVariant::fromValue(idx);
         if (!bookmark->note().isEmpty())
             filterEntry.extraInfo = bookmark->note();
         else if (!bookmark->lineText().isEmpty())

src/plugins/coreplugin/actionsfilter.cpp

@@ -215,7 +215,9 @@ void ActionsFilter::collectEntriesForAction(QAction *action,
         }
     } else if (!text.isEmpty()) {
         const ActionFilterEntryData data{action, {}};
-        LocatorFilterEntry filterEntry(this, text, QVariant::fromValue(data), action->icon());
+        LocatorFilterEntry filterEntry(this, text);
+        filterEntry.internalData = QVariant::fromValue(data);
+        filterEntry.displayIcon = action->icon();
         filterEntry.extraInfo = path.join(" > ");
         updateEntry(action, filterEntry);
     }
@@ -241,8 +243,10 @@ void ActionsFilter::collectEntriesForCommands()
 
         const QString identifier = command->id().toString();
         const QStringList path = identifier.split(QLatin1Char('.'));
-        const ActionFilterEntryData data{action, command->id()};
+        const ActionFilterEntryData data{};
-        LocatorFilterEntry filterEntry(this, text, QVariant::fromValue(data), action->icon());
+        LocatorFilterEntry filterEntry(this, text);
+        filterEntry.internalData = QVariant::fromValue(ActionFilterEntryData{action, command->id()});
+        filterEntry.displayIcon = action->icon();
         filterEntry.displayExtra = command->keySequence().toString(QKeySequence::NativeText);
         if (path.size() >= 2)
             filterEntry.extraInfo = path.mid(0, path.size() - 1).join(" > ");
@@ -260,7 +264,9 @@ void ActionsFilter::collectEntriesForLastTriggered()
         if (!data.action || !m_enabledActions.contains(data.action))
             continue;
         const QString text = actionText(data.action);
-        LocatorFilterEntry filterEntry(this, text, QVariant::fromValue(data), data.action->icon());
+        LocatorFilterEntry filterEntry(this, text);
+        filterEntry.internalData = QVariant::fromValue(data);
+        filterEntry.displayIcon = data.action->icon();
         updateEntry(data.action, filterEntry);
     }
 }

src/plugins/coreplugin/locator/commandlocator.cpp

@@ -77,7 +77,8 @@ QList<LocatorFilterEntry> CommandLocator::matchesFor(QFutureInterface<LocatorFil
         const QString text = Utils::stripAccelerator(pair.second);
         const int index = text.indexOf(entry, 0, entryCaseSensitivity);
         if (index >= 0) {
-            LocatorFilterEntry filterEntry(this, text, QVariant(pair.first));
+            LocatorFilterEntry filterEntry(this, text);
+            filterEntry.internalData = QVariant(pair.first);
             filterEntry.highlightInfo = {index, int(entry.length())};
 
             if (index == 0)

src/plugins/coreplugin/locator/externaltoolsfilter.cpp

@@ -66,7 +66,8 @@ void ExternalToolsFilter::prepareSearch(const QString &entry)
         }
 
         if (index >= 0) {
-            LocatorFilterEntry filterEntry(this, tool->displayName(), QVariant::fromValue(tool));
+            LocatorFilterEntry filterEntry(this, tool->displayName());
+            filterEntry.internalData = QVariant::fromValue(tool);
             filterEntry.extraInfo = tool->description();
             filterEntry.highlightInfo = LocatorFilterEntry::HighlightInfo(index, entry.length(), hDataType);
 
@@ -78,7 +79,7 @@ void ExternalToolsFilter::prepareSearch(const QString &entry)
                 goodEntries.append(filterEntry);
         }
     }
-    LocatorFilterEntry configEntry(this, "Configure External Tool...", {});
+    LocatorFilterEntry configEntry(this, "Configure External Tool...");
     configEntry.extraInfo = "Opens External Tool settings";
     m_results = {};
     m_results << bestEntries << betterEntries << goodEntries << configEntry;

src/plugins/coreplugin/locator/ilocatorfilter.h

@@ -72,14 +72,9 @@ public:
 
     LocatorFilterEntry() = default;
 
-    LocatorFilterEntry(ILocatorFilter *fromFilter,
+    LocatorFilterEntry(ILocatorFilter *fromFilter, const QString &name)
-                       const QString &name,
-                       const QVariant &data = {},
-                       std::optional<QIcon> icon = std::nullopt)
         : filter(fromFilter)
         , displayName(name)
-        , internalData(data)
-        , displayIcon(icon)
     {}
 
     /* backpointer to creating filter */
@@ -93,7 +88,7 @@ public:
     /* additional tooltip */
     QString toolTip;
     /* can be used by the filter to save more information about the entry */
-    QVariant internalData;
+    QVariant internalData; // DON'T USE IN NEW CODE, IT'S GOING TO BE REMOVED, SOON...
     /* icon to display along with the entry */
     std::optional<QIcon> displayIcon;
     /* file path, if the entry is related to a file, is used e.g. for resolving a file icon */

src/plugins/coreplugin/locator/javascriptfilter.cpp

@@ -52,17 +52,25 @@ QList<LocatorFilterEntry> JavaScriptFilter::matchesFor(
 
     QList<LocatorFilterEntry> entries;
     if (entry.trimmed().isEmpty()) {
-        entries.append({this, Tr::tr("Reset Engine"), QVariant::fromValue(EngineAction::Reset)});
+        LocatorFilterEntry entry{this, Tr::tr("Reset Engine")};
+        entry.internalData = QVariant::fromValue(EngineAction::Reset);
+        entries.append(entry);
     } else {
         const QString result = m_engine->evaluate(entry).toString();
         if (m_aborted) {
             const QString message = entry + " = " + Tr::tr("Engine aborted after timeout.");
-            entries.append({this, message, QVariant::fromValue(EngineAction::Abort)});
+            LocatorFilterEntry entry(this, message);
+            entry.internalData = QVariant::fromValue(EngineAction::Abort);
+            entries.append(entry);
         } else {
             const QString expression = entry + " = " + result;
             entries.append({this, expression});
-            entries.append({this, Tr::tr("Copy to clipboard: %1").arg(result), result});
+            LocatorFilterEntry resultEntry(this, Tr::tr("Copy to clipboard: %1").arg(result));
-            entries.append({this, Tr::tr("Copy to clipboard: %1").arg(expression), expression});
+            resultEntry.internalData = result;
+            entries.append(resultEntry);
+            LocatorFilterEntry expressionEntry(this, Tr::tr("Copy to clipboard: %1").arg(expression));
+            resultEntry.internalData = expression;
+            entries.append(expressionEntry);
         }
     }
 

src/plugins/coreplugin/locator/locatorfiltersfilter.cpp

@@ -60,10 +60,9 @@ QList<LocatorFilterEntry> LocatorFiltersFilter::matchesFor(QFutureInterface<Loca
     for (int i = 0; i < m_filterShortcutStrings.size(); ++i) {
         if (future.isCanceled())
             break;
-        LocatorFilterEntry filterEntry(this,
+        LocatorFilterEntry filterEntry(this, m_filterShortcutStrings.at(i));
-                                m_filterShortcutStrings.at(i),
+        filterEntry.internalData = i;
-                                i,
+        filterEntry.displayIcon = m_icon;
-                                m_icon);
         filterEntry.extraInfo = m_filterDisplayNames.at(i);
         filterEntry.toolTip = m_filterDescriptions.at(i);
         filterEntry.displayExtra = m_filterKeyboardShortcuts.at(i);

src/plugins/cppeditor/cppcurrentdocumentfilter.cpp

@@ -87,7 +87,8 @@ QList<LocatorFilterEntry> CppCurrentDocumentFilter::matchesFor(
                 }
             }
 
-            LocatorFilterEntry filterEntry(this, name, {}, info->icon());
+            LocatorFilterEntry filterEntry(this, name);
+            filterEntry.displayIcon = info->icon();
             filterEntry.linkForEditor = {info->filePath(), info->line(), info->column()};
             filterEntry.extraInfo = extraInfo;
             if (match.hasMatch()) {

src/plugins/cppeditor/cpplocatorfilter.cpp

@@ -25,7 +25,8 @@ CppLocatorFilter::CppLocatorFilter()
 
 LocatorFilterEntry CppLocatorFilter::filterEntryFromIndexItem(IndexItem::Ptr info)
 {
-    LocatorFilterEntry filterEntry(this, info->scopedSymbolName(), {}, info->icon());
+    LocatorFilterEntry filterEntry(this, info->scopedSymbolName());
+    filterEntry.displayIcon = info->icon();
     filterEntry.linkForEditor = {info->filePath(), info->line(), info->column()};
     if (info->type() == IndexItem::Class || info->type() == IndexItem::Enum)
         filterEntry.extraInfo = info->shortNativeFilePath();
@@ -120,7 +121,8 @@ CppClassesFilter::CppClassesFilter()
 
 LocatorFilterEntry CppClassesFilter::filterEntryFromIndexItem(IndexItem::Ptr info)
 {
-    LocatorFilterEntry filterEntry(this, info->symbolName(), {}, info->icon());
+    LocatorFilterEntry filterEntry(this, info->symbolName());
+    filterEntry.displayIcon = info->icon();
     filterEntry.linkForEditor = {info->filePath(), info->line(), info->column()};
     filterEntry.extraInfo = info->symbolScope().isEmpty()
         ? info->shortNativeFilePath()
@@ -148,7 +150,8 @@ LocatorFilterEntry CppFunctionsFilter::filterEntryFromIndexItem(IndexItem::Ptr i
         extraInfo.append(" (" + info->filePath().fileName() + ')');
     }
 
-    LocatorFilterEntry filterEntry(this, name + info->symbolType(), {}, info->icon());
+    LocatorFilterEntry filterEntry(this, name + info->symbolType());
+    filterEntry.displayIcon = info->icon();
     filterEntry.linkForEditor = {info->filePath(), info->line(), info->column()};
     filterEntry.extraInfo = extraInfo;
 

src/plugins/help/helpindexfilter.cpp

@@ -78,7 +78,8 @@ QList<LocatorFilterEntry> HelpIndexFilter::matchesFor(QFutureInterface<LocatorFi
     QList<LocatorFilterEntry> entries;
     for (const QString &keyword : std::as_const(m_lastIndicesCache)) {
         const int index = keyword.indexOf(entry, 0, cs);
-        LocatorFilterEntry filterEntry(this, keyword, {}, m_icon);
+        LocatorFilterEntry filterEntry(this, keyword);
+        filterEntry.displayIcon = m_icon;
         filterEntry.highlightInfo = {index, int(entry.length())};
         entries.append(filterEntry);
     }

src/plugins/macros/macrolocatorfilter.cpp

@@ -50,7 +50,8 @@ QList<Core::LocatorFilterEntry> MacroLocatorFilter::matchesFor(QFutureInterface<
         }
 
         if (index >= 0) {
-            Core::LocatorFilterEntry filterEntry(this, displayName, {}, m_icon);
+            Core::LocatorFilterEntry filterEntry(this, displayName);
+            filterEntry.displayIcon = m_icon;
             filterEntry.extraInfo = description;
             filterEntry.highlightInfo = Core::LocatorFilterEntry::HighlightInfo(index, entry.length(), hDataType);
 

src/plugins/texteditor/linenumberfilter.cpp

@@ -64,7 +64,9 @@ QList<LocatorFilterEntry> LineNumberFilter::matchesFor(QFutureInterface<LocatorF
             text = Tr::tr("Line %1").arg(line);
         else
             text = Tr::tr("Column %1").arg(column);
-        value.append(LocatorFilterEntry(this, text, QVariant::fromValue(data)));
+        LocatorFilterEntry entry(this, text);
+        entry.internalData = QVariant::fromValue(data);
+        value.append(entry);
     }
     return value;
 }