Try harder to find out the correct location for evaluation errors

Task-number: QBS-946
Change-Id: I2fb5d573f7caf44f96fd4a96fd5069a44cdfb16a
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

10 files changed, 45 insertions, 29 deletions

diff --git a/src/lib/corelib/buildgraph/depscanner.cpp b/src/lib/corelib/buildgraph/depscanner.cpp
index 8dc99f5ed..fddacfc5d 100644
--- a/src/lib/corelib/buildgraph/depscanner.cpp
+++ b/src/lib/corelib/buildgraph/depscanner.cpp
@@ -250,8 +250,9 @@ QStringList UserDependencyScanner::evaluate(Artifact *artifact, const ScriptFunc
     m_engine->clearRequestedProperties();
     if (Q_UNLIKELY(m_engine->hasErrorOrException(result))) {
         QString msg = Tr::tr("evaluating scan script: ") + m_engine->lastErrorString(result);
+        const CodeLocation loc = m_engine->lastErrorLocation(result, script->location);
         m_engine->clearExceptions();
-        throw ErrorInfo(msg, script->location);
+        throw ErrorInfo(msg, loc);
     }
     QStringList list;
     if (result.isArray()) {
diff --git a/src/lib/corelib/buildgraph/processcommandexecutor.cpp b/src/lib/corelib/buildgraph/processcommandexecutor.cpp
index 25a80f5be..6fecb73a4 100644
--- a/src/lib/corelib/buildgraph/processcommandexecutor.cpp
+++ b/src/lib/corelib/buildgraph/processcommandexecutor.cpp
@@ -217,7 +217,8 @@ QString ProcessCommandExecutor::filterProcessOutput(const QByteArray &_output,
     QScriptValue filteredOutput = filterFunction.call(scriptEngine()->undefinedValue(), outputArg);
     if (scriptEngine()->hasErrorOrException(filteredOutput)) {
         logger().printWarning(ErrorInfo(Tr::tr("Error when calling output filter function: %1")
-                         .arg(scriptEngine()->lastErrorString(filteredOutput))));
+                         .arg(scriptEngine()->lastErrorString(filteredOutput)),
+                                        scriptEngine()->lastErrorLocation(filteredOutput)));
         return output;
     }
 
diff --git a/src/lib/corelib/buildgraph/rulesapplicator.cpp b/src/lib/corelib/buildgraph/rulesapplicator.cpp
index 1aaf38fa0..271c9c325 100644
--- a/src/lib/corelib/buildgraph/rulesapplicator.cpp
+++ b/src/lib/corelib/buildgraph/rulesapplicator.cpp
@@ -241,7 +241,8 @@ void RulesApplicator::doApply(const ArtifactSet &inputArtifacts, QScriptValue &p
             if (Q_UNLIKELY(engine()->hasErrorOrException(scriptValue))) {
                 QString msg = QLatin1String("evaluating rule binding '%1': %2");
                 throw ErrorInfo(msg.arg(binding.name.join(QLatin1Char('.')),
-                                        engine()->lastErrorString(scriptValue)), binding.location);
+                                        engine()->lastErrorString(scriptValue)),
+                                engine()->lastErrorLocation(scriptValue, binding.location));
             }
             setConfigProperty(artifactModulesCfg, binding.name, scriptValue.toVariant());
         }
