feat(fe): add a warning when comparing typeof to invalid string literals

docs/errors/E0721.md

@@ -0,0 +1,22 @@
+# E0721: typeof comparison with invalid string literal
+
+The `typeof` operator will always return a string that can take a value from `["undefined", "object", "boolean", "number", "bigint", "string", "symbol", "function"]`. Therefore, comparing to another sting has a high chance of being an error
+([MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/typeof))
+
+```javascript
+let x = foo;
+if (typeof x === "strng") {
+    // this will not run!
+    alert("x is a string!");
+}
+```
+Instead, compare against the string `"string"`;
+
+```javascript
+let x = "foo"
+if (typeof x === "string") {
+    // this will run now :)
+    alert("x is a string");
+}
+```
+

po/messages.pot

@@ -2429,6 +2429,10 @@ msgstr ""
 msgid "function 'let' call may be confused for destructuring; remove parentheses to declare a variable"
 msgstr ""
 
+#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
+msgid "typeof comparison with invalid string literal: {0}"
+msgstr ""
+
 #: test/test-diagnostic-formatter.cpp
 #: test/test-vim-qflist-json-diag-reporter.cpp
 msgid "something happened"

src/quick-lint-js/diag/diagnostic-metadata-generated.cpp

@@ -6975,6 +6975,20 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = {
         },
       },
     },
+
+    // Diag_Typeof_Invalid_String_Comparison
+    {
+      .code = 721,
+      .severity = Diagnostic_Severity::warning,
+      .message_formats = {
+        QLJS_TRANSLATABLE("typeof comparison with invalid string literal: {0}"),
+      },
+      .message_args = {
+        {
+          Diagnostic_Message_Arg_Info(offsetof(Diag_Typeof_Invalid_String_Comparison, literal), Diagnostic_Arg_Type::source_code_span),
+        },
+      },
+    },
 };
 }
 

src/quick-lint-js/diag/diagnostic-metadata-generated.h

@@ -476,10 +476,11 @@ namespace quick_lint_js {
   QLJS_DIAG_TYPE_NAME(Diag_Unintuitive_Bitshift_Precedence) \
   QLJS_DIAG_TYPE_NAME(Diag_TypeScript_Namespace_Alias_Cannot_Use_Import_Type) \
   QLJS_DIAG_TYPE_NAME(Diag_Confusing_Let_Call) \
+  QLJS_DIAG_TYPE_NAME(Diag_Typeof_Invalid_String_Comparison) \
   /* END */
 // clang-format on
 
-inline constexpr int Diag_Type_Count = 465;
+inline constexpr int Diag_Type_Count = 466;
 
 extern const Diagnostic_Info all_diagnostic_infos[Diag_Type_Count];
 }

src/quick-lint-js/diag/diagnostic-types-2.h

