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 1 commit
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
39 changes: 39 additions & 0 deletions docs/errors/E0720.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# E0720: calling `let` on an assignment is ambiguous
msharipov marked this conversation as resolved.
Show resolved Hide resolved

If `let` is declared as a function, then this code:

```javascript
function let(arg) {
console.log(arg);
}

const y = {x: "hello", z: "world"};
let ({x} = y);
```

is actually equivalent to:

```javascript
function let(arg) {
console.log(arg);
}

const y = {x: "hello", z: "world"};
let(y);
```

instead of destructuring `y`.
msharipov marked this conversation as resolved.
Show resolved Hide resolved

To remove the ambiguity, separate the declaration and the assignment into two
statements:
msharipov marked this conversation as resolved.
Show resolved Hide resolved

```javascript
function let(arg) {
console.log(arg);
}

const y = {x: "hello", z: "world"};
let x;
({x} = y);

```
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 "ambiguous use of the keyword 'let'; let is a function in this scope"
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_Ambiguous_Let_Call
{
.code = 720,
.severity = Diagnostic_Severity::warning,
.message_formats = {
QLJS_TRANSLATABLE("ambiguous use of the keyword 'let'; let is a function in this scope"),
},
.message_args = {
{
Diagnostic_Message_Arg_Info(offsetof(Diag_Ambiguous_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_Ambiguous_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_Ambiguous_Let_Call {
[[qljs::diag("E0720", Diagnostic_Severity::warning)]] //
[
[qljs::message("ambiguous use of the keyword 'let'; "
"let is a function in this scope",
msharipov marked this conversation as resolved.
Show resolved Hide resolved
ARG(let_function_call))]] //
Source_Code_Span let_function_call;
};

}
QLJS_WARNING_POP

Expand Down
12 changes: 12 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,18 @@ 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); // Ambiguous 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_1()->kind() == Expression_Kind::Assignment &&
call->child_1()->child_0()->kind() == Expression_Kind::Object) {
this->diag_reporter_->report(
Diag_Ambiguous_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 @@ -196,7 +196,8 @@ const Translation_Table translation_data = {
{0, 0, 0, 58, 0, 50}, //
{0, 0, 0, 0, 0, 57}, //
{0, 0, 0, 0, 0, 37}, //
{14, 14, 0, 64, 0, 29}, //
{0, 0, 0, 0, 0, 29}, //
{14, 14, 0, 64, 0, 68}, //
{0, 0, 0, 0, 0, 18}, //
{0, 0, 0, 0, 0, 8}, //
{0, 0, 0, 0, 0, 16}, //
Expand Down Expand Up @@ -2067,6 +2068,7 @@ const Translation_Table translation_data = {
u8"abstract properties are only allowed in abstract classes\0"
u8"abstract properties cannot be static\0"
u8"accessors cannot be optional\0"
u8"ambiguous use of the keyword 'let'; let is a function in this scope\0"
u8"an 'if' statement\0"
u8"an enum\0"
u8"an import alias\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 = 82476;
constexpr std::size_t translation_table_locale_table_size = 35;

QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
Expand Down Expand Up @@ -211,6 +211,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
"abstract properties are only allowed in abstract classes"sv,
"abstract properties cannot be static"sv,
"accessors cannot be optional"sv,
"ambiguous use of the keyword 'let'; let is a function in this scope"sv,
"an 'if' statement"sv,
"an enum"sv,
"an import alias"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 @@ -2063,6 +2063,17 @@ inline const Translated_String test_translation_table[604] = {
u8"accessors cannot be optional",
},
},
{
"ambiguous use of the keyword 'let'; let is a function in this scope"_translatable,
u8"ambiguous use of the keyword 'let'; let is a function in this scope",
{
u8"ambiguous use of the keyword 'let'; let is a function in this scope",
u8"ambiguous use of the keyword 'let'; let is a function in this scope",
u8"ambiguous use of the keyword 'let'; let is a function in this scope",
u8"ambiguous use of the keyword 'let'; let is a function in this scope",
u8"ambiguous use of the keyword 'let'; let is a function in this scope",
},
},
{
"an 'if' statement"_translatable,
u8"an 'if' statement",
Expand Down
5 changes: 5 additions & 0 deletions test/test-parse-warning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,11 @@ TEST_F(Test_Parse_Warning, early_exit_does_not_trigger_fallthrough_warning) {
no_diags);
}
}

TEST_F(Test_Parse_Warning, warn_on_ambiguous_let_use) {
test_parse_and_visit_statement(u8"let ({x} = y);"_sv, //
u8"^^^ Diag_Ambiguous_Let_Call"_diag);
}
}
}

Expand Down
Loading