Skip to content

Commit

Permalink
LibXML: Include line and column numbers in parse error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@tcl3
tcl3 committed Aug 3, 2024
1 parent e746b2b commit 6f93e19
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 29 deletions.
3 changes: 2 additions & 1 deletion Tests/LibWeb/Text/data/bad.xml
View file
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
<this-will-not-parse
<?xml version="1.0" encoding="UTF-8"?>
<this-will-not-parse</this-will-not-parse>
2 changes: 1 addition & 1 deletion Tests/LibWeb/Text/expected/XML/error-page.txt
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Got load event
[object HTMLDocument]
Failed to parse XML document: Expected '>' at offset 21
Failed to parse XML document: Expected '>' at line: 1, col: 20 (offset 59)
48 changes: 24 additions & 24 deletions Userland/Libraries/LibXML/Parser/Parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ ErrorOr<void, ParseError> Parser::skip_whitespace(Required required)
// S ::= (#x20 | #x9 | #xD | #xA)+
auto matched = m_lexer.consume_while(is_any_of("\x20\x09\x0d\x0a"sv));
if (required == Required::Yes && matched.is_empty())
return parse_error(m_lexer.tell(), "Expected whitespace");
return parse_error(m_lexer.current_position(), "Expected whitespace");

rollback.disarm();
return {};
Expand All @@ -220,12 +220,12 @@ ErrorOr<void, ParseError> Parser::parse_internal()
auto matched_source = m_source.substring_view(0, m_lexer.tell());
if (auto it = find_if(matched_source.begin(), matched_source.end(), s_restricted_characters); !it.is_end()) {
return parse_error(
it.index(),
m_lexer.position_for(it.index()),
ByteString::formatted("Invalid character #{:x} used in document", *it));
}

if (!m_lexer.is_eof())
return parse_error(m_lexer.tell(), "Garbage after document");
return parse_error(m_lexer.current_position(), "Garbage after document");

return {};
}
Expand All @@ -236,7 +236,7 @@ ErrorOr<void, ParseError> Parser::expect(StringView expected)

if (!m_lexer.consume_specific(expected)) {
if (m_options.treat_errors_as_fatal)
return parse_error(m_lexer.tell(), ByteString::formatted("Expected '{}'", expected));
return parse_error(m_lexer.current_position(), ByteString::formatted("Expected '{}'", expected));
}

rollback.disarm();
Expand All @@ -250,7 +250,7 @@ requires(IsCallableWithArguments<Pred, bool, char>) ErrorOr<StringView, ParseErr
auto start = m_lexer.tell();
if (!m_lexer.next_is(predicate)) {
if (m_options.treat_errors_as_fatal)
return parse_error(m_lexer.tell(), ByteString::formatted("Expected {}", description));
return parse_error(m_lexer.current_position(), ByteString::formatted("Expected {}", description));
}

m_lexer.ignore();
Expand All @@ -271,7 +271,7 @@ requires(IsCallableWithArguments<Pred, bool, char>) ErrorOr<StringView, ParseErr

if (m_lexer.tell() == start) {
if (m_options.treat_errors_as_fatal) {
return parse_error(m_lexer.tell(), ByteString::formatted("Expected {}", description));
return parse_error(m_lexer.current_position(), ByteString::formatted("Expected {}", description));
}
}

Expand Down Expand Up @@ -353,7 +353,7 @@ ErrorOr<void, ParseError> Parser::parse_version_info()
m_in_compatibility_mode = true;
} else {
if (version_string != "1.1" && m_options.treat_errors_as_fatal)
return parse_error(m_lexer.tell(), ByteString::formatted("Expected '1.1', found '{}'", version_string));
return parse_error(m_lexer.current_position(), ByteString::formatted("Expected '1.1', found '{}'", version_string));
}

m_version = Version::Version11;
Expand Down Expand Up @@ -415,7 +415,7 @@ ErrorOr<void, ParseError> Parser::parse_standalone_document_decl()

auto value = m_lexer.consume_quoted_string();
if (!value.is_one_of("yes", "no"))
return parse_error(m_lexer.tell() - value.length(), "Expected one of 'yes' or 'no'");
return parse_error(m_lexer.position_for(m_lexer.tell() - value.length()), "Expected one of 'yes' or 'no'");

