TraceParserGen: fix GetTemplateNameForEvent for empty evnt fields

[heeko]

Fixing an issue that happen when the evnt.Fields is not null but empty. Without this fix, if the evnt.Fields is empty, it will generate this kind of event:

public event Action<Template_0> AddAnnotationStart

instead of:

public event Action<EmptyTraceData> AddAnnotationStart

This can be easily observe by trying to generate a parser for the wpf-etw.man.

I just added a check on the count of evnt.Fields. Let me know if you prefer the null conditional form if (evnt.Fields?.Count > 0 ), I can change that.

[codecov-io]

Codecov Report

Merging #712 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #712   +/-   ##
=======================================
  Coverage   17.52%   17.52%           
=======================================
  Files         221      221           
  Lines      126188   126188           
  Branches    12173    12173           
=======================================
  Hits        22116    22116           
  Misses     103202   103202           
  Partials      870      870

Flag	Coverage Δ
#2017	17.52% <ø> (ø)	⬆️
#Debug	17.52% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dcece49...53dbc75. Read the comment docs.

[idg10]

What may not be obvious to casual observer is that this fixes a bug that causes the tool to generate uncompilable code for a fairly widely used ETW provider manifest: the one for the Microsoft-Windows-WPF provider. So the tool cannot currently be used to generate a working parser for the consuming ETW events from WPF.

(Apparently, at one point there was a pre-generated parser class for this provider, which may be why people haven't reported this before, but the current advice is now not to use that old class, and to generate a parser directly from the manifest using TraceParserGen: #554 So people will now start to run into this. I was about to submit a PR for this until I realised @heeko is ahead of me, so people are definitely running into this bug.)

So in the hope of getting this PR merged sooner, I'm adding this explanation. (I'm 100% certain that @heeko already knows all this. I just suspect it's not entirely obvious to anyone who hasn't just spend a big chunk of time diagnosing this problem.)

The manifest for the WPF provider, wpf-etw.man, has an empty template called Template_0. The code in GetTemplateNameForEvent (prior to the fix in this PR) decides that it needs to use a generated Template_0 class for all events that refer to this empty template. But the code that generates the classes corresponding to templates decides that it does not need to generate a Template_0 class.

This problem occurs because two different but interconnected parts of the code apply incompatible policies here: the GetTemplateNameForEvent and GenerateEventPayloadClass methods need to agree on whether or not a particular event will use a generated custom payload class. Unfortunately, with the code as it is, GetTemplateNameForEvent decides that a custom class is required whenever an event uses a template (even an empty one like Template_0), which causes lots of code that refers to this custom class to be generated. But GenerateEventPayloadClass then decides that because the template defines no fields, no such custom class is required.

GenerateEventPayloadClass performs this check at https://github.com/Microsoft/perfview/blob/master/src/TraceParserGen/TraceParserGen.cs#L339

The allFields.Count <= 0 test is the one that decides that no custom class is required when a template defines no fields.

@heeko's proposed modifies GetTemplateNameForEvent to apply the same policy. It would also be possible to fix this by bringing GenerateEventPayloadClass into line with the policy applied by GetTemplateNameForEvent. However, while that would get rid of the compiler errors, it @heeko's fix is better because it avoids generating an unneeded class.