diff options
author | Christian Kandeler <christian.kandeler@qt.io> | 2016-11-07 13:34:54 +0100 |
---|---|---|
committer | Joerg Bornemann <joerg.bornemann@qt.io> | 2016-11-11 10:33:24 +0000 |
commit | ab14d325eaa1ee762dacc8b9fbbc902dae4d5e6e (patch) | |
tree | 6f8d2e8c15376eaf6cf1795bcf5506034faa7180 /src | |
parent | 18ea9077c0aff50ae1d891bea4d4bbe2ba20bab7 (diff) |
Warn against possibly improper use of JS extensions in qbs project files
For instance, we inform users when they are doing potentially slow File
I/O operations during project resolving and advise them to use a Probe
instead.
Task-number: QBS-1033
Change-Id: I3e9d0eb36c6ddebc5f4a112c329f96d25856ac0a
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>
Diffstat (limited to 'src')
-rw-r--r-- | src/lib/corelib/jsextensions/file.cpp | 29 | ||||
-rw-r--r-- | src/lib/corelib/jsextensions/process.cpp | 6 | ||||
-rw-r--r-- | src/lib/corelib/jsextensions/propertylist.mm | 7 | ||||
-rw-r--r-- | src/lib/corelib/jsextensions/temporarydir.cpp | 8 | ||||
-rw-r--r-- | src/lib/corelib/jsextensions/textfile.cpp | 7 | ||||
-rw-r--r-- | src/lib/corelib/language/scriptengine.cpp | 31 | ||||
-rw-r--r-- | src/lib/corelib/language/scriptengine.h | 12 |
7 files changed, 99 insertions, 1 deletions
diff --git a/src/lib/corelib/jsextensions/file.cpp b/src/lib/corelib/jsextensions/file.cpp index 071c2a9da..1a6d259e4 100644 --- a/src/lib/corelib/jsextensions/file.cpp +++ b/src/lib/corelib/jsextensions/file.cpp @@ -131,6 +131,13 @@ QScriptValue File::js_copy(QScriptContext *context, QScriptEngine *engine) Tr::tr("copy expects 2 arguments")); } + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts({ + DubiousContext(EvalContext::PropertyEvaluation), + DubiousContext(EvalContext::RuleExecution, DubiousContext::SuggestMoving) + }); + se->checkContext(QLatin1String("File.copy()"), dubiousContexts); + const QString sourceFile = context->argument(0).toString(); const QString targetFile = context->argument(1).toString(); QString errorMessage; @@ -160,11 +167,17 @@ QScriptValue File::js_directoryEntries(QScriptContext *context, QScriptEngine *e return context->throwError(QScriptContext::SyntaxError, Tr::tr("directoryEntries expects 2 arguments")); } + + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts({ + DubiousContext(EvalContext::PropertyEvaluation, DubiousContext::SuggestMoving) + }); + se->checkContext(QLatin1String("File.directoryEntries()"), dubiousContexts); + const QString path = context->argument(0).toString(); const QDir::Filters filters = static_cast<QDir::Filters>(context->argument(1).toUInt32()); QDir dir(path); const QStringList entries = dir.entryList(filters, QDir::Name); - ScriptEngine * const se = static_cast<ScriptEngine *>(engine); se->addDirectoryEntriesResult(path, filters, entries); return qScriptValueFromSequence(engine, entries); } @@ -176,6 +189,11 @@ QScriptValue File::js_remove(QScriptContext *context, QScriptEngine *engine) return context->throwError(QScriptContext::SyntaxError, Tr::tr("remove expects 1 argument")); } + + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts({ DubiousContext(EvalContext::PropertyEvaluation) }); + se->checkContext(QLatin1String("File.remove()"), dubiousContexts); + QString fileName = context->argument(0).toString(); QString errorMessage; @@ -205,6 +223,11 @@ QScriptValue File::js_makePath(QScriptContext *context, QScriptEngine *engine) return context->throwError(QScriptContext::SyntaxError, Tr::tr("makePath expects 1 argument")); } + + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts({ DubiousContext(EvalContext::PropertyEvaluation) }); + se->checkContext(QLatin1String("File.makePath()"), dubiousContexts); + return QDir::root().mkpath(context->argument(0).toString()); } @@ -216,6 +239,10 @@ QScriptValue File::js_move(QScriptContext *context, QScriptEngine *engine) Tr::tr("move expects 2 arguments")); } + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts({ DubiousContext(EvalContext::PropertyEvaluation) }); + se->checkContext(QLatin1String("File.move()"), dubiousContexts); + const QString sourceFile = context->argument(0).toString(); const QString targetFile = context->argument(1).toString(); const bool overwrite = context->argumentCount() > 2 ? context->argument(2).toBool() : true; diff --git a/src/lib/corelib/jsextensions/process.cpp b/src/lib/corelib/jsextensions/process.cpp index ad67999b6..ab4df57b8 100644 --- a/src/lib/corelib/jsextensions/process.cpp +++ b/src/lib/corelib/jsextensions/process.cpp @@ -73,6 +73,12 @@ QScriptValue Process::ctor(QScriptContext *context, QScriptEngine *engine) return context->throwError(QLatin1String("Process()")); } + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts ({ + DubiousContext(EvalContext::PropertyEvaluation, DubiousContext::SuggestMoving) + }); + se->checkContext(QLatin1String("qbs.Process"), dubiousContexts); + QScriptValue obj = engine->newQObject(t, QScriptEngine::ScriptOwnership); // Get environment diff --git a/src/lib/corelib/jsextensions/propertylist.mm b/src/lib/corelib/jsextensions/propertylist.mm index 4e431e499..2bf2acacc 100644 --- a/src/lib/corelib/jsextensions/propertylist.mm +++ b/src/lib/corelib/jsextensions/propertylist.mm @@ -40,6 +40,7 @@ #include "propertylist.h" +#include <language/scriptengine.h> #include <tools/hostosinfo.h> #include <QFile> @@ -80,6 +81,12 @@ void initializeJsExtensionPropertyList(QScriptValue extensionObject) QScriptValue PropertyList::ctor(QScriptContext *context, QScriptEngine *engine) { + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts({ + DubiousContext(EvalContext::PropertyEvaluation, DubiousContext::SuggestMoving) + }); + se->checkContext(QLatin1String("qbs.PropertyList"), dubiousContexts); + PropertyList *p = new PropertyList(context); QScriptValue obj = engine->newQObject(p, QScriptEngine::ScriptOwnership); return obj; diff --git a/src/lib/corelib/jsextensions/temporarydir.cpp b/src/lib/corelib/jsextensions/temporarydir.cpp index 1fb4c34ec..e7b811f31 100644 --- a/src/lib/corelib/jsextensions/temporarydir.cpp +++ b/src/lib/corelib/jsextensions/temporarydir.cpp @@ -40,6 +40,8 @@ #include "temporarydir.h" +#include <language/scriptengine.h> + #include <QScriptEngine> #include <QScriptValue> #include <QTemporaryDir> @@ -57,6 +59,12 @@ void initializeJsExtensionTemporaryDir(QScriptValue extensionObject) QScriptValue TemporaryDir::ctor(QScriptContext *context, QScriptEngine *engine) { + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts({ + DubiousContext(EvalContext::PropertyEvaluation, DubiousContext::SuggestMoving) + }); + se->checkContext(QLatin1String("qbs.TemporaryDir"), dubiousContexts); + TemporaryDir *t = new TemporaryDir(context); QScriptValue obj = engine->newQObject(t, QScriptEngine::ScriptOwnership); return obj; diff --git a/src/lib/corelib/jsextensions/textfile.cpp b/src/lib/corelib/jsextensions/textfile.cpp index a631366b1..02166c6a2 100644 --- a/src/lib/corelib/jsextensions/textfile.cpp +++ b/src/lib/corelib/jsextensions/textfile.cpp @@ -39,6 +39,7 @@ #include "textfile.h" +#include <language/scriptengine.h> #include <logging/translator.h> #include <tools/hostosinfo.h> @@ -84,6 +85,12 @@ QScriptValue TextFile::ctor(QScriptContext *context, QScriptEngine *engine) return context->throwError(Tr::tr("TextFile constructor takes at most three parameters.")); } + ScriptEngine * const se = static_cast<ScriptEngine *>(engine); + const DubiousContextList dubiousContexts({ + DubiousContext(EvalContext::PropertyEvaluation, DubiousContext::SuggestMoving) + }); + se->checkContext(QLatin1String("qbs.TextFile"), dubiousContexts); + return engine->newQObject(t, QScriptEngine::ScriptOwnership); } diff --git a/src/lib/corelib/language/scriptengine.cpp b/src/lib/corelib/language/scriptengine.cpp index b383504b8..3e83fe6cb 100644 --- a/src/lib/corelib/language/scriptengine.cpp +++ b/src/lib/corelib/language/scriptengine.cpp @@ -159,6 +159,37 @@ void ScriptEngine::clearImportsCache() m_jsImportCache.clear(); } +void ScriptEngine::checkContext(const QString &operation, + const DubiousContextList &dubiousContexts) +{ + for (auto it = dubiousContexts.cbegin(); it != dubiousContexts.cend(); ++it) { + const DubiousContext &info = *it; + if (info.context != evalContext()) + continue; + QString warning; + switch (info.context) { + case EvalContext::PropertyEvaluation: + warning = Tr::tr("Suspicious use of %1 during property evaluation.").arg(operation); + if (info.suggestion == DubiousContext::SuggestMoving) + warning += QLatin1Char(' ') + Tr::tr("Should this call be in a Probe instead?"); + break; + case EvalContext::RuleExecution: + warning = Tr::tr("Suspicious use of %1 during rule execution.").arg(operation); + if (info.suggestion == DubiousContext::SuggestMoving) { + warning += QLatin1Char(' ') + + Tr::tr("Should this call be in a JavaScriptCommand instead?"); + } + break; + case EvalContext::ProbeExecution: + case EvalContext::JsCommand: + QBS_ASSERT(false, continue); + break; + } + m_logger.printWarning(ErrorInfo(warning, currentContext()->backtrace())); + return; + } +} + void ScriptEngine::addPropertyRequestedFromArtifact(const Artifact *artifact, const Property &property) { diff --git a/src/lib/corelib/language/scriptengine.h b/src/lib/corelib/language/scriptengine.h index 3bbce2c93..837ba4cd3 100644 --- a/src/lib/corelib/language/scriptengine.h +++ b/src/lib/corelib/language/scriptengine.h @@ -53,6 +53,8 @@ #include <QStack> #include <QString> +#include <vector> + namespace qbs { namespace Internal { class Artifact; @@ -61,6 +63,15 @@ class ScriptImporter; class ScriptPropertyObserver; enum class EvalContext { PropertyEvaluation, ProbeExecution, RuleExecution, JsCommand }; +class DubiousContext +{ +public: + enum Suggestion { NoSuggestion, SuggestMoving }; + DubiousContext(EvalContext c, Suggestion s = NoSuggestion) : context(c), suggestion(s) { } + EvalContext context; + Suggestion suggestion; +}; +using DubiousContextList = std::vector<DubiousContext>; class ScriptEngine : public QScriptEngine { @@ -76,6 +87,7 @@ public: void setEvalContext(EvalContext c) { m_evalContext = c; } EvalContext evalContext() const { return m_evalContext; } + void checkContext(const QString &operation, const DubiousContextList &dubiousContexts); void addPropertyRequestedInScript(const Property &property) { m_propertiesRequestedInScript += property; |