Skip to content
Open
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
1 change: 1 addition & 0 deletions src/extension/alterschema/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ sourcemeta_library(NAMESPACE sourcemeta PROJECT core NAME alterschema
linter/modern_official_dialect_with_empty_fragment.h
linter/then_empty.h
linter/else_empty.h
linter/else_false.h
linter/then_without_if.h
linter/property_names_type_default.h
linter/property_names_default.h
Expand Down
2 changes: 2 additions & 0 deletions src/extension/alterschema/alterschema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ contains_any(const Vocabularies &container,
#include "linter/duplicate_enum_values.h"
#include "linter/duplicate_required_values.h"
#include "linter/else_empty.h"
#include "linter/else_false.h"
#include "linter/else_without_if.h"
#include "linter/enum_to_const.h"
#include "linter/enum_with_type.h"
Expand Down Expand Up @@ -106,6 +107,7 @@ auto add(SchemaTransformer &bundle, const AlterSchemaMode mode)
bundle.add<MinContainsWithoutContains>();
bundle.add<ThenEmpty>();
bundle.add<ElseEmpty>();
bundle.add<ElseFalse>();
bundle.add<ThenWithoutIf>();
bundle.add<DependenciesPropertyTautology>();
bundle.add<DependentRequiredTautology>();
Expand Down
54 changes: 54 additions & 0 deletions src/extension/alterschema/linter/else_false.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
class ElseFalse final : public SchemaTransformRule {
public:
ElseFalse()
: SchemaTransformRule{"else_false",
"`if: S, else: false` collapses to just `S`"} {};

[[nodiscard]] auto
condition(const JSON &schema, const JSON &, const Vocabularies &vocabularies,
const SchemaFrame &, const SchemaFrame::Location &,
const SchemaWalker &, const SchemaResolver &) const
-> SchemaTransformRule::Result override {
Copy link
Member

Choose a reason for hiding this comment

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

I think at this point you can simplify the condition by making it a one liner on a return. That will make it easier to read, as we won't be reading negated conditionals

return contains_any(
vocabularies,
{"https://json-schema.org/draft/2020-12/vocab/applicator",
"https://json-schema.org/draft/2019-09/vocab/applicator",
"http://json-schema.org/draft-07/schema#"}) &&
schema.is_object() && schema.defines("if") &&
schema.defines("else") && is_schema(schema.at("else")) &&
schema.at("else").is_boolean() && !schema.at("else").to_boolean() &&
is_schema(schema.at("if")) &&
(!schema.defines("then") || (schema.at("then").is_boolean() &&
schema.at("then").to_boolean())) &&
(!schema.at("if").is_object() || [&schema](const JSON &if_schema) {
for (const auto &entry : if_schema.as_object()) {
if (schema.defines(entry.first)) {
return false;
}
}
return true;
}(schema.at("if")));
}

auto transform(JSON &schema) const -> void override {
auto if_schema = schema.at("if");
schema.erase("if");
schema.erase("else");
schema.erase("then");

if (if_schema.is_object()) {
for (auto &entry : if_schema.as_object()) {
schema.assign(entry.first, entry.second);
}
} else if (if_schema.is_boolean() && !if_schema.to_boolean()) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this branch will be problematic. You are only keeping certain keywords like $schema but we are not being exhaustive. There could be anchors, or any other thing too. Can we just check against this case in the condition? I think we would need another rule to properly handle this and many of its edge cases

auto metadata = JSON::make_object();
for (const auto &entry : schema.as_object()) {
if (entry.first.starts_with("$")) {
metadata.assign(entry.first, entry.second);
}
}
schema = std::move(metadata);
schema.assign("not", JSON::make_object());
}
}
};
221 changes: 221 additions & 0 deletions test/alterschema/alterschema_lint_2019_09_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2349,3 +2349,224 @@ TEST(AlterSchema_lint_2019_09, non_applicable_type_specific_keywords_3) {

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_1) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"if": {
"properties": {
"flag": {
"const": true
}
}
},
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"properties": {
"flag": {
"const": true
}
}
})JSON");

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_2) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"if": {
"properties": {
"flag": {
"const": true
}
}
},
"then": true,
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"properties": {
"flag": {
"const": true
}
}
})JSON");

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_3) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"if": true,
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema"
})JSON");

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_4) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"if": false,
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"not": {}
})JSON");

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_5) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"type": "object",
"properties": {
"name": { "type": "string" }
},
"if": {
"properties": {
"name": { "type": "number" }
}
},
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"type": "object",
"properties": {
"name": { "type": "string" }
},
"if": {
"properties": {
"name": { "type": "number" }
}
},
"else": false
})JSON");

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_6) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"if": {
"properties": {
"flag": { "const": true }
}
},
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"properties": {
"flag": { "const": true }
}
})JSON");

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_7) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"if": {
"type": "object"
},
"then": false,
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"if": {
"type": "object"
},
"then": false,
"else": false
})JSON");

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_8) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$id": "https://example.com/schema",
"$anchor": "root",
"$comment": "Test schema with metadata",
"if": false,
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$id": "https://example.com/schema",
"$anchor": "root",
"$comment": "Test schema with metadata",
"not": {}
})JSON");

EXPECT_EQ(document, expected);
}

TEST(AlterSchema_lint_2019_09, else_false_9) {
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$id": "https://example.com/complex",
"$anchor": "main",
"$ref": "#/$defs/base",
"$defs": {
"base": { "type": "string" }
},
"if": false,
"else": false
})JSON");

LINT_AND_FIX_FOR_READABILITY(document);

const sourcemeta::core::JSON expected = sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2019-09/schema",
"$id": "https://example.com/complex",
"$anchor": "main",
"$ref": "#/$defs/base",
"$defs": {
"base": { "type": "string" }
},
"not": {}
})JSON");

EXPECT_EQ(document, expected);
}
Loading
Loading