Skip to content

Commit

Permalink
feat(fe): add warning for 'let({x} = y);'
Browse files Browse the repository at this point in the history
Added a new diagnostic E0720 that closes #1203. Issues a warning when a
function let() is called on an object assignment expression.

#1203
  • Loading branch information
msharipov authored and strager committed Mar 16, 2024
1 parent d09a1a7 commit c8b3258
Show file tree
Hide file tree
Showing 10 changed files with 88 additions and 5 deletions.
18 changes: 18 additions & 0 deletions docs/errors/E0720.md
Original file line number Diff line number Diff line change
@@ -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;

```
4 changes: 4 additions & 0 deletions po/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 14 additions & 0 deletions src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
},
};
}

Expand Down
3 changes: 2 additions & 1 deletion src/quick-lint-js/diag/diagnostic-metadata-generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
10 changes: 10 additions & 0 deletions src/quick-lint-js/diag/diagnostic-types-2.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions src/quick-lint-js/fe/parse-statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Expression::Call *>(ast);
if (call->child_count() == 2 &&
call->child_0()->kind() == Expression_Kind::Variable &&
expression_cast<Expression::Variable *>(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.
Expand Down
4 changes: 3 additions & 1 deletion src/quick-lint-js/i18n/translation-table-generated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}, //
Expand Down Expand Up @@ -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"
Expand Down
5 changes: 3 additions & 2 deletions src/quick-lint-js/i18n/translation-table-generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand 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,
Expand Down
13 changes: 12 additions & 1 deletion src/quick-lint-js/i18n/translation-table-test-generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions test/test-parse-warning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down

0 comments on commit c8b3258

Please sign in to comment.