m_standalone = value == "yes";

Expand Down Expand Up @@ -445,7 +445,7 @@ ErrorOr<void, ParseError> Parser::parse_misc()
return {};
}

return parse_error(m_lexer.tell(), "Expected a match for 'Misc', but found none");
return parse_error(m_lexer.current_position(), "Expected a match for 'Misc', but found none");
}

// 2.5.15 Comment, https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-Comment
Expand Down Expand Up @@ -521,7 +521,7 @@ ErrorOr<Name, ParseError> Parser::parse_processing_instruction_target()

if (target.equals_ignoring_ascii_case("xml"sv) && m_options.treat_errors_as_fatal) {
return parse_error(
m_lexer.tell() - target.length(),
m_lexer.position_for(m_lexer.tell() - target.length()),
"Use of the reserved 'xml' name for processing instruction target name is disallowed");
}

Expand Down Expand Up @@ -579,7 +579,7 @@ ErrorOr<void, ParseError> Parser::parse_doctype_decl()
auto declarations = TRY(parse_external_subset());
if (!m_lexer.is_eof()) {
return parse_error(
m_lexer.tell(),
m_lexer.current_position(),
ByteString::formatted("Failed to resolve external subset '{}': garbage after declarations", doctype.external_id->system_id.system_literal));
}
doctype.markup_declarations.extend(move(declarations));
Expand Down Expand Up @@ -634,7 +634,7 @@ ErrorOr<void, ParseError> Parser::parse_element()

// Well-formedness constraint: The Name in an element's end-tag MUST match the element type in the start-tag.
if (m_options.treat_errors_as_fatal && closing_name != tag.name)
return parse_error(tag_location, "Invalid closing tag");
return parse_error(m_lexer.position_for(tag_location), "Invalid closing tag");

rollback.disarm();
return {};
Expand Down Expand Up @@ -720,7 +720,7 @@ ErrorOr<ByteString, ParseError> Parser::parse_attribute_value_inner(StringView d

if (m_lexer.next_is('<')) {
// Not allowed, return a nice error to make it easier to debug.
return parse_error(m_lexer.tell(), "Unescaped '<' not allowed in attribute values");
return parse_error(m_lexer.current_position(), "Unescaped '<' not allowed in attribute values");
}

if (m_lexer.next_is('&')) {
Expand Down Expand Up @@ -774,7 +774,7 @@ ErrorOr<Variant<Parser::EntityReference, ByteString>, ParseError> Parser::parse_
}

if (!code_point.has_value() || !s_characters.contains(*code_point))
return parse_error(reference_start, "Invalid character reference");
return parse_error(m_lexer.position_for(reference_start), "Invalid character reference");

TRY(expect(";"sv));

Expand Down Expand Up @@ -995,7 +995,7 @@ ErrorOr<Optional<MarkupDeclaration>, ParseError> Parser::parse_markup_declaratio
return Optional<MarkupDeclaration> {};
}

return parse_error(m_lexer.tell(), "Expected one of elementdecl, attlistdecl, entitydecl, notationdecl, PI or comment");
return parse_error(m_lexer.current_position(), "Expected one of elementdecl, attlistdecl, entitydecl, notationdecl, PI or comment");
}

// 2.8.28a DeclSep, https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-DeclSep
Expand All @@ -1016,7 +1016,7 @@ ErrorOr<Optional<ByteString>, ParseError> Parser::parse_declaration_separator()
return Optional<ByteString> {};
}

return parse_error(m_lexer.tell(), "Expected either whitespace, or a PEReference");
return parse_error(m_lexer.current_position(), "Expected either whitespace, or a PEReference");
}

