Simplify errors and diagnostics

We only need two classes to describe all possible diagnostics:

* A low-level private POD DiagnosticMessage. This is easily copied and
  passed around internally. It doesn't need to adhere to a stable API
  and it doesn't carry any extra baggage.

* The high-level public QQmlError with its stable interface. This can
  internally also use a DiagnosticMessage as storage.

Change-Id: I52be88d9b5d9855a661b8032b01eedb43a0fb0b3
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
 
 
 

4 files changed, 51 insertions, 76 deletions

diff --git a/src/qml/parser/qqmljs.g b/src/qml/parser/qqmljs.g
index 4ad9057ced..aada928dea 100644
--- a/src/qml/parser/qqmljs.g
+++ b/src/qml/parser/qqmljs.g
@@ -245,6 +245,7 @@
 #include <private/qqmljsgrammar_p.h>
 #include <private/qqmljsast_p.h>
 #include <private/qqmljsengine_p.h>
+#include <private/qqmljsdiagnosticmessage_p.h>
 
 #include <QtCore/qlist.h>
 #include <QtCore/qstring.h>
@@ -365,7 +366,7 @@ public:
     inline DiagnosticMessage diagnosticMessage() const
     {
         for (const DiagnosticMessage &d : diagnostic_messages) {
-            if (d.kind != DiagnosticMessage::Warning)
+            if (d.type != QtWarningMsg)
                 return d;
         }
 
@@ -376,10 +377,10 @@ public:
     { return diagnosticMessage().message; }
 
     inline int errorLineNumber() const
-    { return diagnosticMessage().loc.startLine; }
+    { return diagnosticMessage().line; }
 
     inline int errorColumnNumber() const
-    { return diagnosticMessage().loc.startColumn; }
+    { return diagnosticMessage().column; }
 
 protected:
     bool parse(int startToken);
@@ -403,11 +404,22 @@ protected:
     void pushToken(int token);
     int lookaheadToken(Lexer *lexer);
 
+    static DiagnosticMessage compileError(const AST::SourceLocation &location,
+                                          const QString &message, QtMsgType kind = QtCriticalMsg)
+    {
+        DiagnosticMessage error;
+        error.line = location.startLine;
+        error.column = location.startColumn;
+        error.message = message;
+        error.type = kind;
+        return error;
+    }
+
     void syntaxError(const AST::SourceLocation &location, const char *message) {
-        diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, location, QLatin1String(message)));
+        diagnostic_messages.append(compileError(location, QLatin1String(message)));
      }
      void syntaxError(const AST::SourceLocation &location, const QString &message) {
-         diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, location, message));
+        diagnostic_messages.append(compileError(location, message));
       }
 
 protected:
@@ -840,7 +852,7 @@ UiImportHead: T_IMPORT ImportId;
         if (node) {
             node->importToken = loc(1);
         } else {
-           diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, loc(1),
+            diagnostic_messages.append(compileError(loc(1),
              QLatin1String("Expected a qualified name id or a string literal")));
 
             return false; // ### remove me
@@ -1332,8 +1344,8 @@ UiQualifiedId: MemberExpression;
 /.
     case $rule_number: {
       if (AST::ArrayMemberExpression *mem = AST::cast<AST::ArrayMemberExpression *>(sym(1).Expression)) {
-        diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Warning, mem->lbracketToken,
-          QLatin1String("Ignored annotation")));
+        diagnostic_messages.append(compileError(mem->lbracketToken,
+          QLatin1String("Ignored annotation"), QtWarningMsg));
 
         sym(1).Expression = mem->base;
       }
@@ -1343,7 +1355,7 @@ UiQualifiedId: MemberExpression;
       } else {
         sym(1).UiQualifiedId = 0;
 
-        diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, loc(1),
+        diagnostic_messages.append(compileError(loc(1),
           QLatin1String("Expected a qualified name id")));
 
         return false; // ### recover
@@ -1590,7 +1602,7 @@ RegularExpressionLiteral: T_DIVIDE_EQ;
     scan_regexp: {
         bool rx = lexer->scanRegExp(prefix);
         if (!rx) {
-            diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, location(lexer), lexer->errorMessage()));
+            diagnostic_messages.append(compileError(location(lexer), lexer->errorMessage()));
             return false;
         }
 
@@ -4420,7 +4432,7 @@ ExportSpecifier: IdentifierName T_AS IdentifierName;
             yylloc.length = 0;
 
             //const QString msg = QCoreApplication::translate("QQmlParser", "Missing `;'");
-            //diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Warning, yylloc, msg));
+            //diagnostic_messages.append(compileError(yyloc, msg, QtWarningMsg));
 
             first_token = &token_buffer[0];
             last_token = &token_buffer[1];
@@ -4457,7 +4469,7 @@ ExportSpecifier: IdentifierName T_AS IdentifierName;
                 msg = QCoreApplication::translate("QQmlParser", "Syntax error");
             else
                 msg = QCoreApplication::translate("QQmlParser", "Unexpected token `%1'").arg(QLatin1String(spell[token]));
