Skip to content

Commit 62788ab

Browse files
dmitriplotnikovcopybara-github
dmitriplotnikov
authored and
copybara-github
committed
[Pratt Parser] Add support for options.error_recovery_limit
PiperOrigin-RevId: 953512384
1 parent bc1e3e6 commit 62788ab

5 files changed

Lines changed: 109 additions & 58 deletions

File tree

‎parser/internal/pratt_parser.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,11 @@ namespace {
5050
std::string DisplayParserError(const cel::Source& source,
5151
SourceLocation location,
5252
std::string_view message) {
53+
int32_t display_column =
54+
location.column >= 0 ? location.column + 1 : location.column;
5355
return absl::StrCat(
54-
absl::StrFormat("ERROR: %s:%zu:%zu: %s", source.description(),
55-
location.line, location.column + 1, message),
56+
absl::StrFormat("ERROR: %s:%d:%d: %s", source.description(),
57+
location.line, display_column, message),
5658
source.DisplayErrorLocation(location));
5759
}
5860

‎parser/internal/pratt_parser_test.cc‎

Lines changed: 73 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,7 +1055,7 @@ std::string FormatIssues(const cel::Source& source,
10551055
issues, "\n", [&source](std::string* out, const cel::ParseIssue& issue) {
10561056
absl::StrAppend(
10571057
out,
1058-
absl::StrFormat("ERROR: %s:%zu:%zu: %s", source.description(),
1058+
absl::StrFormat("ERROR: %s:%d:%d: %s", source.description(),
10591059
issue.location().line, issue.location().column + 1,
10601060
issue.message()),
10611061
source.DisplayErrorLocation(issue.location()));
@@ -1576,53 +1576,6 @@ INSTANTIATE_TEST_SUITE_P(PrattParserMacroTest, PrattParserMacroTest,
15761576
testing::ValuesIn(GetMacroTestCases()),
15771577
TestName<MacroTestCase>);
15781578

1579-
TEST(PrattParserMacroErrorTest, ReportError) {
1580-
auto builder = NewPrattParserBuilder();
1581-
ASSERT_OK_AND_ASSIGN(
1582-
auto error_macro,
1583-
Macro::Global("bad_macro", 1,
1584-
[](MacroExprFactory& macro_factory,
1585-
absl::Span<Expr> args) -> std::optional<Expr> {
1586-
return macro_factory.ReportError("custom macro error");
1587-
}));
1588-
1589-
ASSERT_THAT(builder->AddMacro(error_macro), IsOk());
1590-
ASSERT_OK_AND_ASSIGN(auto parser, builder->Build());
1591-
1592-
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("42 + bad_macro(x)"));
1593-
std::vector<cel::ParseIssue> issues;
1594-
auto ast = parser->Parse(*source, &issues);
1595-
EXPECT_THAT(ast, StatusIs(absl::StatusCode::kInvalidArgument));
1596-
EXPECT_EQ(FormatIssues(*source, issues),
1597-
"ERROR: <input>:1:6: custom macro error\n"
1598-
" | 42 + bad_macro(x)\n"
1599-
" | .....^");
1600-
}
1601-
1602-
TEST(PrattParserMacroErrorTest, ReportErrorAt) {
1603-
auto builder = NewPrattParserBuilder();
1604-
ASSERT_OK_AND_ASSIGN(
1605-
auto error_at_macro,
1606-
Macro::Global("bad_macro_at", 1,
1607-
[](MacroExprFactory& macro_factory,
1608-
absl::Span<Expr> args) -> std::optional<Expr> {
1609-
return macro_factory.ReportErrorAt(args[0],
1610-
"custom error at arg");
1611-
}));
1612-
1613-
ASSERT_THAT(builder->AddMacro(error_at_macro), IsOk());
1614-
ASSERT_OK_AND_ASSIGN(auto parser, builder->Build());
1615-
1616-
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("bad_macro_at(x)"));
1617-
std::vector<cel::ParseIssue> issues;
1618-
auto ast = parser->Parse(*source, &issues);
1619-
EXPECT_THAT(ast, StatusIs(absl::StatusCode::kInvalidArgument));
1620-
EXPECT_EQ(FormatIssues(*source, issues),
1621-
"ERROR: <input>:1:14: custom error at arg\n"
1622-
" | bad_macro_at(x)\n"
1623-
" | .............^");
1624-
}
1625-
16261579
TEST(PrattParserMacroCallsTest, MacroCallsDisabledByDefault) {
16271580
cel::ParserOptions options;
16281581
options.add_macro_calls = false;
@@ -1737,5 +1690,77 @@ TEST(PrattParserMacroCallsTest, NestedMacroCallsUseCopyAndReplaceReplacer) {
17371690
)"));
17381691
}
17391692

1693+
TEST(PrattParserMacroErrorTest, ReportError) {
1694+
auto builder = NewPrattParserBuilder();
1695+
ASSERT_OK_AND_ASSIGN(
1696+
auto error_macro,
1697+
Macro::Global("bad_macro", 1,
1698+
[](MacroExprFactory& macro_factory,
1699+
absl::Span<Expr> args) -> std::optional<Expr> {
1700+
return macro_factory.ReportError("custom macro error");
1701+
}));
1702+
1703+
ASSERT_THAT(builder->AddMacro(error_macro), IsOk());
1704+
ASSERT_OK_AND_ASSIGN(auto parser, builder->Build());
1705+
1706+
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("42 + bad_macro(x)"));
1707+
std::vector<cel::ParseIssue> issues;
1708+
auto ast = parser->Parse(*source, &issues);
1709+
EXPECT_THAT(ast, StatusIs(absl::StatusCode::kInvalidArgument));
1710+
EXPECT_EQ(FormatIssues(*source, issues),
1711+
"ERROR: <input>:1:6: custom macro error\n"
1712+
" | 42 + bad_macro(x)\n"
1713+
" | .....^");
1714+
}
1715+
1716+
TEST(PrattParserMacroErrorTest, ReportErrorAt) {
1717+
auto builder = NewPrattParserBuilder();
1718+
ASSERT_OK_AND_ASSIGN(
1719+
auto error_at_macro,
1720+
Macro::Global("bad_macro_at", 1,
1721+
[](MacroExprFactory& macro_factory,
1722+
absl::Span<Expr> args) -> std::optional<Expr> {
1723+
return macro_factory.ReportErrorAt(args[0],
1724+
"custom error at arg");
1725+
}));
1726+
1727+
ASSERT_THAT(builder->AddMacro(error_at_macro), IsOk());
1728+
ASSERT_OK_AND_ASSIGN(auto parser, builder->Build());
1729+
1730+
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("bad_macro_at(x)"));
1731+
std::vector<cel::ParseIssue> issues;
1732+
auto ast = parser->Parse(*source, &issues);
1733+
EXPECT_THAT(ast, StatusIs(absl::StatusCode::kInvalidArgument));
1734+
EXPECT_EQ(FormatIssues(*source, issues),
1735+
"ERROR: <input>:1:14: custom error at arg\n"
1736+
" | bad_macro_at(x)\n"
1737+
" | .............^");
1738+
}
1739+
1740+
TEST(PrattParserErrorRecoveryTest, ErrorRecoveryLimitZero) {
1741+
cel::ParserOptions options;
1742+
options.error_recovery_limit = 0;
1743+
std::vector<cel::ParseIssue> issues;
1744+
auto result = Parse("......", options, &issues);
1745+
EXPECT_THAT(result, StatusIs(absl::StatusCode::kInvalidArgument));
1746+
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("......"));
1747+
EXPECT_EQ(FormatIssues(*source, issues),
1748+
"ERROR: <input>:-1:0: Error recovery limit (0) exceeded");
1749+
}
1750+
1751+
TEST(PrattParserErrorRecoveryTest, ErrorRecoveryLimitOne) {
1752+
cel::ParserOptions options;
1753+
options.error_recovery_limit = 1;
1754+
std::vector<cel::ParseIssue> issues;
1755+
auto result = Parse("......", options, &issues);
1756+
EXPECT_THAT(result, StatusIs(absl::StatusCode::kInvalidArgument));
1757+
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("......"));
1758+
EXPECT_EQ(FormatIssues(*source, issues),
1759+
"ERROR: <input>:1:2: expected identifier\n"
1760+
" | ......\n"
1761+
" | .^\n"
1762+
"ERROR: <input>:-1:0: Error recovery limit (1) exceeded");
1763+
}
1764+
17401765
} // namespace
17411766
} // namespace cel::parser_internal

