Skip to content

SyntaxContext's GlobalStatementContext changes#59957

Merged
akhera99 merged 5 commits intodotnet:mainfrom
akhera99:bugs/59931_syntaxcontext
Mar 7, 2022
Merged

SyntaxContext's GlobalStatementContext changes#59957
akhera99 merged 5 commits intodotnet:mainfrom
akhera99:bugs/59931_syntaxcontext

Conversation

@akhera99
Copy link
Copy Markdown
Member

@akhera99 akhera99 commented Mar 4, 2022

#59931

1: Returns false instead of true if we recognize we're in a filescoped namespace
2: abstracts out the global statement context concept

@akhera99 akhera99 requested a review from a team as a code owner March 4, 2022 22:40
@ghost ghost added the Area-IDE label Mar 4, 2022
@@ -223,7 +223,7 @@ public static bool IsBeginningOfGlobalStatementContext(this SyntaxToken token)
return true;

if (token.Parent is FileScopedNamespaceDeclarationSyntax namespaceDeclaration && namespaceDeclaration.SemicolonToken == token)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CyrusNajmabadi is this line necessary at all?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have no idea. did anything break? :)

Comment on lines 225 to +226
if (token.Parent is FileScopedNamespaceDeclarationSyntax namespaceDeclaration && namespaceDeclaration.SemicolonToken == token)
return true;
return false;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is changing to false, we can remove the condition completely, then the code will fallback to the return false; at the end of the method.

But first need to confirm if the new behavior of keyword recommenders is correct.

Copy link
Copy Markdown
Member Author

@akhera99 akhera99 Mar 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I don't believe the 'new' keyword should be available directly after the file-scoped namespace declaration anyways. The check doesn't really seem to add anything.

Edit: the check is still needed because otherwise it gets to the next check and returns true, since FileScopedNamespaceDeclarationSyntax is a MemberDeclarationSyntax

@akhera99 akhera99 enabled auto-merge March 7, 2022 21:17
@akhera99 akhera99 merged commit 724400b into dotnet:main Mar 7, 2022
@ghost ghost added this to the Next milestone Mar 7, 2022
@allisonchou allisonchou modified the milestones: Next, 17.2.P3 Mar 28, 2022
@akhera99 akhera99 deleted the bugs/59931_syntaxcontext branch June 18, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SyntaxContexts IsGlobalStatementContext should take file scoped namespaces into consideration

5 participants