From 77965450f24da5e027579db689be59f483c8da42 Mon Sep 17 00:00:00 2001 From: "Matthew \"strager\" Glazar" Date: Thu, 25 Jan 2024 01:40:37 -0500 Subject: [PATCH 1/2] fix(fe): parse decorators on abstract classes --- docs/CHANGELOG.md | 2 ++ src/quick-lint-js/fe/parse-class.cpp | 14 ++++++++++++++ src/quick-lint-js/fe/parse-statement.cpp | 1 + test/test-parse-decorator.cpp | 11 +++++++++++ 4 files changed, 28 insertions(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index df78615f44..edd93b8219 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -13,6 +13,7 @@ Semantic Versioning. * VS Code: You can now make quick-lint-js messages fun and insulting with the `quick-lint-js.snarky` setting (disabled by default). (Implemented by [vegerot][].) +* TypeScript: Decorators on abstract classes are now parsed. ([#1194][]) ### Fixed @@ -1415,6 +1416,7 @@ Beta release. [#1168]: https://github.com/quick-lint/quick-lint-js/pull/1168 [#1171]: https://github.com/quick-lint/quick-lint-js/issues/1171 [#1180]: https://github.com/quick-lint/quick-lint-js/issues/1180 +[#1194]: https://github.com/quick-lint/quick-lint-js/issues/1194 [E0001]: https://quick-lint-js.com/errors/E0001/ [E0003]: https://quick-lint-js.com/errors/E0003/ diff --git a/src/quick-lint-js/fe/parse-class.cpp b/src/quick-lint-js/fe/parse-class.cpp index b8251db7ea..18e676dd82 100644 --- a/src/quick-lint-js/fe/parse-class.cpp +++ b/src/quick-lint-js/fe/parse-class.cpp @@ -25,6 +25,20 @@ namespace quick_lint_js { void Parser::parse_and_visit_class(Parse_Visitor_Base &v, Parse_Class_Options options) { + if (this->peek().type == Token_Type::kw_abstract) { + if (options.abstract_keyword_span.has_value()) { + // abstract abstract class??? + QLJS_PARSER_UNIMPLEMENTED(); + } + options.abstract_keyword_span = this->peek().span(); + this->skip(); + if (this->peek().has_leading_newline) { + this->diag_reporter_->report( + Diag_Newline_Not_Allowed_After_Abstract_Keyword{ + .abstract_keyword = *options.abstract_keyword_span, + }); + } + } QLJS_ASSERT(this->peek().type == Token_Type::kw_class); Source_Code_Span class_keyword_span = this->peek().span(); diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index df3c959349..740a82f04b 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2999,6 +2999,7 @@ void Parser::parse_and_visit_decorator_statement(Parse_Visitor_Base &v) { this->parse_and_visit_one_or_more_decorators(decorator_visits.visitor()); switch (this->peek().type) { + case Token_Type::kw_abstract: case Token_Type::kw_class: this->parse_and_visit_class( v, Parse_Class_Options{ diff --git a/test/test-parse-decorator.cpp b/test/test-parse-decorator.cpp index 543cfc4b4c..48050329aa 100644 --- a/test/test-parse-decorator.cpp +++ b/test/test-parse-decorator.cpp @@ -456,6 +456,17 @@ TEST_F(Test_Parse_Decorator, typescript_options); } +TEST_F(Test_Parse_Decorator, decorator_on_typescript_abstract_class) { + test_parse_and_visit_module( + u8"@decorator abstract class C { abstract m(); }"_sv, no_diags, + typescript_options); + + test_parse_and_visit_module( + u8"@decorator abstract\nclass C { abstract m(); }"_sv, // + u8" ^^^^^^^^ Diag_Newline_Not_Allowed_After_Abstract_Keyword.abstract_keyword"_diag, + typescript_options); +} + TEST_F(Test_Parse_Decorator, typescript_parameter_decorator) { { Spy_Visitor p = test_parse_and_visit_statement( From 434d08514d2ab0bc70b5ecb40f1e1e7c1256b6ae Mon Sep 17 00:00:00 2001 From: "Matthew \"strager\" Glazar" Date: Thu, 25 Jan 2024 01:49:12 -0500 Subject: [PATCH 2/2] fix(fe): don't diagnose x?.y.z quick-lint-js reports a diagnostic for x?.y.z. The diagnostic is incorrect. Even the JavaScript example in the documentation (written by me) does not do what the documentation claims it does. x?.y.z does not throw an error if x is null or undefined. Fix quick-lint-js's false positives by reverting commit bdfccbdc8c405c5f0d9d40ce8c6736d9c31a6a4a. Leave a tombstone so that the diagnostic's code is not reused. --- docs/errors/E0718.md | 23 +-------- po/messages.pot | 4 -- .../diag/diagnostic-metadata-generated.cpp | 15 ------ .../diag/diagnostic-metadata-generated.h | 3 +- src/quick-lint-js/diag/diagnostic-types-2.h | 12 +---- src/quick-lint-js/fe/parse-expression.cpp | 2 - src/quick-lint-js/fe/parse.cpp | 51 ------------------- src/quick-lint-js/fe/parse.h | 1 - .../i18n/translation-table-generated.cpp | 2 - .../i18n/translation-table-generated.h | 5 +- .../i18n/translation-table-test-generated.h | 13 +---- test/test-parse-warning.cpp | 25 --------- 12 files changed, 7 insertions(+), 149 deletions(-) diff --git a/docs/errors/E0718.md b/docs/errors/E0718.md index 79a565ece6..392ff36a6a 100644 --- a/docs/errors/E0718.md +++ b/docs/errors/E0718.md @@ -1,24 +1,5 @@ # E0718: using a '.' after a '?.' might fail, since '?.' might return 'undefined' -```config-for-examples -{ - "globals": { - } -} -``` +This diagnostic has been removed in quick-lint-js version 3.2.0. -In JavaScript, `x?.y` ignores `y` and returns `undefined` if `x` is `null` or `undefined`. If `?.` returns `undefined`, then using `.` after will throw an error: - -```js -let bug = { milestone: null }; -console.log(bug.milestone); // null -console.log(bug.milestone?.name); // undefined -console.log(bug.milestone?.name.trim()); // throws an error -``` - -Replacing the `.` with `?.` will prevent the errors from being thrown at runtime: - -```js -let bug = { milestone: null }; -console.log(bug.milestone?.name?.trim()); // undefined -``` + diff --git a/po/messages.pot b/po/messages.pot index cfca0327e2..94c27e0037 100644 --- a/po/messages.pot +++ b/po/messages.pot @@ -2413,10 +2413,6 @@ msgstr "" msgid "namespace alias cannot use 'import type'" msgstr "" -#: src/quick-lint-js/diag/diagnostic-metadata-generated.cpp -msgid "using a '.' after a '?.' might fail, since '?.' might return 'undefined'." -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 f29b70bfbd..4abe4aea0d 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp @@ -6912,21 +6912,6 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = { }, }, }, - - // Diag_Using_Dot_After_Optional_Chaining - { - .code = 718, - .severity = Diagnostic_Severity::warning, - .message_formats = { - QLJS_TRANSLATABLE("using a '.' after a '?.' might fail, since '?.' might return 'undefined'."), - }, - .message_args = { - { - Diagnostic_Message_Arg_Info(offsetof(Diag_Using_Dot_After_Optional_Chaining, dot_op), Diagnostic_Arg_Type::source_code_span), - Diagnostic_Message_Arg_Info(offsetof(Diag_Using_Dot_After_Optional_Chaining, optional_chain_op), 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 bbd9239b0e..b8569eef2c 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.h +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.h @@ -472,11 +472,10 @@ 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_Using_Dot_After_Optional_Chaining) \ /* END */ // clang-format on -inline constexpr int Diag_Type_Count = 462; +inline constexpr int Diag_Type_Count = 461; 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 b858eae431..850dca9f0d 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -31,6 +31,7 @@ QLJS_RESERVED_DIAG("E0271") QLJS_RESERVED_DIAG("E0279") QLJS_RESERVED_DIAG("E0391") QLJS_RESERVED_DIAG("E0707") +QLJS_RESERVED_DIAG("E0718") struct Diag_Abstract_Field_Cannot_Have_Initializer { [[qljs::diag("E0295", Diagnostic_Severity::error)]] // @@ -3588,17 +3589,6 @@ struct Diag_TypeScript_Namespace_Alias_Cannot_Use_Import_Type { ARG(type_keyword))]] // Source_Code_Span type_keyword; }; - -struct Diag_Using_Dot_After_Optional_Chaining { - [[qljs::diag("E0718", Diagnostic_Severity::warning)]] // - // clang-format off - [[qljs::message("using a '.' after a '?.' might fail, since '?.' might " - "return 'undefined'.", - ARG(dot_op), ARG(optional_chain_op))]] // - // clang-format on - Source_Code_Span dot_op; - Source_Code_Span optional_chain_op; -}; } QLJS_WARNING_POP diff --git a/src/quick-lint-js/fe/parse-expression.cpp b/src/quick-lint-js/fe/parse-expression.cpp index b2f5de8aab..da00833c2b 100644 --- a/src/quick-lint-js/fe/parse-expression.cpp +++ b/src/quick-lint-js/fe/parse-expression.cpp @@ -172,8 +172,6 @@ void Parser::visit_expression(Expression* ast, Parse_Visitor_Base& v, break; case Expression_Kind::Dot: this->visit_expression(ast->child_0(), v, Variable_Context::rhs); - this->warn_on_dot_operator_after_optional_chain( - expression_cast(ast)); break; case Expression_Kind::Index: this->visit_expression(ast->child_0(), v, Variable_Context::rhs); diff --git a/src/quick-lint-js/fe/parse.cpp b/src/quick-lint-js/fe/parse.cpp index d7844d11e9..f8ab440d94 100644 --- a/src/quick-lint-js/fe/parse.cpp +++ b/src/quick-lint-js/fe/parse.cpp @@ -494,57 +494,6 @@ void Parser::error_on_invalid_as_const(Expression* ast, } } -void Parser::warn_on_dot_operator_after_optional_chain(Expression::Dot* ast) { - Expression* lhs = ast->child_; - Source_Code_Span operator_span = ast->operator_span_; - auto is_optional_chain = [](String8_View s) -> bool { return s[0] == u8'?'; }; - - // we know the current node is a dot operator. - // If it is a '.' and its parent is a '?.' or a '?.(' or a '?.[' - // then we can report a warning - if (!is_optional_chain(operator_span.string_view())) { - switch (lhs->kind()) { - case Expression_Kind::Dot: { - auto lhs_dot = expression_cast(lhs); - Source_Code_Span lhs_operator_span = lhs_dot->operator_span_; - if (is_optional_chain(lhs_operator_span.string_view())) { - this->diag_reporter_->report(Diag_Using_Dot_After_Optional_Chaining{ - .dot_op = operator_span, - .optional_chain_op = lhs_operator_span, - }); - } - break; - } - case Expression_Kind::Call: { - auto lhs_call = expression_cast(lhs); - std::optional lhs_operator_span = - lhs_call->optional_chaining_operator_span(); - if (lhs_operator_span.has_value()) { - this->diag_reporter_->report(Diag_Using_Dot_After_Optional_Chaining{ - .dot_op = operator_span, - .optional_chain_op = *lhs_operator_span, - }); - } - break; - } - case Expression_Kind::Index: { - auto lhs_index = expression_cast(lhs); - std::optional lhs_operator_span = - lhs_index->optional_chaining_operator_span(); - if (lhs_operator_span.has_value()) { - this->diag_reporter_->report(Diag_Using_Dot_After_Optional_Chaining{ - .dot_op = operator_span, - .optional_chain_op = *lhs_operator_span, - }); - } - break; - } - default: - break; - } - } -} - void Parser::warn_on_xor_operator_as_exponentiation( Expression::Binary_Operator* ast) { auto is_xor_operator = [](String8_View s) -> bool { return s == u8"^"_sv; }; diff --git a/src/quick-lint-js/fe/parse.h b/src/quick-lint-js/fe/parse.h index 28bf98cca1..fdc157233d 100644 --- a/src/quick-lint-js/fe/parse.h +++ b/src/quick-lint-js/fe/parse.h @@ -607,7 +607,6 @@ class Parser { void warn_on_comma_operator_in_conditional_statement(Expression *); void warn_on_comma_operator_in_index(Expression *, Source_Code_Span); void warn_on_xor_operator_as_exponentiation(Expression::Binary_Operator *); - void warn_on_dot_operator_after_optional_chain(Expression::Dot *); void warn_on_unintuitive_bitshift_precedence(Expression *ast); void error_on_pointless_string_compare(Expression::Binary_Operator *); void error_on_pointless_compare_against_literal( diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp index 5c8c06916d..e224f00985 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.cpp +++ b/src/quick-lint-js/i18n/translation-table-generated.cpp @@ -594,7 +594,6 @@ const Translation_Table translation_data = { {0, 0, 0, 47, 0, 60}, // {0, 0, 0, 55, 0, 59}, // {0, 0, 0, 0, 0, 59}, // - {0, 0, 0, 0, 0, 74}, // {57, 29, 48, 46, 41, 9}, // {37, 26, 31, 33, 35, 31}, // {0, 0, 0, 0, 0, 41}, // @@ -2465,7 +2464,6 @@ const Translation_Table translation_data = { u8"using '{0}' against an array literal does not compare items\0" u8"using '{0}' against an arrow function always returns '{1}'\0" u8"using '{0}' against an object literal always returns '{1}'\0" - u8"using a '.' after a '?.' might fail, since '?.' might return 'undefined'.\0" u8"variable\0" u8"variable already declared here\0" u8"variable assigned before its declaration\0" diff --git a/src/quick-lint-js/i18n/translation-table-generated.h b/src/quick-lint-js/i18n/translation-table-generated.h index 02355e8fb5..881b73ee6b 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 = 606; -constexpr std::size_t translation_table_string_table_size = 82482; +constexpr std::uint16_t translation_table_mapping_table_size = 605; +constexpr std::size_t translation_table_string_table_size = 82408; constexpr std::size_t translation_table_locale_table_size = 35; QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( @@ -608,7 +608,6 @@ QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( "using '{0}' against an array literal does not compare items"sv, "using '{0}' against an arrow function always returns '{1}'"sv, "using '{0}' against an object literal always returns '{1}'"sv, - "using a '.' after a '?.' might fail, since '?.' might return 'undefined'."sv, "variable"sv, "variable already declared here"sv, "variable assigned before its declaration"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 db9a0443c5..c56c46e69a 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[605] = { +inline const Translated_String test_translation_table[604] = { { "\"global-groups\" entries must be strings"_translatable, u8"\"global-groups\" entries must be strings", @@ -6430,17 +6430,6 @@ inline const Translated_String test_translation_table[605] = { u8"using '{0}' against an object literal always returns '{1}'", }, }, - { - "using a '.' after a '?.' might fail, since '?.' might return 'undefined'."_translatable, - u8"using a '.' after a '?.' might fail, since '?.' might return 'undefined'.", - { - u8"using a '.' after a '?.' might fail, since '?.' might return 'undefined'.", - u8"using a '.' after a '?.' might fail, since '?.' might return 'undefined'.", - u8"using a '.' after a '?.' might fail, since '?.' might return 'undefined'.", - u8"using a '.' after a '?.' might fail, since '?.' might return 'undefined'.", - u8"using a '.' after a '?.' might fail, since '?.' might return 'undefined'.", - }, - }, { "variable"_translatable, u8"variable", diff --git a/test/test-parse-warning.cpp b/test/test-parse-warning.cpp index 28452dbf2a..932072ccb3 100644 --- a/test/test-parse-warning.cpp +++ b/test/test-parse-warning.cpp @@ -507,31 +507,6 @@ TEST_F(Test_Parse_Warning, early_exit_does_not_trigger_fallthrough_warning) { no_diags); } } - -TEST_F(Test_Parse_Warning, Diag_Using_Dot_After_Optional_Chaining) { - test_parse_and_visit_expression( - u8"a?.b.c"_sv, - u8" ^ Diag_Using_Dot_After_Optional_Chaining.dot_op\n"_diag - u8" ^^ .optional_chain_op"_diag); - test_parse_and_visit_expression(u8"a?.b?.c"_sv, no_diags); - test_parse_and_visit_expression( - u8"a?.b?.a?.c.d"_sv, - u8" ^ Diag_Using_Dot_After_Optional_Chaining.dot_op\n"_diag - u8" ^^ .optional_chain_op"_diag); - test_parse_and_visit_expression(u8"a?.b?.a?.c"_sv, no_diags); - - test_parse_and_visit_expression(u8"_a?.(a)?.(_a)?.(a)"_sv, no_diags); - test_parse_and_visit_expression( - u8"_a?.(a).b"_sv, - u8" ^ Diag_Using_Dot_After_Optional_Chaining.dot_op\n"_diag - u8" ^^ .optional_chain_op"_diag); - - test_parse_and_visit_expression(u8"g?.[1]"_sv, no_diags); - test_parse_and_visit_expression( - u8"g?.[1]?.[2].b"_sv, - u8" ^ Diag_Using_Dot_After_Optional_Chaining.dot_op\n"_diag - u8" ^^ .optional_chain_op"_diag); -} } }