[WIP] Sync namespace and folder refactoring (C#)

[genlu]

[Marked as WIP avoid accidental merge, this change should target master. A new PR is created for that but I'd like to keep this around for review purpose]

Implement #14341

As mentioned in dotnet/vblang#351, this will not be provided for VB.

Remaining issues/work that will be addressed in separate PRs

• Fix "undo" for "move file to folder" refactoring
• Enable this refactoring for CPS based project (this requires a crosscutting change between roslyn and project system)
• refactor the language service to support "Move type to new namespace" and "change namespace (selected declaration only)" refactoring. (we might implement those for VB as well, still under discussion)

Just to give you an idea, here's how current implementation works with some basic scenarios. The root namespace for the sample project is ClassLibrary4

• Move file to a new folder to match declared namespace:

• Rename namespace to match folder hierarchy:

[CyrusNajmabadi]

This is still a work-in-progess, but I have enough in place so people can at least comment on the approach I'm using here.

Can you provide a little more info? Just a high level overview of the design, and maybe any particularly interesting points.

For example, does this make your namespaces match your folder structure? Or your folder structure match your namespaces? (or both?).

How do you deal with assembly-names/default-namespace names.

How would this work in something like Roslyn. For example, we have a ton of 'Feature' assembly code with namespace like "namespace Microsoft.CodeAnalysis.SomeFeature". However, we don't create namespace for "Microsoft" or "CodeAnalysis" because of how excessive it would be. Is that handled at all?

[CyrusNajmabadi]

definitely worth calling out in the original description.

[CyrusNajmabadi]

Done with pass. Note:

I then leave it to the person receiving the feedback to decide what to do about each issue and to communicate it. i.e. "will do", "issue resolved with such and such change", "this is what we want, won't fix", etc.

There was a bunch of feedback left in the PR that was never addressed. I would definitely appreciate it if it can be. The feedback is not given lightly :) Outside of nits, it's often there because with my best attempt of understanding the code there were things that were not clear. And that can be a big problem with code maintenance. If someone has to come 6-12 months from now to fixup something, they may often fix things at the wrong level or in hte wrong way if it's not super clear to them what's going on.

Thanks!

[genlu]

Note: i would prefer to just get this into a 'passing' PR state. Then you can just rebase over to master. That way we can keep the comments. I've noticed several of my PR comments were not addressed (or even responded to). So i definitely don't want to lose them.

@CyrusNajmabadi Yes, let's keep using this PR until you signed off. I will rebase & retarget then.

There was a bunch of feedback left in the PR that was never addressed.

For some of your unaddressed comments at least I probably just missed them, I will try to look from the beginning again to make sure. Also it'd be great if you can ping on those to bring them to my attention. Thanks! :)

[genlu]

@CyrusNajmabadi I think I have responded to all your comments. As mentioned, things related to moving language specific logic to attract layer will be done later. Please take a another look and let me know if I missed anything.

[CyrusNajmabadi]

Great! I'm doing async-completion and pull-up first (since i was asked first) :) Will try to review this afterrwards.

[CyrusNajmabadi]

Overall, things are looking quite good. Has there been verification with cases inside roslyn itself (say, at the workspace layer)? We havea bunch of types that are in the wrong namespace. I'd really like to know how well this works in a real-world project.

[genlu]

@CyrusNajmabadi Comments addressed. Please take another look. Thanks!

[genlu]

@CyrusNajmabadi I just tried the "change namespace" action on Roslyn 2.0 snapshot, it seems at least for some of the less prevalent types I tried (e.g. RuleSet, ObjectData, etc), it definitely feels the refactoring is dominated by the time to find all references (i.e. takes no longer than a "find all reference", at least perceptually)

[genlu]

@heejaechang I have made a few more iterations of changes since you last reviewed, do you mind to take another quick look please? I have rebased the PR to target Preview 2 branch here, so you don't have to look at all the existing comments :)

[CyrusNajmabadi]

@CyrusNajmabadi I just tried the "change namespace" action on Roslyn 2.0 snapshot, it seems at least for some of the less prevalent types I tried (e.g. RuleSet, ObjectData, etc), it definitely feels the refactoring is dominated by the time to find all references (i.e. takes no longer than a "find all reference", at least perceptually)

1. What does profiling reveal?
2. Do you need cascading behavior in FindReferences? If not, we should consider prioritizing: I would like a way to find references to a symbol, without cascading being involved. #25628

[genlu]

@CyrusNajmabadi Do you have any concerns that's still blocking? I will profile this and make perf improvement later, given that I will be continue working on this service and implement some related code refactorings.

[CyrusNajmabadi]

consider test where this references a method as well off off an alias qualified name.

[genlu]

Will add in the rebased PR

[CyrusNajmabadi]

I think i'm good at thsi point. Any additional concerns ca come through later PRs. Awesome job!

[genlu]

@CyrusNajmabadi Thanks for the code review! i will compile all remaining issues and file a bug for them