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

fix: new diag for async/generator on getter/setter #1087

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
36 changes: 36 additions & 0 deletions docs/errors/E0713.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# E0713: getters and setters cannot be generators

Use of the '*' character, defining generator functions, is not allowed on getters or setters.
Getters and setters are synchronous operations and do not support the generator functionality.

```javascript
class C {
constructor() {
this._value = 0;
}

get *value() {
return this._value;
}
set *value(newValue) {
this._value = newValue;
}
}
```

To fix this error define a getter or setter, using regular function syntax.

```javascript
class C {
constructor() {
this._value = 0;
}

get value() {
return this._value;
}
set value(newValue) {
this._value = newValue;
}
}
```
61 changes: 61 additions & 0 deletions docs/errors/E0714.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# E0714: 'async' keyword is not allowed on getters or setters

Use of 'async' keyword, defining asynchronous functions, is not allowed on getters or setters.
Getters and setters are synchronous operations and do not support the asynchronous functionality.


```javascript
class C {
constructor() {
this._value = 0;
}

async get value() {
return this._value;
}

async set value(newValue) {
this._value = newValue;
}
}
```

To fix this error simply remove 'async' keyword from getters and setters,
so they can function properly as synchronous operations.


```javascript
class C {
constructor() {
this._value = 0;
}

get value() {
return this._value;
}

set value(newValue) {
this._value = newValue;
}
}
```

However, if you require asynchronous behavior within getters or setters,
you can achieve this by implementing separate asynchronous methods.


```javascript
class C {
constructor() {
this._value = 0;
}

async getValueAsync() {
return await fetch(this._value);
}

async setValueAsync(newValue) {
this._value = await fetch(newValue);
}
}
```
8 changes: 8 additions & 0 deletions po/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -2057,6 +2057,14 @@ msgstr ""
msgid "missing ',' between array elements"
msgstr ""

#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
msgid "getters and setters cannot be generators"
msgstr ""

#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
msgid "'async' keyword is not allowed on getters or setters"
msgstr ""

#: test/test-diagnostic-formatter.cpp
#: test/test-vim-qflist-json-diag-reporter.cpp
msgid "something happened"
Expand Down
36 changes: 36 additions & 0 deletions src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6325,6 +6325,42 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = {
},
},
},

// Diag_Class_Generator_On_Getter_Or_Setter
{
.code = 713,
.severity = Diagnostic_Severity::error,
.message_formats = {
QLJS_TRANSLATABLE("getters and setters cannot be generators"),
QLJS_TRANSLATABLE("'{0}' here"),
},
.message_args = {
{
Diagnostic_Message_Arg_Info(offsetof(Diag_Class_Generator_On_Getter_Or_Setter, star_token), Diagnostic_Arg_Type::source_code_span),
},
{
Diagnostic_Message_Arg_Info(offsetof(Diag_Class_Generator_On_Getter_Or_Setter, getter_setter_keyword), Diagnostic_Arg_Type::source_code_span),
},
},
},

// Diag_Class_Async_On_Getter_Or_Setter
{
.code = 714,
.severity = Diagnostic_Severity::error,
.message_formats = {
QLJS_TRANSLATABLE("'async' keyword is not allowed on getters or setters"),
QLJS_TRANSLATABLE("'{0}' here"),
},
.message_args = {
{
Diagnostic_Message_Arg_Info(offsetof(Diag_Class_Async_On_Getter_Or_Setter, async_keyword), Diagnostic_Arg_Type::source_code_span),
},
{
Diagnostic_Message_Arg_Info(offsetof(Diag_Class_Async_On_Getter_Or_Setter, getter_setter_keyword), Diagnostic_Arg_Type::source_code_span),
},
},
},
};
}

Expand Down
4 changes: 3 additions & 1 deletion src/quick-lint-js/diag/diagnostic-metadata-generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,12 @@ namespace quick_lint_js {
QLJS_DIAG_TYPE_NAME(Diag_Xor_Used_As_Exponentiation) \
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) \
QLJS_DIAG_TYPE_NAME(Diag_Class_Async_On_Getter_Or_Setter) \
/* END */
// clang-format on

inline constexpr int Diag_Type_Count = 423;
inline constexpr int Diag_Type_Count = 425;

extern const Diagnostic_Info all_diagnostic_infos[Diag_Type_Count];
}
Expand Down
18 changes: 18 additions & 0 deletions src/quick-lint-js/diag/diagnostic-types-2.h
Original file line number Diff line number Diff line change
Expand Up @@ -3261,6 +3261,24 @@ struct Diag_Missing_Comma_Between_Array_Elements {
ARG(expected_comma))]] //
Source_Code_Span expected_comma;
};

