Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

@stephentoub @joperezr I'm not able to build the repository locally:

Can you help here? I need to build locally to work on the codefix side of this change and be able to debug.

Does build.cmd clr+libs -rc release work?

@stephentoub Unfortunately it doesn't:

I don't know what those 7.0.0-beta* versions are.

@ViktorHofer, can you assist?

The Arcade SDK (which is versioned as 7.0.0-beta) intentionally restores projects that use the Microsoft.NET.Sdk.

The restore is failing presumably because of a transient network issue or because of a proxy. @nkolev92 is there a way to gather more information in this case?

@ViktorHofer The error appears too fast, so I'm not feeling it tries to connect to anything over the network. Maybe it did try to connect the first time I tried to build, then some bad cache is causing this?

The Arcade SDK (which is versioned as 7.0.0-beta) intentionally restores projects that use the Microsoft.NET.Sdk.

The restore is failing presumably because of a transient network issue or because of a proxy. @nkolev92 is there a way to gather more information in this case?

NuGet tries not to do too many fancy things with networking. Usually unless there's auth, a console app should behave similarly.

@Youssef1313 can you try cleaning your local clone and try running build.cmd -s clr+libs -rc Release again? You can do it by running git clean -fdx which will get your local clone to be completely clean. I believe what you are seeing might be because the dotnet sdk restored by our build.cmd command might have failed half-way through, so it left you in a torn state. I would expect that cleaning and re-running build.cmd should fix this.

Thanks a bunch for the contribution, I'm glad we'll be able to support top-level statements with the analyzer now. Quick question though, is there a special reason why you decided to make this PR a draft? Usually we prefer non-draft PRs for contributions like this 😃

@sharwell PTAL Actually scratch that, this is not ready, sorry for the ping.

@joperezr git clean didn't fix it for me. It turned out when I recently updated to a newer .NET 7 preview, the MSBuildSdksPath environment variable was kept pointing to the older SDK, whose directory no longer exists. But not sure why building other repositories still worked. It's restoring now locally, which I hope completes to the end without more problems.

Quick question though, is there a special reason why you decided to make this PR a draft? Usually we prefer non-draft PRs for contributions like this 😃

Roslyn team likes work-in-progress PRs that are not ready for review to be draft. So I thought you would be following the same policy 😄

@sharwell @joperezr This is ready for review. I made some simplifications along the way. It turned into an almost rewrite, I think this is best reviewed by having a general quick look at the old implementation, then reviewing the new code in details. The diff doesn't seem easy to review.

It turned into an almost rewrite, I think this is best reviewed by having a general quick look at the old implementation, then reviewing the new code in details. The diff doesn't seem easy to review.

Would it be possible to split the refactoring/rewrite out of this PR and into a separate PR instead? The original PR was just about fixing support for top-level statements and I think it would be best to just keep this PR about that if possible, and then having the refactoring work separate. That way, if the refactoring work causes an issue or needs to be reverted for some reason, the functional fix would still be kept.

@joperezr I found it harder to support top-level statements without doing some clean up. Maybe a commit by commit review, followed by reading the new codefix code (it's not a big code) will ease reviewing.

@joperezr How can we move forward with this?

sorry @Youssef1313 this fell through my radar.

without doing some clean up.

Would it be possible to have only the required cleanup as part of this PR and then have another one for refactoring? One of the reasons why I'm asking for this is because we are also in the middle of doing some performance measurements on built-in analyzers, fixers and source generators, but don't have any benchmarks check in yet so most of the testing so far has been manual. I would like to avoid if possible a major re-write of the code fixer at the same time we are adding a feature, since the re-write could cause performance issues that we could detect later, at which point we would have to revert and we would lose the feature support.

@joperezr Is it possible we could do the needed benchmarking and see if the PR introduces any performance regressions before the PR is merged?

It is for sure possible; I just don't know how soon I'll be able to run these.

@joperezr I took another look, and it doesn't seem that hard to support top-level with very minimal changes.

I'm going to keep this PR open as a draft to track adding more of the changes here (which include other bug fixes)

Hey @Youssef1313, now that the other PRs are merged, I suppose it is ok to go ahead and close this. If there are still parts of this that we want to take, we can cherry-pick the change even if the PR is closed. I'm closing mainly for house-keeping purposes, but let me know if there is a reason why you want to keep it open instead.