Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source generators run very unreliably (especially with multi-targeting) #67123

Closed
Sergio0694 opened this issue Mar 1, 2023 · 48 comments · Fixed by #68451
Closed

Source generators run very unreliably (especially with multi-targeting) #67123

Sergio0694 opened this issue Mar 1, 2023 · 48 comments · Fixed by #68451

Comments

@Sergio0694
Copy link
Contributor

Sergio0694 commented Mar 1, 2023

Version Used: 17.4.3 to 17.6 Preview 1

We're seeing multiple issues being reported of the MVVM Toolkit source generators not working correctly, especially in the 8.1 release, which is the first one that uses Roslyn multi-targeting. This is a major issue for us, as it (1) causes frustration to users and sometimes outright prevents them from using the MVVM Toolkit, and (2) leads them to assume the MVVM Toolkit is at fault here, and then report issues in our repo, where there isn't really much we can do (and I suspect the issue is rather somewhere in Roslyn/VS).

The common symptoms are:

  • IntelliSense not working correctly (red squiggly lines all over the place for generated members)
  • Builds outright being broken with various source generator errors (common one being that they run twice)

We've noticed reports in several configurations, from MAUI to WinUI 3, etc. I have tried to reproduce this and so far I've just been completely unable to do so. This included setting up test projects on various frameworks and inspecting our unit tests. I genuinely have no idea what's going on and why things are breaking up for so many people, and could use some help 🥲

Related issues:

Note: this seems unrelated to the previous issue caused by CsWin32 bundling System.Memory assemblies (which @jaredpar fixed in Roslyn, and the CsWin32 team also addressed on their end), as that was just causing the generators to completely fail to be loaded. This is different: they are loaded but then either result in IntelliSense being broken, or they throw errors, or they just seem to not run at all. I have been unable to find a setup/configuration that consistently reproduces the issue.

Interestingly, several people seem to also report downgrading to 8.0 fixes the issue. The main difference I can see in 8.1 is that we're now using Roslyn multi-targeting. I wonder if there might be some bug in that area that causes Roslyn to sometimes trip up? Not saying this is 100% the cause of the issue, but in some cases it does seem downgrading solves it.

cc. @333fred @sharwell @chsienki

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Mar 1, 2023
@Sastum
Copy link

Sastum commented Mar 5, 2023

Yes, downgrading to 8.0 fixes the issue, at least form me.

@RokG
Copy link

RokG commented Mar 6, 2023

The only issue I had was the one where IntelliSense was broken.
Can also confirm that downgrading to 8.0.0 has resolved this issue for me.
My solution is of type WPF App with three WPF Class Library projects.
The issue was the same on .net5, .net6 and .net7

To sum up my experience on a Related issue:

This seemed completely random, but it looked like it always started happening with properties like:

[ObservableProperty]
[NotifyCanExecuteChangedFor(nameof(SomeMethodCommand))]
private string someProperty

