Skip to content
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

Fix cpp constraints #2614

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

PMLavigne
Copy link

Description

  • Fix code in CPlusPlusRenderer that was detecting the C++ type incorrectly, as it was doing it in a way where optional types will never have constraints applied. The code was assuming the cppType function would only return a single string, but when an optional value is provided it actually returns an array such as [ 'std::optional', '<', 'int64_t', '>']. Comparing that against a string will always fail, and whether or not it's optional doesn't actually matter in this case anyway as we're just using cppType to detect which kind of constraints to apply to the underlying type.

  • Fix code in CPlusPlusRenderer where constraints with a value of zero (such as a minimum value of 0) would not get applied because the code was just checking for truthiness. Code now checks for truthiness or if the value is explicitly zero, so null and undefined should still be excluded correctly.

Motivation and Context

Generating C++ code from a JSON schema that had constraints would generate code to handle the constraints, but set all the constraint values to null (thus bypassing them entirely).

Previous Behaviour / Output

Given this JSON Schema as input:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$ref": "#/definitions/Test",
  "definitions": {
    "Test": {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "testProp": {
          "type": "number",
          "minimum": 0.0,
          "maximum": 1.0
        },
        "testPropInt": {
          "type": "integer",
          "minimum": 0,
          "maximum": 1
        },
        "testPropString": {
          "type": "string",
          "minLength": 0,
          "maxLength": 1
        }
      },
      "required": [
      ],
      "title": "Test"
    }
  }
}

Quicktype would generate the following C++ class initializer (details vary depending on settings, but the symptom is always the same):

...
class Test {
    public:
    Test() :
        test_prop_constraint(boost::none, boost::none, boost::none, boost::none, boost::none, boost::none, boost::none),
        test_prop_int_constraint(boost::none, boost::none, boost::none, boost::none, boost::none, boost::none, boost::none),
        test_prop_string_constraint(boost::none, boost::none, boost::none, boost::none, boost::none, 1, boost::none)
    {}
...

Note how the constraint objects are initialized entirely with "none" values, or in the case of test_prop_string_constraint the 0 is ignored (in the string's case, a minimum length check of 0 isn't actually that useful, but it shows another hidden bug that I can't really demonstrate on the other types in the original code: that if a value of 0 is specified, it's ignored due to only checking for truthiness)

New Behaviour / Output

Given the same input above, quicktype now generates the following:

...
class Test {
    public:
    Test() :
        test_prop_constraint(boost::none, boost::none, 0, 1, boost::none, boost::none, boost::none),
        test_prop_int_constraint(0, 1, boost::none, boost::none, boost::none, boost::none, boost::none),
        test_prop_string_constraint(boost::none, boost::none, boost::none, boost::none, 0, 1, boost::none)
    {}
...

Note that the constraints are actually correctly filled out this time.

How Has This Been Tested?

Tested using the above test schema, as well as the actual schema for my project (which I can't post publicly, sorry).

 - Fix code in CPlusPlusRenderer that was detecting the C++ type incorrectly, as it was doing it in a way where optional types will never have constraints applied
 - Fix code in CPlusPlusRenderer where constraints with a value of zero (such as a minimum value of 0) would not get applied because the code was just checking for falsyness rather than whether a value was undefined.
 - Explicitly check for truthy value or zero instead of comparing to undefined, as the value could possibly be null as well
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant