From d221738e207c22c3369c40922d168b0aa76a5af6 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 11 Jan 2021 21:09:22 +0000 Subject: [PATCH] Add diag::Formatter::Style::print_newline_at_end Automatically prints a newline at the end of the last diagnostic in a list. Defaults to true. Disabled for many tests that assume no newline at end of string. Change-Id: Id1c2f7771f03f22d926fafc2bebebcef056ac5e8 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/37260 Reviewed-by: dan sinclair Commit-Queue: Ben Clayton --- src/diagnostic/formatter.cc | 3 +++ src/diagnostic/formatter.h | 2 ++ src/diagnostic/formatter_test.cc | 25 +++++++++++++------ src/reader/reader.h | 2 +- src/reader/wgsl/parser_impl.h | 2 +- src/reader/wgsl/parser_impl_error_msg_test.cc | 23 ++++++++++------- .../wgsl/parser_impl_error_resync_test.cc | 21 ++++++++++------ src/transform/test_helper.h | 4 ++- src/validator/validator.h | 2 +- src/validator/validator_impl.h | 2 +- 10 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/diagnostic/formatter.cc b/src/diagnostic/formatter.cc index 94affb6360..6529eecb5e 100644 --- a/src/diagnostic/formatter.cc +++ b/src/diagnostic/formatter.cc @@ -121,6 +121,9 @@ void Formatter::format(const List& list, Printer* printer) const { format(diag, state); first = false; } + if (style_.print_newline_at_end) { + state.newline(); + } } void Formatter::format(const Diagnostic& diag, State& state) const { diff --git a/src/diagnostic/formatter.h b/src/diagnostic/formatter.h index 2df249fa36..8ed06398c4 100644 --- a/src/diagnostic/formatter.h +++ b/src/diagnostic/formatter.h @@ -36,6 +36,8 @@ class Formatter { bool print_severity = true; /// include the source line(s) for the diagnostic bool print_line = true; + /// print a newline at the end of a diagnostic list + bool print_newline_at_end = true; }; /// Constructor for the formatter using a default style. diff --git a/src/diagnostic/formatter_test.cc b/src/diagnostic/formatter_test.cc index 45b89941e4..6c4518f4f0 100644 --- a/src/diagnostic/formatter_test.cc +++ b/src/diagnostic/formatter_test.cc @@ -45,7 +45,7 @@ class DiagFormatterTest : public testing::Test { }; TEST_F(DiagFormatterTest, Simple) { - Formatter fmt{{false, false, false}}; + Formatter fmt{{false, false, false, false}}; auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); auto* expect = R"(1:14: purr 2:14: grrr @@ -54,8 +54,19 @@ TEST_F(DiagFormatterTest, Simple) { ASSERT_EQ(expect, got); } +TEST_F(DiagFormatterTest, SimpleNewlineAtEnd) { + Formatter fmt{{false, false, false, true}}; + auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); + auto* expect = R"(1:14: purr +2:14: grrr +3:16 abc123: hiss +4:16: nothing +)"; + ASSERT_EQ(expect, got); +} + TEST_F(DiagFormatterTest, SimpleNoSource) { - Formatter fmt{{false, false, false}}; + Formatter fmt{{false, false, false, false}}; Diagnostic diag{Severity::Info, Source{}, "no source!"}; auto got = fmt.format(List{diag}); auto* expect = "no source!"; @@ -63,7 +74,7 @@ TEST_F(DiagFormatterTest, SimpleNoSource) { } TEST_F(DiagFormatterTest, WithFile) { - Formatter fmt{{true, false, false}}; + Formatter fmt{{true, false, false, false}}; auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); auto* expect = R"(file.name:1:14: purr file.name:2:14: grrr @@ -73,7 +84,7 @@ file.name:4:16: nothing)"; } TEST_F(DiagFormatterTest, WithSeverity) { - Formatter fmt{{false, true, false}}; + Formatter fmt{{false, true, false, false}}; auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); auto* expect = R"(1:14 info: purr 2:14 warning: grrr @@ -83,7 +94,7 @@ TEST_F(DiagFormatterTest, WithSeverity) { } TEST_F(DiagFormatterTest, WithLine) { - Formatter fmt{{false, false, true}}; + Formatter fmt{{false, false, true, false}}; auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); auto* expect = R"(1:14: purr the cat says meow @@ -105,7 +116,7 @@ the snail says ??? } TEST_F(DiagFormatterTest, BasicWithFileSeverityLine) { - Formatter fmt{{true, true, true}}; + Formatter fmt{{true, true, true, false}}; auto got = fmt.format(List{diag_info, diag_warn, diag_err, diag_fatal}); auto* expect = R"(file.name:1:14 info: purr the cat says meow @@ -130,7 +141,7 @@ TEST_F(DiagFormatterTest, BasicWithMultiLine) { Diagnostic multiline{Severity::Warning, Source{Source::Range{{2, 9}, {4, 15}}, &file}, "multiline"}; - Formatter fmt{{false, false, true}}; + Formatter fmt{{false, false, true, false}}; auto got = fmt.format(List{multiline}); auto* expect = R"(2:9: multiline the dog says woof diff --git a/src/reader/reader.h b/src/reader/reader.h index 8fa4c92016..9c71184f6f 100644 --- a/src/reader/reader.h +++ b/src/reader/reader.h @@ -39,7 +39,7 @@ class Reader { /// @returns the parser error string std::string error() const { - diag::Formatter formatter{{false, false, false}}; + diag::Formatter formatter{{false, false, false, false}}; return formatter.format(diags_); } diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index c523d06d51..9a4e4788fc 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -297,7 +297,7 @@ class ParserImpl { /// @returns the parser error string std::string error() const { - diag::Formatter formatter{{false, false, false}}; + diag::Formatter formatter{{false, false, false, false}}; return formatter.format(diags_); } diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc index d43d60f170..d969e720c2 100644 --- a/src/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/reader/wgsl/parser_impl_error_msg_test.cc @@ -21,17 +21,22 @@ namespace reader { namespace wgsl { namespace { +const diag::Formatter::Style formatter_style{ + /* print_file: */ true, /* print_severity: */ true, + /* print_line: */ true, /* print_newline_at_end: */ false}; + class ParserImplErrorTest : public ParserImplTest {}; -#define EXPECT(SOURCE, EXPECTED) \ - do { \ - std::string source = SOURCE; \ - std::string expected = EXPECTED; \ - auto p = parser(source); \ - p->set_max_errors(5); \ - EXPECT_EQ(false, p->Parse()); \ - EXPECT_EQ(true, p->diagnostics().contains_errors()); \ - EXPECT_EQ(expected, diag::Formatter().format(p->diagnostics())); \ +#define EXPECT(SOURCE, EXPECTED) \ + do { \ + std::string source = SOURCE; \ + std::string expected = EXPECTED; \ + auto p = parser(source); \ + p->set_max_errors(5); \ + EXPECT_EQ(false, p->Parse()); \ + EXPECT_EQ(true, p->diagnostics().contains_errors()); \ + EXPECT_EQ(expected, \ + diag::Formatter(formatter_style).format(p->diagnostics())); \ } while (false) TEST_F(ParserImplErrorTest, AdditiveInvalidExpr) { diff --git a/src/reader/wgsl/parser_impl_error_resync_test.cc b/src/reader/wgsl/parser_impl_error_resync_test.cc index e420bcddfe..be671af000 100644 --- a/src/reader/wgsl/parser_impl_error_resync_test.cc +++ b/src/reader/wgsl/parser_impl_error_resync_test.cc @@ -21,16 +21,21 @@ namespace reader { namespace wgsl { namespace { +const diag::Formatter::Style formatter_style{ + /* print_file: */ true, /* print_severity: */ true, + /* print_line: */ true, /* print_newline_at_end: */ false}; + class ParserImplErrorResyncTest : public ParserImplTest {}; -#define EXPECT(SOURCE, EXPECTED) \ - do { \ - std::string source = SOURCE; \ - std::string expected = EXPECTED; \ - auto p = parser(source); \ - EXPECT_EQ(false, p->Parse()); \ - EXPECT_EQ(true, p->diagnostics().contains_errors()); \ - EXPECT_EQ(expected, diag::Formatter().format(p->diagnostics())); \ +#define EXPECT(SOURCE, EXPECTED) \ + do { \ + std::string source = SOURCE; \ + std::string expected = EXPECTED; \ + auto p = parser(source); \ + EXPECT_EQ(false, p->Parse()); \ + EXPECT_EQ(true, p->diagnostics().contains_errors()); \ + EXPECT_EQ(expected, \ + diag::Formatter(formatter_style).format(p->diagnostics())); \ } while (false) TEST_F(ParserImplErrorResyncTest, BadFunctionDecls) { diff --git a/src/transform/test_helper.h b/src/transform/test_helper.h index a9eb584083..3868143a88 100644 --- a/src/transform/test_helper.h +++ b/src/transform/test_helper.h @@ -59,8 +59,10 @@ class TransformTest : public testing::Test { auto result = manager.Run(&module); if (result.diagnostics.contains_errors()) { + diag::Formatter::Style style; + style.print_newline_at_end = false; return "manager().Run() errored:\n" + - diag::Formatter().format(result.diagnostics); + diag::Formatter(style).format(result.diagnostics); } // Release the source module to ensure there's no uncloned data in result diff --git a/src/validator/validator.h b/src/validator/validator.h index 2180de6aaa..8b8f9a9b5e 100644 --- a/src/validator/validator.h +++ b/src/validator/validator.h @@ -41,7 +41,7 @@ class Validator { /// @returns error messages from the validator std::string error() { - diag::Formatter formatter{{false, false, false}}; + diag::Formatter formatter{{false, false, false, false}}; return formatter.format(diags_); } /// @returns true if an error was encountered diff --git a/src/validator/validator_impl.h b/src/validator/validator_impl.h index c0a8d2b333..0a77654e42 100644 --- a/src/validator/validator_impl.h +++ b/src/validator/validator_impl.h @@ -54,7 +54,7 @@ class ValidatorImpl { /// @returns error messages from the validator std::string error() { - diag::Formatter formatter{{false, false, false}}; + diag::Formatter formatter{{false, false, false, false}}; return formatter.format(diags_); } /// @returns true if an error was encountered