LibJS: Fix "use strict" directive false positives

By having the "is this a use strict directive?" logic in
parse_string_literal() we would apply it to *any* string literal, which
is incorrect and would lead to false positives - e.g.:

    "use strict" + 1
    `"use strict"`
    "\123"; ({"use strict": ...})

Relevant part from the spec which is now implemented properly:

[...] and where each ExpressionStatement in the sequence consists
entirely of a StringLiteral token [...]

I also got rid of UseStrictDirectiveState which is not needed anymore.

Fixes #3903.

Libraries/LibJS/AST.h

@@ -64,6 +64,8 @@ public:
     virtual bool is_call_expression() const { return false; }
     virtual bool is_new_expression() const { return false; }
     virtual bool is_super_expression() const { return false; }
+    virtual bool is_expression_statement() const { return false; };
+    virtual bool is_string_literal() const { return false; };
 
 protected:
     ASTNode() { }
@@ -99,11 +101,15 @@ public:
     {
     }
 
-    Value execute(Interpreter&, GlobalObject&) const override;
+    virtual Value execute(Interpreter&, GlobalObject&) const override;
-    const char* class_name() const override { return "ExpressionStatement"; }
     virtual void dump(int indent) const override;
+    virtual bool is_expression_statement() const override { return true; }
+
+    const Expression& expression() const { return m_expression; };
 
 private:
+    virtual const char* class_name() const override { return "ExpressionStatement"; }
+
     NonnullRefPtr<Expression> m_expression;
 };
 
