dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.71k stars 3.98k forks source link

SourceGenerator not generating the latest code #70771

Open WeihanLi opened 8 months ago

WeihanLi commented 8 months ago

Keep generating legacy code which I had cleaned in the generator code even I delete all the artifacts

code in my generator:

public static Microsoft.Extensions.DependencyInjection.IServiceScope ScopeActivityInterceptorMethod(this System.IServiceProvider provider)
{
    System.Console.WriteLine("scope creating...");
    var scope = provider.CreateScope();
    var activity = scope.ServiceProvider.GetRequiredService<CSharp12Sample.ActivityScope>();
    System.Console.WriteLine("scope created...");
    return scope;
}

Generated code:

public static Microsoft.Extensions.DependencyInjection.IServiceScope ScopeActivityInterceptorMethod(this System.IServiceProvider provider)
{
    System.Console.WriteLine("logging before...");
    var scope = provider.CreateScope();
    var activity = scope.ServiceProvider.GetRequiredService<CSharp12Sample.ActivityScope>();
    System.Console.WriteLine("logging after...");
}

The generated code is based on the legacy source code changed ~10min ago, is there any cache I could clean up?

WeihanLi commented 8 months ago

Works in another 30 min later, nothing changed

Blokyk commented 7 months ago

If the consuming project <ProjectReference>s the generator, I'm pretty sure this is a known issue (though i'm just a random stranger, don't take my word for it), since those types of references to generators are poorly supported and not really the target scenario.

