Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/linter/include/sourcemeta/blaze/linter_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ namespace sourcemeta::blaze {
#pragma warning(disable : 4251 4275)
#endif

/// @ingroup linter
/// An error that represents a missing rule name
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says this is a “missing rule name”, but the error is specifically about a missing schema title (and what() also says “title”); consider aligning the terminology so the public API/docs/message are consistent.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

class SOURCEMETA_BLAZE_LINTER_EXPORT LinterMissingNameError
: public std::exception {
public:
[[nodiscard]] auto what() const noexcept -> const char * override {
return "The schema rule is missing a title";
}
};

/// @ingroup linter
/// An error that represents an invalid schema rule name. The name must
/// consist only of lowercase ASCII letters, digits, underscores, or slashes.
Expand Down
12 changes: 9 additions & 3 deletions src/linter/schema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,15 @@ static auto extract_description(const sourcemeta::core::JSON &schema)
}

static auto extract_title(const sourcemeta::core::JSON &schema) -> std::string {
if (!schema.defines("title") || !schema.at("title").is_string()) {
throw LinterInvalidNameError(
"", "The schema rule title is missing or not a string");
if (!schema.defines("title")) {
throw LinterMissingNameError{};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the exception type for a missing title from LinterInvalidNameError to LinterMissingNameError; consider whether any existing callers (outside tests) rely on catching LinterInvalidNameError for this case.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

}

if (!schema.at("title").is_string()) {
std::ostringstream result;
sourcemeta::core::stringify(schema.at("title"), result);
throw LinterInvalidNameError(std::move(result).str(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For non-string title values, LinterInvalidNameError::identifier() will now carry a stringified JSON value (previously it was empty); consider whether this observable behavior change should be documented and/or locked in via a test assertion.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

"The schema rule title is not a string");
}

auto title{schema.at("title").to_string()};
Expand Down
12 changes: 5 additions & 7 deletions test/linter/linter_schema_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ TEST(Linter, schema_rule_missing_title_throws) {
rule_schema, sourcemeta::core::schema_walker,
sourcemeta::core::schema_resolver,
sourcemeta::blaze::default_schema_compiler),
sourcemeta::blaze::LinterInvalidNameError);
sourcemeta::blaze::LinterMissingNameError);
}

TEST(Linter, schema_rule_non_string_title_throws) {
Expand Down Expand Up @@ -406,7 +406,7 @@ TEST(Linter, schema_rule_invalid_name_error_preserves_name) {
}
}

TEST(Linter, schema_rule_missing_title_error_preserves_empty_name) {
TEST(Linter, schema_rule_missing_title_error_message) {
const auto rule_schema{sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2020-12/schema",
"type": "object"
Expand All @@ -418,11 +418,9 @@ TEST(Linter, schema_rule_missing_title_error_preserves_empty_name) {
rule_schema, sourcemeta::core::schema_walker,
sourcemeta::core::schema_resolver,
sourcemeta::blaze::default_schema_compiler);
FAIL() << "Expected LinterInvalidNameError";
} catch (const sourcemeta::blaze::LinterInvalidNameError &error) {
EXPECT_EQ(error.identifier(), "");
EXPECT_STREQ(error.what(),
"The schema rule title is missing or not a string");
FAIL() << "Expected LinterMissingNameError";
} catch (const sourcemeta::blaze::LinterMissingNameError &error) {
EXPECT_STREQ(error.what(), "The schema rule is missing a title");
}
}

Expand Down
Loading