LibJS: Let parser keep track of errors

Rather than printing them to stderr directly the parser now keeps a
Vector<Error>, which allows the "owner" of the parser to consume them
individually after parsing.

The Error struct has a message, line number, column number and a
to_string() helper function to format this information into a meaningful
error message.

The Function() constructor will now include an error message when
throwing a SyntaxError.

Libraries/LibJS/Parser.cpp

@@ -29,7 +29,6 @@
 #include <AK/HashMap.h>
 #include <AK/ScopeGuard.h>
 #include <AK/StdLibExtras.h>
-#include <stdio.h>
 
 namespace JS {
 
@@ -1385,12 +1384,11 @@ void Parser::expected(const char* what)
 
 void Parser::syntax_error(const String& message, size_t line, size_t column)
 {
-    m_parser_state.m_has_errors = true;
     if (line == 0 || column == 0) {
         line = m_parser_state.m_current_token.line_number();
         column = m_parser_state.m_current_token.line_column();
     }
-    fprintf(stderr, "Syntax Error: %s (line: %zu, column: %zu)\n", message.characters(), line, column);
+    m_parser_state.m_errors.append({ message, line, column });
 }
 
 void Parser::save_state()

Libraries/LibJS/Parser.h

@@ -29,6 +29,7 @@
 #include "AST.h"
 #include "Lexer.h"
 #include <AK/NonnullRefPtr.h>
+#include <stdio.h>
 
 namespace JS {
 
@@ -75,7 +76,26 @@ public:
     NonnullRefPtr<NewExpression> parse_new_expression();
     RefPtr<FunctionExpression> try_parse_arrow_function_expression(bool expect_parens);
 
-    bool has_errors() const { m_parser_state.m_has_errors; }
+    struct Error {
+        String message;
+        size_t line;
+        size_t column;
+
+        String to_string() const
+        {
+            if (line == 0 || column == 0)
+                return message;
+            return String::format("%s (line: %zu, column: %zu)", message.characters(), line, column);
+        }
+    };
+
+    bool has_errors() const { return m_parser_state.m_errors.size(); }
+    const Vector<Error>& errors() const { return m_parser_state.m_errors; }
+    void print_errors() const
+    {
+        for (auto& error : m_parser_state.m_errors)
+            fprintf(stderr, "SyntaxError: %s\n", error.to_string().characters());
+    }
 
 private:
     friend class ScopePusher;
@@ -101,7 +121,7 @@ private:
     struct ParserState {
         Lexer m_lexer;
         Token m_current_token;
-        bool m_has_errors = false;
+        Vector<Error> m_errors;
         Vector<NonnullRefPtrVector<VariableDeclaration>> m_var_scopes;
         Vector<NonnullRefPtrVector<VariableDeclaration>> m_let_scopes;
 

Libraries/LibJS/Runtime/FunctionConstructor.cpp

@@ -70,8 +70,8 @@ Value FunctionConstructor::construct(Interpreter& interpreter)
     auto parser = Parser(Lexer(source));
     auto function_expression = parser.parse_function_node<FunctionExpression>();
     if (parser.has_errors()) {
-        // FIXME: The parser should expose parsing error strings rather than just fprintf()'ing them
+        auto error = parser.errors()[0];
-        interpreter.throw_exception<SyntaxError>("");
+        interpreter.throw_exception<SyntaxError>(error.to_string());
         return {};
     }
     return function_expression->execute(interpreter);

Libraries/LibJS/Tests/Function.js

@@ -29,7 +29,11 @@ try {
     assertThrowsError(() => {
         new Function("[");
     }, {
-        error: SyntaxError
+        error: SyntaxError,
+        // This might be confusing at first but keep in mind it's actually parsing
+        // function anonymous() { [ }
+        // This is in line with what other engines are reporting.
+        message: "Unexpected token CurlyClose. Expected BracketClose (line: 1, column: 26)"
     });
 
     console.log("PASS");

Libraries/LibWeb/DOM/Document.cpp

@@ -378,6 +378,7 @@ JS::Value Document::run_javascript(const StringView& source)
     auto parser = JS::Parser(JS::Lexer(source));
     auto program = parser.parse_program();
     if (parser.has_errors()) {
+        parser.print_errors();
         return JS::js_undefined();
     }
     dbg() << "Document::run_javascript('" << source << "') will run:";

Libraries/LibWeb/DOM/HTMLScriptElement.cpp

@@ -61,9 +61,10 @@ void HTMLScriptElement::children_changed()
 
     auto parser = JS::Parser(JS::Lexer(source));
     auto program = parser.parse_program();
-    if (parser.has_errors())
+    if (parser.has_errors()) {
+        parser.print_errors();
         return;
-
+    }
     document().interpreter().run(*program);
 }
 
@@ -96,9 +97,10 @@ void HTMLScriptElement::inserted_into(Node& new_parent)
 
     auto parser = JS::Parser(JS::Lexer(source));
     auto program = parser.parse_program();
-    if (parser.has_errors())
+    if (parser.has_errors()) {
+        parser.print_errors();
         return;
-
+    }
     document().interpreter().run(*program);
 }
 

Meta/Lagom/Fuzzers/FuzzJs.cpp

@@ -36,5 +36,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
     auto lexer = JS::Lexer(js);
     auto parser = JS::Parser(lexer);
     parser.parse_program();
+    if (parser.has_errors())
+        parser.print_errors();
     return 0;
 }