Nice generator, but a couple questions

[CyberBotX]

Hi there. I started using this source generator to replace using PostSharp's NotifyPropertyChanged support and I think it works out great. But I do have a few things I wanted to point out.

First, it seems that the source generator works fully when compiling but during design-time, it seems to break after the first time of applying it to a field. As in, the first field has the proper Intellisense and the generated source is available to see. But once I apply it on a second field, suddenly Visual Studio thinks that the generated code is gone, Intellisense no longer sees the attribute or the generated property. I don't know if this is a problem because of how this source generator works or some other reason.

Second, it seems that the source generator does not transfer the XML documentation from the field to the generated property. Was it intended that the documentation was not transferred or is this something that could be done?

I do like using the source generator as I think it is better than having PostSharp try to add all its INotifyPropertyChanged post-compile and also that their support has a few issues here and there.

[canton7]

First, it seems that the source generator works fully when compiling but during design-time, it seems to break after the first time of applying it to a field. As in, the first field has the proper Intellisense and the generated source is available to see. But once I apply it on a second field, suddenly Visual Studio thinks that the generated code is gone, Intellisense no longer sees the attribute or the generated property

Interesting, I haven't seen anything like that myself.

When you say that VS thinks the generated code is gone, are you looking at the generated code under the 'Analysers' node of the Solution Explorer, or are you referring to something else?

If it works properly during a full build, I rather suspect that any design-time issues have to be a Visual Studio thing. The source generator itself doesn't know the difference between a full build and design-time -- it just gets called on a Compilation.

Second, it seems that the source generator does not transfer the XML documentation from the field to the generated property. Was it intended that the documentation was not transferred or is this something that could be done?

I hadn't even considered that. Should be an easy fix, thanks.

[canton7]

Interesting, I'll have a shot at reproing, thanks for the details. It might be a week or two before I have time though

[CyberBotX]

That's fine. One thing I might suggest, although I haven't written my own SGs and only found this via searching, is maybe using IIncrementalGenerator instead of ISourceGenerator. I've no idea if this would change anything, though. I think it is also a larger change than just changing the interface.

[canton7]

Yeah I was looking into that. There's a bit of re-architecting required (there's already a nice spilt between the analysis and generation already which works well with incremental SGs). The main pain points are going to be 1) The EventArgs cache: the generated code for one class depends on the generated code for all other classes, which won't play well with incremental SGs, and 2) We need to analyse both the semantic model (for most of our info) and the syntax tree (to tell whether a type is partial), which complicates things a bit.

[canton7]

So, I managed to reproduce the "This generator is not generating files" thing. Interestingly, it doesn't generate Attribute.cs either, and that's entirely independent of all of the other generation

I found this issue, which looks similar: dotnet/aspnetcore#38238. Indeed, adding <EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles> shows that the SG is generating updated files as you add new [Notify] properties, as they're present in obj/generated. The problem does seem to be that the VS solution explorer and Intellisense aren't picking them up for some reason.

I'm not sure what I'm doing to trip over this issue -- I'm certain this wasn't happening the version of VS I used to initially develop and test the SG.

[canton7]

Since then, I've had a lot of problems with Microsoft-authored Source Generators hitting this same problem, and have seen other reports on StackOverflow/etc of other users hitting the same, so I'm pretty confident this isn't a problem with PropertyChanged.SourceGenerator.

It also doesn't seem like incremental SGs handle generating from class hierarchies, which would be a hard stop on this SG using them. Discussion here: dotnet/roslyn#60273

[canton7]

Should be an easy fix, thanks.

1d40b8f

I'll hold off on a new release for now, in case anything needs to be done for your first point.

[canton7]

So it looks like my hunch was probably right: dotnet/roslyn#49249 (comment)

I'd still like to understand what exception PropertyChanged.SourceGenerator is throwing here, as it's probably indicative of something I'm mis-handling. So if you have any luck reproing (and I think my suggestion of putting a Debugger.Break() inside a catch there is still a good one), or you manage to create a repro which I can run, I'm still very interested!

[CyberBotX]

So I added a Debugger.Launch() (so I could have it launch a new debugger instance as soon as something happened) and was getting an XML exception about root element not found (I didn't copy the exception at the time because I wanted to dig into it more first). So after digging in more (by launching the debugger before the try block and stepping through lines one at a time until it hit the exception), turns out it was one of my fields that had [Notify] on it had ill-formed XML documentation in the form of a missing </summary>. So technically it was my fault, not the generator's fault. Once I fixed that, everything started working as expected, but I can see why it wasn't failing with 1.0.1 but was with 1.0.2, since you didn't have it parsing XML documentation before.

Still a shame that the current version of Visual Studio can't cope with an exception being thrown by a source generator and also gives no errors in the Error List. Good to see that they'll have it fixed in the future, though.

[canton7]

Nice find, thank you!

I'll fix the xml parsing thing, and I'll also catch all exceptions rather than letting them bubble up to Roslyn (and raise a diagnostic)

[canton7]

Fixed, and I've tweaked the behaviour if we get an unexpected exception while processing a type: we now only fail to generate that type and its children, and we report the exception ourselves rather than chucking it up to Roslyn.

I'll hold off on the release for a couple of days: got one or two more things I want to squeeze into it.

[canton7]

Right, that's released.