diff --git a/docs/errors/E0717.md b/docs/errors/E0717.md new file mode 100644 index 0000000000..48134551dd --- /dev/null +++ b/docs/errors/E0717.md @@ -0,0 +1,24 @@ +# E0717: using a '.' after a '?.' might fail, since '?.' might return 'undefined' + +```config-for-examples +{ + "globals": { + } +} +``` + +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 a3f6633a21..72ff207b57 100644 --- a/po/messages.pot +++ b/po/messages.pot @@ -2401,6 +2401,10 @@ msgstr "" msgid "'&' here" 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 15f87ab6aa..45111572eb 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.cpp @@ -6870,6 +6870,21 @@ const QLJS_CONSTINIT Diagnostic_Info all_diagnostic_infos[] = { }, }, }, + + // Diag_Using_Dot_After_Optional_Chaining + { + .code = 717, + .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 12d28d2fdd..f7cbb4a5b3 100644 --- a/src/quick-lint-js/diag/diagnostic-metadata-generated.h +++ b/src/quick-lint-js/diag/diagnostic-metadata-generated.h @@ -469,10 +469,11 @@ namespace quick_lint_js { QLJS_DIAG_TYPE_NAME(Diag_Class_Async_On_Getter_Or_Setter) \ QLJS_DIAG_TYPE_NAME(Diag_Multiple_Export_Defaults) \ QLJS_DIAG_TYPE_NAME(Diag_Unintuitive_Bitshift_Precedence) \ + QLJS_DIAG_TYPE_NAME(Diag_Using_Dot_After_Optional_Chaining) \ /* END */ // clang-format on -inline constexpr int Diag_Type_Count = 458; +inline constexpr int Diag_Type_Count = 459; 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 4397a0dd66..38766cb135 100644 --- a/src/quick-lint-js/diag/diagnostic-types-2.h +++ b/src/quick-lint-js/diag/diagnostic-types-2.h @@ -3567,6 +3567,15 @@ struct Diag_Unintuitive_Bitshift_Precedence { Source_Code_Span bitshift_operator; Source_Code_Span and_operator; }; + +struct Diag_Using_Dot_After_Optional_Chaining { + [[qljs::diag("E0717", Diagnostic_Severity::warning)]] + [[qljs::message("using a '.' after a '?.' might fail, " + "since '?.' might return 'undefined'.", + ARG(dot_op), ARG(optional_chain_op))]] + Source_Code_Span dot_op; + Source_Code_Span optional_chain_op; +}; } QLJS_WARNING_POP diff --git a/src/quick-lint-js/fe/expression.h b/src/quick-lint-js/fe/expression.h index cefb018e09..def75ff949 100644 --- a/src/quick-lint-js/fe/expression.h +++ b/src/quick-lint-js/fe/expression.h @@ -612,11 +612,13 @@ class Expression::Call final : public Expression { static constexpr Expression_Kind kind = Expression_Kind::Call; explicit Call(Expression_Arena::Array_Ptr children, - Source_Code_Span left_paren_span, const Char8 *span_end) + Source_Code_Span left_paren_span, const Char8 *span_end, + std::optional optional_chaining_op_) : Expression(kind), call_left_paren_begin_(left_paren_span.begin()), span_end_(span_end), - children_(children) { + children_(children), + optional_chaining_operator_(optional_chaining_op_) { QLJS_ASSERT(left_paren_span.size() == 1); } @@ -628,6 +630,7 @@ class Expression::Call final : public Expression { const Char8 *call_left_paren_begin_; const Char8 *span_end_; Expression_Arena::Array_Ptr children_; + std::optional optional_chaining_operator_; }; static_assert(Expression_Arena::is_allocatable); @@ -647,11 +650,12 @@ class Expression::Dot final : public Expression { public: static constexpr Expression_Kind kind = Expression_Kind::Dot; - explicit Dot(Expression *lhs, Identifier rhs) - : Expression(kind), variable_identifier_(rhs), child_(lhs) {} + explicit Dot(Expression *lhs, Identifier rhs, Source_Code_Span op_span) + : Expression(kind), variable_identifier_(rhs), child_(lhs), operator_span_(op_span) {} Identifier variable_identifier_; Expression *child_; + Source_Code_Span operator_span_; }; static_assert(Expression_Arena::is_allocatable); @@ -682,15 +686,18 @@ class Expression::Index final : public Expression { static constexpr Expression_Kind kind = Expression_Kind::Index; explicit Index(Expression *container, Expression *subscript, - Source_Code_Span left_square_span, const Char8 *subscript_end) + Source_Code_Span left_square_span, const Char8 *subscript_end, + std::optional optional_chaining_op_) : Expression(kind), index_subscript_end_(subscript_end), left_square_span(left_square_span), - children_{container, subscript} {} + children_{container, subscript}, + optional_chaining_operator_(optional_chaining_op_) {} const Char8 *index_subscript_end_; Source_Code_Span left_square_span; std::array children_; + std::optional optional_chaining_operator_; }; static_assert(Expression_Arena::is_allocatable); diff --git a/src/quick-lint-js/fe/parse-expression.cpp b/src/quick-lint-js/fe/parse-expression.cpp index eb3f9f270f..4805612c86 100644 --- a/src/quick-lint-js/fe/parse-expression.cpp +++ b/src/quick-lint-js/fe/parse-expression.cpp @@ -172,6 +172,8 @@ 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); @@ -964,7 +966,8 @@ Expression* Parser::parse_async_expression_only( Expression* call_ast = this->make_expression( this->expressions_.make_array(std::move(call_children)), /*left_paren_span=*/left_paren_span, - /*span_end=*/right_paren_span.end()); + /*span_end=*/right_paren_span.end(), + /*optional_chaining_op=*/std::nullopt); return call_ast; } else if (is_await) { // await is an operator. @@ -1650,7 +1653,7 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, .func_start = func_start_span}); } Expression* callee = binary_builder.last_expression(); - Expression::Call* call = this->parse_call_expression_remainder(v, callee); + Expression::Call* call = this->parse_call_expression_remainder(v, callee, std::nullopt); binary_builder.replace_last(call); if (this->peek().type == Token_Type::equal_greater) { @@ -1748,7 +1751,7 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, }); } binary_builder.replace_last(this->make_expression( - binary_builder.last_expression(), this->peek().identifier_name())); + binary_builder.last_expression(), this->peek().identifier_name(), dot_span)); this->skip(); goto next; @@ -1779,7 +1782,7 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, case Token_Type::semicolon: { Source_Code_Span empty_property_name(dot_span.end(), dot_span.end()); binary_builder.replace_last(this->make_expression( - binary_builder.last_expression(), Identifier(empty_property_name))); + binary_builder.last_expression(), Identifier(empty_property_name), dot_span)); this->diag_reporter_->report(Diag_Missing_Property_Name_For_Dot_Operator{ .dot = dot_span, }); @@ -1813,6 +1816,7 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, // array?.[index] // f?.(x, y) case Token_Type::question_dot: { + Source_Code_Span question_dot_span = this->peek().span(); this->skip(); switch (this->peek().type) { // x?.y @@ -1821,7 +1825,7 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, case Token_Type::reserved_keyword_with_escape_sequence: QLJS_CASE_KEYWORD: binary_builder.replace_last(this->make_expression( - binary_builder.last_expression(), this->peek().identifier_name())); + binary_builder.last_expression(), this->peek().identifier_name(), question_dot_span)); this->skip(); goto next; @@ -1836,7 +1840,8 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, // f?.(x, y) case Token_Type::left_paren: binary_builder.replace_last(this->parse_call_expression_remainder( - v, binary_builder.last_expression())); + v, binary_builder.last_expression(), + std::optional(question_dot_span))); goto next; // f?.(x, y) // TypeScript only @@ -1852,13 +1857,13 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, } this->parse_and_visit_typescript_generic_arguments(v, /*in_jsx=*/false); binary_builder.replace_last(this->parse_call_expression_remainder( - v, binary_builder.last_expression())); + v, binary_builder.last_expression(), std::nullopt)); goto next; // array?.[index] case Token_Type::left_square: binary_builder.replace_last(this->parse_index_expression_remainder( - v, binary_builder.last_expression())); + v, binary_builder.last_expression(), std::optional(question_dot_span))); goto next; default: @@ -1871,7 +1876,7 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v, // o[key] // Indexing expression. case Token_Type::left_square: { binary_builder.replace_last(this->parse_index_expression_remainder( - v, binary_builder.last_expression())); + v, binary_builder.last_expression(), std::nullopt)); goto next; } @@ -2622,7 +2627,8 @@ Expression* Parser::parse_arrow_function_expression_remainder( } Expression::Call* Parser::parse_call_expression_remainder(Parse_Visitor_Base& v, - Expression* callee) { + Expression* callee, + std::optional optional_chaining_op) { Source_Code_Span left_paren_span = this->peek().span(); Expression_Arena::Vector call_children( "parse_expression_remainder call children", @@ -2663,11 +2669,13 @@ Expression::Call* Parser::parse_call_expression_remainder(Parse_Visitor_Base& v, this->make_expression( this->expressions_.make_array(std::move(call_children)), /*left_paren_span=*/left_paren_span, - /*span_end=*/call_span_end)); + /*span_end=*/call_span_end, + /*optional_chaining_op=*/optional_chaining_op)); } Expression* Parser::parse_index_expression_remainder(Parse_Visitor_Base& v, - Expression* lhs) { + Expression* lhs, + std::optional optional_chaining_op) { QLJS_ASSERT(this->peek().type == Token_Type::left_square); Source_Code_Span left_square_span = this->peek().span(); this->skip(); @@ -2695,7 +2703,8 @@ Expression* Parser::parse_index_expression_remainder(Parse_Visitor_Base& v, break; } return this->make_expression(lhs, subscript, - left_square_span, end); + left_square_span, end, + optional_chaining_op); } Expression_Arena::Vector diff --git a/src/quick-lint-js/fe/parse-statement.cpp b/src/quick-lint-js/fe/parse-statement.cpp index 03c7852b92..f2b48b8a4f 100644 --- a/src/quick-lint-js/fe/parse-statement.cpp +++ b/src/quick-lint-js/fe/parse-statement.cpp @@ -2941,6 +2941,7 @@ void Parser::parse_and_visit_decorator(Parse_Visitor_Base &v) { this->skip(); while (this->peek().type == Token_Type::dot) { + Source_Code_Span op_span_ = this->peek().span(); this->skip(); switch (this->peek().type) { // @myNamespace.myDecorator @@ -2948,7 +2949,7 @@ void Parser::parse_and_visit_decorator(Parse_Visitor_Base &v) { case Token_Type::identifier: case Token_Type::private_identifier: ast = this->make_expression( - ast, this->peek().identifier_name()); + ast, this->peek().identifier_name(), op_span_); this->skip(); break; @@ -2961,7 +2962,7 @@ void Parser::parse_and_visit_decorator(Parse_Visitor_Base &v) { if (this->peek().type == Token_Type::left_paren) { // @decorator() // @decorator(arg1, arg2) - ast = this->parse_call_expression_remainder(v, ast); + ast = this->parse_call_expression_remainder(v, ast, std::nullopt); } this->visit_expression(ast, v, Variable_Context::rhs); diff --git a/src/quick-lint-js/fe/parse.cpp b/src/quick-lint-js/fe/parse.cpp index ebabc00eee..eb41bdf90e 100644 --- a/src/quick-lint-js/fe/parse.cpp +++ b/src/quick-lint-js/fe/parse.cpp @@ -469,6 +469,61 @@ 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_; + 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_; + 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 b9744d53b2..49433ccde2 100644 --- a/src/quick-lint-js/fe/parse.h +++ b/src/quick-lint-js/fe/parse.h @@ -587,6 +587,7 @@ 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( @@ -851,9 +852,11 @@ class Parser { Expression *parameters_expression, Buffering_Visitor *return_type_visits, Precedence); Expression::Call *parse_call_expression_remainder(Parse_Visitor_Base &, - Expression *callee); + Expression *callee, + std::optional); Expression *parse_index_expression_remainder(Parse_Visitor_Base &, - Expression *lhs); + Expression *lhs, + std::optional); Expression_Arena::Vector parse_arrow_function_parameters_or_call_arguments(Parse_Visitor_Base &v); Expression *parse_arrow_function_body( diff --git a/src/quick-lint-js/i18n/translation-table-generated.cpp b/src/quick-lint-js/i18n/translation-table-generated.cpp index 653b6f43a0..ef9225a606 100644 --- a/src/quick-lint-js/i18n/translation-table-generated.cpp +++ b/src/quick-lint-js/i18n/translation-table-generated.cpp @@ -591,6 +591,7 @@ 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}, // @@ -2458,6 +2459,7 @@ 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 f410cfda3e..2d145babc3 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 = 602; -constexpr std::size_t translation_table_string_table_size = 82276; +constexpr std::uint16_t translation_table_mapping_table_size = 603; +constexpr std::size_t translation_table_string_table_size = 82350; constexpr std::size_t translation_table_locale_table_size = 35; QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up( @@ -605,6 +605,7 @@ 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 00551ea639..90b3cb9ee9 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[601] = { +inline const Translated_String test_translation_table[602] = { { "\"global-groups\" entries must be strings"_translatable, u8"\"global-groups\" entries must be strings", @@ -6397,6 +6397,17 @@ inline const Translated_String test_translation_table[601] = { 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 2ec6e3c231..8ab1bc858c 100644 --- a/test/test-parse-warning.cpp +++ b/test/test-parse-warning.cpp @@ -509,6 +509,39 @@ 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); +} } }