From 012f4735a7cb7206171c90cad5295a18366ff04f Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Tue, 22 Jul 2025 17:26:34 +0100 Subject: [PATCH] LibWeb/CSS: Introduce structured reporting for CSS errors Instead of random dbgln_if(CSS_PARSER_DEBUG) messages, this lets us report what kind of error it was. Repeated errors are combined instead of spamming the console. Ideally this would also record where the error occurred, but not yet. --- Libraries/LibWeb/CMakeLists.txt | 1 + Libraries/LibWeb/CSS/Parser/ErrorReporter.cpp | 67 ++++++++ Libraries/LibWeb/CSS/Parser/ErrorReporter.h | 150 ++++++++++++++++++ 3 files changed, 218 insertions(+) create mode 100644 Libraries/LibWeb/CSS/Parser/ErrorReporter.cpp create mode 100644 Libraries/LibWeb/CSS/Parser/ErrorReporter.h diff --git a/Libraries/LibWeb/CMakeLists.txt b/Libraries/LibWeb/CMakeLists.txt index a93627d54bc..4f2a13c23ed 100644 --- a/Libraries/LibWeb/CMakeLists.txt +++ b/Libraries/LibWeb/CMakeLists.txt @@ -145,6 +145,7 @@ set(SOURCES CSS/Parser/ArbitrarySubstitutionFunctions.cpp CSS/Parser/ComponentValue.cpp CSS/Parser/DescriptorParsing.cpp + CSS/Parser/ErrorReporter.cpp CSS/Parser/GradientParsing.cpp CSS/Parser/Helpers.cpp CSS/Parser/MediaParsing.cpp diff --git a/Libraries/LibWeb/CSS/Parser/ErrorReporter.cpp b/Libraries/LibWeb/CSS/Parser/ErrorReporter.cpp new file mode 100644 index 00000000000..f5cc24181aa --- /dev/null +++ b/Libraries/LibWeb/CSS/Parser/ErrorReporter.cpp @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2025, Sam Atkins + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include + +namespace Web::CSS::Parser { + +String serialize_parsing_error(ParsingError const& error) +{ + return error.visit( + [](UnknownPropertyError const& error) { + return MUST(String::formatted("Unknown property '{}' in {} rule.", error.property_name, error.rule_name)); + }, + [](UnknownRuleError const& error) { + return MUST(String::formatted("Unknown rule '{}'.", error.rule_name)); + }, + [](UnknownMediaFeatureError const& error) { + return MUST(String::formatted("Unknown media feature '{}'.", error.media_feature_name)); + }, + [](UnknownPseudoClassOrElementError const& error) { + return MUST(String::formatted("Unknown pseudo class or element '{}' in {} selector.", error.name, error.rule_name)); + }, + [](InvalidPropertyError const& error) { + return MUST(String::formatted("Property '{}' in {} rule has invalid value `{}`.", error.property_name, error.rule_name, error.value_string)); + }, + [](InvalidValueError const& error) { + return MUST(String::formatted("Unable to parse {} from `{}`: {}", error.value_type, error.value_string, error.description)); + }, + [](InvalidRuleError const& error) { + return MUST(String::formatted("'{}' rule with prelude `{}` is invalid: {}", error.rule_name, error.prelude, error.description)); + }, + [](InvalidQueryError const& error) { + return MUST(String::formatted("'{}' query `{}` is invalid: {}", error.query_type, error.value_string, error.description)); + }, + [](InvalidSelectorError const& error) { + return MUST(String::formatted("{} selector `{}` is invalid: {}", error.rule_name, error.value_string, error.description)); + }, + [](InvalidPseudoClassOrElementError const& error) { + return MUST(String::formatted("Pseudo '{}' value `{}` is invalid: {}", error.name, error.value_string, error.description)); + }, + [](InvalidRuleLocationError const& error) { + return MUST(String::formatted("'{}' rule is invalid inside {}", error.inner_rule_name, error.outer_rule_name)); + }); +} + +ErrorReporter& ErrorReporter::the() +{ + static ErrorReporter s_error_reporter {}; + return s_error_reporter; +} + +void ErrorReporter::report(ParsingError&& error) +{ + if (auto existing = m_errors.get(error); existing.has_value()) { + existing->occurrences++; + return; + } + + dbgln_if(CSS_PARSER_DEBUG, "CSS parsing error: {}", serialize_parsing_error(error)); + m_errors.set(move(error), { .occurrences = 1 }); +} + +} diff --git a/Libraries/LibWeb/CSS/Parser/ErrorReporter.h b/Libraries/LibWeb/CSS/Parser/ErrorReporter.h new file mode 100644 index 00000000000..736d2451c43 --- /dev/null +++ b/Libraries/LibWeb/CSS/Parser/ErrorReporter.h @@ -0,0 +1,150 @@ +/* + * Copyright (c) 2025, Sam Atkins + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include + +namespace Web::CSS::Parser { + +struct UnknownPropertyError { + FlyString rule_name { "style"_fly_string }; + FlyString property_name; + bool operator==(UnknownPropertyError const&) const = default; + unsigned hash() const + { + return pair_int_hash(rule_name.hash(), property_name.hash()); + } +}; + +struct UnknownRuleError { + FlyString rule_name; + bool operator==(UnknownRuleError const&) const = default; + unsigned hash() const { return rule_name.hash(); } +}; + +struct UnknownMediaFeatureError { + FlyString media_feature_name; + bool operator==(UnknownMediaFeatureError const&) const = default; + unsigned hash() const { return media_feature_name.hash(); } +}; + +struct UnknownPseudoClassOrElementError { + FlyString rule_name { "style"_fly_string }; + FlyString name; + bool operator==(UnknownPseudoClassOrElementError const&) const = default; + unsigned hash() const + { + return pair_int_hash(rule_name.hash(), name.hash()); + } +}; + +struct InvalidPropertyError { + FlyString rule_name { "style"_fly_string }; + FlyString property_name; + String value_string; + String description; + bool operator==(InvalidPropertyError const&) const = default; + unsigned hash() const + { + return pair_int_hash(rule_name.hash(), pair_int_hash(property_name.hash(), pair_int_hash(value_string.hash(), description.hash()))); + } +}; + +struct InvalidValueError { + FlyString value_type; + String value_string; + String description; + bool operator==(InvalidValueError const&) const = default; + unsigned hash() const + { + return pair_int_hash(value_type.hash(), pair_int_hash(value_string.hash(), description.hash())); + } +}; + +struct InvalidRuleError { + FlyString rule_name; + String prelude; + String description; + bool operator==(InvalidRuleError const&) const = default; + unsigned hash() const + { + return pair_int_hash(rule_name.hash(), pair_int_hash(prelude.hash(), description.hash())); + } +}; + +struct InvalidQueryError { + FlyString query_type { "@media"_fly_string }; + String value_string; + String description; + bool operator==(InvalidQueryError const&) const = default; + unsigned hash() const + { + return pair_int_hash(query_type.hash(), pair_int_hash(value_string.hash(), description.hash())); + } +}; + +struct InvalidSelectorError { + FlyString rule_name { "style"_fly_string }; + String value_string; + String description; + bool operator==(InvalidSelectorError const&) const = default; + unsigned hash() const + { + return pair_int_hash(rule_name.hash(), pair_int_hash(value_string.hash(), description.hash())); + } +}; + +struct InvalidPseudoClassOrElementError { + FlyString name; + String value_string; + String description; + bool operator==(InvalidPseudoClassOrElementError const&) const = default; + unsigned hash() const + { + return pair_int_hash(name.hash(), pair_int_hash(value_string.hash(), description.hash())); + } +}; + +struct InvalidRuleLocationError { + FlyString outer_rule_name; + FlyString inner_rule_name; + bool operator==(InvalidRuleLocationError const&) const = default; + unsigned hash() const + { + return pair_int_hash(outer_rule_name.hash(), inner_rule_name.hash()); + } +}; + +using ParsingError = Variant; + +String serialize_parsing_error(ParsingError const&); + +class ErrorReporter { +public: + static ErrorReporter& the(); + + void report(ParsingError&&); + +private: + explicit ErrorReporter() = default; + struct Metadata { + u32 occurrences { 1 }; + }; + HashMap m_errors; +}; + +} + +template<> +struct AK::Traits : public DefaultTraits { + static unsigned hash(Web::CSS::Parser::ParsingError const& error) + { + return error.visit([](auto const& it) { return it.hash(); }); + } +};