struct Diag_Class_Generator_On_Getter_Or_Setter {
[[qljs::diag("E0713", Diagnostic_Severity::error)]] //
[[qljs::message("getters and setters cannot be generators",
ARG(star_token))]] //
[[qljs::message("'{0}' here", ARG(getter_setter_keyword))]] //
Source_Code_Span star_token;
Source_Code_Span getter_setter_keyword;
};

struct Diag_Class_Async_On_Getter_Or_Setter {
[[qljs::diag("E0714", Diagnostic_Severity::error)]] //
[[qljs::message("'async' keyword is not allowed on getters or setters",
ARG(async_keyword))]] //
[[qljs::message("'{0}' here", ARG(getter_setter_keyword))]] //
Source_Code_Span async_keyword;
Source_Code_Span getter_setter_keyword;
};
}
QLJS_WARNING_POP

Expand Down
37 changes: 37 additions & 0 deletions src/quick-lint-js/fe/parse-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,8 @@ void Parser::parse_and_visit_class_or_interface_member(
void check_modifiers_for_method() {
error_if_accessor_method();
error_if_declare_method();
error_if_generator_getter_or_setter();
error_if_async_getter_or_setter();
error_if_readonly_method();
error_if_async_or_generator_without_method_body();
error_if_invalid_access_specifier();
Expand Down Expand Up @@ -1324,6 +1326,41 @@ void Parser::parse_and_visit_class_or_interface_member(
}
}

void error_if_generator_getter_or_setter() {
if (const Modifier *star_modifier = find_modifier(Token_Type::star)) {
if (const Modifier *get_modifier = find_modifier(Token_Type::kw_get)) {
p->diag_reporter_->report(Diag_Class_Generator_On_Getter_Or_Setter{
.star_token = star_modifier->span,
.getter_setter_keyword = get_modifier->span,
});
} else if (const Modifier *set_modifier =
find_modifier(Token_Type::kw_set)) {
p->diag_reporter_->report(Diag_Class_Generator_On_Getter_Or_Setter{
.star_token = star_modifier->span,
.getter_setter_keyword = set_modifier->span,
});
}
}
}

void error_if_async_getter_or_setter() {
if (const Modifier *async_modifier =
find_modifier(Token_Type::kw_async)) {
if (const Modifier *get_modifier = find_modifier(Token_Type::kw_get)) {
p->diag_reporter_->report(Diag_Class_Async_On_Getter_Or_Setter{
.async_keyword = async_modifier->span,
.getter_setter_keyword = get_modifier->span,
});
} else if (const Modifier *set_modifier =
find_modifier(Token_Type::kw_set)) {
p->diag_reporter_->report(Diag_Class_Async_On_Getter_Or_Setter{
.async_keyword = async_modifier->span,
.getter_setter_keyword = set_modifier->span,
});
}
}
}

void error_if_readonly_method() {
if (const Modifier *readonly_modifier =
find_modifier(Token_Type::kw_readonly)) {
Expand Down
6 changes: 5 additions & 1 deletion src/quick-lint-js/i18n/translation-table-generated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ const Translation_Table translation_data = {
{0, 0, 0, 100, 0, 89}, //
{0, 0, 0, 0, 0, 24}, //
{50, 77, 41, 27, 0, 60}, //
{70, 31, 69, 53, 0, 60}, //
{0, 0, 0, 0, 0, 60}, //
{70, 31, 69, 53, 0, 53}, //
{93, 15, 80, 68, 26, 69}, //
{0, 0, 0, 0, 0, 43}, //
{0, 0, 0, 0, 0, 44}, //
Expand Down Expand Up @@ -282,6 +283,7 @@ const Translation_Table translation_data = {
{84, 28, 66, 75, 25, 54}, //
{0, 0, 0, 70, 0, 52}, //
{0, 0, 0, 0, 0, 45}, //
{0, 0, 0, 0, 0, 41}, //
{70, 27, 0, 57, 0, 52}, //
{0, 0, 0, 5, 0, 5}, //
{5, 11, 66, 52, 54, 42}, //
Expand Down Expand Up @@ -1827,6 +1829,7 @@ const Translation_Table translation_data = {
u8"'as const' located here\0"
u8"'async export' is not allowed; write 'export async' instead\0"
u8"'async static' is not allowed; write 'static async' instead\0"
u8"'async' keyword is not allowed on getters or setters\0"
u8"'await' cannot be followed by an arrow function; use 'async' instead\0"
u8"'await' is only allowed in async functions\0"
u8"'declare class' cannot contain static block\0"
Expand Down Expand Up @@ -2068,6 +2071,7 @@ const Translation_Table translation_data = {
u8"generator function '*' belongs after keyword function\0"
u8"generator function '*' belongs before function name\0"
u8"generic arrow function needs ',' here in TSX\0"
u8"getters and setters cannot be generators\0"
u8"getters and setters cannot have overload signatures\0"
u8"here\0"
u8"here is the assignment assertion operator\0"
Expand Down
6 changes: 4 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 = 516;
constexpr std::size_t translation_table_string_table_size = 79639;
constexpr std::uint16_t translation_table_mapping_table_size = 518;
constexpr std::size_t translation_table_string_table_size = 79733;
constexpr std::size_t translation_table_locale_table_size = 35;

QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
Expand Down Expand Up @@ -55,6 +55,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
"'as const' located here"sv,
"'async export' is not allowed; write 'export async' instead"sv,
"'async static' is not allowed; write 'static async' instead"sv,
"'async' keyword is not allowed on getters or setters"sv,
"'await' cannot be followed by an arrow function; use 'async' instead"sv,
"'await' is only allowed in async functions"sv,
"'declare class' cannot contain static block"sv,
Expand Down Expand Up @@ -296,6 +297,7 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
"generator function '*' belongs after keyword function"sv,
"generator function '*' belongs before function name"sv,
"generic arrow function needs ',' here in TSX"sv,
"getters and setters cannot be generators"sv,
"getters and setters cannot have overload signatures"sv,
"here"sv,
"here is the assignment assertion operator"sv,
Expand Down
24 changes: 23 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[515] = {
inline const Translated_String test_translation_table[517] = {
{
"\"global-groups\" entries must be strings"_translatable,
u8"\"global-groups\" entries must be strings",
Expand Down Expand Up @@ -347,6 +347,17 @@ inline const Translated_String test_translation_table[515] = {
u8"'async static' is not allowed; write 'static async' instead",
},
},
{
"'async' keyword is not allowed on getters or setters"_translatable,
u8"'async' keyword is not allowed on getters or setters",
{
u8"'async' keyword is not allowed on getters or setters",
u8"'async' keyword is not allowed on getters or setters",
u8"'async' keyword is not allowed on getters or setters",
u8"'async' keyword is not allowed on getters or setters",
u8"'async' keyword is not allowed on getters or setters",
},
},
{
"'await' cannot be followed by an arrow function; use 'async' instead"_translatable,
u8"'await' cannot be followed by an arrow function; use 'async' instead",
Expand Down Expand Up @@ -2998,6 +3009,17 @@ inline const Translated_String test_translation_table[515] = {
u8"generic arrow function needs ',' here in TSX",
},
},
{
"getters and setters cannot be generators"_translatable,
u8"getters and setters cannot be generators",
{
u8"getters and setters cannot be generators",
u8"getters and setters cannot be generators",
u8"getters and setters cannot be generators",
u8"getters and setters cannot be generators",
u8"getters and setters cannot be generators",
},
},
{
"getters and setters cannot have overload signatures"_translatable,
u8"getters and setters cannot have overload signatures",
Expand Down
36 changes: 36 additions & 0 deletions test/test-parse-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,42 @@ TEST_F(Test_Parse_Class, accessor_cannot_be_method) {
javascript_options);
}

TEST_F(Test_Parse_Class, async_keyword_on_getters_and_setters) {
test_parse_and_visit_statement(
u8"class C { get async myMethod() { } }"_sv, //
u8" ^^^^^ Diag_Class_Async_On_Getter_Or_Setter.async_keyword\n"_diag
u8" ^^^ .getter_setter_keyword"_diag,
javascript_options);
test_parse_and_visit_statement(
u8"class C { set async myMethod() { } }"_sv, //
u8" ^^^^^ Diag_Class_Async_On_Getter_Or_Setter.async_keyword\n"_diag
u8" ^^^ .getter_setter_keyword"_diag,
javascript_options);
test_parse_and_visit_statement(
u8"class C { async get myMethod() { } }"_sv, //
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

u8" ^^^ Diag_Class_Async_On_Getter_Or_Setter.getter_setter_keyword\n"_diag
u8" ^^^^^ .async_keyword"_diag,
javascript_options);
test_parse_and_visit_statement(
u8"class C { async set myMethod() { } }"_sv, //
u8" ^^^ Diag_Class_Async_On_Getter_Or_Setter.getter_setter_keyword\n"_diag
u8" ^^^^^ .async_keyword"_diag,
javascript_options);
}

TEST_F(Test_Parse_Class, getters_and_setters_cannot_be_generator) {
test_parse_and_visit_statement(
u8"class C { get *myMethod() { } }"_sv, //
u8" ^ Diag_Class_Generator_On_Getter_Or_Setter.star_token\n"_diag
u8" ^^^ .getter_setter_keyword"_diag,
javascript_options);
test_parse_and_visit_statement(
u8"class C { set *myMethod() { } }"_sv, //
u8" ^ Diag_Class_Generator_On_Getter_Or_Setter.star_token\n"_diag
u8" ^^^ .getter_setter_keyword"_diag,
javascript_options);
}

TEST_F(Test_Parse_Class, class_methods_should_not_use_function_keyword) {
{
Spy_Visitor p = test_parse_and_visit_statement(
Expand Down
Loading