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

Analyzers not being recognized in Dev17 Preview 6 #57293

Closed
akhera99 opened this issue Oct 21, 2021 · 8 comments
Closed

Analyzers not being recognized in Dev17 Preview 6 #57293

akhera99 opened this issue Oct 21, 2021 · 8 comments

Comments

@akhera99
Copy link
Member

Version Used:
Version 17.0.0 Preview 6.0 [31815.197.d17.0]
Steps to Reproduce:
Insert:

public class Class
{
    public void Method()
    {
        int x = 5;
        x = 2;
     }
}

Expected Behavior:

public class Class
{
    public void Method()
    {
        int x = 5;
        x = 2; <----- analyzer on this line indicating redundant assignment
     }
}

Actual Behavior:
No diagnostic is shown.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Oct 21, 2021
mavasani added a commit to mavasani/roslyn that referenced this issue Oct 21, 2021
…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

Fixed with #57294

@mavasani
Copy link
Contributor

Actually, I am going to re-activate this issue to track why we did not report any warning diagnostics for analyzer load failure in IDE, we should have ideally called this method to report diagnostics:

public static DiagnosticData CreateAnalyzerLoadFailureDiagnostic(AnalyzerLoadFailureEventArgs e, string fullPath, ProjectId? projectId, string? language)

@mavasani mavasani reopened this Oct 22, 2021
@mavasani
Copy link
Contributor

Investigated a bit about failure to report analyzer load diagnostics. This is not a regression, we have never had support to report analyzer load failure diagnostics in OOP. Analyzers are loaded/instantiated in both devenv.exe and OOP process, and we do have code to report load failure diagnostics if analyzer fails to load in devenv.exe. For all third party analyzers, any analyzer that fails to load, say due to missing dependency accessed in its ctor, it will fail in both devenv.exe and OOP and we will report those failures as diagnostics in devenv.exe. IDE CodeStyle analyzers are special in the sense that they are defined in Roslyn assemblies (Features with dependencies to Workspaces and Compiler), so the analyzer load is slightly different in both processes. Also, if any analyzer throws an exception during analyzer callbacks while executing in OOP, we do report these as AD0001 diagnostics in error list, we also have tests for it.

In short, I don't think we need to any more work here. This is just a very special case where we don't report load failure diagnostics, and it doesn't affect any other analyzer packages.

@dibarbet
Copy link
Member

dibarbet commented Oct 26, 2021

@mavasani I'm hitting this bug (or at least one with the same symptoms in preview 7 still, even with your change.
Using this build
image
Which is using this commit - https://github.com/dotnet/roslyn/commits/68d3c0e77ff8607adca62a883197a5637a596438

This does not repro in p5.

To repro, download \\mlangfs1\public\v-irjain\RealWorldSoln\MaterialSkin-master.zip , open the sln, open ColorScheme.cs and change Color on line 7 to Colors and note the lack of diagnostics

no_diagnostics

Was found via https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1393716

@dibarbet dibarbet reopened this Oct 26, 2021
@jinujoseph jinujoseph added Bug Urgency-Soon and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 26, 2021
@jinujoseph jinujoseph added this to the 17.0 milestone Oct 26, 2021
@mavasani
Copy link
Contributor

mavasani commented Oct 27, 2021

I have tried to repro this in a bunch of ways, but unable to repro it:

  1. Open solution from VS using the desktop CLR host for OOP
  2. Open solution from VS using the core CLR host for OOP
  3. Open solution from VS started from RoslynDevHive using the core CLR host for OOP (based off Roslyn sources in release/dev17.0 branch)

I always get the red squiggle and lightbulb for Colors. I'll now try the liveshare repro to see if I see any different results.

NOTE: I installed Dev17.0 Preview7 from https://docs.microsoft.com/en-us/visualstudio/releases/2022/release-notes-preview#17.0.0-pre.7.0 and tried, but still unable to repro.

@mavasani
Copy link
Contributor

@jinujoseph @dibarbet @vatsalyaagrawal I have tried to repro this issue on Dev17.0 Preview7 build installed from https://docs.microsoft.com/en-us/visualstudio/releases/2022/release-notes-preview#17.0.0-pre.7.0 and can confirm that I can only repro it under liveshare session. Squiggles work fine in regular mode, seem to just be missing in liveshare mode. See the below gif confirming the same:

ColorSchemeBug

@dibarbet
Copy link
Member

@mavasani I just figured out the repro - it only happens when you re-open the solution with the file already open. When I open VS without the file already open, it does not reproduce. See the gif below.

no_diagnostics_file_open

@mavasani
Copy link
Contributor

Closing this issue based on the offline discussion with @dibarbet

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

No branches or pull requests

4 participants