-            diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, token_buffer[0].loc, msg));
+            diagnostic_messages.append(compileError(token_buffer[0].loc, msg));
 
             action = errorState;
             goto _Lcheck_token;
@@ -4488,7 +4500,7 @@ ExportSpecifier: IdentifierName T_AS IdentifierName;
                 qDebug() << "Parse error, trying to recover (2).";
 #endif
                 const QString msg = QCoreApplication::translate("QQmlParser", "Expected token `%1'").arg(QLatin1String(spell[*tk]));
-                diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, token_buffer[0].loc, msg));
+                diagnostic_messages.append(compileError(token_buffer[0].loc, msg));
 
                 pushToken(*tk);
                 goto _Lcheck_token;
@@ -4503,7 +4515,7 @@ ExportSpecifier: IdentifierName T_AS IdentifierName;
             int a = t_action(errorState, tk);
             if (a > 0 && t_action(a, yytoken)) {
                 const QString msg = QCoreApplication::translate("QQmlParser", "Expected token `%1'").arg(QLatin1String(spell[tk]));
-                diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, token_buffer[0].loc, msg));
+                diagnostic_messages.append(compileError(token_buffer[0].loc, msg));
 
                 pushToken(tk);
                 goto _Lcheck_token;
@@ -4511,7 +4523,7 @@ ExportSpecifier: IdentifierName T_AS IdentifierName;
         }
 
         const QString msg = QCoreApplication::translate("QQmlParser", "Syntax error");
-        diagnostic_messages.append(DiagnosticMessage(DiagnosticMessage::Error, token_buffer[0].loc, msg));
+        diagnostic_messages.append(compileError(token_buffer[0].loc, msg));
     }
 
     return false;
diff --git a/src/qml/parser/qqmljsengine_p.h b/src/qml/parser/qqmljsengine_p.h
index b7f7da9478..6a754fc236 100644
--- a/src/qml/parser/qqmljsengine_p.h
+++ b/src/qml/parser/qqmljsengine_p.h
@@ -91,28 +91,6 @@ public:
     }
 };
 
