From 52a5a89ea482b986560f7c50737a8634ebbcf152 Mon Sep 17 00:00:00 2001 From: Aleix Pol Date: Mon, 18 Sep 2023 17:05:37 +0200 Subject: [PATCH] QCommandLineParser: Warn invalid value calls If the QCommandLineOption doesn't have a valueName, the parser won't read the argument, therefore returning an empty value. If the developers are calling ::value on the option, they clearly think it's expected to get a value but won't ever be getting one, so we better warn them about it. Change-Id: I434b94c0b817b5d9d137c17f32b92af363f93eb8 Reviewed-by: David Faure --- src/corelib/tools/qcommandlineparser.cpp | 12 +++-- .../tst_qcommandlineparser.cpp | 45 ++++++++++++------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/corelib/tools/qcommandlineparser.cpp b/src/corelib/tools/qcommandlineparser.cpp index 94370f4938..2880eedf77 100644 --- a/src/corelib/tools/qcommandlineparser.cpp +++ b/src/corelib/tools/qcommandlineparser.cpp @@ -820,7 +820,7 @@ bool QCommandLineParser::isSet(const QString &name) const that option is returned. If the option wasn't specified on the command line, the default value is returned. - An empty string is returned if the option does not take a value. + If the option does not take a value, a warning is printed, and an empty string is returned. \sa values(), QCommandLineOption::setDefaultValue(), QCommandLineOption::setDefaultValues() */ @@ -861,8 +861,14 @@ QStringList QCommandLineParser::values(const QString &optionName) const if (it != d->nameHash.cend()) { const qsizetype optionOffset = *it; QStringList values = d->optionValuesHash.value(optionOffset); - if (values.isEmpty()) - values = d->commandLineOptionList.at(optionOffset).defaultValues(); + if (values.isEmpty()) { + const auto &option = d->commandLineOptionList.at(optionOffset); + if (option.valueName().isEmpty()) { + qWarning("QCommandLineParser: option not expecting values: \"%ls\"", + qUtf16Printable(optionName)); + } + values = option.defaultValues(); + } return values; } diff --git a/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp b/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp index a536926724..da31bf4538 100644 --- a/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp +++ b/tests/auto/corelib/tools/qcommandlineparser/tst_qcommandlineparser.cpp @@ -126,6 +126,7 @@ void tst_QCommandLineParser::testBooleanOption() QVERIFY(parser.parse(args)); QCOMPARE(parser.optionNames(), expectedOptionNames); QCOMPARE(parser.isSet("b"), expectedIsSet); + QTest::ignoreMessage(QtWarningMsg, "QCommandLineParser: option not expecting values: \"b\""); QCOMPARE(parser.values("b"), QStringList()); QCOMPARE(parser.positionalArguments(), QStringList()); // Should warn on typos @@ -163,6 +164,7 @@ void tst_QCommandLineParser::testOptionsAndPositional() QVERIFY(parser.parse(args)); QCOMPARE(parser.optionNames(), expectedOptionNames); QCOMPARE(parser.isSet("b"), expectedIsSet); + QTest::ignoreMessage(QtWarningMsg, "QCommandLineParser: option not expecting values: \"b\""); QCOMPARE(parser.values("b"), QStringList()); QCOMPARE(parser.positionalArguments(), expectedPositionalArguments); } @@ -361,6 +363,7 @@ void tst_QCommandLineParser::testProcessNotCalled() QTest::ignoreMessage(QtWarningMsg, "QCommandLineParser: call process() or parse() before isSet"); QVERIFY(!parser.isSet("b")); QTest::ignoreMessage(QtWarningMsg, "QCommandLineParser: call process() or parse() before values"); + QTest::ignoreMessage(QtWarningMsg, "QCommandLineParser: option not expecting values: \"b\""); QCOMPARE(parser.values("b"), QStringList()); } @@ -448,37 +451,40 @@ void tst_QCommandLineParser::testSingleDashWordOptionModes_data() QTest::addColumn("commandLine"); QTest::addColumn("expectedOptionNames"); QTest::addColumn("expectedOptionValues"); + QTest::addColumn("invalidOptionValues"); QTest::newRow("collapsed") << QCommandLineParser::ParseAsCompactedShortOptions << (QStringList() << "-abc" << "val") - << (QStringList() << "a" << "b" << "c") << (QStringList() << QString() << QString() << "val"); + << (QStringList() << "a" << "b" << "c") << (QStringList() << QString() << QString() << "val") + << (QStringList() << "a" << "b"); QTest::newRow("collapsed_with_equalsign_value") << QCommandLineParser::ParseAsCompactedShortOptions << (QStringList() << "-abc=val") - << (QStringList() << "a" << "b" << "c") << (QStringList() << QString() << QString() << "val"); + << (QStringList() << "a" << "b" << "c") << (QStringList() << QString() << QString() << "val") + << (QStringList() << "a" << "b"); QTest::newRow("collapsed_explicit_longoption") << QCommandLineParser::ParseAsCompactedShortOptions << QStringList("--nn") - << QStringList("nn") << QStringList(); + << QStringList("nn") << QStringList() << QStringList(); QTest::newRow("collapsed_longoption_value") << QCommandLineParser::ParseAsCompactedShortOptions << (QStringList() << "--abc" << "val") - << QStringList("abc") << QStringList("val"); + << QStringList("abc") << QStringList("val") << QStringList(); QTest::newRow("compiler") << QCommandLineParser::ParseAsCompactedShortOptions << QStringList("-cab") - << QStringList("c") << QStringList("ab"); + << QStringList("c") << QStringList("ab") << QStringList(); QTest::newRow("compiler_with_space") << QCommandLineParser::ParseAsCompactedShortOptions << (QStringList() << "-c" << "val") - << QStringList("c") << QStringList("val"); + << QStringList("c") << QStringList("val") << QStringList(); QTest::newRow("implicitlylong") << QCommandLineParser::ParseAsLongOptions << (QStringList() << "-abc" << "val") - << QStringList("abc") << QStringList("val"); + << QStringList("abc") << QStringList("val") << QStringList(); QTest::newRow("implicitlylong_equal") << QCommandLineParser::ParseAsLongOptions << (QStringList() << "-abc=val") - << QStringList("abc") << QStringList("val"); + << QStringList("abc") << QStringList("val") << QStringList(); QTest::newRow("implicitlylong_longoption") << QCommandLineParser::ParseAsLongOptions << (QStringList() << "--nn") - << QStringList("nn") << QStringList(); + << QStringList("nn") << QStringList() << QStringList(); QTest::newRow("implicitlylong_longoption_value") << QCommandLineParser::ParseAsLongOptions << (QStringList() << "--abc" << "val") - << QStringList("abc") << QStringList("val"); + << QStringList("abc") << QStringList("val") << QStringList(); QTest::newRow("implicitlylong_with_space") << QCommandLineParser::ParseAsCompactedShortOptions << (QStringList() << "-c" << "val") - << QStringList("c") << QStringList("val"); + << QStringList("c") << QStringList("val") << QStringList(); QTest::newRow("forceshort_detached") << QCommandLineParser::ParseAsLongOptions << (QStringList() << "-I" << "45") - << QStringList("I") << QStringList("45"); + << QStringList("I") << QStringList("45") << QStringList(); QTest::newRow("forceshort_attached") << QCommandLineParser::ParseAsLongOptions << (QStringList() << "-I46") - << QStringList("I") << QStringList("46"); + << QStringList("I") << QStringList("46") << QStringList(); QTest::newRow("forceshort_mixed") << QCommandLineParser::ParseAsLongOptions << (QStringList() << "-I45" << "-nn") - << (QStringList() << "I" << "nn") << QStringList("45"); + << (QStringList() << "I" << "nn") << QStringList("45") << QStringList(); } void tst_QCommandLineParser::testSingleDashWordOptionModes() @@ -487,6 +493,7 @@ void tst_QCommandLineParser::testSingleDashWordOptionModes() QFETCH(QStringList, commandLine); QFETCH(QStringList, expectedOptionNames); QFETCH(QStringList, expectedOptionValues); + QFETCH(QStringList, invalidOptionValues); commandLine.prepend("tst_QCommandLineParser"); @@ -503,8 +510,14 @@ void tst_QCommandLineParser::testSingleDashWordOptionModes() QVERIFY(parser.addOption(forceShort)); QVERIFY(parser.parse(commandLine)); QCOMPARE(parser.optionNames(), expectedOptionNames); - for (int i = 0; i < expectedOptionValues.size(); ++i) - QCOMPARE(parser.value(parser.optionNames().at(i)), expectedOptionValues.at(i)); + for (int i = 0; i < expectedOptionValues.size(); ++i) { + const QString option = parser.optionNames().at(i); + if (invalidOptionValues.contains(option)) { + QByteArray msg = QLatin1String("QCommandLineParser: option not expecting values: \"%1\"").arg(option).toLatin1(); + QTest::ignoreMessage(QtWarningMsg, msg.data()); + } + QCOMPARE(parser.value(option), expectedOptionValues.at(i)); + } QCOMPARE(parser.unknownOptionNames(), QStringList()); }