From c8b32584f1ad8d1bc88a293e451886880c6d0bb9 Mon Sep 17 00:00:00 2001 From: msharipov Date: Fri, 15 Mar 2024 21:47:32 -0400 Subject: [PATCH] feat(fe): add warning for 'let({x} = y);' Added a new diagnostic E0720 that closes #1203. Issues a warning when a function let() is called on an object assignment expression. https://github.com/quick-lint/quick-lint-js/issues/1203 --- docs/errors/E0720.md | 18 ++++++++++++++++++ po/messages.pot | 4 ++++ .../diag/diagnostic-metadata-generated.cpp | 14 ++++++++++++++ .../diag/diagnostic-metadata-generated.h | 3 ++- src/quick-lint-js/diag/diagnostic-types-2.h | 10 ++++++++++ src/quick-lint-js/fe/parse-statement.cpp | 15 +++++++++++++++ .../i18n/translation-table-generated.cpp | 4 +++- .../i18n/translation-table-generated.h | 5 +++-- .../i18n/translation-table-test-generated.h | 13 ++++++++++++- test/test-parse-warning.cpp | 7 +++++++ 10 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 docs/errors/E0720.md diff --git a/docs/errors/E0720.md b/docs/errors/E0720.md new file mode 100644 index 000000000..ede4a69aa --- /dev/null +++ b/docs/errors/E0720.md @@ -0,0 +1,18 @@ +# E0720: function 'let' call may be confused for destructuring; remove parentheses to declare a variable + +In JavaScript, variables can be named `let` and interpreted as a function +call if it is followed by parentheses. This code calls function `let` +instead of destructuring an object: + +```javascript +const words = {first: "hello", second: "world"}; +let ({first} = words); +``` + +If you want to declare a variable, remove the parentheses: + +```javascript +const words = {first: "hello", second: "world"}; +let {first} = words; + +``` diff --git a/po/messages.pot b/po/messages.pot index 21777243d..6c591842f 100644 --- a/po/messages.pot +++ b/po/messages.pot @@ -2425,6 +2425,10 @@ msgstr "" msgid "namespace alias cannot use 'import type'" msgstr "" +#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +msgid "function 'let' call may be confused for destructuring; remove parentheses to declare a variable" +msgstr "" + #: test/test-diagnostic-formatter.cpp #: test/test-vim-qflist-json-diag-reporter.cpp msgid "something happened" diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp index ca09e1578..24f01f806 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp @@ -6961,6 +6961,20 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = { }, }, }, + + // Diag_Confusing_Let_Call + { + .code = 720, + .severity = Diagnostic_Severity::warning, + .message_formats = { + QLJS_TRANSLATABLE("function 'let' call may be confused for destructuring; remove parentheses to declare a variable"), + }, + .message_args = { + { + Diagnostic_Message_Arg_Info(offsetof(Diag_Confusing_Let_Call, let_function_call), Diagnostic_Arg_Type::source_code_span), + }, + }, + }, }; } diff --git a/src/quick-lint-js/diag/diagnostic-metadata-generated.h b/src/quick-lint-js/diag/diagnostic-metadata-generated.h index 5ae1000bd..d9eadc717 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.h +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.h @@ -475,10 +475,11 @@ namespace quick_lint_js { QLJS_DIAG_TYPE_NAME(Diag_Multiple_Export_Defaults) \ 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) \ /* END */ // clang-format on -inline constexpr int Diag_Type_Count = 464; +inline constexpr int Diag_Type_Count = 465; 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 index 90e462028..60ff0ceef 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -3618,6 +3618,16 @@ struct Diag_TypeScript_Namespace_Alias_Cannot_Use_Import_Type { ARG(type_keyword))]] // Source_Code_Span type_keyword; }; + +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))]] // + Source_Code_Span let_function_call; +}; + } QLJS_WARNING_POP diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index ad362b8ec..bc283f071 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -162,6 +162,21 @@ bool Parser::parse_and_visit_statement(Parse_Visitor_Base &v, Expression *ast = this->parse_expression(v, Precedence{.in_operator = true}); this->visit_expression(ast, v, Variable_Context::rhs); + + // let ({x} = y); // Confusing, if 'let' is a function + if (ast->kind() == Expression_Kind::Call) { + Expression::Call *call = expression_cast(ast); + if (call->child_count() == 2 && + call->child_0()->kind() == Expression_Kind::Variable && + expression_cast(call->child_0())->type_ == + Token_Type::kw_let && + call->child_1()->kind() == Expression_Kind::Assignment && + call->child_1()->child_0()->kind() == Expression_Kind::Object) { + this->diag_reporter_->report( + Diag_Confusing_Let_Call{.let_function_call = let_token.span()}); + } + } + this->parse_end_of_expression_statement(); } else { // Variable declaration. diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp index 4c1c98370..74c41a186 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.cpp +++ b/src/quick-lint-js/i18n/translation-table-generated.cpp @@ -318,7 +318,8 @@ const Translation_Table translation_data = { {61, 26, 69, 43, 41, 46}, // {53, 51, 0, 0, 0, 51}, // {0, 0, 0, 0, 0, 25}, // - {27, 25, 64, 54, 59, 9}, // + {0, 0, 0, 0, 0, 9}, // + {27, 25, 64, 54, 59, 96}, // {29, 17, 31, 33, 0, 27}, // {68, 28, 70, 45, 30, 55}, // {0, 0, 0, 24, 0, 23}, // @@ -2192,6 +2193,7 @@ const Translation_Table translation_data = { u8"forwarding exports are only allowed in export-from\0" u8"free {1} and {0} {1} {2}\0" u8"function\0" + u8"function 'let' call may be confused for destructuring; remove parentheses to declare a variable\0" u8"function call started here\0" u8"function called before declaration in block scope: {0}\0" u8"function declared here\0" diff --git a/src/quick-lint-js/i18n/translation-table-generated.h b/src/quick-lint-js/i18n/translation-table-generated.h index 4647accdc..8d74ec6f4 100644 --- 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 = 608; -constexpr std::size_t translation_table_string_table_size = 82591; +constexpr std::uint16_t translation_table_mapping_table_size = 609; +constexpr std::size_t translation_table_string_table_size = 82687; constexpr std::size_t translation_table_locale_table_size = 35; QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( @@ -333,6 +333,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "forwarding exports are only allowed in export-from"sv, "free {1} and {0} {1} {2}"sv, "function"sv, + "function 'let' call may be confused for destructuring; remove parentheses to declare a variable"sv, "function call started here"sv, "function called before declaration in block scope: {0}"sv, "function declared here"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 index 893ecf9f9..504c22755 100644 --- 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[607] = { +inline const Translated_String test_translation_table[608] = { { "\"global-groups\" entries must be strings"_translatable, u8"\"global-groups\" entries must be strings", @@ -3405,6 +3405,17 @@ inline const Translated_String test_translation_table[607] = { u8"function", }, }, + { + "function 'let' call may be confused for destructuring; remove parentheses to declare a variable"_translatable, + u8"function 'let' call may be confused for destructuring; remove parentheses to declare a variable", + { + u8"function 'let' call may be confused for destructuring; remove parentheses to declare a variable", + u8"function 'let' call may be confused for destructuring; remove parentheses to declare a variable", + u8"function 'let' call may be confused for destructuring; remove parentheses to declare a variable", + u8"function 'let' call may be confused for destructuring; remove parentheses to declare a variable", + u8"function 'let' call may be confused for destructuring; remove parentheses to declare a variable", + }, + }, { "function call started here"_translatable, u8"function call started here", diff --git a/test/test-parse-warning.cpp b/test/test-parse-warning.cpp index 217beee50..22d76141b 100644 --- a/test/test-parse-warning.cpp +++ b/test/test-parse-warning.cpp @@ -531,6 +531,13 @@ TEST_F(Test_Parse_Warning, early_exit_does_not_trigger_fallthrough_warning) { no_diags); } } + +TEST_F(Test_Parse_Warning, warn_on_confusing_let_use) { + test_parse_and_visit_statement(u8"let ({x} = y);"_sv, // + u8"^^^ Diag_Confusing_Let_Call"_diag); + test_parse_and_visit_statement(u8"let.prop({x} = y);"_sv, no_diags); + test_parse_and_visit_statement(u8"let (x = y);"_sv, no_diags); +} } }