Teacher tool: first pass of nested validators

[srietkerk]

This sets up the ability to test for nested blocks in student projects. I haven't tested this extensively yet, but in order to do that, I need to create a lot of new validators, and I didn't want to put all of that work in one PR.

In this PR:

• The message that gets sent to the editor from the teacher tool has been modified to include all of the validator plans that have been loaded into the teacher tool app. I know we talked about possibly just including the extra plans only when needed, but this had annoying implications on checks everywhere. If we made sending additional plans optional, we would have to check they weren't undefined both before we send the message to the editor and in the editorcontroller, and I found this tedious and unnecessary. We can revisit this, but I would like to leave as is for now.
• I modified the BlocksExist validator to return the list of blocks that match the requiredBlockId specified in blockCounts. This way we can look at the children of a BlocksExist validator when it has a childValidatorPlans list. I also added the passed field so we don't have to check for success on the validator side, we can just look for that in the returned result.
• I changed runValidatorPlanAsync to use a for loop. It was easier for me to conceptualize the nested checks this way. Also, the in the switch statement in the anonymous function in promisePool , the run function results were getting returned, and I wasn't sure how to work within that pattern while running nested checks. So, checks are now run sequentially.
• The ValidatorCheckBase has been updated to include the optional field of childValidatorPlans. Now, any check can have nested checks.
• I changed the return types of the runBlocks__ExistValidation functions in runValidatorPlanAysnc to be a tuple of the blocks that were successful with the validation and the boolean pass/fail of the validation. We can arguably simplify this by just returning the list and checking its length, but I was planning on leaving the validators where we don't want to allow nesting to just return booleans, and I would like there to be some commonality between all of the run functions. I didn't change the return types of the comment validators because the specific block comments validator returns the blocks that don't have comments on them, so there's a discrepancy there.
• A couple more shared validators to test that nesting works were added.

Some screenshots:

[eanders-ms]

Small naming suggestion for clarity.

Suggested change

	planBank: pxt.blocks.ValidatorPlan[];
	planLib: pxt.blocks.ValidatorPlan[];

[eanders-ms]

This method has several array accesses without range checks. Are these arrays guaranteed to be non-empty?

[srietkerk]

For blockId in particular, if Object.keys(inputs.blockCounts) returned nothing, then the validation would be essentially pointless. With this array access, I'm getting the block that we're checking exists for that validator run.

So, for example, if you have this validator:

    {
        ".desc": "A custom variable's value is set to a random number",
        "name": "variable_set_random",
        "threshold": 1,
        "checks": [
            {
                "validator": "blocksExist",
                "blockCounts": {
                    "variables_set": 1
                },
                "childValidatorPlans": ["device_random_used"]
            }
        ]
    }

The block that we're actually wanting to check for is variables_set, and you can only know this in this function by looking in the JSON. For tutorial validation, you can have multiple different blocks in the blockCounts object, but for our validation, we should only ever have one entry in the blockCounts object. Either way, if Object.keys(inputs.blockCounts) was empty at the point that we do an array access in this function, validation would be essentially pointless. However, you're right in that I should check whether the array returned from Object.keys() has length before accessing.

[eanders-ms]

Potential bug here. In the course of iterating over child validator plans, checkPassed can become false then true again with no consequences. Only the last-assigned value of checkPassed is used when computing checksSucceeded below.

[srietkerk]

Yeah, good catch. I think "anding" this will do the trick..

If we have any of the children validators fail, then the whole validator plan should fail, so, if at any moment, checkPassed is false, then that false value should be maintained.

[eanders-ms]

Looks great!