Remove unneeded project references in EditorFeatures layer

[Youssef1313]

While working on #51469, I noticed that C#-specific projects were being restored when running dotnet test on these VB-specific tests.

These package references shouldn't be needed. But let's see if this breaks anything.

[Youssef1313]

I actually noticed that this is the only reference that is no longer being built as part of this change. So other references will still be existing as transitive references. Should I only remove this one? or it's okay to remove all?

[jaredpar]

I don't have strong opinions on whether we should or should not explicitly include transitive references anymore. Historically I was strongly for it but that was for technical reasons. There was a lot of tooling that simply failed to function properly if we didn't have them. That is why I pushed for them to be included and added verification that we did so. Now though the bugs are gone and there is no longer a technical need to do this.

My only issue though is that people understand what the change is doing. For instance, as you noted, this change in no way removes the dependency on the VB libraries from this project. As long as this project continues to reference the core compilers testing project the dependency remains, it's just hidden.

[Youssef1313]

There is a single reference that is completely removed. I can update the PR to only remove this one. But as those technical reasons gone and those bugs are fixed. I personally lean towards removing all that are not needed.

While the removal feels redundant (except for this single reference), it may be beneficial in case whatever project causing the transitive dependency didn't do that any more in future.

But certainly you'd know better than me in this area. So I'm open to both options. Either remove only the single reference that isn't a transitive dependency or remove all unneeded references.

[Youssef1313]

@jasonmalinowski Can you take a look? I'm not sure whether those references should be removed or not. There is no harm removing them I think, but also no much benefit (my initial opinion has changed)

[jasonmalinowski]

@Youssef1313 Yep, will take a look, hopefully this week if that's OK?

[sharwell]

I'm torn; I don't see a lot of value in removing these references, but if someone else does 🤷‍♂️

[jasonmalinowski]

So if the only reason things are added is because we were placating the build validation, I'm OK with removing. I find there's two benefits:

1. It is a lot less confusing when you're trying to understand the actual meaningful set of dependencies. Consider

   roslyn/src/Features/Lsif/Generator/Microsoft.CodeAnalysis.LanguageServerIndexFormat.Generator.csproj

   Lines 61 to 77 in c5713c6

   	<!-- Reference to EditorFeatures should be removed once Hover is moved into the Features layer.
   	See https://github.com/dotnet/roslyn/issues/55142 for details -->
   	<ProjectReference Include="..\..\..\EditorFeatures\Core\Microsoft.CodeAnalysis.EditorFeatures.csproj" />
   	<ProjectReference Include="..\..\..\Workspaces\Core\MSBuild\Microsoft.CodeAnalysis.Workspaces.MSBuild.csproj" />
   	<ProjectReference Include="..\..\..\Workspaces\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.Workspaces.csproj" />
   	<ProjectReference Include="..\..\..\Workspaces\VisualBasic\Portable\Microsoft.CodeAnalysis.VisualBasic.Workspaces.vbproj" />
   	<ProjectReference Include="..\..\Core\Portable\Microsoft.CodeAnalysis.Features.csproj" />
   	<!-- Below is the transitive closure of the project references above to placate BuildBoss. If changes are made above this line,
   	please update the stuff here accordingly. -->
   	<ProjectReference Include="..\..\..\Compilers\Core\Portable\Microsoft.CodeAnalysis.csproj" />
   	<ProjectReference Include="..\..\..\Compilers\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.csproj" />
   	<ProjectReference Include="..\..\..\Compilers\VisualBasic\Portable\Microsoft.CodeAnalysis.VisualBasic.vbproj" />
   	<ProjectReference Include="..\..\..\Workspaces\Core\Portable\Microsoft.CodeAnalysis.Workspaces.csproj" />
   	<ProjectReference Include="..\..\LanguageServer\Protocol\Microsoft.CodeAnalysis.LanguageServer.Protocol.csproj" />
   	<ProjectReference Include="..\..\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.Features.csproj" />
   	<ProjectReference Include="..\..\VisualBasic\Portable\Microsoft.CodeAnalysis.VisualBasic.Features.vbproj" />

   where the extra closure that's obvious is larger than the regular set.
2. It means we don't end up with stale stuff just slowing down builds down the road. If some reference between projects is cleaned up, then not having duplicate references means incremental builds are now faster.

In this case, it's not obvious why we are having these binaries pulled along. If we have tests that are doing cross-language references then absolutely we should keep the references because those tests absolutely need it as a direct reference. If it's not being used but is some tangle then harder to say.

(I'm looking at your other PR first since presumably that one is easier to sort out why stuff is coming along...)