add Diag_Equality_Check_Used_As_Statement

quick-lint · Apr 28, 2024 · 079f917 · 079f917
1 parent 456e774
commit 079f917
Show file tree

Hide file tree

Showing 13 changed files with 147 additions and 27 deletions.
diff --git a/docs/errors/E0459.md b/docs/errors/E0459.md
@@ -0,0 +1,23 @@
+# E0459: equality check result is unused; did you mean to use assignment (=) instead?
+
+Using `==` as a statement rather than a comparison is almost never intended.
+
+```javascript
+let x = 4;
+let y = 5;
+
+// set x to y - oops, one = instead of two so no assignment is made!
+x == y;
+```
+
+To resolve this, either correct to `=` instead:
+```javascript
+x = y; // no error
+```
+
+Or use/discard the result:
+```javascript
+let _ = x == y;
+void (x == y);
+f(x == y);
+```
diff --git a/po/messages.pot b/po/messages.pot
@@ -2389,6 +2389,10 @@ msgstr ""
 msgid "typeof result is of type string and so will never equal undefined; use 'undefined' instead"
 msgstr ""
 
+#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
+msgid "equality check result is unused; did you mean to use assignment (=) instead?"
+msgstr ""
+
 #: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
 msgid "missing expression in placeholder within template literal"
 msgstr ""

diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
@@ -6848,6 +6848,20 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = {
       },
     },
 
