IDE shoudl not attempt to replicate the compiler to generate diagnostics inside lambdas for constructors. #53234
Conversation
CyrusNajmabadi
changed the title
CyrusNajmabadi
changed the title
src/Features/Core/Portable/Diagnostics/Analyzers/UnboundIdentifiersDiagnosticAnalyzerBase.cs
Outdated
Show resolved
Hide resolved
| @@ -6124,5 +6124,82 @@ public class Goo | |||
| } | |||
| }", testHost); | |||
| } | |||
|
|
|||
| [WorkItem(1239, @"https://github.com/dotnet/roslyn/issues/1239")] | |||
There was a problem hiding this comment.
these tests moved. teh standard analyzer supports these once neal did his work to still bind lambdas with syntax errors in them.
| } | ||
| }"); | ||
| } | ||
|
|
||
| [WorkItem(829970, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/829970")] | ||
| [Fact] | ||
| public async Task TestUnknownIdentifierGenericName() |
There was a problem hiding this comment.
we still need these tests for incomplete members until compiler fixes: #7536
| } | ||
|
|
||
| internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace) | ||
| => (new CSharpUnboundIdentifiersDiagnosticAnalyzer(), new GenerateConstructorCodeFixProvider()); |
There was a problem hiding this comment.
same as above. these are not supported by the normal analzyer/fixprovider
|
|
||
| protected override DiagnosticDescriptor DiagnosticDescriptor2 => GetDiagnosticDescriptor(IDEDiagnosticIds.UnboundConstructorId, _constructorOverloadResolutionFailureMessageFormat); | ||
|
|
||
| protected override bool ConstructorDoesNotExist(SyntaxNode node, SymbolInfo info, SemanticModel model) |
There was a problem hiding this comment.
this is horrible. we were effectively adding a pass saying: "oh... compiler didn't report diagnostics. so let's just try to simulate the rules of the language to report the diagnostics we think tehy should be reporting". We should never ever ever ever ever ever ever do this.
There was a problem hiding this comment.
I agree, I see that #32628 is still open. Is the intention to just remove this analyzer and file additional bugs at the compiler level to appropriately report these diagnostics? Either way, I think it makes more sense to remove this analyzer and not introduce bugs in the users code then to keep it in and catch a niche case.
There was a problem hiding this comment.
Is the intention to just remove this analyzer and file additional bugs at the compiler level to appropriately report these diagnostics?
Yes :)
src/EditorFeatures/CSharpTest/GenerateConstructor/GenerateConstructorTests.cs
Show resolved
Hide resolved
| @@ -168,7 +168,6 @@ internal static class IDEDiagnosticIds | |||
| public const string InvokeDelegateWithConditionalAccessId = "IDE1005"; | |||
| public const string NamingRuleId = "IDE1006"; | |||
| public const string UnboundIdentifierId = "IDE1007"; | |||
| public const string UnboundConstructorId = "IDE1008"; | |||
There was a problem hiding this comment.
UnboundIdentifierId still needs to go too, but that's harder and we need compiler support here for that.
ghost
added this to the 
Fixes #38822
Opening this to see what fails, adn thus what we can file semantic-model issues on.