Fellow qml hackers,
I wasn't too happy with type shadowing and resolving in current qml.
Background: the implicit "." import has been moved to be the last import
checked. This was done in order to support our previous coding convention with
case insensitive file systems.
Example:
Foo/foo.qml imports "content" and instantiates Foo{}
content/Foo.qml implements Foo
with the correct lookup order, foo.qml would instantiate itself as Foo.qml -
which breaks.
This creates one big problem: If you add a type MyType.qml to your local
directory, you cannot be sure that it isn't shadowed by some import. If so,
the shadowing would be completely silent. This is unexpected behaviour and
hard to debug.
My suggestion (as implemented in the patch) is to disallow shadowing of types
completely. We don't need it since qml has excellent namespace and versioning
capabilities.
In addition I suggest to forbid recursive instantiations as well. A file
Button.qml should not instantiate the same Button.qml inside.
The above use case with the lowercase main file can be supported with a tiny
little trick: if a file would instantiate itself recursively, but the "same"
type can be found elsewhere, take it from elsewhere and continue.
Works like a charm :)
The patch breaks a few testcases in qdeclarativelanguage due to its more
explicit error messages and the strict no-shadowing policy. Warwick, Aaron et.
al., if you like the idea, I will fix the tests.
Matthias
diff --git a/src/declarative/qml/qdeclarativecompositetypemanager.cpp b/src/declarative/qml/qdeclarativecompositetypemanager.cpp
index 133b71f..5ca0892 100644
--- a/src/declarative/qml/qdeclarativecompositetypemanager.cpp
+++ b/src/declarative/qml/qdeclarativecompositetypemanager.cpp
@@ -530,32 +530,6 @@ int QDeclarativeCompositeTypeManager::resolveTypes(QDeclarativeCompositeTypeData
int waiting = 0;
- /*
- For local urls, add an implicit import "." as first (most overridden) lookup. This will also trigger
- the loading of the qmldir and the import of any native types from available plugins.
- */
- {
-
- QDeclarativeDirComponents qmldircomponentsnetwork;
- if (QDeclarativeCompositeTypeResource *resource
- = resources.value(unit->imports.baseUrl().resolved(QUrl(QLatin1String("./qmldir"))))) {
- QDeclarativeDirParser parser;
- parser.setSource(QString::fromUtf8(resource->data));
- parser.parse();
- qmldircomponentsnetwork = parser.components();
- }
-
- QDeclarativeEnginePrivate::get(engine)->
- addToImport(&unit->imports,
- qmldircomponentsnetwork,
- QLatin1String("."),
- QString(),
- -1, -1,
- QDeclarativeScriptParser::Import::File,
- 0); // error ignored (just means no fallback)
- }
-
-
foreach (QDeclarativeScriptParser::Import imp, unit->data.imports()) {
QDeclarativeDirComponents qmldircomponentsnetwork;
if (imp.type == QDeclarativeScriptParser::Import::Script)
@@ -605,6 +579,32 @@ int QDeclarativeCompositeTypeManager::resolveTypes(QDeclarativeCompositeTypeData
}
}
+ /*
+ For local urls, add an implicit import "." as last (first looked at) lookup. This will also trigger
+ the loading of the qmldir and the import of any native types from available plugins.
+ */
+ {
+
+ QDeclarativeDirComponents qmldircomponentsnetwork;
+ if (QDeclarativeCompositeTypeResource *resource
+ = resources.value(unit->imports.baseUrl().resolved(QUrl(QLatin1String("./qmldir"))))) {
+ QDeclarativeDirParser parser;
+ parser.setSource(QString::fromUtf8(resource->data));
+ parser.parse();
+ qmldircomponentsnetwork = parser.components();
+ }
+
+ QDeclarativeEnginePrivate::get(engine)->
+ addToImport(&unit->imports,
+ qmldircomponentsnetwork,
+ QLatin1String("."),
+ QString(),
+ -1, -1,
+ QDeclarativeScriptParser::Import::File,
+ 0); // error ignored (just means no fallback)
+ }
+
+
QList<QDeclarativeScriptParser::TypeReference*> types = unit->data.referencedTypes();
@@ -618,7 +618,8 @@ int QDeclarativeCompositeTypeManager::resolveTypes(QDeclarativeCompositeTypeData
int majorVersion;
int minorVersion;
QDeclarativeEnginePrivate::ImportedNamespace *typeNamespace = 0;
- if (!QDeclarativeEnginePrivate::get(engine)->resolveType(unit->imports, typeName, &ref.type, &url, &majorVersion, &minorVersion, &typeNamespace)
+ QString errorString;
+ if (!QDeclarativeEnginePrivate::get(engine)->resolveType(unit->imports, typeName, &ref.type, &url, &majorVersion, &minorVersion, &typeNamespace, &errorString)
|| typeNamespace)
{
// Known to not be a type:
@@ -631,7 +632,8 @@ int QDeclarativeCompositeTypeManager::resolveTypes(QDeclarativeCompositeTypeData
if (typeNamespace)
error.setDescription(tr("Namespace %1 cannot be used as a type").arg(userTypeName));
else
- error.setDescription(tr("%1 is not a type").arg(userTypeName));
+ error.setDescription(tr("%1 %2").arg(userTypeName).arg(errorString));
+
if (!parserRef->refObjects.isEmpty()) {
QDeclarativeParser::Object *obj = parserRef->refObjects.first();
error.setLine(obj->location.start.line);
diff --git a/src/declarative/qml/qdeclarativeengine.cpp b/src/declarative/qml/qdeclarativeengine.cpp
index 96145fb..ec269d2 100644
--- a/src/declarative/qml/qdeclarativeengine.cpp
+++ b/src/declarative/qml/qdeclarativeengine.cpp
@@ -1425,55 +1425,65 @@ struct QDeclarativeEnginePrivate::ImportedNamespace {
QList<bool> isLibrary;
QList<QDeclarativeDirComponents> qmlDirComponents;
- bool find(const QByteArray& type, int *vmajor, int *vminor, QDeclarativeType** type_return, QUrl* url_return,
- QUrl *base = 0)
+
+ bool find_helper(int i, const QByteArray& type, int *vmajor, int *vminor,
+ QDeclarativeType** type_return, QUrl* url_return,
+ QUrl *base = 0, bool *typeRecursionDetected = 0)
{
- for (int i=0; i<urls.count(); ++i) {
- int vmaj = majversions.at(i);
- int vmin = minversions.at(i);
-
- QByteArray qt = uris.at(i).toUtf8();
- qt += '/';
- qt += type;
-
- QDeclarativeType *t = QDeclarativeMetaType::qmlType(qt,vmaj,vmin);
- if (t) {
- if (vmajor) *vmajor = vmaj;
- if (vminor) *vminor = vmin;
- if (type_return)
- *type_return = t;
- return true;
- }
+ int vmaj = majversions.at(i);
+ int vmin = minversions.at(i);
+
+ QByteArray qt = uris.at(i).toUtf8();
+ qt += '/';
+ qt += type;
+
+ QDeclarativeType *t = QDeclarativeMetaType::qmlType(qt,vmaj,vmin);
+ if (t) {
+ if (vmajor) *vmajor = vmaj;
+ if (vminor) *vminor = vmin;
+ if (type_return)
+ *type_return = t;
+ return true;
+ }
- QUrl url = QUrl(urls.at(i) + QLatin1Char('/') + QString::fromUtf8(type) + QLatin1String(".qml"));
- QDeclarativeDirComponents qmldircomponents = qmlDirComponents.at(i);
-
- bool typeWasDeclaredInQmldir = false;
- if (!qmldircomponents.isEmpty()) {
- const QString typeName = QString::fromUtf8(type);
- foreach (const QDeclarativeDirParser::Component &c, qmldircomponents) {
- if (c.typeName == typeName) {
- typeWasDeclaredInQmldir = true;
-
- // importing version -1 means import ALL versions
- if ((vmaj == -1) || (c.majorVersion < vmaj || (c.majorVersion == vmaj && vmin >= c.minorVersion))) {
- QUrl candidate = url.resolved(QUrl(c.fileName));
- if (c.internal && base) {
- if (base->resolved(QUrl(c.fileName)) != candidate)
- continue; // failed attempt to access an internal type
- }
- if (url_return)
- *url_return = candidate;
- return true;
+ QUrl url = QUrl(urls.at(i) + QLatin1Char('/') + QString::fromUtf8(type) + QLatin1String(".qml"));
+ QDeclarativeDirComponents qmldircomponents = qmlDirComponents.at(i);
+
+ bool typeWasDeclaredInQmldir = false;
+ if (!qmldircomponents.isEmpty()) {
+ const QString typeName = QString::fromUtf8(type);
+ foreach (const QDeclarativeDirParser::Component &c, qmldircomponents) {
+ if (c.typeName == typeName) {
+ typeWasDeclaredInQmldir = true;
+
+ // importing version -1 means import ALL versions
+ if ((vmaj == -1) || (c.majorVersion < vmaj || (c.majorVersion == vmaj && vmin >= c.minorVersion))) {
+ QUrl candidate = url.resolved(QUrl(c.fileName));
+ if (c.internal && base) {
+ if (base->resolved(QUrl(c.fileName)) != candidate)
+ continue; // failed attempt to access an internal type
+ }
+ if (base && *base == candidate) {
+ if (typeRecursionDetected)
+ *typeRecursionDetected = true;
+ continue; // no recursion
}
+ if (url_return)
+ *url_return = candidate;
+ return true;
}
}
}
+ }
- if (!typeWasDeclaredInQmldir && !isLibrary.at(i)) {
- // XXX search non-files too! (eg. zip files, see QT-524)
- QFileInfo f(toLocalFileOrQrc(url));
- if (f.exists()) {
+ if (!typeWasDeclaredInQmldir && !isLibrary.at(i)) {
+ // XXX search non-files too! (eg. zip files, see QT-524)
+ QFileInfo f(toLocalFileOrQrc(url));
+ if (f.exists()) {
+ if (base && *base == url) { // no recursion
+ if (typeRecursionDetected)
+ *typeRecursionDetected = true;
+ } else {
if (url_return)
*url_return = url;
return true;
@@ -1482,6 +1492,34 @@ struct QDeclarativeEnginePrivate::ImportedNamespace {
}
return false;
}
+
+ bool find(const QByteArray& type, int *vmajor, int *vminor, QDeclarativeType** type_return,
+ QUrl* url_return, QUrl *base = 0, QString *errorString = 0)
+ {
+ bool typeRecursionDetected = false;
+ for (int i=0; i<urls.count(); ++i) {
+ if (find_helper(i, type, vmajor, vminor, type_return, url_return, base, &typeRecursionDetected)) {
+#if 1
+ for (int j = i+1; j<urls.count(); ++j) {
+ if (find_helper(j, type, vmajor, vminor, 0, 0, base)) {
+ // clash
+ if (errorString)
+ *errorString = QString(QLatin1String("is ambigous. Found in %1 and in %2")).arg(urls.at(i)).arg(urls.at(j));
+ return false;
+ }
+ }
+#endif
+ return true;
+ }
+ }
+ if (errorString) {
+ if (typeRecursionDetected)
+ *errorString = QLatin1String("is instantiated recursively");
+ else
+ *errorString = QLatin1String("is not a type");
+ }
+ return false;
+ }
};
static bool greaterThan(const QString &s1, const QString &s2)
@@ -1691,30 +1729,37 @@ public:
return true;
}
- bool find(const QByteArray& type, int *vmajor, int *vminor, QDeclarativeType** type_return, QUrl* url_return)
+ bool find(const QByteArray& type, int *vmajor, int *vminor, QDeclarativeType** type_return,
+ QUrl* url_return, QString *errorString)
{
QDeclarativeEnginePrivate::ImportedNamespace *s = 0;
int slash = type.indexOf('/');
if (slash >= 0) {
- s = set.value(QString::fromUtf8(type.left(slash)));
- if (!s)
- return false; // qualifier must be a namespace
+ QString namespaceName = QString::fromUtf8(type.left(slash));
+ s = set.value(namespaceName);
+ if (!s) {
+ if (errorString)
+ *errorString = QString(QLatin1String("- %1 is not a namespace")).arg(namespaceName);
+ return false;
+ }
int nslash = type.indexOf('/',slash+1);
- if (nslash > 0)
- return false; // only single qualification allowed
+ if (nslash > 0) {
+ if (errorString)
+ *errorString = QLatin1String("- nested namespaces not allowed");
+ return false;
+ }
} else {
s = &unqualifiedset;
}
QByteArray unqualifiedtype = slash < 0 ? type : type.mid(slash+1); // common-case opt (QString::mid works fine, but slower)
if (s) {
- if (s->find(unqualifiedtype,vmajor,vminor,type_return,url_return, &base))
+ if (s->find(unqualifiedtype,vmajor,vminor,type_return,url_return, &base, errorString))
return true;
if (s->urls.count() == 1 && !s->isLibrary[0] && url_return && s != &unqualifiedset) {
// qualified, and only 1 url
*url_return = QUrl(s->urls[0]+QLatin1Char('/')).resolved(QUrl(QString::fromUtf8(unqualifiedtype) + QLatin1String(".qml")));
return true;
}
-
}
return false;
@@ -2208,7 +2253,7 @@ bool QDeclarativeEnginePrivate::addToImport(Imports* imports, const QDeclarative
\sa addToImport()
*/
-bool QDeclarativeEnginePrivate::resolveType(const Imports& imports, const QByteArray& type, QDeclarativeType** type_return, QUrl* url_return, int *vmaj, int *vmin, ImportedNamespace** ns_return) const
+bool QDeclarativeEnginePrivate::resolveType(const Imports& imports, const QByteArray& type, QDeclarativeType** type_return, QUrl* url_return, int *vmaj, int *vmin, ImportedNamespace** ns_return, QString *errorString) const
{
ImportedNamespace* ns = imports.d->findNamespace(QString::fromUtf8(type));
if (ns) {
@@ -2217,7 +2262,7 @@ bool QDeclarativeEnginePrivate::resolveType(const Imports& imports, const QByteA
return true;
}
if (type_return || url_return) {
- if (imports.d->find(type,vmaj,vmin,type_return,url_return)) {
+ if (imports.d->find(type,vmaj,vmin,type_return,url_return, errorString)) {
if (qmlImportTrace()) {
if (type_return && *type_return && url_return && !url_return->isEmpty())
qDebug() << "QDeclarativeEngine::resolveType" << type << '=' << (*type_return)->typeName() << *url_return;
diff --git a/src/declarative/qml/qdeclarativeengine_p.h b/src/declarative/qml/qdeclarativeengine_p.h
index b3bba43..b2afdc4 100644
--- a/src/declarative/qml/qdeclarativeengine_p.h
+++ b/src/declarative/qml/qdeclarativeengine_p.h
@@ -288,7 +288,8 @@ public:
bool resolveType(const Imports&, const QByteArray& type,
QDeclarativeType** type_return, QUrl* url_return,
int *version_major, int *version_minor,
- ImportedNamespace** ns_return) const;
+ ImportedNamespace** ns_return,
+ QString *errorString = 0) const;
void resolveTypeInNamespace(ImportedNamespace*, const QByteArray& type,
QDeclarativeType** type_return, QUrl* url_return,
int *version_major, int *version_minor ) const;
_______________________________________________
Qt-qml mailing list
Qt-qml@trolltech.com
http://lists.trolltech.com/mailman/listinfo/qt-qml