// 4.1.69 PEReference, https://www.w3.org/TR/2006/REC-xml11-20060816/#NT-PEReference
Expand Down Expand Up @@ -1269,7 +1269,7 @@ ErrorOr<ElementDeclaration::ContentSpec, ParseError> Parser::parse_content_spec(
if (auto result = parse_name(); !result.is_error())
names.set(result.release_value());
else
return parse_error(m_lexer.tell(), "Expected a Name");
return parse_error(m_lexer.current_position(), "Expected a Name");
}
TRY(skip_whitespace());
TRY(expect(")*"sv));
Expand Down Expand Up @@ -1331,7 +1331,7 @@ ErrorOr<ElementDeclaration::ContentSpec, ParseError> Parser::parse_content_spec(
TRY(expect(")"sv));

if (choices.size() < 2)
return parse_error(m_lexer.tell(), "Expected more than one choice");
return parse_error(m_lexer.current_position(), "Expected more than one choice");

TRY(skip_whitespace());
auto qualifier = parse_qualifier();
Expand Down Expand Up @@ -1709,7 +1709,7 @@ ErrorOr<ByteString, ParseError> Parser::resolve_reference(EntityReference const&
{
static HashTable<Name> reference_lookup {};
if (reference_lookup.contains(reference.name))
return parse_error(m_lexer.tell(), ByteString::formatted("Invalid recursive definition for '{}'", reference.name));
return parse_error(m_lexer.current_position(), ByteString::formatted("Invalid recursive definition for '{}'", reference.name));

reference_lookup.set(reference.name);
ScopeGuard remove_lookup {
Expand Down Expand Up @@ -1737,17 +1737,17 @@ ErrorOr<ByteString, ParseError> Parser::resolve_reference(EntityReference const&
},
[&](EntityDefinition const& definition) -> ErrorOr<void, ParseError> {
if (placement == ReferencePlacement::AttributeValue)
return parse_error(m_lexer.tell(), ByteString::formatted("Attribute references external entity '{}'", reference.name));
return parse_error(m_lexer.current_position(), ByteString::formatted("Attribute references external entity '{}'", reference.name));

if (definition.notation.has_value())
return parse_error(0u, ByteString::formatted("Entity reference to unparsed entity '{}'", reference.name));
return parse_error(m_lexer.position_for(0), ByteString::formatted("Entity reference to unparsed entity '{}'", reference.name));

if (!m_options.resolve_external_resource)
return parse_error(0u, ByteString::formatted("Failed to resolve external entity '{}'", reference.name));
return parse_error(m_lexer.position_for(0), ByteString::formatted("Failed to resolve external entity '{}'", reference.name));

auto result = m_options.resolve_external_resource(definition.id.system_id, definition.id.public_id);
if (result.is_error())
return parse_error(0u, ByteString::formatted("Failed to resolve external entity '{}': {}", reference.name, result.error()));
return parse_error(m_lexer.position_for(0), ByteString::formatted("Failed to resolve external entity '{}': {}", reference.name, result.error()));

resolved = result.release_value();
return {};
Expand All @@ -1767,7 +1767,7 @@ ErrorOr<ByteString, ParseError> Parser::resolve_reference(EntityReference const&
return "'";
if (reference.name == "quot")
return "\"";
return parse_error(0u, ByteString::formatted("Reference to undeclared entity '{}'", reference.name));
return parse_error(m_lexer.position_for(0), ByteString::formatted("Reference to undeclared entity '{}'", reference.name));
}

StringView resolved_source = *resolved;
Expand Down
6 changes: 3 additions & 3 deletions Userland/Libraries/LibXML/Parser/Parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
namespace XML {

struct ParseError {
size_t offset;
LineTrackingLexer::Position position {};
ByteString error;
};

Expand Down Expand Up @@ -185,7 +185,7 @@ class Parser {
if (rule_name.starts_with("parse_"sv))
rule_name = rule_name.substring_view(6);
m_parse_errors.append({
error.offset,
error.position,
ByteString::formatted("{}: {}", rule_name, error.error),
});
}
Expand Down Expand Up @@ -219,6 +219,6 @@ template<>
struct AK::Formatter<XML::ParseError> : public AK::Formatter<FormatString> {
ErrorOr<void> format(FormatBuilder& builder, XML::ParseError const& error)
{
return Formatter<FormatString>::format(builder, "{} at offset {}"sv, error.error, error.offset);
return Formatter<FormatString>::format(builder, "{} at line: {}, col: {} (offset {})"sv, error.error, error.position.line, error.position.column, error.position.offset);
}
};

0 comments on commit 6f93e19

Please sign in to comment.