@@ -788,10 +788,10 @@ struct Diag_Duplicated_Cases_In_Switch_Statement {
 
 struct Diag_Fallthrough_Without_Comment_In_Switch {
   [[qljs::diag("E0427", Diagnostic_Severity::warning)]]  //
-  [
-      [qljs::message("missing 'break;' or '// fallthrough' comment between "
-                     "statement and 'case'",
-                     ARG(end_of_case))]]  //
+  [[qljs::message(
+      "missing 'break;' or '// fallthrough' comment between "
+      "statement and 'case'",
+      ARG(end_of_case))]]  //
   Source_Code_Span end_of_case;
 };
 
@@ -2281,10 +2281,10 @@ struct Diag_TypeScript_Delete_Cannot_Delete_Variables {
 
 struct Diag_TypeScript_Definite_Assignment_Assertion_In_Ambient_Context {
   [[qljs::diag("E0445", Diagnostic_Severity::error)]]  //
-  [
-      [qljs::message("'!' (definite assignment assertion) is not allowed on "
-                     "'declare' variables",
-                     ARG(definite_assignment_assertion))]]  //
+  [[qljs::message(
+      "'!' (definite assignment assertion) is not allowed on "
+      "'declare' variables",
+      ARG(definite_assignment_assertion))]]  //
   [[qljs::message("'declare' here",
                   ARG(declare_keyword))]]  //
   Source_Code_Span definite_assignment_assertion;
@@ -2309,10 +2309,10 @@ struct Diag_TypeScript_Definite_Assignment_Assertion_On_Const {
 
 struct Diag_TypeScript_Definite_Assignment_Assertion_With_Initializer {
   [[qljs::diag("E0442", Diagnostic_Severity::error)]]  //
-  [
-      [qljs::message("'!' (definite assignment assertion) cannot be used with "
-                     "an initial value",
-                     ARG(definite_assignment_assertion))]]  //
+  [[qljs::message(
+      "'!' (definite assignment assertion) cannot be used with "
+      "an initial value",
+      ARG(definite_assignment_assertion))]]  //
   [[qljs::message("initial value was given here",
                   ARG(equal))]]  //
   Source_Code_Span definite_assignment_assertion;
@@ -2321,10 +2321,10 @@ struct Diag_TypeScript_Definite_Assignment_Assertion_With_Initializer {
 
 struct Diag_TypeScript_Definite_Assignment_Assertion_Without_Type_Annotation {
   [[qljs::diag("E0443", Diagnostic_Severity::error)]]  //
-  [
-      [qljs::message("type annotation is required when using '!' (definite "
-                     "assignment assertion)",
-                     ARG(definite_assignment_assertion))]]  //
+  [[qljs::message(
+      "type annotation is required when using '!' (definite "
+      "assignment assertion)",
+      ARG(definite_assignment_assertion))]]  //
   Source_Code_Span definite_assignment_assertion;
 };
 
@@ -2586,10 +2586,10 @@ struct Diag_TypeScript_Declare_Field_Not_Allowed_In_JavaScript {
 
 struct Diag_TypeScript_Declare_Field_Cannot_Use_Private_Identifier {
   [[qljs::diag("E0416", Diagnostic_Severity::error)]]  //
-  [
-      [qljs::message("private identifiers are not allowed for 'declare' "
-                     "fields; use 'private' instead",
-                     ARG(private_identifier_hash))]]         //
+  [[qljs::message(
+      "private identifiers are not allowed for 'declare' "
+      "fields; use 'private' instead",
+      ARG(private_identifier_hash))]]                        //
   [[qljs::message("'declare' here", ARG(declare_keyword))]]  //
   Source_Code_Span private_identifier_hash;
   Source_Code_Span declare_keyword;
@@ -3621,14 +3621,21 @@ struct Diag_TypeScript_Namespace_Alias_Cannot_Use_Import_Type {
 
 struct Diag_Confusing_Let_Call {
   [[qljs::diag("E0720", Diagnostic_Severity::warning)]]  //
-  [
-      [qljs::message("function 'let' call may be confused for destructuring; "
-                     "remove parentheses to declare a variable",
-                     ARG(let_function_call))]]  //
+  [[qljs::message(
+      "function 'let' call may be confused for destructuring; "
+      "remove parentheses to declare a variable",
+      ARG(let_function_call))]]  //
   Source_Code_Span let_function_call;
 };
 
+struct Diag_Typeof_Invalid_String_Comparison {
+  [[qljs::diag("E0721", Diagnostic_Severity::warning)]]  //
+  [[qljs::message("typeof comparison with invalid string literal: {0}",
+                  ARG(literal))]]  //
+  Source_Code_Span literal;
+};
 }
+
 QLJS_WARNING_POP
 
 // quick-lint-js finds bugs in JavaScript programs.

src/quick-lint-js/fe/parse-expression.cpp

@@ -75,6 +75,8 @@ void Parser::visit_expression(Expression* ast, Parse_Visitor_Base& v,
         expression_cast<Expression::Binary_Operator*>(ast));
     this->warn_on_typeof_variable_equals_undefined(
         expression_cast<Expression::Binary_Operator*>(ast));
+    this->warn_on_typeof_variable_equals_invalid_literal(
+        expression_cast<Expression::Binary_Operator*>(ast));
     this->warn_on_unintuitive_bitshift_precedence(
         expression_cast<Expression::Binary_Operator*>(ast));
     break;
@@ -518,11 +520,10 @@ Expression* Parser::parse_primary_expression(Parse_Visitor_Base& v,
     Expression* ast =
         type == Token_Type::kw_delete
             ? this->make_expression<Expression::Delete>(child, operator_span)
-            : type == Token_Type::kw_typeof
-                  ? this->make_expression<Expression::Typeof>(child,
-                                                              operator_span)
-                  : this->make_expression<Expression::Unary_Operator>(
-                        child, operator_span);
+        : type == Token_Type::kw_typeof
+            ? this->make_expression<Expression::Typeof>(child, operator_span)
+            : this->make_expression<Expression::Unary_Operator>(child,
+                                                                operator_span);
     return ast;
   }
 
@@ -1683,7 +1684,7 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v,
   // x += y
   // x[y] &&= z
   QLJS_CASE_COMPOUND_ASSIGNMENT_OPERATOR:
-  QLJS_CASE_CONDITIONAL_ASSIGNMENT_OPERATOR : {
+  QLJS_CASE_CONDITIONAL_ASSIGNMENT_OPERATOR: {
     if (!prec.math_or_logical_or_assignment) {
       break;
     }
@@ -3016,7 +3017,7 @@ Expression* Parser::parse_object_literal(Parse_Visitor_Base& v) {
           break;
         }
 
-        QLJS_CASE_RESERVED_KEYWORD_EXCEPT_AWAIT_AND_YIELD : {
+        QLJS_CASE_RESERVED_KEYWORD_EXCEPT_AWAIT_AND_YIELD: {
           Expression* value =
               this->make_expression<Expression::Missing>(key_token.span());
           this->diags_.add(Diag_Missing_Value_For_Object_Literal_Entry{
@@ -3990,10 +3991,11 @@ Expression* Parser::parse_jsx_element_or_fragment(Parse_Visitor_Base& v,
             .opening_tag_name =
                 tag_namespace
                     ? Source_Code_Span(tag_namespace->span().begin(), tag_end)
-                    : !tag_members.empty()
-                          ? Source_Code_Span(tag_members.front().span().begin(),
-                                             tag_end)
-                          : tag ? tag->span() : Source_Code_Span::unit(tag_end),
+                : !tag_members.empty()
+                    ? Source_Code_Span(tag_members.front().span().begin(),
+                                       tag_end)
+                : tag ? tag->span()
+                      : Source_Code_Span::unit(tag_end),
             .closing_tag_name =
                 closing_tag_begin <= closing_tag_end
                     ? Source_Code_Span(closing_tag_begin, closing_tag_end)

src/quick-lint-js/fe/parse.cpp

@@ -538,6 +538,60 @@ void Parser::warn_on_typeof_variable_equals_undefined(
   }
 }
 
+void Parser::warn_on_typeof_variable_equals_invalid_literal(
+    Expression::Binary_Operator* ast) {
+  static const auto existingLiterals =
+      std::to_array<std::basic_string_view<char8_t>>(
+          {u8"undefined", u8"object", u8"boolean", u8"number", u8"bigint",
+           u8"string", u8"symbol", u8"function"});
+
+  auto is_comparison_operator = [](String8_View s) -> bool {
+    return s == u8"=="_sv || s == u8"==="_sv || s == u8"!="_sv ||
+           s == u8"!=="_sv;
+  };
+
+  auto normalize = [](String8_View str) -> String8_View {
+    auto start = str.find_first_not_of(u8"'\"\\");
+    if (start == String8_View::npos) {
+      return {};
+    }
+    auto end = str.find_last_not_of(u8"'\"\\");
+    if (end == String8_View::npos || end < start) {
+      return {};
+    }
+    return str.substr(start, end - start + 1);
+  };
+
+  for (Span_Size i = 0; i < ast->child_count() - 1; i++) {
+    Source_Code_Span eq_span = ast->operator_spans_[i];
+    if (is_comparison_operator(eq_span.string_view())) {
+      Expression* typeof_op = ast->child(i)->without_paren();
+      Expression* literal = ast->child(i + 1)->without_paren();
+      if (typeof_op->kind() == Expression_Kind::Typeof &&
+          literal->kind() == Expression_Kind::Literal) {
+        // fallthrough
+      } else if (literal->kind() == Expression_Kind::Typeof &&
+                 typeof_op->kind() == Expression_Kind::Literal) {
+        std::swap(literal, typeof_op);
+        // falltrough
+      } else {
+        continue;  // does not match
+      }
+
+      Expression::Literal* type_literal =
+          expression_cast<Expression::Literal*>(literal);
+      String8_View literal_value =
+          normalize(type_literal->span().string_view());
+
+      if (std::find(existingLiterals.begin(), existingLiterals.end(),
+                    literal_value) == existingLiterals.end()) {
+        this->diags_.add(Diag_Typeof_Invalid_String_Comparison{
+            .literal = type_literal->span()});
+      }
+    }
+  }
+}
+
 void Parser::error_on_pointless_compare_against_literal(
     Expression::Binary_Operator* ast) {
   auto is_comparison_operator = [](String8_View s) -> bool {

src/quick-lint-js/fe/parse.h

@@ -105,7 +105,7 @@ struct Parser_Transaction {
 // Parse_Visitor (visit_variable_declaration, visit_enter_function_scope, etc.).
 class Parser {
  private:
-  template <bool Parser::*Member>
+  template <bool Parser:: *Member>
   class Bool_Guard;
 
  public:
@@ -604,6 +604,8 @@ class Parser {
   void warn_on_comma_operator_in_index(Expression *, Source_Code_Span);
   void warn_on_xor_operator_as_exponentiation(Expression::Binary_Operator *);
   void warn_on_typeof_variable_equals_undefined(Expression::Binary_Operator *);
+  void warn_on_typeof_variable_equals_invalid_literal(
+      Expression::Binary_Operator *);
   void warn_on_unintuitive_bitshift_precedence(Expression *ast);
   void error_on_pointless_string_compare(Expression::Binary_Operator *);
   void error_on_pointless_compare_against_literal(
@@ -1011,7 +1013,7 @@ class Parser {
 
  private:
   template <class New_Expression, class... Args>
-  Expression *make_expression(Args &&... args) {
+  Expression *make_expression(Args &&...args) {
     return this->expressions_.make_expression<New_Expression>(
         std::forward<Args>(args)...);
   }
@@ -1040,7 +1042,7 @@ class Parser {
   };
 
  private:
-  template <bool Parser::*Member>
+  template <bool Parser:: *Member>
   class Bool_Guard {
    public:
     explicit Bool_Guard(Parser *p, bool old_value)

src/quick-lint-js/i18n/translation-table-generated.cpp

@@ -544,6 +544,7 @@ const Translation_Table translation_data = {
         {0, 0, 0, 0, 0, 75},                 //
         {0, 0, 0, 0, 0, 58},                 //
         {0, 0, 0, 0, 0, 31},                 //
+        {0, 0, 0, 0, 0, 51},                 //
         {27, 19, 30, 29, 22, 91},            //
         {25, 50, 0, 36, 0, 23},              //
         {66, 43, 31, 36, 30, 44},            //
@@ -2418,6 +2419,7 @@ const Translation_Table translation_data = {
         u8"type annotation is required when using '!' (definite assignment assertion)\0"
         u8"type predicates are only allowed as function return types\0"
         u8"type {0} is being defined here\0"
+        u8"typeof comparison with invalid string literal: {0}\0"
         u8"typeof result is of type string and so will never equal undefined; use 'undefined' instead\0"
         u8"unclosed block comment\0"
         u8"unclosed class; expected '}' by end of file\0"

src/quick-lint-js/i18n/translation-table-generated.h

@@ -18,8 +18,8 @@ namespace quick_lint_js {
 using namespace std::literals::string_view_literals;
 
 constexpr std::uint32_t translation_table_locale_count = 5;
-constexpr std::uint16_t translation_table_mapping_table_size = 609;
-constexpr std::size_t translation_table_string_table_size = 82687;
+constexpr std::uint16_t translation_table_mapping_table_size = 610;
+constexpr std::size_t translation_table_string_table_size = 82738;
 constexpr std::size_t translation_table_locale_table_size = 35;
 
 QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
@@ -558,6 +558,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
           "type annotation is required when using '!' (definite assignment assertion)"sv,
           "type predicates are only allowed as function return types"sv,
           "type {0} is being defined here"sv,
+          "typeof comparison with invalid string literal: {0}"sv,
           "typeof result is of type string and so will never equal undefined; use 'undefined' instead"sv,
           "unclosed block comment"sv,
           "unclosed class; expected '}' by end of file"sv,

src/quick-lint-js/i18n/translation-table-test-generated.h

@@ -27,7 +27,7 @@ struct Translated_String {
 };
 
 // clang-format off
-inline const Translated_String test_translation_table[608] = {
+inline const Translated_String test_translation_table[609] = {
     {
         "\"global-groups\" entries must be strings"_translatable,
         u8"\"global-groups\" entries must be strings",
@@ -5880,6 +5880,17 @@ inline const Translated_String test_translation_table[608] = {
             u8"type {0} is being defined here",
         },
     },
+    {
+        "typeof comparison with invalid string literal: {0}"_translatable,
+        u8"typeof comparison with invalid string literal: {0}",
+        {
+            u8"typeof comparison with invalid string literal: {0}",
+            u8"typeof comparison with invalid string literal: {0}",
+            u8"typeof comparison with invalid string literal: {0}",
+            u8"typeof comparison with invalid string literal: {0}",
+            u8"typeof comparison with invalid string literal: {0}",
+        },
+    },
     {
         "typeof result is of type string and so will never equal undefined; use 'undefined' instead"_translatable,
         u8"typeof result is of type string and so will never equal undefined; use 'undefined' instead",