qmllint: don't set error exit code if there are warnings
[ChangeLog][qmllint][Important Behavior Change] qmllint no longer exits with an error code if there are warnings by default. Actual errors will still cause the exit code to be set. [ChangeLog][qmllint] qmllint now accepts a max-warning flag. When set, it will set an error exit code if there are more warnings than specified. Pick-to: 6.8 Fixes: QTBUG-115856 Change-Id: I88810795a494ba249b0b39fd7f245229b1f0c959 Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io> Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
parent
90b693dfd7
commit
74ec76d5ed
|
@ -14,10 +14,11 @@ It also warns about some QML anti-patterns. If you want to disable a specific
|
||||||
warning type, you can find the appropriate flag for doing so by passing
|
warning type, you can find the appropriate flag for doing so by passing
|
||||||
\c{--help} on the command line.
|
\c{--help} on the command line.
|
||||||
|
|
||||||
By default, some issues will result in warnings that will be printed and result
|
By default, some issues will result in warnings that will be printed. If there
|
||||||
in a non-zero exit code.
|
are more warnings than a limit which can be configured with \c{--max-warnings},
|
||||||
|
the exit code will be non-zero.
|
||||||
Minor issues however (such as unused imports) are just informational messages
|
Minor issues however (such as unused imports) are just informational messages
|
||||||
by default and will not affect the exit code.
|
by default and will never affect the exit code.
|
||||||
qmllint is very configurable and allows for disabling warnings or changing how
|
qmllint is very configurable and allows for disabling warnings or changing how
|
||||||
they are treated.
|
they are treated.
|
||||||
Users may freely turn any issue into a warning, informational message, or
|
Users may freely turn any issue into a warning, informational message, or
|
||||||
|
|
|
@ -110,6 +110,8 @@ private Q_SLOTS:
|
||||||
void environment_data();
|
void environment_data();
|
||||||
void environment();
|
void environment();
|
||||||
|
|
||||||
|
void maxWarnings();
|
||||||
|
|
||||||
#if QT_CONFIG(library)
|
#if QT_CONFIG(library)
|
||||||
void testPlugin();
|
void testPlugin();
|
||||||
void quickPlugin();
|
void quickPlugin();
|
||||||
|
@ -125,6 +127,11 @@ private:
|
||||||
|
|
||||||
enum LintType { LintFile, LintModule };
|
enum LintType { LintFile, LintModule };
|
||||||
|
|
||||||
|
static QStringList warningsShouldFailArgs() {
|
||||||
|
static QStringList args {"-W", "0"};
|
||||||
|
return args;
|
||||||
|
}
|
||||||
|
|
||||||
QString runQmllint(const QString &fileToLint, std::function<void(QProcess &)> handleResult,
|
QString runQmllint(const QString &fileToLint, std::function<void(QProcess &)> handleResult,
|
||||||
const QStringList &extraArgs = QStringList(), bool ignoreSettings = true,
|
const QStringList &extraArgs = QStringList(), bool ignoreSettings = true,
|
||||||
bool addImportDirs = true, bool absolutePath = true,
|
bool addImportDirs = true, bool absolutePath = true,
|
||||||
|
@ -411,7 +418,7 @@ void TestQmllint::autoqmltypes()
|
||||||
{
|
{
|
||||||
QProcess process;
|
QProcess process;
|
||||||
process.setWorkingDirectory(testFile("autoqmltypes"));
|
process.setWorkingDirectory(testFile("autoqmltypes"));
|
||||||
process.start(m_qmllintPath, { QStringLiteral("test.qml") });
|
process.start(m_qmllintPath, warningsShouldFailArgs() << QStringLiteral("test.qml") );
|
||||||
|
|
||||||
process.waitForFinished();
|
process.waitForFinished();
|
||||||
|
|
||||||
|
@ -425,7 +432,7 @@ void TestQmllint::autoqmltypes()
|
||||||
{
|
{
|
||||||
QProcess bare;
|
QProcess bare;
|
||||||
bare.setWorkingDirectory(testFile("autoqmltypes"));
|
bare.setWorkingDirectory(testFile("autoqmltypes"));
|
||||||
bare.start(m_qmllintPath, { QStringLiteral("--bare"), QStringLiteral("test.qml") });
|
bare.start(m_qmllintPath, warningsShouldFailArgs() << QStringLiteral("--bare") << QStringLiteral("test.qml") );
|
||||||
bare.waitForFinished();
|
bare.waitForFinished();
|
||||||
|
|
||||||
const QByteArray errors = bare.readAllStandardError();
|
const QByteArray errors = bare.readAllStandardError();
|
||||||
|
@ -1795,17 +1802,17 @@ void TestQmllint::requiredProperty()
|
||||||
|
|
||||||
void TestQmllint::settingsFile()
|
void TestQmllint::settingsFile()
|
||||||
{
|
{
|
||||||
QVERIFY(runQmllint("settings/unqualifiedSilent/unqualified.qml", true, QStringList(), false)
|
QVERIFY(runQmllint("settings/unqualifiedSilent/unqualified.qml", true, warningsShouldFailArgs(), false)
|
||||||
.isEmpty());
|
.isEmpty());
|
||||||
QVERIFY(runQmllint("settings/unusedImportWarning/unused.qml", false, QStringList(), false)
|
QVERIFY(runQmllint("settings/unusedImportWarning/unused.qml", false, warningsShouldFailArgs(), false)
|
||||||
.contains(QStringLiteral("Warning: %1:2:1: Unused import")
|
.contains(QStringLiteral("Warning: %1:2:1: Unused import")
|
||||||
.arg(testFile("settings/unusedImportWarning/unused.qml"))));
|
.arg(testFile("settings/unusedImportWarning/unused.qml"))));
|
||||||
QVERIFY(runQmllint("settings/bare/bare.qml", false, {}, false, false)
|
QVERIFY(runQmllint("settings/bare/bare.qml", false, warningsShouldFailArgs(), false, false)
|
||||||
.contains(QStringLiteral("Failed to find the following builtins: "
|
.contains(QStringLiteral("Failed to find the following builtins: "
|
||||||
"jsroot.qmltypes, builtins.qmltypes")));
|
"jsroot.qmltypes, builtins.qmltypes")));
|
||||||
QVERIFY(runQmllint("settings/qmltypes/qmltypes.qml", false, QStringList(), false)
|
QVERIFY(runQmllint("settings/qmltypes/qmltypes.qml", false, warningsShouldFailArgs(), false)
|
||||||
.contains(QStringLiteral("not a qmldir file. Assuming qmltypes.")));
|
.contains(QStringLiteral("not a qmldir file. Assuming qmltypes.")));
|
||||||
QVERIFY(runQmllint("settings/qmlimports/qmlimports.qml", true, QStringList(), false).isEmpty());
|
QVERIFY(runQmllint("settings/qmlimports/qmlimports.qml", true, warningsShouldFailArgs(), false).isEmpty());
|
||||||
}
|
}
|
||||||
|
|
||||||
void TestQmllint::additionalImplicitImport()
|
void TestQmllint::additionalImplicitImport()
|
||||||
|
@ -1915,8 +1922,8 @@ void TestQmllint::missingBuiltinsNoCrash()
|
||||||
|
|
||||||
void TestQmllint::absolutePath()
|
void TestQmllint::absolutePath()
|
||||||
{
|
{
|
||||||
QString absPathOutput = runQmllint("memberNotFound.qml", false, {}, true, true, true);
|
QString absPathOutput = runQmllint("memberNotFound.qml", false, warningsShouldFailArgs(), true, true, true);
|
||||||
QString relPathOutput = runQmllint("memberNotFound.qml", false, {}, true, true, false);
|
QString relPathOutput = runQmllint("memberNotFound.qml", false, warningsShouldFailArgs(), true, true, false);
|
||||||
const QString absolutePath = QFileInfo(testFile("memberNotFound.qml")).absoluteFilePath();
|
const QString absolutePath = QFileInfo(testFile("memberNotFound.qml")).absoluteFilePath();
|
||||||
|
|
||||||
QVERIFY(absPathOutput.contains(absolutePath));
|
QVERIFY(absPathOutput.contains(absolutePath));
|
||||||
|
@ -2208,7 +2215,7 @@ void TestQmllint::environment_data()
|
||||||
const QString noWarningExpected;
|
const QString noWarningExpected;
|
||||||
|
|
||||||
QTest::addRow("missing-import-dir")
|
QTest::addRow("missing-import-dir")
|
||||||
<< fileThatNeedsImportPath << false << QStringList{}
|
<< fileThatNeedsImportPath << false << warningsShouldFailArgs()
|
||||||
<< Environment{ { u"QML_IMPORT_PATH"_s, importPath } } << noWarningExpected;
|
<< Environment{ { u"QML_IMPORT_PATH"_s, importPath } } << noWarningExpected;
|
||||||
|
|
||||||
QTest::addRow("import-dir-via-arg")
|
QTest::addRow("import-dir-via-arg")
|
||||||
|
@ -2242,6 +2249,18 @@ void TestQmllint::environment()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void TestQmllint::maxWarnings()
|
||||||
|
{
|
||||||
|
// warnings are not fatal by default
|
||||||
|
runQmllint(testFile("badScript.qml"), true);
|
||||||
|
// or when max-warnings is set to -1
|
||||||
|
runQmllint(testFile("badScript.qml"), true, {"-W", "-1"});
|
||||||
|
// 1 warning => should fail
|
||||||
|
runQmllint(testFile("badScript.qml"), false, {"--max-warnings", "0"});
|
||||||
|
// only 1 warning => should exit normally
|
||||||
|
runQmllint(testFile("badScript.qml"), true, {"--max-warnings", "1"});
|
||||||
|
}
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
void TestQmllint::ignoreSettingsNotCommandLineOptions()
|
void TestQmllint::ignoreSettingsNotCommandLineOptions()
|
||||||
|
|
|
@ -187,6 +187,18 @@ All warnings can be set to three levels:
|
||||||
QLatin1String("directory"));
|
QLatin1String("directory"));
|
||||||
parser.addOption(pluginPathsOption);
|
parser.addOption(pluginPathsOption);
|
||||||
|
|
||||||
|
QCommandLineOption maxWarnings(
|
||||||
|
QStringList() << "W"
|
||||||
|
<< "max-warnings",
|
||||||
|
QLatin1String("Exit with an error code if more than \"count\" many"
|
||||||
|
"warnings are found by qmllint. By default or if \"count\" "
|
||||||
|
"is -1, warnings do not cause qmllint "
|
||||||
|
"to return with an error exit code."),
|
||||||
|
"count"
|
||||||
|
);
|
||||||
|
parser.addOption(maxWarnings);
|
||||||
|
settings.addOption("MaxWarnings", -1);
|
||||||
|
|
||||||
auto levelToString = [](const QQmlJS::LoggerCategory &category) -> QString {
|
auto levelToString = [](const QQmlJS::LoggerCategory &category) -> QString {
|
||||||
Q_ASSERT(category.isIgnored() || category.level() != QtCriticalMsg);
|
Q_ASSERT(category.isIgnored() || category.level() != QtCriticalMsg);
|
||||||
if (category.isIgnored())
|
if (category.isIgnored())
|
||||||
|
@ -463,7 +475,13 @@ All warnings can be set to three levels:
|
||||||
useJson ? &jsonFiles : nullptr, qmlImportPaths,
|
useJson ? &jsonFiles : nullptr, qmlImportPaths,
|
||||||
qmldirFiles, resourceFiles, categories);
|
qmldirFiles, resourceFiles, categories);
|
||||||
}
|
}
|
||||||
success &= (lintResult == QQmlJSLinter::LintSuccess);
|
success &= (lintResult == QQmlJSLinter::LintSuccess || lintResult == QQmlJSLinter::HasWarnings);
|
||||||
|
if (success && parser.isSet(maxWarnings))
|
||||||
|
{
|
||||||
|
int value = parser.value(maxWarnings).toInt();
|
||||||
|
if (value != -1 && value < linter.logger()->warnings().size())
|
||||||
|
success = false;
|
||||||
|
}
|
||||||
|
|
||||||
if (isFixing) {
|
if (isFixing) {
|
||||||
if (lintResult != QQmlJSLinter::LintSuccess && lintResult != QQmlJSLinter::HasWarnings)
|
if (lintResult != QQmlJSLinter::LintSuccess && lintResult != QQmlJSLinter::HasWarnings)
|
||||||
|
|
Loading…
Reference in New Issue