-
-class QML_PARSER_EXPORT DiagnosticMessage
-{
-public:
-    enum Kind { Hint, Warning, Error };
-
-    DiagnosticMessage() {}
-
-    DiagnosticMessage(Kind kind, const AST::SourceLocation &loc, const QString &message)
-        : kind(kind), loc(loc), message(message) {}
-
-    bool isWarning() const
-    { return kind == Warning; }
-
-    bool isError() const
-    { return kind == Error; }
-
-    Kind kind = Error;
-    AST::SourceLocation loc;
-    QString message;
-};
-
 class QML_PARSER_EXPORT Engine
 {
     Lexer *_lexer;
diff --git a/src/qml/parser/qqmljslexer.cpp b/src/qml/parser/qqmljslexer.cpp
index c53b13f64d..1b3b129c6a 100644
--- a/src/qml/parser/qqmljslexer.cpp
+++ b/src/qml/parser/qqmljslexer.cpp
@@ -42,6 +42,8 @@
 #include "qqmljsmemorypool_p.h"
 #include "qqmljskeywords_p.h"
 
+#include <private/qqmljsdiagnosticmessage_p.h>
+
 #include <QtCore/qcoreapplication.h>
 #include <QtCore/qvarlengtharray.h>
 #include <QtCore/qdebug.h>
@@ -1402,6 +1404,12 @@ static inline bool isUriToken(int token)
 
 bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
 {
+    auto setError = [error, this](QString message) {
+        error->message = std::move(message);
+        error->line = tokenStartLine();
+        error->column = tokenStartColumn();
+    };
+
     Q_ASSERT(!_qmlMode);
 
     lex(); // fetch the first token
@@ -1422,9 +1430,7 @@ bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
 
         if (! (directiveName == QLatin1String("pragma") ||
                directiveName == QLatin1String("import"))) {
-            error->message = QCoreApplication::translate("QQmlParser", "Syntax error");
-            error->loc.startLine = tokenStartLine();
-            error->loc.startColumn = tokenStartColumn();
+            setError(QCoreApplication::translate("QQmlParser", "Syntax error"));
             return false; // not a valid directive name
         }
 
@@ -1432,9 +1438,7 @@ bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
         if (directiveName == QLatin1String("pragma")) {
             // .pragma library
             if (! (lex() == T_IDENTIFIER && tokenText() == QLatin1String("library"))) {
-                error->message = QCoreApplication::translate("QQmlParser", "Syntax error");
-                error->loc.startLine = tokenStartLine();
-                error->loc.startColumn = tokenStartColumn();
+                setError(QCoreApplication::translate("QQmlParser", "Syntax error"));
                 return false; // expected `library
             }
 
@@ -1456,9 +1460,7 @@ bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
                 pathOrUri = tokenText();
 
                 if (!pathOrUri.endsWith(QLatin1String("js"))) {
-                    error->message = QCoreApplication::translate("QQmlParser","Imported file must be a script");
-                    error->loc.startLine = tokenStartLine();
-                    error->loc.startColumn = tokenStartColumn();
+                    setError(QCoreApplication::translate("QQmlParser","Imported file must be a script"));
                     return false;
                 }
 
@@ -1467,9 +1469,7 @@ bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
 
                 while (true) {
                     if (!isUriToken(_tokenKind)) {
-                        error->message = QCoreApplication::translate("QQmlParser","Invalid module URI");
-                        error->loc.startLine = tokenStartLine();
-                        error->loc.startColumn = tokenStartColumn();
+                        setError(QCoreApplication::translate("QQmlParser","Invalid module URI"));
                         return false;
                     }
 
@@ -1477,9 +1477,7 @@ bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
 
                     lex();
                     if (tokenStartLine() != lineNumber) {
-                        error->message = QCoreApplication::translate("QQmlParser","Invalid module URI");
-                        error->loc.startLine = tokenStartLine();
-                        error->loc.startColumn = tokenStartColumn();
+                        setError(QCoreApplication::translate("QQmlParser","Invalid module URI"));
                         return false;
                     }
                     if (_tokenKind != QQmlJSGrammar::T_DOT)
@@ -1489,17 +1487,13 @@ bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
 
                     lex();
                     if (tokenStartLine() != lineNumber) {
-                        error->message = QCoreApplication::translate("QQmlParser","Invalid module URI");
-                        error->loc.startLine = tokenStartLine();
-                        error->loc.startColumn = tokenStartColumn();
+                        setError(QCoreApplication::translate("QQmlParser","Invalid module URI"));
                         return false;
                     }
                 }
 
                 if (_tokenKind != T_NUMERIC_LITERAL) {
-                    error->message = QCoreApplication::translate("QQmlParser","Module import requires a version");
-                    error->loc.startLine = tokenStartLine();
-                    error->loc.startColumn = tokenStartColumn();
+                    setError(QCoreApplication::translate("QQmlParser","Module import requires a version"));
                     return false; // expected the module version number
                 }
 
@@ -1511,34 +1505,27 @@ bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
             //
             if (! (lex() == T_AS && tokenStartLine() == lineNumber)) {
                 if (fileImport)
-                    error->message = QCoreApplication::translate("QQmlParser", "File import requires a qualifier");
+                    setError(QCoreApplication::translate("QQmlParser", "File import requires a qualifier"));
                 else
-                    error->message = QCoreApplication::translate("QQmlParser", "Module import requires a qualifier");
+                    setError(QCoreApplication::translate("QQmlParser", "Module import requires a qualifier"));
                 if (tokenStartLine() != lineNumber) {
-                    error->loc.startLine = lineNumber;
-                    error->loc.startColumn = column;
-                } else {
-                    error->loc.startLine = tokenStartLine();
-                    error->loc.startColumn = tokenStartColumn();
+                    error->line = lineNumber;
+                    error->line = column;
                 }
                 return false; // expected `as'
             }
 
             if (lex() != T_IDENTIFIER || tokenStartLine() != lineNumber) {
                 if (fileImport)
-                    error->message = QCoreApplication::translate("QQmlParser", "File import requires a qualifier");
+                    setError(QCoreApplication::translate("QQmlParser", "File import requires a qualifier"));
                 else
-                    error->message = QCoreApplication::translate("QQmlParser", "Module import requires a qualifier");
-                error->loc.startLine = tokenStartLine();
-                error->loc.startColumn = tokenStartColumn();
+                    setError(QCoreApplication::translate("QQmlParser", "Module import requires a qualifier"));
                 return false; // expected module name
             }
 
             const QString module = tokenText();
             if (!module.at(0).isUpper()) {
-                error->message = QCoreApplication::translate("QQmlParser","Invalid import qualifier");
-                error->loc.startLine = tokenStartLine();
-                error->loc.startColumn = tokenStartColumn();
+                setError(QCoreApplication::translate("QQmlParser","Invalid import qualifier"));
                 return false;
             }
 
@@ -1549,9 +1536,7 @@ bool Lexer::scanDirectives(Directives *directives, DiagnosticMessage *error)
         }
 
         if (tokenStartLine() != lineNumber) {
-            error->message = QCoreApplication::translate("QQmlParser", "Syntax error");
-            error->loc.startLine = tokenStartLine();
-            error->loc.startColumn = tokenStartColumn();
+            setError(QCoreApplication::translate("QQmlParser", "Syntax error"));
             return false; // the directives cannot span over multiple lines
         }
 
diff --git a/src/qml/parser/qqmljslexer_p.h b/src/qml/parser/qqmljslexer_p.h
index 51152bfd6e..4fe40948e3 100644
--- a/src/qml/parser/qqmljslexer_p.h
+++ b/src/qml/parser/qqmljslexer_p.h
@@ -62,7 +62,7 @@ QT_BEGIN_NAMESPACE
 namespace QQmlJS {
 
 class Engine;
-class DiagnosticMessage;
+struct DiagnosticMessage;
 class Directives;
 
 class QML_PARSER_EXPORT Lexer: public QQmlJSGrammar