-
-
Notifications
You must be signed in to change notification settings - Fork 5
Linter: create rule to remove additional items when items is an object #1805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
767c38f
bb30244
f600c3b
e692ce3
4407107
1cdd055
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
class DanglingAdditionalItems final : public SchemaTransformRule { | ||
public: | ||
DanglingAdditionalItems() | ||
: SchemaTransformRule{ | ||
"dangling_additional_items", | ||
"`additionalItems` is ignored when `items` is an object"} {}; | ||
Karan-Palan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
[[nodiscard]] auto | ||
condition(const sourcemeta::core::JSON &schema, | ||
const sourcemeta::core::JSON &, | ||
const sourcemeta::core::Vocabularies &vocabularies, | ||
const sourcemeta::core::SchemaFrame &, | ||
const sourcemeta::core::SchemaFrame::Location &, | ||
const sourcemeta::core::SchemaWalker &, | ||
const sourcemeta::core::SchemaResolver &) const | ||
-> sourcemeta::core::SchemaTransformRule::Result override { | ||
return contains_any( | ||
vocabularies, | ||
{"https://json-schema.org/draft/2019-09/vocab/applicator", | ||
"http://json-schema.org/draft-07/schema#", | ||
"http://json-schema.org/draft-06/schema#", | ||
"http://json-schema.org/draft-04/schema#"}) && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this rule applies to Draft 3 as well. Can you confirm on the spec? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it does, adding tests for it as well |
||
schema.is_object() && schema.defines("items") && | ||
schema.defines("additionalItems") && schema.at("items").is_object(); | ||
Karan-Palan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
auto transform(JSON &schema) const -> void override { | ||
schema.erase("additionalItems"); | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1661,8 +1661,7 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_5) { | |
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ | ||
"$schema": "https://json-schema.org/draft/2019-09/schema", | ||
"allOf": [ | ||
{ | ||
"type": "integer", | ||
{ "type": "integer", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unintended change? |
||
"$ref": "https://example.com" | ||
} | ||
] | ||
|
@@ -1680,3 +1679,47 @@ TEST(AlterSchema_lint_2019_09, unnecessary_allof_ref_wrapper_5) { | |
|
||
EXPECT_EQ(document, expected); | ||
} | ||
|
||
TEST(AlterSchema_lint_2019_09, dangling_additional_items_1) { | ||
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ | ||
"$schema": "https://json-schema.org/draft/2019-09/schema", | ||
"items": { | ||
"type": "number" | ||
}, | ||
"additionalItems": 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", | ||
"items": { | ||
"type": "number" | ||
} | ||
})JSON"); | ||
|
||
EXPECT_EQ(document, expected); | ||
} | ||
|
||
TEST(AlterSchema_lint_2019_09, dangling_additional_items_2) { | ||
sourcemeta::core::JSON document = sourcemeta::core::parse_json(R"JSON({ | ||
"$schema": "https://json-schema.org/draft/2019-09/schema", | ||
"items": { | ||
"unevaluatedProperties": false | ||
}, | ||
"additionalItems": { | ||
"type": "string" | ||
} | ||
})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", | ||
"items": { | ||
"unevaluatedProperties": false | ||
} | ||
})JSON"); | ||
|
||
EXPECT_EQ(document, expected); | ||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we all it
additional_items_with_schema_items
? Thedangling
part seems confusing at first and other rules follow thewith
andwithout
convention. I thinkadditional_items_with_schema_items
is still short enough and very self-explanatoryThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, you have to update the
CMakeLists.txt
file to list this file too