Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(fe): add warning for 'let({x} = y);' #1211

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -2413,6 +2413,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 @@ -6912,6 +6912,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 @@ -472,10 +472,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 = 461;
inline constexpr int Diag_Type_Count = 462;

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 @@ -3589,6 +3589,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 &&
msharipov marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -317,7 +317,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 @@ -2188,6 +2189,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 = 605;
constexpr std::size_t translation_table_string_table_size = 82408;
constexpr std::uint16_t translation_table_mapping_table_size = 606;
constexpr std::size_t translation_table_string_table_size = 82504;
constexpr std::size_t translation_table_locale_table_size = 35;

QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
Expand Down Expand Up @@ -332,6 +332,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[604] = {
inline const Translated_String test_translation_table[605] = {
{
"\"global-groups\" entries must be strings"_translatable,
u8"\"global-groups\" entries must be strings",
Expand Down Expand Up @@ -3394,6 +3394,17 @@ inline const Translated_String test_translation_table[604] = {
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 @@ -507,6 +507,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
Loading