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

Fix compliation errors in Testing README sample custom verifier #1166

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

MattKotsenas
Copy link
Member

Copy / pasting the sample custom verifier from the bottom of the Testing README results in 9 errors and 2 warnings:

Severity Code Description
Error CS0117 'CSharpAnalyzerVerifier.Test' does not contain a definition for 'TestCode'
Error CS1061 'CSharpAnalyzerVerifier.Test' does not contain a definition for 'ExpectedDiagnostics' and no accessible extension method 'ExpectedDiagnostics' accepting a first argument of type 'CSharpAnalyzerVerifier.Test' could be found (are you missing a using directive or an assembly reference?)
Error CS1061 'CSharpAnalyzerVerifier.Test' does not contain a definition for 'ExpectedDiagnostics' and no accessible extension method 'ExpectedDiagnostics' accepting a first argument of type 'CSharpAnalyzerVerifier.Test' could be found (are you missing a using directive or an assembly reference?)
Error CS1061 'CSharpAnalyzerVerifier.Test' does not contain a definition for 'RunAsync' and no accessible extension method 'RunAsync' accepting a first argument of type 'CSharpAnalyzerVerifier.Test' could be found (are you missing a using directive or an assembly reference?)
Error CS1061 'CSharpAnalyzerVerifier.Test' does not contain a definition for 'RunAsync' and no accessible extension method 'RunAsync' accepting a first argument of type 'CSharpAnalyzerVerifier.Test' could be found (are you missing a using directive or an assembly reference?)
Error CS0117 'CSharpAnalyzerVerifier.Test' does not contain a definition for 'TestCode'
Warning CS8625 Cannot convert null literal to non-nullable reference type.
Warning CS8625 Cannot convert null literal to non-nullable reference type.
Error CS0080 Constraints are not allowed on non-generic declarations
Error CS1520 Method must have a return type
Error CS0305 Using the generic type 'CSharpCodeFixVerifier<TAnalyzer, TCodeFix>' requires 2 type arguments

Which can be addressed with these minor edits:

  1. Remove the default null value for Diagnostic, given that the delegated method is also non-nullable
  2. Fix the location of the generic params on the nested CSharpCodeFixVerifier.Test type
  3. Remove generic constraints on the inner type CSharpCodeFixVerifier.Test (because they belong on the outer type)
  4. Fix constructor name on CSharpCodeFixVerifier.Test

@MattKotsenas MattKotsenas requested a review from a team as a code owner June 21, 2024 14:41
@sharwell
Copy link
Member

sharwell commented Jun 21, 2024

The main issue I have is the discrepancy between the README and the actual code we produce with the new analyzer/code fix project template. The true code is split across several files here:

https://github.com/dotnet/roslyn-sdk/tree/main/src/VisualStudio.Roslyn.SDK/Roslyn.SDK/ProjectTemplates/CSharp/Diagnostic/Test/Verifiers

@MattKotsenas
Copy link
Member Author

MattKotsenas commented Jun 21, 2024

The main issue I have is the discrepancy between the README and the actual code we produce with the new analyzer/code fix project template. The true code is split across several files here:

https://github.com/dotnet/roslyn-sdk/tree/main/src/VisualStudio.Roslyn.SDK/Roslyn.SDK/ProjectTemplates/CSharp/Diagnostic/Test/Verifiers

The code we're currently pointing people to doesn't compile, so I'm happy to make any requested changes, but I don't think the status quo is sufficient here.

Edit: Re-read your comment and looked at the template. Edit below:

Now I understand: these helpers are created as part of the Roslyn Analyzer project template. I hadn't seen that before because I was working on an existing analyzer project, and hence didn't go through the template flow.

In that case, does it make sense to remove this section of the README entirely, and instead point to the code with an explanation that these are provided as part of the template?

@MattKotsenas
Copy link
Member Author

OK, replaced the snippet entirely with a link to the template and a link to instructions on instantiating the template using VS instead.

I don't love this solution because it's another layer of indirection and relies on devs either parsing the template by hand or using Visual Studio, but it addresses the potential for drift between docs.

@sharwell, mind taking another look? Thanks!

@sharwell sharwell merged commit e9aff57 into dotnet:main Jul 2, 2024
9 checks passed
@MattKotsenas MattKotsenas deleted the bugfix/readme-custom-verifier branch July 2, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants