-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Report diagnostics when top level statements are used out of order. #42008
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
Report diagnostics when top level statements are used out of order. #42008
Conversation
|
@cston, @RikkiGibson, @dotnet/roslyn-compiler Please review. |
| // extern alias A; | ||
| Diagnostic(ErrorCode.ERR_BadExternAlias, "A").WithArguments("A").WithLocation(2, 14) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} [](start = 8, length = 1)
Is there a test of a type (or namespace) followed by a local function? #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a test of a type (or namespace) followed by a local function?
No, those are no different than any other top-level statements - all of them are wrapped into GlobalStatementSyntax
In reply to: 386529251 [](ancestors = 386529251)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be useful to have such a test so it's clear that is that top-level methods are treated as top-level statements.
It would also make clear the error message is "Top-level statements must precede namespace and type declarations" which might be a little confusing when a member method is treated as a top-level statement because of a misplaced } before the method.
In reply to: 386553202 [](ancestors = 386553202,386529251)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also make clear the error message is "Top-level statements must precede namespace and type declarations" which might be a little confusing when a member method is treated as a top-level statement because of a misplaced } before the method.
Is there any specific suggestion in this statement? Could you please elaborate?
In reply to: 386558178 [](ancestors = 386558178,386553202,386529251)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably a common mistake to copy and paste methods unintentionally as top-level methods so perhaps the diagnostic message should mention methods explicitly. For instance: "Top-level statements and functions must precede namespace and type declarations."
In reply to: 386563032 [](ancestors = 386563032,386558178,386553202,386529251)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance: "Top-level statements and functions must precede namespace and type declarations."
"Top-level functions" is a different feature. Local function are not them. I think, the message is clear enough, it points out that the statement is a top-level statement, so if that is not expected it is pretty clear what to look for (unexpected end of type declaration, etc.). I'll add a PROTOTYPE comment to follow-up if that would be necessary.
In reply to: 386566431 [](ancestors = 386566431,386563032,386558178,386553202,386529251)
|
@RikkiGibson, @dotnet/roslyn-compiler Please review. |
|
@RikkiGibson, @dotnet/roslyn-compiler Please review. |
1 similar comment
|
@RikkiGibson, @dotnet/roslyn-compiler Please review. |
Related to #41704.