After some time of coding, squiggly lines first appeared under SomeMethodCommand. Then more and more squiggly lines started to appear (for example, under SomeProperty wherever it was used in code.

I tried to find a pattern, to reproduce the issue but I didn't find anything.. The only repeatable thing was that the squiggly lines first appeared under [NotifyCanExecuteChangedFor(nameof(SomeMethodCommand))].

If I used F12 to get to the autogenerated file on some other property within the same .cs file, I could see that there really were no properties that were underlined with the squiggly lines there. So the IntelliSense was working "correctly". Regardless, I could still build and ran the solution without any problems, apart from Hot Reload not working due to these false Errors.

The AutoGenerated files were all in C:\Users\<MyUser>\AppData\Local\Temp\VSGeneratedDocuments\<SomeRandomString>

@majora2007
Copy link

I also had a similar issue with Regex Source Generators ([GeneratedRegex]) showing a squiggly line in Rider, but the solution would build and run completely fine (in dotnet 7).

This was reported and I believe it is indeed a Roslyn issue.
https://youtrack.jetbrains.com/issue/RIDER-85361#focus=Comments-27-6869809.0-0

@CyrusNajmabadi
Copy link
Member

FYI that SGs are not a current priority for the team. So best bet for any progress here would be if someone could determine what is potentially going wrong and submit a fix externally. Not sure when we'll have resources taht would be able to look into this issue. Generally speaking though, i would recommend only minimal use of SGs and not try to make them part of the inner loop where you want IDE features working for them.

@CyrusNajmabadi CyrusNajmabadi added this to the Backlog milestone Mar 6, 2023
@Sergio0694
Copy link
Contributor Author

"i would recommend only minimal use of SGs and not try to make them part of the inner loop where you want IDE features working for them"

This is not really a viable suggestion for us, as it's essentially saying not to use the MVVM Toolkit at all. It's a core component of our dev inner loop both for external consumers and first party apps, so not really advice we can follow 🥲

And I will say again, I do think this hasn't been made clear well enough at all when source generators (even incremental ones) were made available as an API.

@CyrusNajmabadi
Copy link
Member

Source generators should still run fine from teh command line. Just that hte IDE experience may not be good. Right now we don't have any resources we can allocate to that, and we may not be able to get around to it for quite a while (lots of other things happening). It's possible the issues you're facing are due to #67123 and that may improve things.

But outside of that i would expect improvements in the IDE space to be on a best-effort basis. In other words, if someone has spare cycles they're willing to invest into looking at things here, then maybe it gets attention. But otherwise, it will just be on the backlog.

I do think this hasn't been made clear well enough at all when source generators (even incremental ones) were made available as an API.

i can't change the past on this (lots of stuff here was decided/announced/etc. without our involvement). i can only tell you the current state of things, and the resource allocation issues that will likely impact things here.

If interested community members want to pitch in, we'd def appreciate it. But attention here will only be possible after many other high-order divisional items get into a suitable state.

@Balkoth
Copy link

Balkoth commented Mar 7, 2023

If what you are saying is true, then any new feature you guys are implementing should just be ignored, because it might be released in a half-finished state and never get fixed. You guys have a serious communication problem if stuff like this creeps out into the wild.

@TomOeser
Copy link

TomOeser commented Mar 7, 2023

If what you are saying is true, then any new feature you guys are implementing should just be ignored, because it might be released in a half-finished state and never get fixed. You guys have a serious communication problem if stuff like this creeps out into the wild.

So you're saying you can do all this much better? Go ahead - no one is stopping you ;)

Back to topic:
What worked wonderfully for me was specifying a particular platform (32/64) and removing the Any Platform specification from the project settings.

@Balkoth
Copy link

Balkoth commented Mar 7, 2023

I have not said anything like this. It is just you trying to dismiss valid criticism about a released feature that breaks when used in a supported way. If you are fine with it, have it your way. /Ignored

@polymorphicshade
Copy link

I'm here to confirm I have the same issue.
I mistakenly thought it was related to something else, see my outline in #493.

Downgrading to 8.0.0 did not seem to resolve anything, even after deleting my bin/obj folders.

@borrmann
Copy link

borrmann commented Mar 9, 2023

+1, downgrading is the only way I can work with VS and MVVM effectively at the moment. When everything is fine in my solution the SG works fine, but once I have an error somewhere, the SG adds hundreds to thousands more to my solution, so that I have to read the build output to find my actual errors.
The SG is the main reason I am using this toolkit and I would imagine this applies to many other people, so giving this a low priority doesn't make sense to me.
Maybe you could consider to rollback these changes, if you cant solve them at the moment?
Lastly, I really appreciate this great toolkit, so thank you for all your hard work!

@RokG
Copy link

RokG commented Mar 9, 2023

I think I managed to reproduce it..

Pseudocode:

// Property:
[ObservableProperty]
[NotifyCanExecuteChangedFor(nameof(SomeMethodCommand))]
private SomeClass someClassProperty;

// Method:
[RelayCommand(CanExecute = nameof(CanExecuteSomeMethod))]
private async void SomeMethod()
{}

// CanExecute Method
private bool CanExecuteSomeMethod
{
   return SomeClassProperty != null;
}

Code analyzer suggested making the CanExecuteSomeMethod to static, (Which is wrong, because the method was accessing SomeClassProperty), so I did and saved the file.
This underlined the SomeClassProperty and also underlined ALL [NotifyCanExecuteChangedFor(nameof(...))] above any other properties in red.
Then I removed the static from CanExecuteSomeMethod and saved the file again. Now the red line under SomeClassProperty inside the method was gone, but all [NotifyCanExecuteChangedFor(nameof(...))] were still underlined

NOTE: Only the method inside the nameof() of the [NotifyCanExecuteChangedFor(nameof(...))] is underlined in red, not the entire line

EDIT: I can reproduce it like this (from fresh start of Visual Studio and loading solution with the file I'm about to edit is already opened):

  1. Run solution in Debug
  2. Change method to static (properties within method and ...Command methods within nameof() get underlined
  3. Wait for IntelliSense to show errors
  4. Remove static
  5. Wait for IntelliSense to remove errors

Now the ...Command methods within the nameof() stay underlined in red. And this is 100% repeatable.

@polymorphicshade
Copy link

FYI that SGs are not a current priority for the team. So best bet for any progress here would be if someone could determine what is potentially going wrong and submit a fix externally. Not sure when we'll have resources taht would be able to look into this issue. Generally speaking though, i would recommend only minimal use of SGs and not try to make them part of the inner loop where you want IDE features working for them.

I think it's worth noting anecdotally, my team and I can no longer use this toolkit. For us, we almost exclusively use the SGs with the toolkit since we primarily develop XAML-focused MVVM apps. Without restarting Visual Studio, we are forced to use classical INotifyPropertyChanged implementations which basically triples our development time.

Downgrading doesn't seem to fix anything but we will continue to try to find workarounds.

@0xF6
Copy link

0xF6 commented Mar 11, 2023

Why SG still very problematic like a year ago, like now, it works every other time, I constantly have to restart VS
Not ready to using in business? If so, why is this feature not under the feature flag\beta\alpha\etc?

@CyrusNajmabadi
Copy link
Member

Why SG still very problematic like a year ago,

There currently no resources available to invest in the source generator space. We would happily take community involvement to help in areas we're not able to currently focus on. Thanks!

@VincentH-Net
Copy link

@RokG mentioned:

Hot Reload not working due to these false Errors

This is the main problem this issue causes for me: it makes CommunityToolkit.Mvvm above version 8.0.0 unusable for working with C# Markup 2 for Windows App SDK and/or Uno Platform.

Hot Reload is essential for the inner dev loop with C# UI.

Workaround is to downgrade to CommunityToolkit.Mvvm 8.0.0

@Sergio0694
Copy link
Contributor Author

@chsienki @sharwell I know that as @CyrusNajmabadi mentioned there's currently no resources to significantly invest in SG tooling, but this issue is becoming particularly problematic for us, as it's essentially harming our adoption of the new CommunityToolkit.Mvvm release significantly. Like, the amount of people having problems and needing to rollback is just way too high, and I genuinely have no clue on what's going on or how we could help at all.

Given the issues seems to be particularly widespread with 8.1 and not 8.0, would it be possible for someone from the Roslyn team to look into this (I'd be more than happy to assist if needed) so that we could at least maybe narrow down what change we made that's causing Roslyn to choke up so much, and possibly come up with some workaround for the time being? 🥲

@chsienki
Copy link
Contributor

Are these errors specific to the IDE, or do they happen on the command line too?

@VincentH-Net
Copy link

VincentH-Net commented Mar 17, 2023

@chsienki

Are these errors specific to the IDE, or do they happen on the command line too?

In VS Windows, I experience Intellisense errors - there are no build errors. So I would expect they do not happen on the cli
Hard to repro quickly since after switching CommunityToolkit.Mvvm NuGet package version the issue disappears for a while

@Sergio0694
Copy link
Contributor Author

Question for everyone that has experienced issues with the MVVM Toolkit 8.1 and not on 8.0: are you also using any other generators that are working fine? Because from what I can see, there's two main differences between 8.0 and 8.1:

My hypothesis is: it's FAWMN causing problems, so people rolling back to 8.0 are fine because that API isn't used anymore.

Essentially I'd be curious to know whether for people that stopped having issues going back to 8.0, whether they are referencing any other generators that are using FAWMN, and not just eg. ISourceGenerator or other APIs 🤔

cc. @CyrusNajmabadi @jjonescz

@HavenDV
Copy link

HavenDV commented Mar 20, 2023

Is ForAttributeWithMetadataName resistant to copy-paste operations? I notice that Intellisence often breaks when adding new attributes through copy-paste of existing code.

@CyrusNajmabadi
Copy link
Member

There is no concept in generators of what sort of edit caused the change. All changes are processed in the same fashion.

@Sergio0694
Copy link
Contributor Author

"I notice that Intellisence often breaks when adding new attributes through copy-paste of existing code."

@HavenDV just to double check: is that not the case on 8.0? Or does IntelliSense also break down there?

@CyrusNajmabadi
Copy link
Member

FAWMN stresses the underlying incremental engine (lots of interesting things it does). I could easily see it doing something that causes a break in that engine, which then leads to things getting bad.

@christosk92
Copy link

christosk92 commented Mar 21, 2023

I just get IntelliSense errors.

I trust the MVVM toolkit to work fine so I just ignore them and rarely get any build errors. If I do I just do a clean and it's resolved. I don't think it has ever been breaking for me. I'm on the MVVM toolkit 8.2 preview 1 and .NET 8 preview 2

@Sergio0694
Copy link
Contributor Author

Sergio0694 commented Mar 25, 2023

As mentioned before, potentially related to the problem fixed by #66643, which has now been merged and I assume will be in VS 17.6 (and even if it happens to not completely solve all issues here, for sure it seems like a very good improvement) 🎉

cc. @jjonescz since you contributed that fix (thank you!) 😄

@huynhsontung
Copy link

huynhsontung commented Mar 26, 2023

I got the issue with Intellisense using MVVM Toolkit version 8.1 or above in a UWP app project. This also breaks hot reload as others have mentioned. Downgraded to MVVM 8.0 for now.

@jasonmsmith
Copy link

I can confirm that downgrading MVVM Toolkit from 8.1 to 8.0 fixed this problem for me. As others have noted, the problems I experienced were "bogus" errors reported due to the source generators crashing for various reasons. The project always built successfully, but intellisense and error reporting during development would become useless after 10-15 minutes of development and required restarting VS 2022. I experienced this issue using VS 17.5.3.

@Sergio0694
Copy link
Contributor Author

Sergio0694 commented May 16, 2023

Asking here as well (since not everyone is in CommunityToolkit/dotnet#493). Now that Visual Studio 2022 17.6 is out, that should include a lot of fixes to Roslyn that should hopefully mitigate or solve these issues that people were encountering with the MVVM Toolkit. If anyone has a chance to check this on their end, let us know if it did solve the issues for you! Thank you! 🙂

EDIT: from CommunityToolkit/dotnet#493 (comment), it seems the issues are still there, at least in some cases 🥲

@codengine
Copy link

I observed this behavior:
CommunityToolkit/dotnet#493 (comment)

Could anyone deny or confirm that this is the core of the issue?

@jasonmsmith
Copy link

jasonmsmith commented May 19, 2023

Upgrading to VS 17.6.0 64-bit Community Edition did not fix this problem for me. When I upgraded from CommunityToolkit.MVVM 8.0.0 to 8.2.0, I got the same behavior as before; after a few minutes of coding, bogus errors start showing up. Attached is a screenshot of two representative errors. With a few more minutes of coding, there will be dozens more. The most common errors I see are "Ambiguity between Foo.Bar and Foo.Bar" and "Generator XYZ failed to generate source because the hint name 'FooPackage.Foo.Bar.g.cs' must be unique." Despite the errors in the error list, the project compiles normally and runs fine.
Screenshot 2023-05-19 094545

@codengine
Copy link

Upgrading to VS 17.6.0 64-bit Community Edition did not fix this problem for me. When I upgraded from CommunityToolkit.MVVM 8.0.0 to 8.2.0, I got the same behavior as before; after a few minutes of coding, bogus errors start showing up. Attached is a screenshot of two representative errors. With a few more minutes of coding, there will be dozens more. The most common errors I see are "Ambiguity between Foo.Bar and Foo.Bar" and "Generator XYZ failed to generate source because the hint name 'FooPackage.Foo.Bar.g.cs' must be unique." Despite the errors in the error list, the project compiles normally and runs fine. Screenshot 2023-05-19 094545

Out of curiosity: Do you have commands/fields that have the same name as the classes?
Because this was my observation which might cause the issues to arise.

@KuroiRoy
Copy link

KuroiRoy commented Jun 8, 2023

Can confirm this is still an issue. We're running CommunityToolkit.MVVM 8.2.0 and Visual Studio 17.6.2.

Builds work fine, but Visual Studio does not. Restarting VS doesn't help but switching to 8.0.0 fixes it

@CyrusNajmabadi
Copy link
Member

17.6 is very buggy wrt SGs. Would recommend using 17.7 instead.

@codengine
Copy link

17.6 is very buggy wrt SGs. Would recommend using 17.7 instead.

From my knowledge there is no 17.7 yet

@CyrusNajmabadi
Copy link
Member

You can get it here: https://visualstudio.microsoft.com/vs/preview/

It runs sxs with non-preview VS.

@heartacker
Copy link

When can we earn this fix?

@Sergio0694
Copy link
Contributor Author

It was just merged so I assume it should be in VS 17.7 Preview 4 (maybe 3, but that seems unlikely?).

@polymorphicshade
Copy link

@Sergio0694 I'm excited to try this when it's available. Thank you and the team for working on this 😀

@heartacker
Copy link

heartacker commented Jul 9, 2023

when releasing the update, I want to know what update we need to update/install to solve this problem

  1. vscode's c# extension
  2. THE .NET SDK7
  3. VS 2022

@Sergio0694

or something else?

@xlievo
Copy link

xlievo commented Aug 11, 2023

Visual Studio 17.6.6
SG will cache the generated cs file content, causing project generation to fail, while command-line publishing does not have any issues.

@Sergio0694
Copy link
Contributor Author

Try VS 17.7?

@xlievo
Copy link

xlievo commented Aug 11, 2023

Try VS 17.7?
Unfortunately, 17.7.0 also has the same issue.

I tried to describe as clearly as possible,
a class library project for a multi-objective framework netstandard2.0; Net5.0,
The first vs generation was successful, but the second generation failed due to caching the cs file content of the netstandard2.0 target.

The following situations will succeed
1: Command line generation dotnet build --disable-build-servers
2: Change the class library to any individual target framework

So,,, my conclusion is that SG does not support the generation of multi-objective frameworks or has some unknown issues.

@huynhsontung
Copy link

huynhsontung commented Aug 13, 2023

I am on VS 17.7 and I still get this issue :( I did a solution clean and solution rebuild without any luck.
CommunityToolkit.Mvvm v8.2.1

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.