PostProcessChangesAsync(Document, CancellationToken) is not called anymore

[AlexanderEgurtsovDX]

PostProcessChangesAsync(Document, CancellationToken) is not called anymore

[CyrusNajmabadi]

@AlexanderEgurtsovDX What changes are you trying to make in PostProcessChangesAsync? Thanks!

[AlexanderEgurtsovDX]

@AlexanderEgurtsovDX What changes are you trying to make in PostProcessChangesAsync? Thanks!

@CyrusNajmabadi,
We are usually processing every changed document - getting annotated nodes and perform necessary processing, like adding necessary visibility modifiers, namespace references.

[CyrusNajmabadi]

Can you do that in the fixer itself instead of a post processing pass?

[AlexanderEgurtsovDX]

Our entire logic is built on assumption that some operations are done in the post processing pass.

For example, every refactoring that adds type qualifier, also adds annotation to simplify this type(or add it's namespace part as namespace reference). There are at least 30+ such refactorings, so adding such logic inside them is not a good idea.

We can try to recreate the removed logic in our code - get all changed or added documents and post process them on our side. However, there are few disadvantages in this approach:

• there can be unexpected side effects
• we need to distinguish where to apply such logic(for example, VS 2019 and VS 2022 17.10 are working well) for some versions and that can also cause bugs

[CyrusNajmabadi]

The challenge here is that the prior approach was not scalable. every change would run N steps of 'cleanup', which of course would would then fork individual documents, which would then do things like causing generators to run.

THe new approach is non-extensible, and works by applying a single phase of cleanup to all changed documents at once, getting a new fork, then the entire next phase of cleanup to that fork, and so on.

This allowed cost of something like fix-all to drop from around 30 minutes on large solutions to a couple of minutes. While it was nice to allow arbitrary cleanup on a doc level, it just didn't scale, and effectively made it non-viable to do this sort of work.

--

We can try to recreate the removed logic in our code - get all changed or added documents and post process them on our side. However, there are few disadvantages in this approach:

My recommendation would be to keep the individual fixes the same. But have your compute operations function then do the final pass on them. Similarly, create your own FixAllProvider that produces an operation that does the cleanup on the entire operation as a postpass like you do today. Basically, allow the invidiaul fixes to stay the same, but wrap in a common piece of code that has the semantics you want.

[AlexanderEgurtsovDX]

That's a significant breaking change and it's surprising to see it without any notifications, like deprecating the PostProcessChangesAsync first.

From my point of view, mentioned cleanup changes should be done in a separate method(new one), while still allowing the third-party extensions to apply it's own logic if required.

[CyrusNajmabadi]

Will work on getting a notification/deprecation announcement.

From my point of view, mentioned cleanup changes should be done in a separate method(new one), while still allowing the third-party extensions to apply it's own logic if required.

The issue here is that cleanup being document-based simply isn't tenable. It cannot scale, especially in a world with generators. Preferred route if custom cleanup does need to be done is to do something similar to what we did, and do all the work at once across documents. That way the cleanup can be done in parallel, and so that instead of number of forks scaling with the number of passes times the number of documents, it can scale simply with the number of passes.

[AlexanderEgurtsovDX]

That is not the answer I was hoping for, but it's expected.

Thank you for sharing your thoughts on the issue and for your time!