From 353e43cb6d73ee97ebff1a8c737b2c133b135ab8 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 7 Jan 2015 13:32:33 +0100 Subject: [PATCH] Unify QQmlDirParser version parsing and error reporting Add a parseVersion() helper function to avoid duplicating the same version parsing logic three times. Change-Id: I4e5b6a8c86ee3a26c4eb91c660a81176ac0346cf Reviewed-by: Simon Hausmann Reviewed-by: Shawn Rutledge --- src/qml/qml/qqmldirparser.cpp | 98 +++++++++------------- .../data/invalid-versioned-dependency/qmldir | 1 + .../data/invalid-versioned-script/qmldir | 1 + tests/auto/qml/qqmldirparser/tst_qqmldirparser.cpp | 20 ++++- 4 files changed, 60 insertions(+), 60 deletions(-) create mode 100644 tests/auto/qml/qqmldirparser/data/invalid-versioned-dependency/qmldir create mode 100644 tests/auto/qml/qqmldirparser/data/invalid-versioned-script/qmldir diff --git a/src/qml/qml/qqmldirparser.cpp b/src/qml/qml/qqmldirparser.cpp index 83e1c9c..5995b83 100644 --- a/src/qml/qml/qqmldirparser.cpp +++ b/src/qml/qml/qqmldirparser.cpp @@ -55,6 +55,19 @@ static int parseInt(const QStringRef &str, bool *ok) return number; } +static bool parseVersion(const QString &str, int *major, int *minor) +{ + const int dotIndex = str.indexOf(QLatin1Char('.')); + if (dotIndex != -1 && str.indexOf(QLatin1Char('.'), dotIndex + 1) == -1) { + bool ok = false; + *major = parseInt(QStringRef(&str, 0, dotIndex), &ok); + if (ok) + *minor = parseInt(QStringRef(&str, dotIndex + 1, str.length() - dotIndex - 1), &ok); + return ok; + } + return false; +} + QQmlDirParser::QQmlDirParser() : _designerSupported(false) { } @@ -192,27 +205,14 @@ bool QQmlDirParser::parse(const QString &source) } else { // handle qmldir module listing case where singleton is defined in the following pattern: // singleton TestSingletonType 2.0 TestSingletonType20.qml - const QString &version = sections[2]; - const int dotIndex = version.indexOf(QLatin1Char('.')); - - if (dotIndex == -1) { - reportError(lineNumber, 0, QLatin1String("expected '.'")); - } else if (version.indexOf(QLatin1Char('.'), dotIndex + 1) != -1) { - reportError(lineNumber, 0, QLatin1String("unexpected '.'")); + int major, minor; + if (parseVersion(sections[2], &major, &minor)) { + const QString &fileName = sections[3]; + Component entry(sections[1], fileName, major, minor); + entry.singleton = true; + _components.insertMulti(entry.typeName, entry); } else { - bool validVersionNumber = false; - const int majorVersion = parseInt(QStringRef(&version, 0, dotIndex), &validVersionNumber); - - if (validVersionNumber) { - const int minorVersion = parseInt(QStringRef(&version, dotIndex+1, version.length()-dotIndex-1), &validVersionNumber); - - if (validVersionNumber) { - const QString &fileName = sections[3]; - Component entry(sections[1], fileName, majorVersion, minorVersion); - entry.singleton = true; - _components.insertMulti(entry.typeName, entry); - } - } + reportError(lineNumber, 0, QStringLiteral("invalid version %1, expected .").arg(sections[2])); } } } else if (sections[0] == QLatin1String("typeinfo")) { @@ -238,53 +238,33 @@ bool QQmlDirParser::parse(const QString &source) continue; } - const QString &version = sections[2]; - const int dotIndex = version.indexOf(QLatin1Char('.')); - bool validVersionNumber = false; - const int majorVersion = parseInt(QStringRef(&version, 0, dotIndex), &validVersionNumber); - if (validVersionNumber) { - const int minorVersion = parseInt(QStringRef(&version, dotIndex+1, version.length()-dotIndex-1), &validVersionNumber); - - if (validVersionNumber) { - Component entry(sections[1], QString(), majorVersion, minorVersion); - entry.internal = true; - _dependencies.insert(entry.typeName, entry); - } + int major, minor; + if (parseVersion(sections[2], &major, &minor)) { + Component entry(sections[1], QString(), major, minor); + entry.internal = true; + _dependencies.insert(entry.typeName, entry); } else { - reportError(lineNumber, 0, QString(QLatin1String("invalid version %1")).arg(version)); + reportError(lineNumber, 0, QStringLiteral("invalid version %1, expected .").arg(sections[2])); } } else if (sectionCount == 2) { // No version specified (should only be used for relative qmldir files) const Component entry(sections[0], sections[1], -1, -1); _components.insertMulti(entry.typeName, entry); } else if (sectionCount == 3) { - const QString &version = sections[1]; - const int dotIndex = version.indexOf(QLatin1Char('.')); - - if (dotIndex == -1) { - reportError(lineNumber, 0, QLatin1String("expected '.'")); - } else if (version.indexOf(QLatin1Char('.'), dotIndex + 1) != -1) { - reportError(lineNumber, 0, QLatin1String("unexpected '.'")); - } else { - bool validVersionNumber = false; - const int majorVersion = parseInt(QStringRef(&version, 0, dotIndex), &validVersionNumber); - - if (validVersionNumber) { - const int minorVersion = parseInt(QStringRef(&version, dotIndex+1, version.length()-dotIndex-1), &validVersionNumber); - - if (validVersionNumber) { - const QString &fileName = sections[2]; - - if (fileName.endsWith(QLatin1String(".js"))) { - // A 'js' extension indicates a namespaced script import - const Script entry(sections[0], fileName, majorVersion, minorVersion); - _scripts.append(entry); - } else { - const Component entry(sections[0], fileName, majorVersion, minorVersion); - _components.insertMulti(entry.typeName, entry); - } - } + int major, minor; + if (parseVersion(sections[1], &major, &minor)) { + const QString &fileName = sections[2]; + + if (fileName.endsWith(QLatin1String(".js"))) { + // A 'js' extension indicates a namespaced script import + const Script entry(sections[0], fileName, major, minor); + _scripts.append(entry); + } else { + const Component entry(sections[0], fileName, major, minor); + _components.insertMulti(entry.typeName, entry); } + } else { + reportError(lineNumber, 0, QStringLiteral("invalid version %1, expected .").arg(sections[1])); } } else { reportError(lineNumber, 0, diff --git a/tests/auto/qml/qqmldirparser/data/invalid-versioned-dependency/qmldir b/tests/auto/qml/qqmldirparser/data/invalid-versioned-dependency/qmldir new file mode 100644 index 0000000..4aa5c72 --- /dev/null +++ b/tests/auto/qml/qqmldirparser/data/invalid-versioned-dependency/qmldir @@ -0,0 +1 @@ +depends bar 100 diff --git a/tests/auto/qml/qqmldirparser/data/invalid-versioned-script/qmldir b/tests/auto/qml/qqmldirparser/data/invalid-versioned-script/qmldir new file mode 100644 index 0000000..19b65e3 --- /dev/null +++ b/tests/auto/qml/qqmldirparser/data/invalid-versioned-script/qmldir @@ -0,0 +1 @@ +foo 100 bar.js diff --git a/tests/auto/qml/qqmldirparser/tst_qqmldirparser.cpp b/tests/auto/qml/qqmldirparser/tst_qqmldirparser.cpp index 615df08..5b60ba2 100644 --- a/tests/auto/qml/qqmldirparser/tst_qqmldirparser.cpp +++ b/tests/auto/qml/qqmldirparser/tst_qqmldirparser.cpp @@ -247,7 +247,7 @@ void tst_qqmldirparser::parse_data() QTest::newRow("invalid-versioned-component") << "invalid-versioned-component/qmldir" - << (QStringList() << "qmldir:1: expected '.'") + << (QStringList() << "qmldir:1: invalid version 100, expected .") << QStringList() << QStringList() << QStringList() @@ -263,6 +263,15 @@ void tst_qqmldirparser::parse_data() << QStringList() << false; + QTest::newRow("invalid-versioned-script") + << "invalid-versioned-script/qmldir" + << (QStringList() << "qmldir:1: invalid version 100, expected .") + << QStringList() + << QStringList() + << QStringList() + << QStringList() + << false; + QTest::newRow("versioned-script") << "versioned-script/qmldir" << QStringList() @@ -301,6 +310,15 @@ void tst_qqmldirparser::parse_data() << QStringList() << false; + QTest::newRow("invalid-versioned-dependency") + << "invalid-versioned-dependency/qmldir" + << (QStringList() << "qmldir:1: invalid version 100, expected .") + << QStringList() + << QStringList() + << QStringList() + << QStringList() + << false; + QTest::newRow("dependency") << "dependency/qmldir" << QStringList() -- 2.7.4