Generic highlighter: Fix matching the right definition

No longer uses artificial mime types for highlighting files that do not
specify a mime type. No longer registers mime types that are specified
but that Qt Creator does not know about.

Instead, try to match the mime type, and if that fails, or if the result
does not match the file name pattern, try to match file name patterns
instead.

This also fixes the potential problem that mime types were always only
added, never removed, even if the user removed definitions and triggered
a reparse.

Also fixes that a highlight definition in the fallback location could
overwrite a highlight definition in the preferred location, if it has a
higher priority setting.

Task-number: QTCREATORBUG-13912
Change-Id: I86ce10f4f4341f6add0d2b58a04f080501d0cbf4
Reviewed-by: Eike Ziller <eike.ziller@theqtcompany.com>

8 files changed, 112 insertions, 174 deletions

diff --git a/src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp b/src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp
index 4a34bd6b32..ea904d7286 100644
--- a/src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp
+++ b/src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp
@@ -77,7 +77,7 @@ void HighlighterSettingsPage::HighlighterSettingsPagePrivate::ensureInitialized(
 
 HighlighterSettingsPage::HighlighterSettingsPage(Core::Id id, QObject *parent) :
     TextEditorOptionsPage(parent),
-    m_requestMimeTypeRegistration(false),
+    m_requestHighlightFileRegistration(false),
     m_d(new HighlighterSettingsPagePrivate(id))
 {
     setId(m_d->m_id);
@@ -122,9 +122,9 @@ void HighlighterSettingsPage::apply()
     if (settingsChanged())
         settingsFromUI();
 
-    if (m_requestMimeTypeRegistration) {
-        Manager::instance()->registerMimeTypes();
-        m_requestMimeTypeRegistration = false;
+    if (m_requestHighlightFileRegistration) {
+        Manager::instance()->registerHighlightingFiles();
+        m_requestHighlightFileRegistration = false;
     }
 }
 
@@ -146,12 +146,12 @@ const HighlighterSettings &HighlighterSettingsPage::highlighterSettings() const
 void HighlighterSettingsPage::settingsFromUI()
 {
     m_d->ensureInitialized();
-    if (!m_requestMimeTypeRegistration && (
+    if (!m_requestHighlightFileRegistration && (
         m_d->m_settings.definitionFilesPath() != m_d->m_page->definitionFilesPath->path() ||
         m_d->m_settings.fallbackDefinitionFilesPath() !=
             m_d->m_page->fallbackDefinitionFilesPath->path() ||
         m_d->m_settings.useFallbackLocation() != m_d->m_page->useFallbackLocation->isChecked())) {
-        m_requestMimeTypeRegistration = true;
+        m_requestHighlightFileRegistration = true;
     }
 
     m_d->m_settings.setDefinitionFilesPath(m_d->m_page->definitionFilesPath->path());
@@ -212,8 +212,8 @@ void HighlighterSettingsPage::manageDefinitions(const QList<DefinitionMetaDataPt
     ManageDefinitionsDialog dialog(metaData,
                                    m_d->m_page->definitionFilesPath->path() + QLatin1Char('/'),
                                    m_d->m_page->definitionFilesPath->buttonAtIndex(1)->window());
-    if (dialog.exec() && !m_requestMimeTypeRegistration)
-        m_requestMimeTypeRegistration = true;
+    if (dialog.exec() && !m_requestHighlightFileRegistration)
+        m_requestHighlightFileRegistration = true;
     setDownloadDefinitionsState(m_d->m_page->definitionFilesPath->isValid());
 }
 
diff --git a/src/plugins/texteditor/generichighlighter/highlightersettingspage.h b/src/plugins/texteditor/generichighlighter/highlightersettingspage.h
index f1bd8f6c4a..4c19e67fd0 100644
--- a/src/plugins/texteditor/generichighlighter/highlightersettingspage.h
+++ b/src/plugins/texteditor/generichighlighter/highlightersettingspage.h
@@ -71,7 +71,7 @@ private:
 
     bool settingsChanged() const;
 
-    bool m_requestMimeTypeRegistration;
+    bool m_requestHighlightFileRegistration;
 
     struct HighlighterSettingsPagePrivate;
     HighlighterSettingsPagePrivate *m_d;
diff --git a/src/plugins/texteditor/generichighlighter/manager.cpp b/src/plugins/texteditor/generichighlighter/manager.cpp
index e7fdf65a34..08dc24856b 100644
--- a/src/plugins/texteditor/generichighlighter/manager.cpp
+++ b/src/plugins/texteditor/generichighlighter/manager.cpp
@@ -115,7 +115,7 @@ Manager::Manager() :
     m_multiDownloader(0),
     m_hasQueuedRegistration(false)
 {
-    connect(&m_registeringWatcher, SIGNAL(finished()), this, SLOT(registerMimeTypesFinished()));
+    connect(&m_registeringWatcher, SIGNAL(finished()), this, SLOT(registerHighlightingFilesFinished()));
 }
 
 Manager::~Manager()
@@ -138,20 +138,68 @@ QString Manager::definitionIdByName(const QString &name) const
     return m_register.m_idByName.value(name);
 }
 
-QString Manager::definitionIdByMimeType(const QString &mimeType) const
+static bool matchesPattern(const QString &fileName, DefinitionMetaDataPtr metaData)
 {
-    return m_register.m_idByMimeType.value(mimeType);
+    if (metaData.isNull())
+        return false;
+    foreach (const QString &pattern, metaData->patterns) {
+        QRegExp reg(pattern, Qt::CaseSensitive, QRegExp::Wildcard);
+        if (reg.exactMatch(fileName))
+            return true;
+    }
+    return false;
 }
 
-QString Manager::definitionIdByAnyMimeType(const QStringList &mimeTypes) const
+QString Manager::definitionIdByMimeType(const Core::MimeType &mimeType) const
 {
-    QString definitionId;
-    foreach (const QString &mimeType, mimeTypes) {
-        definitionId = definitionIdByMimeType(mimeType);
-        if (!definitionId.isEmpty())
-            break;
+    QList<Core::MimeType> queue;
+    queue.append(mimeType);
+    while (!queue.isEmpty()) {
+        const Core::MimeType mt = queue.takeFirst();
+        const QString id = m_register.m_idByMimeType.value(mt.type());
+        if (!id.isEmpty())
+            return id;
+        foreach (const QString &parent, mt.subClassesOf()) {
+            const Core::MimeType parentMt = Core::MimeDatabase::findByType(parent);
+            if (!parentMt.isNull())
+                queue.append(parentMt);
+        }
     }
-    return definitionId;
+    return QString();
+}
+
+QString Manager::definitionIdByFile(const QString &filePath) const
+{
+    const QString fileName = QFileInfo(filePath).fileName();
+    // find best match
+    QString bestId;
+    int bestPriority = -1;
+    auto it = m_register.m_definitionsMetaData.constBegin();
+    while (it != m_register.m_definitionsMetaData.constEnd()) {
+        DefinitionMetaDataPtr metaData = it.value();
+        if (metaData->priority > bestPriority && matchesPattern(fileName, metaData)) {
+            bestId = metaData->id;
+            bestPriority = metaData->priority;
+        }
+        ++it;
+    }
+    return bestId;
+}
+
+QString Manager::definitionIdByMimeTypeAndFile(const MimeType &mimeType, const QString &filePath) const
+{
+    QString id = definitionIdByMimeType(mimeType);
+    if (!filePath.isEmpty()) {
+        QString idByFile;
+        const QString fileName = QFileInfo(filePath).fileName();
+        // if mime type check returned no result, or doesn't match the patterns,
+        // prefer a match by pattern
+        if (id.isEmpty() || !matchesPattern(fileName, m_register.m_definitionsMetaData.value(id)))
+            idByFile = definitionIdByFile(filePath);
+        if (!idByFile.isEmpty())
+            id = idByFile;
+    }
+    return id;
 }
 
 DefinitionMetaDataPtr Manager::availableDefinitionByName(const QString &name) const
@@ -206,51 +254,28 @@ class ManagerProcessor : public QObject
     Q_OBJECT
 public:
     ManagerProcessor();
-    void process(QFutureInterface<QPair<Manager::RegisterData,
-                                        QList<MimeType> > > &future);
+    void process(QFutureInterface<Manager::RegisterData> &future);
 
     QStringList m_definitionsPaths;
-    QSet<QString> m_knownMimeTypes;
-    QSet<QString> m_knownSuffixes;
-    QHash<QString, MimeType> m_userModified;
     static const int kMaxProgress;
 };
 
 const int ManagerProcessor::kMaxProgress = 200;
 
 ManagerProcessor::ManagerProcessor()
-    : m_knownSuffixes(QSet<QString>::fromList(MimeDatabase::suffixes()))
 {
     const HighlighterSettings &settings = TextEditorSettings::highlighterSettings();
     m_definitionsPaths.append(settings.definitionFilesPath());
     if (settings.useFallbackLocation())
         m_definitionsPaths.append(settings.fallbackDefinitionFilesPath());
-
-    foreach (const MimeType &userMimeType, MimeDatabase::readUserModifiedMimeTypes())
-        m_userModified.insert(userMimeType.type(), userMimeType);
-    foreach (const MimeType &mimeType, MimeDatabase::mimeTypes())
-        m_knownMimeTypes.insert(mimeType.type());
 }
 
-void ManagerProcessor::process(QFutureInterface<QPair<Manager::RegisterData,
-                                                      QList<MimeType> > > &future)
+void ManagerProcessor::process(QFutureInterface<Manager::RegisterData> &future)
 {
     future.setProgressRange(0, kMaxProgress);
 
-    // @TODO: Improve MIME database to handle the following limitation.
-    // The generic highlighter only register its types after all other plugins
-    // have populated Creator's MIME database (so it does not override anything).
-    // When the generic highlighter settings change only its internal data is cleaned-up
-    // and rebuilt. Creator's MIME database is not touched. So depending on how the
-    // user plays around with the generic highlighter file definitions (changing
-    // duplicated patterns, for example), some changes might not be reflected.
-    // A definitive implementation would require some kind of re-load or update
-    // (considering hierarchies, aliases, etc) of the MIME database whenever there
-    // is a change in the generic highlighter settings.
-
     Manager::RegisterData data;
-    QList<MimeType> newMimeTypes;
-
+    // iterate through paths in order, high priority > low priority
     foreach (const QString &path, m_definitionsPaths) {
         if (path.isEmpty())
             continue;
@@ -258,93 +283,47 @@ void ManagerProcessor::process(QFutureInterface<QPair<Manager::RegisterData,
         QDir definitionsDir(path);
         QStringList filter(QLatin1String("*.xml"));
         definitionsDir.setNameFilters(filter);
-        QList<DefinitionMetaDataPtr> allMetaData;
         foreach (const QFileInfo &fileInfo, definitionsDir.entryInfoList()) {
-            const DefinitionMetaDataPtr &metaData =
-                    Manager::parseMetadata(fileInfo);
-            if (!metaData.isNull())
-                allMetaData.append(metaData);
-        }
-
-        // Consider definitions with higher priority first.
-        Utils::sort(allMetaData, [](const DefinitionMetaDataPtr &l,
-                                    const DefinitionMetaDataPtr &r) {
-            return l->priority > r->priority;
-        });
-
-        foreach (const DefinitionMetaDataPtr &metaData, allMetaData) {
             if (future.isCanceled())
                 return;
             if (future.progressValue() < kMaxProgress - 1)
                 future.setProgressValue(future.progressValue() + 1);
 
-            if (data.m_idByName.contains(metaData->name))
-                // Name already exists... This is a fallback item, do not consider it.
-                continue;
-
-            const QString &id = metaData->id;
-            data.m_idByName.insert(metaData->name, id);
-            data.m_definitionsMetaData.insert(id, metaData);
-
-            static const QStringList textPlain(QLatin1String("text/plain"));
-
-            // A definition can specify multiple MIME types and file extensions/patterns,
-            // but all on a single string. So associate all patterns with all MIME types.
-            QList<MimeGlobPattern> globPatterns;
-            foreach (const QString &type, metaData->mimeTypes) {
-                if (data.m_idByMimeType.contains(type))
-                    continue;
-
-                data.m_idByMimeType.insert(type, id);
-                if (!m_knownMimeTypes.contains(type)) {
-                    m_knownMimeTypes.insert(type);
-
-                    MimeType mimeType;
-                    mimeType.setType(type);
-                    mimeType.setSubClassesOf(textPlain);
-                    mimeType.setComment(metaData->name);
-
-                    // If there's a user modification for this mime type, we want to use the
-                    // modified patterns and rule-based matchers. If not, just consider what
-                    // is specified in the definition file.
-                    QHash<QString, MimeType>::const_iterator it =
-                        m_userModified.find(mimeType.type());
-                    if (it == m_userModified.end()) {
-                        if (globPatterns.isEmpty()) {
-                            foreach (const QString &pattern, metaData->patterns) {
-                                static const QLatin1String mark("*.");
-                                if (pattern.startsWith(mark)) {
-                                    const QString &suffix = pattern.right(pattern.length() - 2);
-                                    if (!m_knownSuffixes.contains(suffix))
-                                        m_knownSuffixes.insert(suffix);
-                                    else
-                                        continue;
-                                }
-                                globPatterns.append(MimeGlobPattern(pattern, 50));
-                            }
-                        }
-                        mimeType.setGlobPatterns(globPatterns);
-                    } else {
-                        mimeType.setGlobPatterns(it.value().globPatterns());
-                        mimeType.setMagicRuleMatchers(it.value().magicRuleMatchers());
+            const DefinitionMetaDataPtr &metaData =
+                    Manager::parseMetadata(fileInfo);
+            // skip failing or already existing definitions
+            if (!metaData.isNull() && !data.m_idByName.contains(metaData->name)) {
+                const QString id = metaData->id;
+                data.m_idByName.insert(metaData->name, id);
+                data.m_definitionsMetaData.insert(id, metaData);
+                foreach (const QString &mt, metaData->mimeTypes) {
+                    bool insert = true;
+                    // check if there is already a definition registered with higher priority
+                    const QString existingDefinition = data.m_idByMimeType.value(mt);
+                    if (!existingDefinition.isEmpty()) {
+                        // check priorities
+                        DefinitionMetaDataPtr existingMetaData =
+                                data.m_definitionsMetaData.value(existingDefinition);
+                        if (!existingMetaData.isNull() && existingMetaData->priority > metaData->priority)
+                            insert = false;
                     }
-
-                    newMimeTypes.append(mimeType);
+                    if (insert)
+                        data.m_idByMimeType.insert(mt, id);
                 }
             }
         }
     }
 
-    future.reportResult(qMakePair(data, newMimeTypes));
+    future.reportResult(data);
 }
 
-void Manager::registerMimeTypes()
+void Manager::registerHighlightingFiles()
 {
     if (!m_registeringWatcher.isRunning()) {
         clear();
 
         ManagerProcessor *processor = new ManagerProcessor;
-        QFuture<QPair<RegisterData, QList<MimeType> > > future =
+        QFuture<RegisterData> future =
             QtConcurrent::run(&ManagerProcessor::process, processor);
         connect(&m_registeringWatcher, SIGNAL(finished()), processor, SLOT(deleteLater()));
         m_registeringWatcher.setFuture(future);
@@ -354,29 +333,22 @@ void Manager::registerMimeTypes()
     }
 }
 
-void Manager::registerMimeTypesFinished()
+void Manager::registerHighlightingFilesFinished()
 {
     if (m_hasQueuedRegistration) {
         m_hasQueuedRegistration = false;
-        registerMimeTypes();
+        registerHighlightingFiles();
     } else if (!m_registeringWatcher.isCanceled()) {
-        const QPair<RegisterData, QList<MimeType> > &result = m_registeringWatcher.result();
-        m_register = result.first;
+        m_register = m_registeringWatcher.result();
 
-        foreach (const MimeType &mimeType, result.second)
-            MimeDatabase::addMimeType(mimeType);
-
-        emit mimeTypesRegistered();
+        emit highlightingFilesRegistered();
     }
 }
 
 DefinitionMetaDataPtr Manager::parseMetadata(const QFileInfo &fileInfo)
 {
     static const QLatin1Char kSemiColon(';');
-    static const QLatin1Char kSpace(' ');
-    static const QLatin1Char kDash('-');
     static const QLatin1String kLanguage("language");
-    static const QLatin1String kArtificial("text/x-artificial-");
 
     QFile definitionFile(fileInfo.absoluteFilePath());
     if (!definitionFile.open(QIODevice::ReadOnly | QIODevice::Text))
@@ -397,17 +369,8 @@ DefinitionMetaDataPtr Manager::parseMetadata(const QFileInfo &fileInfo)
             metaData->patterns = atts.value(QLatin1String(kExtensions))
                                   .toString().split(kSemiColon, QString::SkipEmptyParts);
 
-            QStringList mimeTypes = atts.value(QLatin1String(kMimeType)).
+            metaData->mimeTypes = atts.value(QLatin1String(kMimeType)).
                                     toString().split(kSemiColon, QString::SkipEmptyParts);
-            if (mimeTypes.isEmpty()) {
-                // There are definitions which do not specify a MIME type, but specify file
-                // patterns. Creating an artificial MIME type is a workaround.
-                QString artificialType(kArtificial);
-                artificialType.append(metaData->name.trimmed().replace(kSpace, kDash));
-                mimeTypes.append(artificialType);
-            }
-            metaData->mimeTypes = mimeTypes;
-
             break;
         }
     }
diff --git a/src/plugins/texteditor/generichighlighter/manager.h b/src/plugins/texteditor/generichighlighter/manager.h
index 0b8bf4dca9..4105f65b64 100644
--- a/src/plugins/texteditor/generichighlighter/manager.h
+++ b/src/plugins/texteditor/generichighlighter/manager.h
@@ -69,8 +69,10 @@ public:
     static Manager *instance();
 
     QString definitionIdByName(const QString &name) const;
-    QString definitionIdByMimeType(const QString &mimeType) const;
-    QString definitionIdByAnyMimeType(const QStringList &mimeTypes) const;
+    QString definitionIdByMimeType(const Core::MimeType &mimeType) const;
+    QString definitionIdByFile(const QString &filePath) const;
+    QString definitionIdByMimeTypeAndFile(const Core::MimeType &mimeType,
+                                          const QString &filePath) const;
     DefinitionMetaDataPtr availableDefinitionByName(const QString &name) const;
 
     bool isBuildingDefinition(const QString &id) const;
@@ -84,15 +86,15 @@ public:
     static DefinitionMetaDataPtr parseMetadata(const QFileInfo &fileInfo);
 
 public slots:
-    void registerMimeTypes();
+    void registerHighlightingFiles();
 
 private slots:
-    void registerMimeTypesFinished();
+    void registerHighlightingFilesFinished();
     void downloadAvailableDefinitionsListFinished();
     void downloadDefinitionsFinished();
 
 signals:
-    void mimeTypesRegistered();
+    void highlightingFilesRegistered();
 
 private:
     Manager();
@@ -114,7 +116,7 @@ private:
     };
     RegisterData m_register;
     bool m_hasQueuedRegistration;
-    QFutureWatcher<QPair<RegisterData, QList<Core::MimeType> > > m_registeringWatcher;
+    QFutureWatcher<RegisterData> m_registeringWatcher;
     friend class ManagerProcessor;
 
 signals:
diff --git a/src/plugins/texteditor/highlighterutils.cpp b/src/plugins/texteditor/highlighterutils.cpp
index 0e84c347e7..2c0dc7b6fa 100644
--- a/src/plugins/texteditor/highlighterutils.cpp
+++ b/src/plugins/texteditor/highlighterutils.cpp
@@ -37,30 +37,10 @@
 using namespace TextEditor;
 using namespace Internal;
 
-QString TextEditor::findDefinitionId(const Core::MimeType &mimeType,
-                         bool considerParents)
-{
-    QString definitionId = Manager::instance()->definitionIdByAnyMimeType(mimeType.aliases());
-    if (definitionId.isEmpty() && considerParents) {
-        definitionId = Manager::instance()->definitionIdByAnyMimeType(mimeType.subClassesOf());
-        if (definitionId.isEmpty()) {
-            foreach (const QString &parent, mimeType.subClassesOf()) {
-                const Core::MimeType &parentMimeType = Core::MimeDatabase::findByType(parent);
-                definitionId = findDefinitionId(parentMimeType, considerParents);
-            }
-        }
-    }
-    return definitionId;
-}
-
 void TextEditor::setMimeTypeForHighlighter(Highlighter *highlighter, const Core::MimeType &mimeType,
-                                           QString *foundDefinitionId)
+                                           const QString &filePath, QString *foundDefinitionId)
 {
-    const QString type = mimeType.type();
-    QString definitionId = Manager::instance()->definitionIdByMimeType(type);
-    if (definitionId.isEmpty())
-        definitionId = findDefinitionId(mimeType, true);
-
+    QString definitionId = Manager::instance()->definitionIdByMimeTypeAndFile(mimeType, filePath);
     if (!definitionId.isEmpty()) {
         const QSharedPointer<HighlightDefinition> &definition =
             Manager::instance()->definition(definitionId);
@@ -72,9 +52,3 @@ void TextEditor::setMimeTypeForHighlighter(Highlighter *highlighter, const Core:
         *foundDefinitionId = definitionId;
 }
 
-SyntaxHighlighter *TextEditor::createGenericSyntaxHighlighter(const Core::MimeType &mimeType)
-{
-    Highlighter *highlighter = new Highlighter();
-    setMimeTypeForHighlighter(highlighter, mimeType);
-    return highlighter;
-}
diff --git a/src/plugins/texteditor/highlighterutils.h b/src/plugins/texteditor/highlighterutils.h
index b0e1b70b69..0d9a9b0942 100644
--- a/src/plugins/texteditor/highlighterutils.h
+++ b/src/plugins/texteditor/highlighterutils.h
@@ -43,12 +43,10 @@ namespace Core { class MimeType; }
 namespace TextEditor {
 
 class Highlighter;
-class SyntaxHighlighter;
 
 void setMimeTypeForHighlighter(Highlighter *highlighter, const Core::MimeType &mimeType,
+                               const QString &filePath,
                                QString *foundDefinitionId = 0);
-QString findDefinitionId(const Core::MimeType &mimeType, bool considerParents);
-TEXTEDITOR_EXPORT SyntaxHighlighter *createGenericSyntaxHighlighter(const Core::MimeType &mimeType);
 
 } // namespace TextEditor
 
diff --git a/src/plugins/texteditor/texteditor.cpp b/src/plugins/texteditor/texteditor.cpp
index c9c824f19a..56adaf2829 100644
--- a/src/plugins/texteditor/texteditor.cpp
+++ b/src/plugins/texteditor/texteditor.cpp
@@ -7178,7 +7178,8 @@ void TextEditorWidget::configureGenericHighlighter()
         d->m_isMissingSyntaxDefinition = true;
 
         QString definitionId;
-        setMimeTypeForHighlighter(highlighter, mimeType, &definitionId);
+        setMimeTypeForHighlighter(highlighter, mimeType, textDocument()->filePath().toString(),
+                                  &definitionId);
 
         if (!definitionId.isEmpty()) {
             d->m_isMissingSyntaxDefinition = false;
@@ -7218,7 +7219,7 @@ void TextEditorWidget::setupGenericHighlighter()
     connect(textDocument(), &IDocument::filePathChanged,
             d, &TextEditorWidgetPrivate::reconfigure);
 
-    connect(Manager::instance(), &Manager::mimeTypesRegistered,
+    connect(Manager::instance(), &Manager::highlightingFilesRegistered,
             d, &TextEditorWidgetPrivate::reconfigure);
 
     updateEditorInfoBar(this);
diff --git a/src/plugins/texteditor/texteditorplugin.cpp b/src/plugins/texteditor/texteditorplugin.cpp
index 0d3b11e144..efa8d8b4bd 100644
--- a/src/plugins/texteditor/texteditorplugin.cpp
+++ b/src/plugins/texteditor/texteditorplugin.cpp
@@ -125,7 +125,7 @@ bool TextEditorPlugin::initialize(const QStringList &arguments, QString *errorMe
     });
 
     // Generic highlighter.
-    connect(ICore::instance(), &ICore::coreOpened, Manager::instance(), &Manager::registerMimeTypes);
+    connect(ICore::instance(), &ICore::coreOpened, Manager::instance(), &Manager::registerHighlightingFiles);
 
     // Add text snippet provider.
     addAutoReleasedObject(new PlainTextSnippetProvider);