feat: new diag for using ‘.’ after ‘?.’

This closes #1128 -- it warns the user about using a dot After an optional chain. This cover optional chain with function call as well as optional chain with index expressions

docs/errors/E0717.md

@@ -16,4 +16,4 @@ of 'import type' when using an alias.
 
 ```typescript
 import A = ns;
-```
+```

docs/errors/E0718.md

@@ -0,0 +1,25 @@
+# E0718: 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
+```
+>>>>>>>

po/messages.pot

@@ -2405,6 +2405,10 @@ 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"

src/quick-lint-js/diag/diagnostic-metadata-generated.cpp

@@ -6884,6 +6884,21 @@ 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),
+        },
+      },
+    },
 };
 }
 

src/quick-lint-js/diag/diagnostic-metadata-generated.h

@@ -470,10 +470,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_Using_Dot_After_Optional_Chaining) \
   /* END */
 // clang-format on
 
-inline constexpr int Diag_Type_Count = 459;
+inline constexpr int Diag_Type_Count = 460;
 
 extern const Diagnostic_Info all_diagnostic_infos[Diag_Type_Count];
 }

src/quick-lint-js/diag/diagnostic-types-2.h

@@ -3574,6 +3574,17 @@ 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
 

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<Expression *> children,
-                Source_Code_Span left_paren_span, const Char8 *span_end)
+                Source_Code_Span left_paren_span, const Char8 *span_end,
+                std::optional<Source_Code_Span> 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<Expression *> children_;
+  std::optional<Source_Code_Span> optional_chaining_operator_;
 };
 static_assert(Expression_Arena::is_allocatable<Expression::Call>);
 
@@ -647,11 +650,15 @@ 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<Expression::Dot>);
 
@@ -682,15 +689,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<Source_Code_Span> 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<Expression *, 2> children_;
+  std::optional<Source_Code_Span> optional_chaining_operator_;
 };
 static_assert(Expression_Arena::is_allocatable<Expression::Index>);
 

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<Expression::Dot*>(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<Expression::Call>(
           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,8 @@ 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 +1752,8 @@ Expression* Parser::parse_expression_remainder(Parse_Visitor_Base& v,
             });
       }
       binary_builder.replace_last(this->make_expression<Expression::Dot>(
-          binary_builder.last_expression(), this->peek().identifier_name()));
+          binary_builder.last_expression(), this->peek().identifier_name(),
+          dot_span));
       this->skip();
       goto next;
 
@@ -1779,7 +1784,8 @@ 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<Expression::Dot>(
-          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 +1819,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 +1828,8 @@ 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<Expression::Dot>(
-          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 +1844,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<Source_Code_Span>(question_dot_span)));
       goto next;
 
     // f?.<T>(x, y)                      // TypeScript only
@@ -1852,13 +1861,14 @@ 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<Source_Code_Span>(question_dot_span)));
       goto next;
 
     default:
@@ -1871,7 +1881,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;
   }
 
@@ -2621,8 +2631,9 @@ Expression* Parser::parse_arrow_function_expression_remainder(
   return arrow_function;
 }
 
-Expression::Call* Parser::parse_call_expression_remainder(Parse_Visitor_Base& v,
-                                                          Expression* callee) {
+Expression::Call* Parser::parse_call_expression_remainder(
+    Parse_Visitor_Base& v, Expression* callee,
+    std::optional<Source_Code_Span> optional_chaining_op) {
   Source_Code_Span left_paren_span = this->peek().span();
   Expression_Arena::Vector<Expression*> call_children(
       "parse_expression_remainder call children",
@@ -2663,11 +2674,13 @@ Expression::Call* Parser::parse_call_expression_remainder(Parse_Visitor_Base& v,
       this->make_expression<Expression::Call>(
           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* Parser::parse_index_expression_remainder(
+    Parse_Visitor_Base& v, Expression* lhs,
+    std::optional<Source_Code_Span> optional_chaining_op) {
   QLJS_ASSERT(this->peek().type == Token_Type::left_square);
   Source_Code_Span left_square_span = this->peek().span();
   this->skip();
@@ -2694,8 +2707,8 @@ Expression* Parser::parse_index_expression_remainder(Parse_Visitor_Base& v,
     end = this->lexer_.end_of_previous_token();
     break;
   }
-  return this->make_expression<Expression::Index>(lhs, subscript,
-                                                  left_square_span, end);
+  return this->make_expression<Expression::Index>(
+      lhs, subscript, left_square_span, end, optional_chaining_op);
 }
 
 Expression_Arena::Vector<Expression*>

src/quick-lint-js/fe/parse-statement.cpp

@@ -2941,14 +2941,15 @@ 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
       QLJS_CASE_KEYWORD:
       case Token_Type::identifier:
       case Token_Type::private_identifier:
         ast = this->make_expression<Expression::Dot>(
-            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);

src/quick-lint-js/fe/parse.cpp

@@ -469,6 +469,57 @@ 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<Expression::Dot*>(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<Expression::Call*>(lhs);
+      std::optional<Source_Code_Span> 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<Expression::Index*>(lhs);
+      std::optional<Source_Code_Span> 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; };