Skip to content

Commit

Permalink
Merge branch 'master' into rename_import_alias_namespace_alias_add_ne…
Browse files Browse the repository at this point in the history
…w_diag_namespace_alias
  • Loading branch information
UnfairBots authored Jan 27, 2024
2 parents 887d8c9 + 434d085 commit d1d27a6
Show file tree
Hide file tree
Showing 16 changed files with 35 additions and 149 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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/
Expand Down
23 changes: 2 additions & 21 deletions docs/errors/E0718.md
Original file line number Diff line number Diff line change
@@ -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
```
<!-- QLJS_NO_CHECK_CODE -->
4 changes: 0 additions & 4 deletions po/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -2417,10 +2417,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"
Expand Down
15 changes: 0 additions & 15 deletions src/quick-lint-js/diag/diagnostic-metadata-generated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6930,21 +6930,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),
},
},
},
};
}

Expand Down
3 changes: 1 addition & 2 deletions src/quick-lint-js/diag/diagnostic-metadata-generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,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 = 463;
inline constexpr int Diag_Type_Count = 462;

extern const Diagnostic_Info all_diagnostic_infos[Diag_Type_Count];
}
Expand Down
12 changes: 1 addition & 11 deletions src/quick-lint-js/diag/diagnostic-types-2.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)]] //
Expand Down Expand Up @@ -3598,17 +3599,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

Expand Down
14 changes: 14 additions & 0 deletions src/quick-lint-js/fe/parse-class.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
2 changes: 0 additions & 2 deletions src/quick-lint-js/fe/parse-expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Expression::Dot*>(ast));
break;
case Expression_Kind::Index:
this->visit_expression(ast->child_0(), v, Variable_Context::rhs);
Expand Down
1 change: 1 addition & 0 deletions src/quick-lint-js/fe/parse-statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
51 changes: 0 additions & 51 deletions src/quick-lint-js/fe/parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<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_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<Expression::Index*>(lhs);
std::optional<Source_Code_Span> 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; };
Expand Down
1 change: 0 additions & 1 deletion src/quick-lint-js/fe/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions src/quick-lint-js/i18n/translation-table-generated.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,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}, //
Expand Down Expand Up @@ -2467,7 +2466,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"
Expand Down
5 changes: 2 additions & 3 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 = 607;
constexpr std::size_t translation_table_string_table_size = 82541;
constexpr std::uint16_t translation_table_mapping_table_size = 606;
constexpr std::size_t translation_table_string_table_size = 82467;
constexpr std::size_t translation_table_locale_table_size = 35;

QLJS_CONSTEVAL std::uint16_t translation_table_const_look_up(
Expand Down Expand Up @@ -609,7 +609,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,
Expand Down
13 changes: 1 addition & 12 deletions 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[606] = {
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 @@ -6441,17 +6441,6 @@ inline const Translated_String test_translation_table[606] = {
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",
Expand Down
11 changes: 11 additions & 0 deletions test/test-parse-decorator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
25 changes: 0 additions & 25 deletions test/test-parse-warning.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down

0 comments on commit d1d27a6

Please sign in to comment.