Don't let run.results be null just because there are no results

[ghost]

If you call ResultLogJsonWriter.WriteResults with an empty results list, it does not write the results property at all. This is because it opens the results array when it encounters the first result.

This is a bug. The spec says that if a scan completes with no results, you should emit an empty results array.

@michaelcfanning FYI

[michaelcfanning]

Bad bug. We need this fixed asap. I will pick it up.

[eddynaka]

@michaelcfanning , I would like to help. In that case an empty least would mean we don't have any result. So, we could throw an ArgunmentException as well. What do you think?

[michaelcfanning]

Hey, @eddynaka, we'd be glad to get your help.

It is actually valid to pass either null or an empty array to WriteResults. A non-nulll empty array is an indicator that analysis completed with no results. A null array can indicate that a catastrophic condition prevented analysis (but a tool can still attempt to complete writing the log file).

So I think the change here should be easy, if you receive a non-null empty array, just call the methods to start and end the results array.

[eddynaka]

Hi @michaelcfanning , well, that solves the problem. But, now I'm facing some issues with the testing. So, what i had to do is change the method CreateCurrentV2SarifLogText to always create the results tag, so when we create the result with no results it would appear.

For the tests where we use the WriteResult / WriteResults methods, those tests work fine. But, for methods that we don't we use that, we don't have the results as expected. For example, ResultLogJsonWriter_WritesAutomationDetails method.

In that case, what do you suggest?

Thanks.

[eddynaka]

What i can also do is create a default parameter to CreateCurrentV2SarifLogText to initialize or not based on a bool value. That would't break the other tests. What do you think?

[michaelcfanning]

Sounds good to me. :)