AK+Everywhere: Fix compiletime format parsing of replacement fields

akimdi · Jun 1, 2021 · 944855c · 944855c
1 parent c0d1a75
commit 944855c
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 12 deletions.
diff --git a/AK/CheckedFormatString.h b/AK/CheckedFormatString.h
@@ -79,6 +79,7 @@ consteval auto count_fmt_params(const char (&fmt)[N])
 
  size_t unclosed_braces { 0 };
  size_t extra_closed_braces { 0 };
+ size_t nesting_level { 0 };
 
  Array<size_t, 4> last_format_specifier_start { 0 };
  size_t total_used_last_format_specifier_start_count { 0 };
@@ -100,13 +101,17 @@ consteval auto count_fmt_params(const char (&fmt)[N])
  result.last_format_specifier_start[result.total_used_last_format_specifier_start_count++] = i + 1;
 
  ++result.unclosed_braces;
+ ++result.nesting_level;
  break;
  case '}':
- if (i + 1 < N && fmt[i + 1] == '}') {
- ++i;
- continue;
+ if (result.nesting_level == 0) {
+ if (i + 1 < N && fmt[i + 1] == '}') {
+ ++i;
+ continue;
+ }
  }
  if (result.unclosed_braces) {
+ --result.nesting_level;
  --result.unclosed_braces;
 
  if (result.total_used_last_format_specifier_start_count == 0)

diff --git a/Tests/AK/TestFormat.cpp b/Tests/AK/TestFormat.cpp
@@ -105,12 +105,12 @@ TEST_CASE(zero_pad)
 
 TEST_CASE(replacement_field)
 {
- // FIXME: Compiletime check bypass: cannot parse '}}' correctly.
- EXPECT_EQ(String::formatted(StringView { "{:*>{1}}" }, 13, static_cast<size_t>(10)), "********13");
- EXPECT_EQ(String::formatted(StringView { "{:*<{1}}" }, 7, 4), "7***");
+ EXPECT_EQ(String::formatted("{:*>{1}}", 13, static_cast<size_t>(10)), "********13");
+ EXPECT_EQ(String::formatted("{:*<{1}}", 7, 4), "7***");
+ // Compiletime check bypass: intentionally ignoring extra arguments
  EXPECT_EQ(String::formatted(StringView { "{:{2}}" }, -5, 8, 16), " -5");
- EXPECT_EQ(String::formatted(StringView { "{{{:*^{1}}}}" }, 1, 3), "{*1*}");
- EXPECT_EQ(String::formatted(StringView { "{:0{}}" }, 1, 3), "001");
+ EXPECT_EQ(String::formatted("{{{:*^{1}}}}", 1, 3), "{*1*}");
+ EXPECT_EQ(String::formatted("{:0{}}", 1, 3), "001");
 }
 
 TEST_CASE(replacement_field_regression)

diff --git a/Userland/Shell/Shell.cpp b/Userland/Shell/Shell.cpp
@@ -1998,16 +1998,16 @@ void Shell::possibly_print_error() const
  warn("\x1b[31m");
  size_t length_written_so_far = 0;
  if (line == (i64)source_position.position->start_line.line_number) {
- warn(StringView { "{:~>{}}" }, "", 5 + source_position.position->start_line.line_column);
+ warn("{:~>{}}", "", 5 + source_position.position->start_line.line_column);
  length_written_so_far += source_position.position->start_line.line_column;
  } else {
- warn(StringView { "{:~>{}}" }, "", 5);
+ warn("{:~>{}}", "", 5);
  }
  if (line == (i64)source_position.position->end_line.line_number) {
- warn(StringView { "{:^>{}}" }, "", source_position.position->end_line.line_column - length_written_so_far);
+ warn("{:^>{}}", "", source_position.position->end_line.line_column - length_written_so_far);
  length_written_so_far += source_position.position->start_line.line_column;
  } else {
- warn(StringView { "{:^>{}}" }, "", current_line.length() - length_written_so_far);
+ warn("{:^>{}}", "", current_line.length() - length_written_so_far);
  }
  warnln("\x1b[0m");
  }