Report CS8802 on every file with top-level statements - #83916
Conversation
|
@dotnet-policy-service agree |
333fred
left a comment
There was a problem hiding this comment.
Overall this looks like an acceptable approach, but I do want to understand what the UX looks like for source generated files.
There was a problem hiding this comment.
Think we should probably test what this looks like with source generated entry point files.
There was a problem hiding this comment.
Good point, from what I understand source generated files are susceptible to having a null filepath, should we fall back to a generic label like generated, or omit the path entirely?
There was a problem hiding this comment.
Sorry, missed that you asked a question and it fell off my radar on vacation. I wouldn't do anything special, I just wanted to see what it looked like and document in a test.
|
Perhaps when multiple files have top-level statements, then we should report a diagnostic in all such files, instead of reporting for "all the files after the first one". That will make it so clicking each diagnostic in the error list will just take you to the right place, rather than having to look at the filename in the message and navigate to it, in order to decide which top-level statements you intended to keep. |
…inerSymbol.cs Co-authored-by: Fred Silberberg <fred@silberberg.xyz>
…sTests.cs Co-authored-by: Fred Silberberg <fred@silberberg.xyz>
|
@RikkiGibson please excuse the late response, some comments escaped my attention. |
No, if you report an error on every file which had top-level statements, then, I would just restore the original message which lacks the file name. |
That makes sense, so we'd emit CS8802 on every file including the first one right? It does seem like a more user friendly approach. Once that's confirmed I'll gladly implement this version of the fix |
Yes, please do. |
|
Thanks @64bitsDuck! Apologies this took so long to review. |
Fixes #83691
Summary
When multiple compilation units have top-level statements, the CS8802 error
message previously only reported the offending file without indicating which
file was already accepted as the entry point. This made it harder to locate
the conflict, especially in larger projects.
This change reports the diagnostic on every compilation unit that has top-level
statements, so clicking each entry in the error list takes you straight to a file
you have to edit.
Changes
SourceMemberContainerSymbol.cs: emit CS8802 for every declaration withtop-level statements when more than one exists, instead of skipping the first
TopLevelStatementsTests.cs: existing tests now expect the additionaldiagnostic on the first file
RemoveUnusedMembersTests.cs: same, for the two analyzer tests that compilemultiple files with top-level statements
Simple_06_02that was conditionalon which tree was reported; both are now always invalid
MultipleTopLevelStatements_ReportedInEveryFileandMultipleTopLevelStatements_SourceGeneratedFileMicrosoft Reviewers: Open in CodeFlow