chalet-org/chalet

Bug and Inconvenience in JSON parsing

HerrNamenlos123 opened this issue · 5 comments

I created this discussion: #339
And I found a few things:

I define this target:

"cppgfx": {
  "kind": "cmakeProject",
  "location": "${external:cppgfx-src}",
  "defines": [
    "test=ON"
  ],
  "targets": [
    "cppgfx"
  ]
},

This works. If I completely remove the "defines" value it also works. However, if I leave it empty:

"cppgfx": {
  "kind": "cmakeProject",
  "location": "${external:cppgfx-src}",
  "defines": [],
  "targets": [
    "cppgfx"
  ]
},
>  Reading Build File [chalet.json] ... 
ERROR: Failed to validate the json file: chalet.json
ERROR: Invalid value type found in: targets.cppgfx

Results in this error. This should not happen, I think this is a bug. It is an array, thus I think it should be allowed to have an arbitrary length, including zero.

Now, this same thing also happens when specifying an invalid option, for example:

"cppgfx": {
  "kind": "cmakeProject",
  "location": "${external:cppgfx-src}",
  "thisObjectStaticallyLinks": false,
  "targets": [
    "cppgfx"
  ]
}

thisObjectStaticallyLinks:false is obviously wrong, but it results in the exact same error message.
When trying things like me, you see a lot of options in the documentation, but don't know in which object they belong,
which means that this thing happens a lot, and there is no way to find out what is wrong, other than by trial and error.
I think the error message should say which key is wrong, and if the key is right but only the datatype is wrong, it should print the key, and what datatype it was expecting.

Re empty arrays: All arrays have a minimum length of 1 to avoid clutter. I'm a little hesitant to allow empty arrays. Comments are supported though, so you can comment out the line if you want to.

Re validation error messages: I'm using a fork of a library for json schema validation, and when you see the same validation error like that, it usually means the name of the current node ("thisObjectStaticallyLinks") didn't get returned by the library. I agree that those errors should be more descriptive, so I'll look into that.

Yes, i see that you want this limitation, but then the error message should also say that empty arrays are not allowed. Interesting that comments are allowed, thank you!

This will be fixed in the next release. There were a number of issues stemming from the oneOf JSON schema type.

In the following example:

{
    "name": "new-project",
    "version": "1.0.0",
    "targets": {
        "new-project": {
            "kind": "executable",
            "language": "C++",
            "settings:Cxx": {
                "cppStandard": "c++23",
                "cppModules": true,
                "warningsPreset": "pedantic",
                "bogusNonsense": false, // invalid property
                "staticLinks": [], // empty array
                "libDirs": [
                    "" // empty string
                ],
                "includeDirs": [
                    "src"
                ]
            },
            "files": "src/**.cc"
        }
    },
    "distribution": {
        "all": {
            "kind": "bundle",
            "buildTargets": "*"
        }
    }
}

chalet will output:

ERROR: Failed to validate the file: chalet.json
ERROR: Array property 'staticLinks' was empty, but requires at least one item
ERROR: A string in 'libDirs' is shorter than the minimum length of 1
ERROR: The object 'settings:Cxx' contains an unknown property 'bogusNonsense': Not allowed.

Available in v0.7.8

I noticed this and it feels a lot better that it now says what's wrong. Thank you very much