-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Fix SyntaxFactory.Identifier to throw ArgumentNullException instead of NullReferenceException #80699
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 SyntaxFactory.Identifier to throw ArgumentNullException instead of NullReferenceException #80699
Changes from all commits
d9623a1
c5a58d3
e8b11cb
3822098
7632720
c139332
d343133
087eb3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -714,5 +714,21 @@ public void TestParseMethodsKeepParseOptionsInTheTree() | |
| var typeName = SyntaxFactory.ParseTypeName("", options: parseOptions); | ||
| Assert.Same(parseOptions, typeName.SyntaxTree.Options); | ||
| } | ||
|
|
||
| [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/17637")] | ||
| public void Identifier_Null_ThrowsArgumentNullException() | ||
| { | ||
| Assert.Throws<ArgumentNullException>(() => SyntaxFactory.Identifier(text: null)); | ||
| Assert.Throws<ArgumentNullException>(() => | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @CyrusNajmabadi Is this something we can automate or ask copilot to do with |
||
| SyntaxFactory.Identifier(SyntaxFactory.TriviaList(), text: null, SyntaxFactory.TriviaList())); | ||
| Assert.Throws<ArgumentNullException>(() => | ||
| SyntaxFactory.Identifier(SyntaxFactory.TriviaList(), SyntaxKind.IdentifierName, text: null, valueText: "value", SyntaxFactory.TriviaList())); | ||
| Assert.Throws<ArgumentNullException>(() => | ||
| SyntaxFactory.Identifier(SyntaxFactory.TriviaList(), SyntaxKind.IdentifierName, text: "text", valueText: null, SyntaxFactory.TriviaList())); | ||
| Assert.Throws<ArgumentNullException>(() => | ||
| SyntaxFactory.VerbatimIdentifier(SyntaxFactory.TriviaList(), text: null, valueText: "value", SyntaxFactory.TriviaList())); | ||
| Assert.Throws<ArgumentNullException>(() => | ||
| SyntaxFactory.VerbatimIdentifier(SyntaxFactory.TriviaList(), text: "text", valueText: null, SyntaxFactory.TriviaList())); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot Consolidate all the added tests into one single test #Closed
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consolidated all 6 tests into a single test method in commit 8f7b5d3. |
||
| } | ||
| } | ||
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.
Nit: do we want to polyfill
ArgumentNullException.ThrowIfNull?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.
ok with not polyfilling.