Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MS.CA.CSharp.Workspaces and MS.CA.VB.Workspaces as VSIX analyzer … #57294

Merged
merged 4 commits into from
Oct 21, 2021

Conversation

mavasani
Copy link
Contributor

…assets

Fixes #57293
With recent changes to move OOP to .NET Core, we seem to require specifying analyzer dependencies as analyzer assets. 6c7f97b added MS.CA.Workspaces as an asset, this change extends it to add the C# and VB Workspaces assemblies as analyzer assets. These assemblies define CSharpCodeStyleOptions and VisualBasicCodeStyleOptions types, which are used in ctors of many of our analyzers.

  • TODO: Add an integration test

…assets

Fixes dotnet#57293
With recent changes to move OOP to .NET Core, we seem to require specifying analyzer dependencies as analyzer assets. dotnet@6c7f97b added MS.CA.Workspaces as an asset, this change extends it to add the C# and VB Workspaces assemblies as analyzer assets.
@mavasani
Copy link
Contributor Author

Tagging @jinujoseph @vatsalyaagrawal for visibility

NOTE: Without this change, all our IDE analyzers that reference CSharpCodeStyleOptions or VisualBasicCodeStyleOptions in their constructor will fail to load, we have large number of such analyzers in our codebase.

Copy link
Member

@akhera99 akhera99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mavasani
Copy link
Contributor Author

@genlu Will probably need your help here, seeing the following build time failure in integration test runs:

##[error].dotnet\sdk\6.0.100-rc.1.21463.6\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(421,5): error NETSDK1112: (NETCORE_ENGINEERING_TELEMETRY=Build) The runtime pack for Microsoft.NETCore.App.Runtime.win-x64 was not downloaded. Try running a NuGet restore with the RuntimeIdentifier 'win-x64'.

@JoeRobich
Copy link
Member

@mavasani Integration tests will be failing until #57281 merges.

@jinujoseph jinujoseph modified the milestones: 17.0.P5, 17.0.P7 Oct 21, 2021
@mavasani
Copy link
Contributor Author

Thank you @JoeRobich

}
");
VisualStudio.Editor.InvokeCodeActionList();
VisualStudio.Editor.Verify.CodeAction("Unnecessary assignment of a value to 'x'");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akhera99 Thanks for adding the test. Shouldn't the code action string here be 'Remove redundant assignment'?

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes sorry, it was late

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akhera99 You may need to fix the added integration test with a separate test-only PR.

@@ -84,6 +84,26 @@ class Program
VisualStudio.Editor.Verify.CodeAction("using System;");
}

[WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedValues)]
[WorkItem(57293, "https://github.com/dotnet/roslyn/issues/57293")]
public void RemoveRedundantAssignmentCodeFix()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to point out that this change is required at the moment to cover Core host sceanrios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants