Skip to content

Commit 04cd4b7

Browse files
stragerstrager
committed
refactor(test): remove Diag_Collector from Spy_Visitor
I want to get rid of Diag_Collector. Delete Spy_Visitor's Diag_Collector. #1154
1 parent dcaf585 commit 04cd4b7

File tree

4 files changed

+60
-45
lines changed

4 files changed

+60
-45
lines changed

‎test/quick-lint-js/spy-visitor.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ inline Visited_Variable_Declaration var_noinit_for_decl(String8_View name) {
233233
Variable_Declaration_Flags::inside_for_loop_head};
234234
}
235235

236-
struct Parse_Visit_Collector : public Parse_Visitor_Base {
236+
struct Parse_Visit_Collector final : public Parse_Visitor_Base {
237237
std::vector<std::string_view> visits;
238238

239239
void visit_end_of_module() override {
@@ -452,9 +452,8 @@ struct Parse_Visit_Collector : public Parse_Visitor_Base {
452452
std::vector<String8> variable_uses;
453453
};
454454

455-
// TODO(strager): Rename this.
456-
struct Spy_Visitor final : public Diag_Collector,
457-
public Parse_Visit_Collector {};
455+
// TODO(strager): Remove this alias.
456+
using Spy_Visitor = Parse_Visit_Collector;
458457

459458
void PrintTo(const Visited_Variable_Declaration &, std::ostream *);
460459
}

‎test/test-parse-statement.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,11 +1306,8 @@ TEST_F(Test_Parse_Statement, if_body_with_semicolon_typescript) {
13061306
}));
13071307
}
13081308

1309-
{
1310-
Spy_Visitor p = test_parse_and_visit_statement(
1311-
u8"if (a) {} else;\n"_sv, no_diags, typescript_options);
1312-
EXPECT_THAT(p.errors, IsEmpty());
1313-
}
1309+
test_parse_and_visit_statement(u8"if (a) {} else;\n"_sv, no_diags,
1310+
typescript_options);
13141311
}
13151312
}
13161313
}

‎test/test-parse-typescript.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,11 @@ TEST_F(Test_Parse_TypeScript, no_crash) {
8585
u8"export declare export"_sv,
8686
u8"export declare()"_sv,
8787
}) {
88-
Spy_Visitor v;
88+
Monotonic_Allocator memory("test");
89+
Diag_List_Diag_Reporter diags(&memory);
8990
Padded_String code_string(code);
90-
Parser p(&code_string, &v, typescript_options);
91+
Parser p(&code_string, &diags, typescript_options);
92+
Spy_Visitor v;
9193
// Should not crash:
9294
p.parse_and_visit_module_catching_fatal_parse_errors(v);
9395
}

‎test/test-parse.cpp

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,25 @@ using ::testing::UnorderedElementsAreArray;
2929