@@ -590,20 +596,24 @@ private:
 
 class StringLiteral final : public Literal {
 public:
-    explicit StringLiteral(String value)
+    explicit StringLiteral(String value, bool is_use_strict_directive = false)
         : m_value(move(value))
+        , m_is_use_strict_directive(is_use_strict_directive)
     {
     }
 
-    StringView value() const { return m_value; }
-
     virtual Value execute(Interpreter&, GlobalObject&) const override;
     virtual void dump(int indent) const override;
+    virtual bool is_string_literal() const override { return true; };
+
+    StringView value() const { return m_value; }
+    bool is_use_strict_directive() const { return m_is_use_strict_directive; };
 
 private:
     virtual const char* class_name() const override { return "StringLiteral"; }
 
     String m_value;
+    bool m_is_use_strict_directive;
 };
 
 class NullLiteral final : public Literal {

Libraries/LibJS/Parser.cpp

@@ -33,6 +33,17 @@
 
 namespace JS {
 
+static bool statement_is_use_strict_directive(NonnullRefPtr<Statement> statement)
+{
+    if (!statement->is_expression_statement())
+        return false;
+    auto& expression_statement = static_cast<ExpressionStatement&>(*statement);
+    auto& expression = expression_statement.expression();
+    if (!expression.is_string_literal())
+        return false;
+    return static_cast<const StringLiteral&>(expression).is_use_strict_directive();
+}
+
 class ScopePusher {
 public:
     enum Type {
@@ -239,28 +250,25 @@ NonnullRefPtr<Program> Parser::parse_program()
     auto program = adopt(*new Program);
 
     bool first = true;
-    m_parser_state.m_use_strict_directive = UseStrictDirectiveState::Looking;
     while (!done()) {
         if (match_declaration()) {
             program->append(parse_declaration());
-            if (first) {
-                first = false;
-                m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
-            }
         } else if (match_statement()) {
-            program->append(parse_statement());
+            auto statement = parse_statement();
-            if (first) {
+            program->append(statement);
-                if (m_parser_state.m_use_strict_directive == UseStrictDirectiveState::Found) {
+            if (statement_is_use_strict_directive(statement)) {
+                if (first) {
                     program->set_strict_mode();
                     m_parser_state.m_strict_mode = true;
                 }
-                first = false;
+                if (m_parser_state.m_string_legacy_octal_escape_sequence_in_scope)
-                m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
+                    syntax_error("Octal escape sequence in string literal not allowed in strict mode");
             }
         } else {
             expected("statement or declaration");
             consume();
         }
+        first = false;
     }
     if (m_parser_state.m_var_scopes.size() == 1) {
         program->add_variables(m_parser_state.m_var_scopes.last());
@@ -862,26 +870,9 @@ NonnullRefPtr<StringLiteral> Parser::parse_string_literal(Token token, bool in_t
             syntax_error(message, token.line_number(), token.line_column());
     }
 
-    auto is_use_strict_directive = token.value() == "'use strict'" || token.value() == "\"use strict\"";
+    auto is_use_strict_directive = !in_template_literal && (token.value() == "'use strict'" || token.value() == "\"use strict\"");
 
-    // It is possible for string literals to precede a Use Strict Directive that places the
+    return create_ast_node<StringLiteral>(string, is_use_strict_directive);
-    // enclosing code in strict mode, and implementations must take care to not use this
-    // extended definition of EscapeSequence with such literals. For example, attempting to
-    // parse the following source text must fail:
-    //
-    // function invalid() { "\7"; "use strict"; }
-
-    if (m_parser_state.m_string_legacy_octal_escape_sequence_in_scope && is_use_strict_directive)
-        syntax_error("Octal escape sequence in string literal not allowed in strict mode");
-
-    if (m_parser_state.m_use_strict_directive == UseStrictDirectiveState::Looking) {
-        if (is_use_strict_directive)
-            m_parser_state.m_use_strict_directive = UseStrictDirectiveState::Found;
-        else
-            m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
-    }
-
-    return create_ast_node<StringLiteral>(string);
 }
 
 NonnullRefPtr<TemplateLiteral> Parser::parse_template_literal(bool is_tagged)
@@ -1238,34 +1229,27 @@ NonnullRefPtr<BlockStatement> Parser::parse_block_statement(bool& is_strict)
 
     bool first = true;
     bool initial_strict_mode_state = m_parser_state.m_strict_mode;
-    if (initial_strict_mode_state) {
+    if (initial_strict_mode_state)
-        m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
         is_strict = true;
-    } else {
-        m_parser_state.m_use_strict_directive = UseStrictDirectiveState::Looking;
-    }
 
     while (!done() && !match(TokenType::CurlyClose)) {
         if (match_declaration()) {
             block->append(parse_declaration());
-
-            if (first && !initial_strict_mode_state)
-                m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
         } else if (match_statement()) {
-            block->append(parse_statement());
+            auto statement = parse_statement();
-
+            block->append(statement);
-            if (first && !initial_strict_mode_state) {
+            if (statement_is_use_strict_directive(statement)) {
-                if (m_parser_state.m_use_strict_directive == UseStrictDirectiveState::Found) {
+                if (first && !initial_strict_mode_state) {
                     is_strict = true;
                     m_parser_state.m_strict_mode = true;
                 }
-                m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
+                if (m_parser_state.m_string_legacy_octal_escape_sequence_in_scope)
+                    syntax_error("Octal escape sequence in string literal not allowed in strict mode");
             }
         } else {
             expected("statement or declaration");
             consume();
         }
-
         first = false;
     }
     m_parser_state.m_strict_mode = initial_strict_mode_state;

Libraries/LibJS/Parser.h

@@ -164,12 +164,6 @@ private:
     void save_state();
     void load_state();
 
-    enum class UseStrictDirectiveState {
-        None,
-        Looking,
-        Found,
-    };
-
     struct ParserState {
         Lexer m_lexer;
         Token m_current_token;
@@ -177,7 +171,6 @@ private:
         Vector<NonnullRefPtrVector<VariableDeclaration>> m_var_scopes;
         Vector<NonnullRefPtrVector<VariableDeclaration>> m_let_scopes;
         Vector<NonnullRefPtrVector<FunctionDeclaration>> m_function_scopes;
-        UseStrictDirectiveState m_use_strict_directive { UseStrictDirectiveState::None };
         HashTable<StringView> m_labels_in_scope;
         bool m_strict_mode { false };
         bool m_allow_super_property_lookup { false };

Libraries/LibJS/Tests/use-strict-directive.js

@@ -45,4 +45,16 @@ test("invalid 'use strict; directive", () => {
             return isStrictMode();
         })()
     ).toBeFalse();
+    expect(
+        (() => {
+            `"use strict"`;
+            return isStrictMode();
+        })()
+    ).toBeFalse();
+    expect(
+        (() => {
+            "use strict" + 1;
+            return isStrictMode();
+        })()
+    ).toBeFalse();
 });