Take into account that TaskResolver can return undefined

[RomanNikitenko]

Signed-off-by: Roman Nikitenko [email protected]

What it does

Fixes #9411

How to test

I'm able to reproduce the bug for master branch using test plugin from #9207 (comment)
Please see #9207 (review)

1. Install test plugin
2. Open "Test View"
3. Click on a "test" item
4. Task should start successfully, terminal should be open and produces output specified in task.sh

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[tsmaeder]

I think we need to handle the undefined return explicitly.

[tsmaeder]

I believe if we overload the signature like so:

    protected toTaskConfiguration(taskDto: TaskDto): TaskConfiguration;
    protected toTaskConfiguration(taskDto: TaskDto | undefined): TaskConfiguration | undefined;
    protected toTaskConfiguration(taskDto: any): any {

We don't have to check the return value for undefined if the parameter can't be undefined.

[tsmaeder]

That would make the changes in provideTasks and $excuteTask unnecessary

[tsmaeder]

I'm not sure using the unresolved tasks is the right thing to do here: looking at the code in VS Code, they seem to treat that condition as an error.
Think about it this way: if we have a task that is not a contributed task of type 'foo', but defined in a tasks.json file, it will not have an execution set (and therefore no 'taskType' field). So looking up a runner using the taskType (undefined) and type ('foo') fields will fall back to using the ProcessTaskRunner, but executing a custom contributed task will fail in unforeseen ways. I think it's better to handle that condition explicitly.

[RomanNikitenko]

I'm not sure using the unresolved tasks is the right thing to do here: looking at the code in VS Code, they seem to treat that condition as an error.

It looks strange for me, because:

• According to the VS Code documentation (please see a description for resolveTask() method):

A valid default implementation for the resolveTask method is to return undefined

• I tested the use case for VS Code using test plugin, which uses a task resolver which returns undefined. The result - a task is run successfully - no errors.

Do you have steps to reproduce which I can use to confirm that behavior you described above?

[tsmaeder]

I have looked at the vscode source. I guess the test would be a task of a contributed type that is not a customization of a contributed task. That should be achievable if you contribute only a type, but don't return anything from 'provideTasks()'

[RomanNikitenko]

what's the goal to

contribute only a type, but don't return anything from 'provideTasks()'

?

Do you know a VS Code extension which has such behavior?

[tsmaeder]

No, but it's valid according to the documentation 🤷

[tsmaeder]

The goal is to make sure we hit the failure case I was describing. If you know a simpler way to reproduce the scenario, feel free to use it.

[RomanNikitenko]

So, if I understand correctly, the your use case is:

• a plugin contribute a type for tasks
• don't provide any tasks
• returns undefined when resolveTask method is called

So the plugin:

• register task provider to provide nothing
• register task resolver to return undefined

correct?

[tsmaeder]

Yes, except the part where you're configuring a task of that type in a tasks.json file.

[tsmaeder]

I haven't tested anything so far, just inspected the code here and in VS Code.

[tsmaeder]

Relevant code starts here: https://github.com/microsoft/vscode/blob/1c01ce84b502457d61efa87c75cf5ddaf6c40f10/src/vs/workbench/api/common/extHostTask.ts