3030
namespace quick_lint_js {
3131
namespace {
32-
class Test_Parse : public Test_Parse_Expression {};
32+
class Test_Parse : public Test_Parse_Expression {
33+
public:
34+
Monotonic_Allocator memory_{"test"};
35+
};
3336

3437
// TODO(strager): Put Test_Escape_First_Character_In_Keyword tests into their
3538
// own test file.
3639
class Test_Escape_First_Character_In_Keyword : public ::testing::Test {};
3740

3841
// TODO(strager): Put Test_No_Overflow and test_overflow tests into their own
3942
// test file.
40-
class Test_No_Overflow : public Test_Parse_Expression {};
41-
class Test_Overflow : public Test_Parse_Expression {};
43+
class Test_No_Overflow : public Test_Parse_Expression {
44+
public:
45+
Monotonic_Allocator memory_{"test"};
46+
};
47+
class Test_Overflow : public Test_Parse_Expression {
48+
public:
49+
Monotonic_Allocator memory_{"test"};
50+
};
4251

4352
TEST_F(Test_Parse, statement_starting_with_invalid_token) {
4453
for (String8_View token : {
@@ -562,9 +571,10 @@ Padded_String unimplemented_token_code(u8"]"_sv);
562571

563572
#if defined(GTEST_HAS_DEATH_TEST) && GTEST_HAS_DEATH_TEST
564573
TEST_F(Test_Parse, unimplemented_token_crashes_SLOW) {
565-
auto check = [] {
574+
auto check = [&] {
575+
Diag_List_Diag_Reporter diags(&this->memory_);
576+
Parser p(&unimplemented_token_code, &diags, javascript_options);
566577
Spy_Visitor v;
567-
Parser p(&unimplemented_token_code, &v, javascript_options);
568578
p.parse_and_visit_module(v);
569579
};
570580
EXPECT_DEATH(check(), "token not implemented");
@@ -573,24 +583,25 @@ TEST_F(Test_Parse, unimplemented_token_crashes_SLOW) {
573583

574584
TEST_F(Test_Parse, unimplemented_token_doesnt_crash_if_caught) {
575585
{
586+
Diag_Collector diags;
587+
Parser p(&unimplemented_token_code, &diags, javascript_options);
576588
Spy_Visitor v;
577-
Parser p(&unimplemented_token_code, &v, javascript_options);
578589
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
579590
EXPECT_FALSE(ok);
580591
EXPECT_THAT(v.visits, IsEmpty());
581-
EXPECT_THAT(v.errors, ElementsAreArray({
582-
DIAG_TYPE_OFFSETS(&unimplemented_token_code,
583-
Diag_Unexpected_Token, //
584-
token, 0, u8"]"_sv),
585-
}));
592+
EXPECT_THAT(diags.errors, ElementsAreArray({
593+
DIAG_TYPE_OFFSETS(&unimplemented_token_code,
594+
Diag_Unexpected_Token, //
595+
token, 0, u8"]"_sv),
596+
}));
586597
}
587598
}
588599

589600
TEST_F(Test_Parse, unimplemented_token_returns_to_innermost_handler) {
590601
{
591602
Padded_String code(u8"hello world"_sv);
592-
Spy_Visitor v;
593-
Parser p(&code, &v, javascript_options);
603+
Diag_List_Diag_Reporter diags(&this->memory_);
604+
Parser p(&code, &diags, javascript_options);
594605
volatile bool inner_catch_returned = false;
595606
bool outer_ok = p.catch_fatal_parse_errors([&] {
596607
bool inner_ok = p.catch_fatal_parse_errors(
@@ -600,7 +611,7 @@ TEST_F(Test_Parse, unimplemented_token_returns_to_innermost_handler) {
600611
});
601612
EXPECT_TRUE(outer_ok);
602613
EXPECT_TRUE(inner_catch_returned);
603-
assert_diagnostics(&code, v.errors,
614+
assert_diagnostics(&code, diags.diags(),
604615
{
605616
u8"Diag_Unexpected_Token"_diag,
606617
});
@@ -611,8 +622,8 @@ TEST_F(Test_Parse,
611622
unimplemented_token_after_handler_ends_returns_to_outer_handler) {
612623
{
613624
Padded_String code(u8"hello world"_sv);
614-
Spy_Visitor v;
615-
Parser p(&code, &v, javascript_options);
625+
Diag_List_Diag_Reporter diags(&this->memory_);
626+
Parser p(&code, &diags, javascript_options);
616627
volatile bool inner_catch_returned = false;
617628
bool outer_ok = p.catch_fatal_parse_errors([&] {
618629
bool inner_ok = p.catch_fatal_parse_errors([] {
@@ -624,7 +635,7 @@ TEST_F(Test_Parse,
624635
});
625636
EXPECT_FALSE(outer_ok);
626637
EXPECT_TRUE(inner_catch_returned);
627-
assert_diagnostics(&code, v.errors,
638+
assert_diagnostics(&code, diags.diags(),
628639
{
629640
u8"Diag_Unexpected_Token"_diag,
630641
});
@@ -634,8 +645,7 @@ TEST_F(Test_Parse,
634645
TEST_F(Test_Parse, unimplemented_token_rolls_back_parser_depth) {
635646
{
636647
Padded_String code(u8"hello world"_sv);
637-
Spy_Visitor v;
638-
Parser p(&code, &v, javascript_options);
648+
Parser p(&code, &Null_Diag_Reporter::instance, javascript_options);
639649
volatile bool inner_catch_returned = false;
640650
bool outer_ok = p.catch_fatal_parse_errors([&] {
641651
Parser::Depth_Guard outer_g(&p);
@@ -657,19 +667,21 @@ TEST_F(Test_Parse, unimplemented_token_rolls_back_parser_depth) {
657667
TEST_F(Test_Parse, unimplemented_token_is_reported_on_outer_diag_reporter) {
658668
{
659669
Padded_String code(u8"hello world"_sv);
660-
Spy_Visitor v;
661-
Parser p(&code, &v, javascript_options);
670+
Diag_List_Diag_Reporter diags(&this->memory_);
671+
Parser p(&code, &diags, javascript_options);
662672

663673
Parser_Transaction transaction = p.begin_transaction();
664674
bool ok = p.catch_fatal_parse_errors(
665675
[&] { QLJS_PARSER_UNIMPLEMENTED_WITH_PARSER(&p); });
666676
EXPECT_FALSE(ok);
667677

668-
EXPECT_THAT(v.errors, IsEmpty())
669-
<< "Diag_Unexpected_Token should be buffered in the transaction";
678+
assert_diagnostics(&code, diags.diags(), {});
679+
// Diag_Unexpected_Token should be buffered in the transaction.
680+
// FIXME(#1154): Instead of buffering, use a rewind mechanism. (No rewinding
681+
// should happen in this test.)
670682
p.commit_transaction(std::move(transaction));
671683
// Diag_Unexpected_Token should be reported when committing the transaction.
672-
assert_diagnostics(&code, v.errors,
684+
assert_diagnostics(&code, diags.diags(),
673685
{
674686
u8"Diag_Unexpected_Token"_diag,
675687
});
@@ -769,23 +781,25 @@ TEST_F(Test_No_Overflow, parser_depth_limit_not_exceeded) {
769781
}) {
770782
Padded_String code(u8"return " + jsx);
771783
SCOPED_TRACE(code);
784+
Diag_List_Diag_Reporter diags(&this->memory_);
785+
Parser p(&code, &diags, jsx_options);
772786
Spy_Visitor v;
773-
Parser p(&code, &v, jsx_options);
774787
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
775788
EXPECT_TRUE(ok);
776-
EXPECT_THAT(v.errors, IsEmpty());
789+
assert_diagnostics(&code, diags.diags(), {});
777790
}
778791

779792
for (const String8& type : {
780793
repeated_str(u8"("_sv, u8"T"_sv, u8")"_sv, Parser::stack_limit - 2),
781794
}) {
782795
Padded_String code(concat(u8"let x: "_sv, type, u8";"_sv));
783796
SCOPED_TRACE(code);
797+
Diag_List_Diag_Reporter diags(&this->memory_);
798+
Parser p(&code, &diags, typescript_options);
784799
Spy_Visitor v;
785-
Parser p(&code, &v, typescript_options);
786800
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
787801
EXPECT_TRUE(ok);
788-
EXPECT_THAT(v.errors, IsEmpty());
802+
assert_diagnostics(&code, diags.diags(), {});
789803
}
790804
}
791805

@@ -836,11 +850,12 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
836850
}) {
837851
Padded_String code(exps);
838852
SCOPED_TRACE(code);
853+
Diag_List_Diag_Reporter diags(&this->memory_);
854+
Parser p(&code, &diags, javascript_options);
839855
Spy_Visitor v;
840-
Parser p(&code, &v, javascript_options);
841856
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
842857
EXPECT_FALSE(ok);
843-
assert_diagnostics(&code, v.errors,
858+
assert_diagnostics(&code, diags.diags(),
844859
{
845860
u8"Diag_Depth_Limit_Exceeded"_diag,
846861
});
@@ -884,11 +899,12 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
884899
}) {
885900
Padded_String code(concat(u8"return "_sv, jsx));
886901
SCOPED_TRACE(code);
902+
Diag_List_Diag_Reporter diags(&this->memory_);
903+
Parser p(&code, &diags, jsx_options);
887904
Spy_Visitor v;
888-
Parser p(&code, &v, jsx_options);
889905
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
890906
EXPECT_FALSE(ok);
891-
assert_diagnostics(&code, v.errors,
907+
assert_diagnostics(&code, diags.diags(),
892908
{
893909
u8"Diag_Depth_Limit_Exceeded"_diag,
894910
});
@@ -899,11 +915,12 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
899915
}) {
900916
Padded_String code(concat(u8"let x: "_sv, type, u8";"_sv));
901917
SCOPED_TRACE(code);
918+
Diag_List_Diag_Reporter diags(&this->memory_);
919+
Parser p(&code, &diags, typescript_options);
902920
Spy_Visitor v;
903-
Parser p(&code, &v, typescript_options);
904921
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
905922
EXPECT_FALSE(ok);
906-
assert_diagnostics(&code, v.errors,
923+
assert_diagnostics(&code, diags.diags(),
907924
{
908925
u8"Diag_Depth_Limit_Exceeded"_diag,
909926
});

0 commit comments

Comments
 (0)