@@ -280,12 +281,8 @@ Artifact *RulesApplicator::createOutputArtifactFromRuleArtifact(
     QScriptValue scriptValue = engine()->evaluate(ruleArtifact->filePath,
                                                   ruleArtifact->filePathLocation.filePath(),
                                                   ruleArtifact->filePathLocation.line());
-    if (Q_UNLIKELY(engine()->hasErrorOrException(scriptValue))) {
-        ErrorInfo errorInfo(engine()->lastErrorString(scriptValue),
-                            engine()->uncaughtExceptionBacktraceOrEmpty());
-        errorInfo.append(QStringLiteral("Artifact.filePath"), ruleArtifact->filePathLocation);
-        throw errorInfo;
-    }
+    if (Q_UNLIKELY(engine()->hasErrorOrException(scriptValue)))
+        throw engine()->lastError(scriptValue, ruleArtifact->filePathLocation);
     QString outputPath = FileInfo::resolvePath(m_product->buildDirectory(), scriptValue.toString());
     if (Q_UNLIKELY(!outputFilePaths->insert(outputPath).second)) {
         throw ErrorInfo(Tr::tr("Rule %1 already created '%2'.")
@@ -395,13 +392,8 @@ QList<Artifact *> RulesApplicator::runOutputArtifactsScript(const ArtifactSet &i
         throw ErrorInfo(QLatin1String("Function expected."),
                         m_rule->outputArtifactsScript->location);
     QScriptValue res = fun.call(QScriptValue(), args);
-    if (engine()->hasErrorOrException(res)) {
-        ErrorInfo errorInfo(engine()->lastErrorString(res),
-                            engine()->uncaughtExceptionBacktraceOrEmpty());
-        errorInfo.append(QStringLiteral("Rule.outputArtifacts"),
-                         m_rule->outputArtifactsScript->location);
-        throw errorInfo;
-    }
+    if (engine()->hasErrorOrException(res))
+        throw engine()->lastError(res, m_rule->outputArtifactsScript->location);
     if (!res.isArray())
         throw ErrorInfo(Tr::tr("Rule.outputArtifacts must return an array of objects."),
                         m_rule->outputArtifactsScript->location);
diff --git a/src/lib/corelib/buildgraph/transformer.cpp b/src/lib/corelib/buildgraph/transformer.cpp
index ef728379b..a9b51f151 100644
--- a/src/lib/corelib/buildgraph/transformer.cpp
+++ b/src/lib/corelib/buildgraph/transformer.cpp
@@ -231,13 +231,8 @@ void Transformer::createCommands(ScriptEngine *engine, const ScriptFunctionConst
     propertiesRequestedInPrepareScript = engine->propertiesRequestedInScript();
     propertiesRequestedFromArtifactInPrepareScript = engine->propertiesRequestedFromArtifact();
     engine->clearRequestedProperties();
-    if (Q_UNLIKELY(engine->hasErrorOrException(scriptValue))) {
-        ErrorInfo errorInfo(engine->lastErrorString(scriptValue),
-                            engine->uncaughtExceptionBacktraceOrEmpty());
-        errorInfo.append(QStringLiteral("Rule.prepare"), script->location);
-        throw errorInfo;
-    }
-
+    if (Q_UNLIKELY(engine->hasErrorOrException(scriptValue)))
+        throw engine->lastError(scriptValue, script->location);
     commands.clear();
     if (scriptValue.isArray()) {
         const int count = scriptValue.property(QLatin1String("length")).toInt32();
diff --git a/src/lib/corelib/language/language.cpp b/src/lib/corelib/language/language.cpp
index 177427fcf..fb920ccad 100644
--- a/src/lib/corelib/language/language.cpp
+++ b/src/lib/corelib/language/language.cpp
@@ -673,7 +673,7 @@ static QProcessEnvironment getProcessEnvironment(ScriptEngine *engine, EnvType e
                                   ? QLatin1String("build") : QLatin1String("run"));
             throw ErrorInfo(Tr::tr("Error while setting up %1 environment: %2")
                             .arg(envTypeStr, engine->lastErrorString(scriptValue)),
-                            setupScript->location);
+                            engine->lastErrorLocation(scriptValue, setupScript->location));
         }
     }
 
diff --git a/src/lib/corelib/language/moduleloader.cpp b/src/lib/corelib/language/moduleloader.cpp
index 11e38518f..fa3dabfb5 100644
--- a/src/lib/corelib/language/moduleloader.cpp
+++ b/src/lib/corelib/language/moduleloader.cpp
@@ -2278,7 +2278,7 @@ void ModuleLoader::resolveProbe(ProductContext *productContext, Item *parent, It
     } else if (!resolvedProbe) {
         QScriptValue sv = engine->evaluate(configureScript->sourceCodeForEvaluation());
         if (Q_UNLIKELY(engine->hasErrorOrException(sv)))
-            evalError = ErrorInfo(engine->lastErrorString(sv), configureScript->location());
+            evalError = engine->lastError(sv, configureScript->location());
     }
     QVariantMap properties;
     for (const ProbeProperty &b : qAsConst(probeBindings)) {
diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp
index 0669f7893..7dec4c978 100644
--- a/src/lib/corelib/language/projectresolver.cpp
+++ b/src/lib/corelib/language/projectresolver.cpp
@@ -1228,8 +1228,8 @@ QVariantMap ProjectResolver::evaluateProperties(const Item *item, const Item *pr
 
             const QScriptValue scriptValue = m_evaluator->property(item, it.key());
             if (Q_UNLIKELY(m_evaluator->engine()->hasErrorOrException(scriptValue))) {
-                throw ErrorInfo(m_evaluator->engine()->lastErrorString(scriptValue),
-                                it.value()->location());
+                throw ErrorInfo(m_evaluator->engine()->lastError(scriptValue,
+                                                                 it.value()->location()));
             }
 
             // NOTE: Loses type information if scriptValue.isUndefined == true,
diff --git a/src/lib/corelib/language/scriptengine.cpp b/src/lib/corelib/language/scriptengine.cpp
index c071b2c14..a1f253bfb 100644
--- a/src/lib/corelib/language/scriptengine.cpp
+++ b/src/lib/corelib/language/scriptengine.cpp
@@ -517,6 +517,29 @@ QScriptValueList ScriptEngine::argumentList(const QStringList &argumentNames,
     return result;
 }
 
+CodeLocation ScriptEngine::lastErrorLocation(const QScriptValue &v,
+                                             const CodeLocation &fallbackLocation) const
+{
+    const QScriptValue &errorVal = lastErrorValue(v);
+    const CodeLocation errorLoc(errorVal.property(QLatin1String("fileName")).toString(),
+            errorVal.property(QLatin1String("lineNumber")).toInt32(),
+            errorVal.property(QLatin1String("expressionCaretOffset")).toInt32(),
+            false);
+    return errorLoc.isValid() ? errorLoc : fallbackLocation;
+}
+
+ErrorInfo ScriptEngine::lastError(const QScriptValue &v, const CodeLocation &fallbackLocation) const
+{
+    const QString msg = lastErrorString(v);
+    CodeLocation errorLocation = lastErrorLocation(v);
+    if (errorLocation.isValid())
+        return ErrorInfo(msg, errorLocation);
+    const QStringList backtrace = uncaughtExceptionBacktraceOrEmpty();
+    if (!backtrace.isEmpty())
+        return ErrorInfo(msg, backtrace);
+    return ErrorInfo(msg, fallbackLocation);
+}
+
 void ScriptEngine::cancel()
 {
     QTimer::singleShot(0, this, [this] { abort(); });
diff --git a/src/lib/corelib/language/scriptengine.h b/src/lib/corelib/language/scriptengine.h
index 323237d88..44e15d0e6 100644
--- a/src/lib/corelib/language/scriptengine.h
+++ b/src/lib/corelib/language/scriptengine.h
@@ -43,6 +43,7 @@
 #include "forward_decls.h"
 #include "property.h"
 #include <logging/logger.h>
+#include <tools/codelocation.h>
 #include <tools/filetime.h>
 #include <tools/set.h>
 
@@ -151,6 +152,10 @@ public:
         return v.isError() ? v : uncaughtException();
     }
     QString lastErrorString(const QScriptValue &v) const { return lastErrorValue(v).toString(); }
+    CodeLocation lastErrorLocation(const QScriptValue &v,
+                                   const CodeLocation &fallbackLocation = CodeLocation()) const;
+    ErrorInfo lastError(const QScriptValue &v,
+                        const CodeLocation &fallbackLocation = CodeLocation()) const;
 
     void cancel();
 
diff --git a/tests/auto/blackbox/tst_blackbox.cpp b/tests/auto/blackbox/tst_blackbox.cpp
index 84d490999..fd0453a6b 100644
--- a/tests/auto/blackbox/tst_blackbox.cpp
+++ b/tests/auto/blackbox/tst_blackbox.cpp
@@ -1783,9 +1783,8 @@ void TestBlackbox::referenceErrorInExport()
     QbsRunParameters params;
     params.expectFailure = true;
     QVERIFY(runQbs(params) != 0);
-    QEXPECT_FAIL(0, "QBS-946", Abort);
     QVERIFY(m_qbsStderr.contains(
-        "referenceErrorInExport.qbs:17:31 ReferenceError: Can't find variable: includePaths"));
+        "referenceErrorInExport.qbs:17:12 ReferenceError: Can't find variable: includePaths"));
 }
 
 void TestBlackbox::reproducibleBuild()