‎parser/internal/pratt_parser_worker.cc‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include "absl/base/nullability.h"
2222
#include "absl/strings/str_cat.h"
23+
#include "absl/strings/str_format.h"
2324
#include "absl/strings/string_view.h"
2425
#include "common/source.h"
2526
#include "parser/internal/lexer.h"
@@ -50,20 +51,30 @@ std::string ParserWorker::GetTokenText(const Token& tok) const {
5051
}
5152

5253
Token ParserWorker::NextSignificantToken() {
54+
if (is_recovery_limit_exceeded()) {
55+
return Token{.type = TokenType::kEnd, .start = 0, .end = 0};
56+
}
5357
while (true) {
5458
Token tok = lexer_.Lex();
5559
if (tok.type == TokenType::kWhitespace || tok.type == TokenType::kComment) {
5660
continue;
5761
}
5862
if (tok.type == TokenType::kError) {
5963
ReportError(tok, lexer_.GetError().message);
64+
if (is_recovery_limit_exceeded()) {
65+
return Token{.type = TokenType::kEnd, .start = 0, .end = 0};
66+
}
6067
}
6168
return tok;
6269
}
6370
}
6471

6572
Token ParserWorker::NextToken() {
6673
current_token_ = peek_token_;
74+
if (is_recovery_limit_exceeded()) {
75+
peek_token_ = Token{.type = TokenType::kEnd, .start = 0, .end = 0};
76+
return current_token_;
77+
}
6778
if (peek_token_.type != TokenType::kEnd) {
6879
peek_token_ = NextSignificantToken();
6980
}
@@ -75,6 +86,9 @@ bool ParserWorker::Expect(TokenType type, absl::string_view msg) {
7586
NextToken();
7687
return true;
7788
}
89+
if (is_recovery_limit_exceeded()) {
90+
return false;
91+
}
7892
if (peek_token_.type != TokenType::kError) {
7993
std::string err_msg;
8094
if (msg.empty()) {
@@ -98,9 +112,7 @@ bool ParserWorker::Expect(TokenType type, absl::string_view msg) {
98112

99113
void ParserWorker::SynchronizeOnDelimiter() {
100114
if (is_recovery_limit_exceeded()) {
101-
while (peek_token_.type != TokenType::kEnd) {
102-
NextToken();
103-
}
115+
peek_token_ = Token{.type = TokenType::kEnd, .start = 0, .end = 0};
104116
return;
105117
}
106118
while (peek_token_.type != TokenType::kEnd) {
@@ -149,8 +161,20 @@ void ParserWorker::ReportError(int32_t position, absl::string_view msg) {
149161

150162
void ParserWorker::ReportError(const SourceLocation& loc,
151163
absl::string_view msg) {
164+
if (error_count_ > options_.error_recovery_limit) {
165+
return;
166+
}
152167
error_count_++;
153-
if (parse_issues_ != nullptr) {
168+
if (error_count_ == options_.error_recovery_limit + 1) {
169+
if (parse_issues_ != nullptr) {
170+
parse_issues_->push_back(
171+
cel::ParseIssue(absl::StrFormat("Error recovery limit (%d) exceeded",
172+
options_.error_recovery_limit)));
173+
}
174+
peek_token_ = Token{.type = TokenType::kEnd, .start = 0, .end = 0};
175+
}
176+
if (parse_issues_ != nullptr &&
177+
error_count_ <= options_.error_recovery_limit) {
154178
parse_issues_->push_back(cel::ParseIssue(loc, std::string(msg)));
155179
}
156180
}

‎parser/internal/pratt_parser_worker.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ class ParserWorker {
8181

8282
// Error reporting and recovery
8383
bool is_recovery_limit_exceeded() const {
84-
return error_count_ >= options_.error_recovery_limit;
84+
return error_count_ > options_.error_recovery_limit;
8585
}
8686
void ReportError(int32_t position, absl::string_view msg);
8787
void ReportError(const SourceLocation& loc, absl::string_view msg);
@@ -214,7 +214,7 @@ class PrattParserWorker : public ParserWorker {
214214
template <typename ExprNode>
215215
ExprNode PrattParserWorker<ExprNode>::Parse() {
216216
ExprNode expr = ParseExpr();
217-
if (is_recursion_limit_exceeded()) {
217+
if (is_recursion_limit_exceeded() || is_recovery_limit_exceeded()) {
218218
return expr;
219219
}
220220
if (peek_token_.type != TokenType::kEnd &&
@@ -226,7 +226,7 @@ ExprNode PrattParserWorker<ExprNode>::Parse() {
226226

227227
template <typename ExprNode>
228228
ExprNode PrattParserWorker<ExprNode>::ParseExpr() {
229-
if (recursion_limit_exceeded_) {
229+
if (recursion_limit_exceeded_ || is_recovery_limit_exceeded()) {
230230
return ExprNode();
231231
}
232232
if (recursion_depth_ > options_.max_recursion_depth) {

‎parser/parser_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1584,7 +1584,7 @@ TEST(ExpressionTest, TsanOom) {
15841584
.IgnoreError();
15851585
}
15861586

1587-
TEST(ExpressionTest, ErrorRecoveryLimits) {
1587+
TEST_P(ExpressionTest, ErrorRecoveryLimits) {
15881588
ParserOptions options;
15891589
options.error_recovery_limit = 1;
15901590
auto result = Parse("......", "", options);

0 commit comments

Comments
 (0)