diff --git a/src/qml/qml/qqmlimport.cpp b/src/qml/qml/qqmlimport.cpp index e17d98d263..69657d1fda 100644 --- a/src/qml/qml/qqmlimport.cpp +++ b/src/qml/qml/qqmlimport.cpp @@ -987,26 +987,6 @@ QString QQmlImports::resolvedUri(const QString &dir_arg, QQmlImportDatabase *dat return stableRelativePath; } -/* removes all file selector occurrences in path - firstPlus is the position of the initial '+' in the path - which we always have as we check for '+' to decide whether - we need to do some work at all -*/ -static QString pathWithoutFileSelectors(QString path, // we want a copy of path - qsizetype firstPlus) -{ - do { - Q_ASSERT(path.at(firstPlus) == u'+'); - const auto eos = path.size(); - qsizetype terminatingSlashPos = firstPlus + 1; - while (terminatingSlashPos != eos && path.at(terminatingSlashPos) != u'/') - ++terminatingSlashPos; - path.remove(firstPlus, terminatingSlashPos - firstPlus + 1); - firstPlus = path.indexOf(u'+', firstPlus); - } while (firstPlus != -1); - return path; -} - /*! \internal @@ -1053,47 +1033,17 @@ QTypeRevision QQmlImports::matchingQmldirVersion( typedef QQmlDirComponents::const_iterator ConstIterator; const QQmlDirComponents &components = qmldir.components(); - QMultiHash baseFileName2ConflictingComponents; - ConstIterator cend = components.constEnd(); for (ConstIterator cit = components.constBegin(); cit != cend; ++cit) { for (ConstIterator cit2 = components.constBegin(); cit2 != cit; ++cit2) { if (cit2->typeName == cit->typeName && cit2->version == cit->version) { - // ugly heuristic to deal with file selectors - const auto comp2PotentialFileSelectorPos = cit2->fileName.indexOf(u'+'); - const bool comp2MightHaveFileSelector = comp2PotentialFileSelectorPos != -1; - /* If we detect conflicting paths, we check if they agree when we remove anything looking like a - file selector. - We need to create copies of the filenames, otherwise QString::replace would modify the - existing file-names - */ - QString compFileName1 = cit->fileName; - QString compFileName2 = cit2->fileName; - if (auto fileSelectorPos1 = compFileName1.indexOf(u'+'); fileSelectorPos1 != -1) { - // existing entry was file selector entry, fix it up - // it could also be the case that _both_ are using file selectors - QString baseName = comp2MightHaveFileSelector ? pathWithoutFileSelectors(compFileName2, - comp2PotentialFileSelectorPos) - : compFileName2; - if (pathWithoutFileSelectors(compFileName1, fileSelectorPos1) == baseName) { - baseFileName2ConflictingComponents.insert(baseName, cit); - baseFileName2ConflictingComponents.insert(baseName, cit2); - continue; - } - // fall through to error case - } else if (comp2MightHaveFileSelector) { - // new entry contains file selector (and we now that cit did not) - if (pathWithoutFileSelectors(compFileName2, comp2PotentialFileSelectorPos) == compFileName1) { - baseFileName2ConflictingComponents.insert(compFileName1, cit2); - continue; - } - // fall through to error case - } // This entry clashes with a predecessor QQmlError error; - error.setDescription(QQmlImportDatabase::tr("\"%1\" version %2.%3 is defined more than once in module \"%4\"") - .arg(cit->typeName).arg(cit->version.majorVersion()) - .arg(cit->version.minorVersion()).arg(uri)); + error.setDescription( + QQmlImportDatabase::tr( + "\"%1\" version %2.%3 is defined more than once in module \"%4\"") + .arg(cit->typeName).arg(cit->version.majorVersion()) + .arg(cit->version.minorVersion()).arg(uri)); errors->prepend(error); return QTypeRevision(); } @@ -1102,14 +1052,6 @@ QTypeRevision QQmlImports::matchingQmldirVersion( addVersion(cit->version); } - // ensure that all components point to the actual base URL, and let the file selectors resolve them correctly during URL resolution - for (auto keyIt = baseFileName2ConflictingComponents.keyBegin(); keyIt != baseFileName2ConflictingComponents.keyEnd(); ++keyIt) { - const QString& baseFileName = *keyIt; - const auto conflictingComponents = baseFileName2ConflictingComponents.values(baseFileName); - for (ConstIterator component: conflictingComponents) - component->fileName = baseFileName; - } - typedef QList::const_iterator SConstIterator; const QQmlDirScripts &scripts = qmldir.scripts(); diff --git a/src/qml/qml/qqmltypeloaderqmldircontent.cpp b/src/qml/qml/qqmltypeloaderqmldircontent.cpp index 57044fcdc3..15767f01ad 100644 --- a/src/qml/qml/qqmltypeloaderqmldircontent.cpp +++ b/src/qml/qml/qqmltypeloaderqmldircontent.cpp @@ -29,6 +29,7 @@ void QQmlTypeLoaderQmldirContent::setContent(const QString &location, const QStr m_hasContent = true; m_location = location; m_parser.parse(content); + m_parser.disambiguateFileSelectors(); } void QQmlTypeLoaderQmldirContent::setError(const QQmlError &error) diff --git a/src/qml/qmldirparser/qqmldirparser.cpp b/src/qml/qmldirparser/qqmldirparser.cpp index a2983995cc..e6a5691d3d 100644 --- a/src/qml/qmldirparser/qqmldirparser.cpp +++ b/src/qml/qmldirparser/qqmldirparser.cpp @@ -376,6 +376,129 @@ bool QQmlDirParser::parse(const QString &source) return hasError(); } +/* removes all file selector occurrences in path + firstPlus is the position of the initial '+' in the path + which we always have as we check for '+' to decide whether + we need to do some work at all +*/ +static QString pathWithoutFileSelectors(QString path, // we want a copy of path + qsizetype firstPlus) +{ + do { + Q_ASSERT(path.at(firstPlus) == u'+'); + const auto eos = path.size(); + qsizetype terminatingSlashPos = firstPlus + 1; + while (terminatingSlashPos != eos && path.at(terminatingSlashPos) != u'/') + ++terminatingSlashPos; + path.remove(firstPlus, terminatingSlashPos - firstPlus + 1); + firstPlus = path.indexOf(u'+', firstPlus); + } while (firstPlus != -1); + return path; +} + +static bool canDisambiguate( + const QString &fileName1, const QString &fileName2, QString *correctedFileName) +{ + // If the entries are exactly the same we can delete one without losing anything. + if (fileName1 == fileName2) + return true; + + // If we detect conflicting paths, we check if they agree when we remove anything + // looking like a file selector. + + // ugly heuristic to deal with file selectors + const qsizetype file2PotentialFileSelectorPos = fileName2.indexOf(u'+'); + const bool file2MightHaveFileSelector = file2PotentialFileSelectorPos != -1; + + if (const qsizetype fileSelectorPos1 = fileName1.indexOf(u'+'); fileSelectorPos1 != -1) { + // existing entry was file selector entry, fix it up + // it could also be the case that _both_ are using file selectors + const QString baseName = file2MightHaveFileSelector + ? pathWithoutFileSelectors(fileName2, file2PotentialFileSelectorPos) + : fileName2; + + if (pathWithoutFileSelectors(fileName1, fileSelectorPos1) != baseName) + return false; + + *correctedFileName = baseName; + return true; + } + + // new entry contains file selector (and we know that fileName1 did not) + if (file2MightHaveFileSelector + && pathWithoutFileSelectors(fileName2, file2PotentialFileSelectorPos) == fileName1) { + *correctedFileName = fileName1; + return true; + } + + return false; +} + +static void disambiguateFileSelectedComponents(QQmlDirComponents *components) +{ + using ConstIterator = QQmlDirComponents::const_iterator; + + // end iterator may get invalidated by the erasing below. + // Therefore, refetch it on each iteration. + for (ConstIterator cit = components->constBegin(); cit != components->constEnd();) { + + // We can erase and re-assign cit if we immediately forget cit2. + // But we cannot erase cit2 without potentially invalidating cit. + + bool doErase = false; + const ConstIterator cend = components->constEnd(); + for (ConstIterator cit2 = ++ConstIterator(cit); cit2 != cend; ++cit2) { + if (cit2.key() != cit.key()) + break; + + Q_ASSERT(cit2->typeName == cit->typeName); + + if (cit2->version != cit->version + || cit2->internal != cit->internal + || cit2->singleton != cit->singleton) { + continue; + } + + // The two components may differ only by fileName now. + + if (canDisambiguate(cit->fileName, cit2->fileName, &(cit2->fileName))) { + doErase = true; + break; + } + } + + if (doErase) + cit = components->erase(cit); + else + ++cit; + } +} + +static void disambiguateFileSelectedScripts(QQmlDirScripts *scripts) +{ + using Iterator = QQmlDirScripts::iterator; + + Iterator send = scripts->end(); + + for (Iterator sit = scripts->begin(); sit != send; ++sit) { + send = std::remove_if(++Iterator(sit), send, [sit](const QQmlDirParser::Script &script2) { + if (sit->nameSpace != script2.nameSpace || sit->version != script2.version) + return false; + + // The two scripts may differ only by fileName now. + return canDisambiguate(sit->fileName, script2.fileName, &(sit->fileName)); + }); + } + + scripts->erase(send, scripts->end()); +} + +void QQmlDirParser::disambiguateFileSelectors() +{ + disambiguateFileSelectedComponents(&_components); + disambiguateFileSelectedScripts(&_scripts); +} + void QQmlDirParser::reportError(quint16 line, quint16 column, const QString &description) { QQmlJS::DiagnosticMessage error; diff --git a/src/qml/qmldirparser/qqmldirparser_p.h b/src/qml/qmldirparser/qqmldirparser_p.h index 9a04997048..43409b7b41 100644 --- a/src/qml/qmldirparser/qqmldirparser_p.h +++ b/src/qml/qmldirparser/qqmldirparser_p.h @@ -30,6 +30,7 @@ class Q_QML_COMPILER_PRIVATE_EXPORT QQmlDirParser public: void clear(); bool parse(const QString &source); + void disambiguateFileSelectors(); bool hasError() const { return !_errors.isEmpty(); } void setError(const QQmlJS::DiagnosticMessage &); diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest/main.qml b/tests/auto/qml/qqmlfileselector/data/qmldirtest/main.qml index d6dd2c9b90..4e09798a84 100644 --- a/tests/auto/qml/qqmlfileselector/data/qmldirtest/main.qml +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest/main.qml @@ -1,10 +1,8 @@ import QtQuick import qmldirtest -Window { - width: 640 - height: 480 - visible: true +Item { + objectName: Name.name property color color: mybutton.color MyButton { diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/+linux/Name.js b/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/+linux/Name.js new file mode 100644 index 0000000000..91ca4f129d --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/+linux/Name.js @@ -0,0 +1 @@ +var name = "linux" diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/+macos/Name.js b/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/+macos/Name.js new file mode 100644 index 0000000000..12e5058285 --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/+macos/Name.js @@ -0,0 +1 @@ +var name = "macos" diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/Name.js b/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/Name.js new file mode 100644 index 0000000000..916a232eb4 --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest/qml/Name.js @@ -0,0 +1 @@ +var name = "base" diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest/qmldir b/tests/auto/qml/qqmlfileselector/data/qmldirtest/qmldir index ac68d9097d..a2efdbf27d 100644 --- a/tests/auto/qml/qqmlfileselector/data/qmldirtest/qmldir +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest/qmldir @@ -2,4 +2,7 @@ module qmldirtest MyButton 1.0 qml/MyButton.qml MyButton 1.0 qml/+linux/MyButton.qml MyButton 1.0 qml/+macos/MyButton.qml +Name 1.0 qml/Name.js +Name 1.0 qml/+linux/Name.js +Name 1.0 qml/+macos/Name.js diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+bar/MyButton.qml b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+bar/MyButton.qml new file mode 100644 index 0000000000..5bf632c48d --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+bar/MyButton.qml @@ -0,0 +1,7 @@ +import QtQuick + +Rectangle { + width: 300 + height: 50 + color: "yellow" +} diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+bar/Name.js b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+bar/Name.js new file mode 100644 index 0000000000..8591795d37 --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+bar/Name.js @@ -0,0 +1 @@ +var name = "bar" diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+foo/MyButton.qml b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+foo/MyButton.qml new file mode 100644 index 0000000000..cc6eb967da --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+foo/MyButton.qml @@ -0,0 +1,7 @@ +import QtQuick + +Rectangle { + width: 300 + height: 50 + color: "blue" +} diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+foo/Name.js b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+foo/Name.js new file mode 100644 index 0000000000..b224ed15ec --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/+foo/Name.js @@ -0,0 +1 @@ +var name = "foo" diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest2/MyButton.qml b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/MyButton.qml new file mode 100644 index 0000000000..32db428c4f --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/MyButton.qml @@ -0,0 +1,7 @@ +import QtQuick + +Rectangle { + width: 300 + height: 50 + color: "green" +} diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest2/Name.js b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/Name.js new file mode 100644 index 0000000000..916a232eb4 --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/Name.js @@ -0,0 +1 @@ +var name = "base" diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest2/main.qml b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/main.qml new file mode 100644 index 0000000000..3ef7df59e0 --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/main.qml @@ -0,0 +1,9 @@ +import QtQuick + +Item { + // TODO: objectName: Name.name + property color color: mybutton.color + MyButton { + id: mybutton + } +} diff --git a/tests/auto/qml/qqmlfileselector/data/qmldirtest2/qmldir b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/qmldir new file mode 100644 index 0000000000..92fefb9806 --- /dev/null +++ b/tests/auto/qml/qqmlfileselector/data/qmldirtest2/qmldir @@ -0,0 +1,5 @@ +module qmldirtest2 +MyButton 1.0 +foo/MyButton.qml +MyButton 1.0 MyButton.qml +MyButton 1.0 +bar/MyButton.qml +Name 1.0 Name.js diff --git a/tests/auto/qml/qqmlfileselector/tst_qqmlfileselector.cpp b/tests/auto/qml/qqmlfileselector/tst_qqmlfileselector.cpp index 46df20378c..4e746ab10e 100644 --- a/tests/auto/qml/qqmlfileselector/tst_qqmlfileselector.cpp +++ b/tests/auto/qml/qqmlfileselector/tst_qqmlfileselector.cpp @@ -72,19 +72,40 @@ void tst_qqmlfileselector::applicationEngineTest() void tst_qqmlfileselector::qmldirCompatibility() { - QQmlApplicationEngine engine; - engine.addImportPath(dataDirectory()); - engine.load(testFileUrl("qmldirtest/main.qml")); - QVERIFY(!engine.rootObjects().isEmpty()); - QObject *object = engine.rootObjects().at(0); - auto color = object->property("color").value(); + { + // No error for multiple files with different selectors, and the matching one is chosen + // for +macos and +linux selectors. + QQmlApplicationEngine engine; + engine.addImportPath(dataDirectory()); + engine.load(testFileUrl("qmldirtest/main.qml")); + QVERIFY(!engine.rootObjects().isEmpty()); + QObject *object = engine.rootObjects().at(0); + auto color = object->property("color").value(); #if defined(Q_OS_LINUX) && !defined(Q_OS_ANDROID) - QCOMPARE(color, QColorConstants::Svg::blue); + QCOMPARE(object->objectName(), "linux"); + QCOMPARE(color, QColorConstants::Svg::blue); #elif defined(Q_OS_DARWIN) - QCOMPARE(color, QColorConstants::Svg::yellow); + QCOMPARE(object->objectName(), "macos"); + QCOMPARE(color, QColorConstants::Svg::yellow); #else - QCOMPARE(color, QColorConstants::Svg::green); + QCOMPARE(object->objectName(), "base"); + QCOMPARE(color, QColorConstants::Svg::green); #endif + } + + { + // If nothing matches, the _base_ file is chosen, not the first or the last one. + // This also holds when using the implicit import. + QQmlApplicationEngine engine; + engine.addImportPath(dataDirectory()); + engine.load(testFileUrl("qmldirtest2/main.qml")); + QVERIFY(!engine.rootObjects().isEmpty()); + QObject *object = engine.rootObjects().at(0); + QCOMPARE(object->property("color").value(), QColorConstants::Svg::green); + + QEXPECT_FAIL("", "scripts in implicit import are not resolved", Continue); + QCOMPARE(object->objectName(), "base"); + } } QTEST_MAIN(tst_qqmlfileselector)