+    // Diag_Equality_Check_Used_As_Statement
+    {
+      .code = 459,
+      .severity = Diagnostic_Severity::warning,
+      .message_formats = {
+        QLJS_TRANSLATABLE("equality check result is unused; did you mean to use assignment (=) instead?"),
+      },
+      .message_args = {
+        {
+          Diagnostic_Message_Arg_Info(offsetof(Diag_Equality_Check_Used_As_Statement, equals_operator), Diagnostic_Arg_Type::source_code_span),
+        },
+      },
+    },
+
     // Diag_Expected_Expression_In_Template_Literal
     {
       .code = 711,

diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.h b/src/quick-lint-js/diag/diagnostic-metadata-generated.h
@@ -468,6 +468,7 @@ namespace quick_lint_js {
   QLJS_DIAG_TYPE_NAME(Diag_Variable_Assigned_To_Self_Is_Noop) \
   QLJS_DIAG_TYPE_NAME(Diag_Xor_Used_As_Exponentiation) \
   QLJS_DIAG_TYPE_NAME(Diag_Typeof_Variable_Equals_Undefined) \
+  QLJS_DIAG_TYPE_NAME(Diag_Equality_Check_Used_As_Statement) \
   QLJS_DIAG_TYPE_NAME(Diag_Expected_Expression_In_Template_Literal) \
   QLJS_DIAG_TYPE_NAME(Diag_Missing_Comma_Between_Array_Elements) \
   QLJS_DIAG_TYPE_NAME(Diag_Class_Generator_On_Getter_Or_Setter) \
@@ -479,7 +480,7 @@ namespace quick_lint_js {
   /* 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];
 }

diff --git a/src/quick-lint-js/diag/diagnostic-types-2.h b/src/quick-lint-js/diag/diagnostic-types-2.h
@@ -3557,6 +3557,16 @@ struct Diag_Typeof_Variable_Equals_Undefined {
   Source_Code_Span undefined;
 };
 
+struct Diag_Equality_Check_Used_As_Statement {
+  [[qljs::diag("E0459", Diagnostic_Severity::warning)]]  //
+  // clang-format off
+  [[qljs::message("equality check result is unused; did you mean to use "
+                  "assignment (=) instead?",
+                  ARG(equals_operator))]]  //
+  // clang-format on
+  Source_Code_Span equals_operator;
+};
+
 struct Diag_Expected_Expression_In_Template_Literal {
   [[qljs::diag("E0711", Diagnostic_Severity::error)]]  //
   [[qljs::message("missing expression in placeholder within template literal",

diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp
@@ -509,6 +509,9 @@ bool Parser::parse_and_visit_statement(Parse_Visitor_Base &v,
       Expression *ast =
           this->make_expression<Expression::Variable>(ident, ident_token_type);
       ast = this->parse_expression_remainder(v, ast, Precedence{});
+
+      this->warn_on_equality_check_used_as_statement(ast);
+
       this->visit_expression(ast, v, Variable_Context::rhs);
       this->parse_end_of_expression_statement();
       break;

diff --git a/src/quick-lint-js/fe/parse.cpp b/src/quick-lint-js/fe/parse.cpp
@@ -406,6 +406,27 @@ void Parser::warn_on_unintuitive_bitshift_precedence(Expression* ast) {
     }
   }
 }
+void Parser::warn_on_equality_check_used_as_statement(Expression* ast) {
+  ast = ast->without_paren();
+  auto is_comparison_operator = [](String8_View s) {
+    return s == u8"=="_sv || s == u8"==="_sv || s == u8"!="_sv ||
+           s == u8"!=="_sv;
+  };
+
+  if (ast->kind() != Expression_Kind::Binary_Operator) return;
+  auto* binary_op = static_cast<Expression::Binary_Operator*>(ast);
+
+  // only interested in the first operator, others may just be ordinary
+  // comparisons
+  if (ast->child_count() <= 1) return;
+  Source_Code_Span eq_span = binary_op->operator_spans_[0];
+
+  if (is_comparison_operator(eq_span.string_view())) {
+    this->diag_reporter_->report(
+        quick_lint_js::Diag_Equality_Check_Used_As_Statement{.equals_operator =
+                                                                 eq_span});
+  }
+}
 void Parser::error_on_pointless_string_compare(
     Expression::Binary_Operator* ast) {
   auto is_comparison_operator = [](String8_View s) {

diff --git a/src/quick-lint-js/fe/parse.h b/src/quick-lint-js/fe/parse.h
@@ -609,6 +609,7 @@ class Parser {
   void warn_on_xor_operator_as_exponentiation(Expression::Binary_Operator *);
   void warn_on_typeof_variable_equals_undefined(Expression::Binary_Operator *);
   void warn_on_unintuitive_bitshift_precedence(Expression *ast);
+  void warn_on_equality_check_used_as_statement(Expression *ast);
   void error_on_pointless_string_compare(Expression::Binary_Operator *);
   void error_on_pointless_compare_against_literal(
       Expression::Binary_Operator *);

diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp
@@ -279,7 +279,8 @@ const Translation_Table translation_data = {
         {0, 0, 0, 0, 0, 43},                 //
         {0, 0, 0, 33, 0, 5},                 //
         {0, 0, 0, 47, 0, 35},                //
-        {31, 20, 35, 43, 34, 30},            //
+        {0, 0, 0, 0, 0, 30},                 //
+        {31, 20, 35, 43, 34, 77},            //
         {64, 53, 0, 54, 0, 48},              //
         {74, 36, 0, 56, 0, 60},              //
         {63, 41, 0, 51, 0, 42},              //
@@ -2154,6 +2155,7 @@ const Translation_Table translation_data = {
         u8"enum\0"
         u8"enum member name cannot be numeric\0"
         u8"enum member needs initializer\0"
+        u8"equality check result is unused; did you mean to use assignment (=) instead?\0"
         u8"escaped character is not allowed in identifiers\0"
         u8"escaping '-' is not allowed in tag names; write '-' instead\0"
         u8"event attributes must be camelCase: '{1}'\0"

diff --git a/src/quick-lint-js/i18n/translation-table-generated.h b/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 = 82764;
 constexpr std::size_t translation_table_locale_table_size = 35;
 
 QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
@@ -294,6 +294,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
           "enum"sv,
           "enum member name cannot be numeric"sv,
           "enum member needs initializer"sv,
+          "equality check result is unused; did you mean to use assignment (=) instead?"sv,
           "escaped character is not allowed in identifiers"sv,
           "escaping '-' is not allowed in tag names; write '-' instead"sv,
           "event attributes must be camelCase: '{1}'"sv,

diff --git a/src/quick-lint-js/i18n/translation-table-test-generated.h b/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",
@@ -2976,6 +2976,17 @@ inline const Translated_String test_translation_table[608] = {
             u8"enum member needs initializer",
         },
     },
+    {
+        "equality check result is unused; did you mean to use assignment (=) instead?"_translatable,
+        u8"equality check result is unused; did you mean to use assignment (=) instead?",
+        {
+            u8"equality check result is unused; did you mean to use assignment (=) instead?",
+            u8"equality check result is unused; did you mean to use assignment (=) instead?",
+            u8"equality check result is unused; did you mean to use assignment (=) instead?",
+            u8"equality check result is unused; did you mean to use assignment (=) instead?",
+            u8"equality check result is unused; did you mean to use assignment (=) instead?",
+        },
+    },
     {
         "escaped character is not allowed in identifiers"_translatable,
         u8"escaped character is not allowed in identifiers",

diff --git a/test/test-parse-typescript.cpp b/test/test-parse-typescript.cpp
@@ -64,11 +64,11 @@ TEST_F(Test_Parse_TypeScript, warn_on_mistyped_strict_inequality_operator) {
 
 TEST_F(Test_Parse_TypeScript,
        mistyped_strict_inequality_operator_is_suppressable) {
-  test_parse_and_visit_statement(u8"(x!) == y"_sv, no_diags,
+  test_parse_and_visit_expression(u8"(x!) == y"_sv, no_diags,
                                  typescript_options);
-  test_parse_and_visit_statement(u8"x! /**/ == y"_sv, no_diags,
+  test_parse_and_visit_expression(u8"x! /**/ == y"_sv, no_diags,
                                  typescript_options);
-  test_parse_and_visit_statement(u8"x!\n== y"_sv, no_diags, typescript_options);
+  test_parse_and_visit_expression(u8"x!\n== y"_sv, no_diags, typescript_options);
 }
 
 TEST_F(Test_Parse_TypeScript, unicode_next_line_is_whitespace) {

diff --git a/test/test-parse-warning.cpp b/test/test-parse-warning.cpp
@@ -153,15 +153,15 @@ TEST_F(Test_Error_Equals_Does_Not_Distribute_Over_Or, non_constant) {
 }
 
 TEST_F(Test_Parse_Warning, warn_on_pointless_string_compare) {
-  test_parse_and_visit_statement(u8"s.toLowerCase() == 'banana'"_sv, no_diags);
-  test_parse_and_visit_statement(
+  test_parse_and_visit_expression(u8"s.toLowerCase() == 'banana'"_sv, no_diags);
+  test_parse_and_visit_expression(
       u8"s.toLowerCase() == 'BANANA'"_sv,  //
       u8"                ^^ Diag_Pointless_String_Comp_Contains_Upper"_diag);
-  test_parse_and_visit_statement(u8"s.toUpperCase() == 'BANANA'"_sv, no_diags);
-  test_parse_and_visit_statement(
+  test_parse_and_visit_expression(u8"s.toUpperCase() == 'BANANA'"_sv, no_diags);
+  test_parse_and_visit_expression(
       u8"s.toUpperCase() == 'banana'"_sv,  //
       u8"                ^^ Diag_Pointless_String_Comp_Contains_Lower"_diag);
-  test_parse_and_visit_statement(
+  test_parse_and_visit_expression(
       u8"s.toLowerCase() == \"BANANA\""_sv,  //
       u8"                ^^ Diag_Pointless_String_Comp_Contains_Upper"_diag);
 }
@@ -171,7 +171,7 @@ TEST_F(Test_Parse_Warning, warn_on_pointless_string_compare_all_operators) {
     for (String8_View op : {u8"=="_sv, u8"==="_sv, u8"!="_sv, u8"!=="_sv}) {
       Test_Parser p(concat(u8"s.toLowerCase() "_sv, op, u8" 'Banana'"_sv),
                     capture_diags);
-      p.parse_and_visit_statement();
+      p.parse_and_visit_expression();
       assert_diagnostics(
           p.code, p.errors,
           {
@@ -186,14 +186,14 @@ TEST_F(Test_Parse_Warning, warn_on_pointless_string_compare_all_operators) {
 
 TEST_F(Test_Parse_Warning,
        warn_on_pointless_string_compare_function_signatures) {
-  test_parse_and_visit_statement(u8"s.tolowercase() == 'BANANA'"_sv, no_diags);
-  test_parse_and_visit_statement(u8"s.myToLowerCase() == 'BANANA'"_sv,
-                                 no_diags);
-  test_parse_and_visit_statement(
+  test_parse_and_visit_expression(u8"s.tolowercase() == 'BANANA'"_sv, no_diags);
+  test_parse_and_visit_expression(u8"s.myToLowerCase() == 'BANANA'"_sv,
+                                  no_diags);
+  test_parse_and_visit_expression(
       u8"stringBuilder.build().toLowerCase() == 'BANANA'"_sv,  //
       u8"                                    ^^ Diag_Pointless_String_Comp_Contains_Upper"_diag);
-  test_parse_and_visit_statement(u8"o.arr[0]() == 'BANANA'"_sv, no_diags);
-  test_parse_and_visit_statement(
+  test_parse_and_visit_expression(u8"o.arr[0]() == 'BANANA'"_sv, no_diags);
+  test_parse_and_visit_expression(
       u8"'BANANA' == s.toLowerCase()"_sv,  //
       u8"         ^^ Diag_Pointless_String_Comp_Contains_Upper"_diag);
 }
@@ -255,23 +255,23 @@ TEST_F(Test_Parse_Warning,
   test_parse_and_visit_expression(
       u8"(((s.toLowerCase())) === ((('BANANA'))))"_sv,  //
       u8"                     ^^^ Diag_Pointless_String_Comp_Contains_Upper"_diag);
-  test_parse_and_visit_statement(
+  test_parse_and_visit_expression(
       u8"s.toLowerCase() == 'BANANA' && s.toUpperCase() !== 'orange'"_sv,  //
       u8"                                               ^^^ Diag_Pointless_String_Comp_Contains_Lower"_diag,  //
       u8"                ^^ Diag_Pointless_String_Comp_Contains_Upper"_diag);
-  test_parse_and_visit_statement(
+  test_parse_and_visit_expression(
       u8"((s.toLowerCase() == 'BANANA') && s.toUpperCase() !== 'orange')"_sv,  //
       u8"                                                  ^^^ Diag_Pointless_String_Comp_Contains_Lower"_diag,  //
       u8"                  ^^ Diag_Pointless_String_Comp_Contains_Upper"_diag);
 }
 
 TEST_F(Test_Parse_Warning, warn_on_pointless_string_compare_literals) {
-  test_parse_and_visit_statement(u8"s.toLowerCase() == 'Ba\\u006Eana'"_sv,
-                                 no_diags);
-  test_parse_and_visit_statement(
+  test_parse_and_visit_expression(u8"s.toLowerCase() == 'Ba\\u006Eana'"_sv,
+                                  no_diags);
+  test_parse_and_visit_expression(
       u8"s.toLowerCase() == 0xeF || s.toUpperCase() == 0xeF"_sv, no_diags);
-  test_parse_and_visit_statement(u8"s.toUpperCase() == 'C:\\User\\tom'"_sv,
-                                 no_diags);
+  test_parse_and_visit_expression(u8"s.toUpperCase() == 'C:\\User\\tom'"_sv,
+                                  no_diags);
 }
 
 TEST_F(Test_Parse_Warning,
@@ -442,6 +442,35 @@ TEST_F(Test_Parse_Warning, warn_on_typeof_variable_equals_undefined) {
                                  no_diags);
 }
 
+TEST_F(Test_Parse_Warning, warn_on_equality_check_used_as_statement) {
+  test_parse_and_visit_statement(
+      u8"x == y;"_sv,  //
+      u8"  ^^ Diag_Equality_Check_Used_As_Statement"_diag);
+  test_parse_and_visit_statement(
+      u8"x != y;"_sv,  //
+      u8"  ^^ Diag_Equality_Check_Used_As_Statement"_diag);
+  test_parse_and_visit_statement(
+      u8"x === y;"_sv,  //
+      u8"  ^^^ Diag_Equality_Check_Used_As_Statement"_diag);
+  test_parse_and_visit_statement(
+      u8"x !== y;"_sv,  //
+      u8"  ^^^ Diag_Equality_Check_Used_As_Statement"_diag);
+  test_parse_and_visit_statement(
+      u8"x == y(42);"_sv,  //
+      u8"  ^^ Diag_Equality_Check_Used_As_Statement"_diag);
+  test_parse_and_visit_statement(
+      u8"x == y == z;"_sv,  //
+      u8"  ^^ Diag_Equality_Check_Used_As_Statement"_diag);
+
+  test_parse_and_visit_statement(u8"let _ = x == y;"_sv, no_diags);
+  test_parse_and_visit_statement(u8"void (x == y);"_sv, no_diags);
+  test_parse_and_visit_statement(u8"f(x == y);"_sv, no_diags);
+
+  test_parse_and_visit_statement(u8"x + y == z;"_sv, no_diags);
+  test_parse_and_visit_statement(u8"x = y;"_sv, no_diags);
+  test_parse_and_visit_statement(u8"x + y;"_sv, no_diags);
+}
+
 TEST_F(Test_Parse_Warning, warn_on_xor_operation_used_as_exponentiation) {
   test_parse_and_visit_expression(
       u8"2 ^ 8"_sv,  //