As a workaround, you can run dotnet build-server shutdown before every build, or set the DOTNET_CLI_USE_MSBUILD_SERVER environment variable to 0 to prevent the dotnet CLI from using the build server (but I'm not sure if all IDEs will respect that setting).

If that doesn't work, another alternative is to create a package for your generator, and give it a constantly increasing version number, e.g. based on the current time (though there's issues with that when using VS or VSCode: https://github.com/microsoft/vscode-dotnettools/issues/698 and for a workaround: https://github.com/dotnet/project-system/issues/1457#issuecomment-510819613). Then, after each build, install that new package version to a local nuget field, and consume the generator's package instead of a project reference (and be sure to use Version="*"). For a more concrete example, you can check this gist.

So yeah, the "Project to Project" scenario is kinda rough around the edges currently, but it's not impossible to workaround. Again, I'm not an expert on MSBuild or Roslyn, I just ran up against this problem earlier and know how confusing and frustrating it can be :/

WeihanLi commented 7 months ago

@Blokyk yeah, I'm using a project reference, thanks very much for the detailed reply, that saved me πŸ‘

glen-84 commented 7 months ago

See also https://twitter.com/glen9184/status/1731583474807525780.

CyrusNajmabadi commented 7 months ago

@glen-84 the recommended way to write a source generator is using tests. You can then easily run, debug, and tweak them easily.

glen-84 commented 7 months ago

@CyrusNajmabadi

I can iterate a bit in a testing context, but having to shut down the build server and perform a clean build each time that I want to see the changes reflected in a dependant project is a really poor DX. Also, testing shouldn't be a requirement, especially during early development/experimentation.

CyrusNajmabadi commented 7 months ago

Testing is realistically multiple orders of magnitude better. It builds faster. Iterates faster. Has no issues around reloading of code. Supplies you with a true regression suite so you don't break things are you iterate. Is trivial to debug. etc. etc. On every metric it is superior (often by an enormous amount).

glen-84 commented 7 months ago

And when you're done testing, and want to see the results in your application? Stop watch-build, shut down build server, clean ... ?

Publishing a package is also not a solution in many cases.

CyrusNajmabadi commented 7 months ago

And when you're done testing, and want to see the results in your application?

Restart and run it. And you see the result. SInce this is rare to happen, the overhead is amortized low.

glen-84 commented 7 months ago

Restarting isn't enough.

Is there no intention to improve this experience?

CyrusNajmabadi commented 7 months ago

Restarting isn't enough.

Why not?

Is there no intention to improve this experience?

We're looking to see if anything can be possible. But it will still always be a much poorer experience than testing, which just makes all of these problems go away, and is incredibly lightweight and easy. There's just so much more machinery involved.

glen-84 commented 7 months ago

Why not?

Look at the OP and my Tweet. πŸ™‚

Changes will not reflect until you run:

People are using all kinds of workarounds to make this work.

I don't use VS myself, but there's also (as you know) #48083 with 54 up-votes.

CyrusNajmabadi commented 7 months ago

Changes will not reflect until you run:

Why do you need to clean? That seems broken.

glen-84 commented 7 months ago

Why do you need to clean? That seems broken.

It's sometimes required if you had run the commands in a different order (f.e. trying to build first, then shut down), but it can be skipped when running shut down followed by build.

This is on WSL (with a Debian distro). Not sure if it'd be different when running directly on Windows.

CyrusNajmabadi commented 7 months ago

Sure. You need to restart the server first. Which is what i was saying. It's sufficient to jsut restart it. Then do your build so it runs the new generator. That's an easy single step. And, if that's too costly, just go to testing, which is orders of magnitude faster :)

glen-84 commented 7 months ago

That's an easy single step.

That's literally 4 steps.

Anyway, I don't know how else to convey this message. The DX is bad, and testing is not the answer to every question. πŸ™‚

CyrusNajmabadi commented 7 months ago

Run dotnet clean. (yes, this seems to be necessary while using this watch workflow)'

This seems to be bug. Someone would have to look at that.

That's literally 4 steps.

Can you just add that to your build script? If you're running it each time, that seems like it would make it simple.

and testing is not the answer to every question

Personally, i disagree. Testing solves all of the above cases, and is fast, reliable, and proven. It's the direction we think people should take. It is expected, from my perspective, that going a different route will not be as smooth and will require additional steps to deal with this.

We can look into improvements here (which would basically be the same as the steps you outlined), but it will still never be as smooth. So it's investing in a poorer experience instead of the prime one.

WeihanLi commented 7 months ago

think testing can be a way, but it's not a required, hoping for a template for the source generator with the test project. Had seen template PR here https://github.com/dotnet/roslyn-sdk/pull/612, hope we could move on and adding the test option for the tests

Blokyk commented 7 months ago

I would also advocate for tests[^1] as the primary catalyst for iterating changes and fixes, although for a small generator (e.g. an SG written only for a specific project) that can be pretty time-consuming to setup.

I do want to add my two cents tho: in my current situation I need to test the behavior of the generated code (since testing the text of the gen'd code would be too cumbersome), and that requires a p2p reference to my generator, which has me run against this issue. While I understand that this might be a relatively rare scenario, it is still pretty frustrating to have to use workarounds like described earlier. Is there an alternative to this?

[^1]: And occasionally using the Workspaces API when encountering bugs in complex projects, although that can also be a subpar experience for source generators

glen-84 commented 7 months ago

We can look into improvements here (which would basically be the same as the steps you outlined), but it will still never be as smooth.

I don't know the internals, but as a developer I would expect:

  1. Make a change to the source generator code.
  2. Build the solution.
  3. The source generator is compiled with the changes.
  4. All projects referencing the updated source generator are compiled while executing the updated source generator code.

(and this would work with watch builds as well)

That would be "smooth".

Although it's different, this is what happens when you make other changes to a referenced project – they are compiled and show up in the referencing projects. Not doing the same is a POLA violation in my opinion, and no amount of testing will fix that.

CyrusNajmabadi commented 7 months ago

I don't know the internals, but as a developer I would expect:

Yes. That would be the restarting as I mentioned.

But, like I also said, it would not be as smooth and would come with its own issues. For example, because the server would have to shut down and restart (to load the newly built generators), you'd lose all the benefits of a compiler server.

Hence why the unit test approach is strictly better and what we will continue to recommend you go by for the best experience.

and no amount of testing will fix that.

I think you'd be surprised. We've gone this route with numerous people and groups and once they get into the better approach they see the value and prefer it.

WeihanLi commented 7 months ago

Is it possible to disable the build server automatically when the generator code changes, do not know how it's implemented internally, just a wonder, so that we do not have to run dotnet build-server shutdown manually each time the generator's code updated

glen-84 commented 7 months ago

For example, because the server would have to shut down and restart (to load the newly built generators), you'd lose all the benefits of a compiler server.

If it's different to how other code changes are handled, could it not be made more similar? I mean, not just internally restarting the server, but actually enabling source generators to be applied without a shut down?

I'm not suggesting that this is a quick fix, but it has to be possible. It's really confusing and frustrating when you're experimenting with a source generator, and the code is never updated. That should not be the default experience. It feels very broken.

Regarding testing, it's like saying from now on, any change to your code will not take effect unless you shut down the build server. But, no worries, you can use testing instead.

CyrusNajmabadi commented 7 months ago

but actually enabling source generators to be applied without a shut down?

No. There is a limitation in .net about loading the same assemblies multiple times in order to execute code in them. We need to shut down the server to be able to load these different versions of the assemblies.

This is not a problem for normal build cases as we're not executing the code within. We're just reading the metadata.

It's really confusing and frustrating when you're experimenting with a source generator, and the code is never updated.

I recommend using unit-tests. they will immediately update, and you can get a fast inner loop of experimenting and validating your changes. Plus, you get tests that then ensure no regressions in behaviors.

Regarding testing, it's like saying from now on, any change to your code will not take effect unless you shut down the build server. But, no worries, you can use testing instead.

Correct. :)

CyrusNajmabadi commented 7 months ago

just a wonder, so that we do not have to run dotnet build-server shutdown manually each time the generator's code updated

You could make this a post build step for your SG project. So once it completes building it kills the server.