Linter: add rule to remove empty then else #1819
Conversation
Signed-off-by: karan-palan <karanpalan007@gmail.com>
Karan-Palan
changed the title
| : SchemaTransformRule{ | ||
| "then_else_empty", | ||
| "`then` or `else` with an empty schema does not restrict " | ||
| "validation and is likely ineffective"} {}; |
There was a problem hiding this comment.
"likely ineffective" is wrong. It is not likely. It does nothing, in all cases.
For consistency with other rules, can you make the message: "Setting the then or else keywords to the empty schema does not add any further constraint"
There was a problem hiding this comment.
Actually, just to make the error simpler by just talking about a single keyword, can you split this rule into two? One of then and one for else similar to how we catch then without if and else without if?
| "http://json-schema.org/draft-07/schema#"}) && | ||
| schema.is_object() && schema.defines("if") && | ||
| ((schema.defines("then") && schema.at("then").is_object() && | ||
| schema.at("then").empty()) || |
There was a problem hiding this comment.
As in the other PR, remember that schemas can be either objects or boolean. You are missing checking if then or else are set to true, which is also equivalent to the empty schema (and tests for those)
…er object and boolean schemas both Signed-off-by: karan-palan <karanpalan007@gmail.com>
Signed-off-by: karan-palan <karanpalan007@gmail.com>
Signed-off-by: karan-palan <karanpalan007@gmail.com>
Signed-off-by: karan-palan <karanpalan007@gmail.com>
|
@jviotti, The tests are passing locally, I'll look into the failing checks and resolve them. Ptal if you have some comments till then 
|
| schema.defines("then") && is_schema(schema.at("then")) && | ||
| ((schema.at("then").is_object() && schema.at("then").empty()) || | ||
| (schema.at("then").is_boolean() && schema.at("then").to_boolean() && | ||
| !(schema.at("if").is_boolean() && schema.at("if").to_boolean()))); |
There was a problem hiding this comment.
Is this line really necessary? I guess the rule applies independently of if (and even if if is not there?)
There was a problem hiding this comment.
Ah, is it to avoid conflicts with the other rule that removes dangling then/else without if? I'm still not sure why you check for if being a boolean. Maybe just the schema.defines("if") is enough?
| schema.defines("else") && is_schema(schema.at("else")) && | ||
| ((schema.at("else").is_object() && schema.at("else").empty()) || | ||
| (schema.at("else").is_boolean() && schema.at("else").to_boolean() && | ||
| !(schema.at("if").is_boolean() && schema.at("if").to_boolean()))); |
No description provided.