Skip to content

Add runtime NullReferenceException validation to foreach null iteration tests #80839

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 23, 2025
Merged

Add runtime NullReferenceException validation to foreach null iteration tests #80839

merged 3 commits into from
Oct 23, 2025

Conversation

Copy link
Contributor

Copilot AI commented Oct 21, 2025 •
edited
Loading

  • Identify all tests added/modified in commit d46a551 that verify no diagnostics for foreach on typed null values
  • Update tests to also execute code and validate NullReferenceException is thrown at runtime
  • Add .VerifyDiagnostics() to all CompileAndVerify calls
  • Build and test the changes
  • Ensure tests follow existing patterns in the repository
Original prompt

This section details on the original issue you should resolve

<issue_title>Consider tests demonstrating a runtime null reference exception when iterating over null.</issue_title>
<issue_description>d46a551 introduced many new tests that demonstrate that it's ok to iterate over a constant null value (though not the null-literal itself). It would be good if the tests that were updated (or added) that have no diagnostics also executed the code and validating that a null-ref exception is thrown. </issue_description>

<agent_instructions>Copilot, can you please take a look at all the tests affected in d46a551 and update those that are reporting no diagnostics to also execute the code and validate that a null-ref exception is thrown at runtime?</agent_instructions>

Comments on the Issue (you are @copilot in this section)

Fixes #80838

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@CyrusNajmabadi
Update foreach null tests to validate runtime NullReferenceException
06293f2 
Updated 9 tests to execute code and verify that NullReferenceException is thrown:
- ForeachOnDefaultArray
- ForeachOnDefaultString
- ForeachOnDefaultIEnumerable
- ForeachOnNullCastToIEnumerable
- ForeachOnNullCastToArray
- ForeachOnNullCastToString
- ForeachOnNullCastToDynamic
- ForeachOnNullCastToMultidimensionalArray
- ForeachOnNullCastToConstrainedTypeParameter

Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
Copilot AI changed the title [WIP] Update tests to validate null reference exception at runtime Add runtime NullReferenceException validation to foreach null iteration tests Oct 21, 2025
Copilot AI requested a review from CyrusNajmabadi October 21, 2025 18:17
Copilot finished work on behalf of CyrusNajmabadi October 21, 2025 18:17
@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review October 21, 2025 18:54
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner October 21, 2025 18:54
@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Oct 21, 2025

@dotnet/roslyn-compiler ptal.

jcouv
jcouv reviewed Oct 21, 2025
View reviewed changes
src/Compilers/CSharp/Test/Semantic/Semantics/ForEachTests.cs Outdated
""";

CreateCompilation(source).VerifyEmitDiagnostics();
CompileAndVerify(source, expectedOutput: "NullReferenceException");
Copy link
Member

@jcouv jcouv Oct 21, 2025

Choose a reason for hiding this comment

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

@copilot Add .VerifyDiagnostics() to all the CompileAndVerify calls you added. For example: CompileAndVerify(source, expectedOutput: "NullReferenceException").VerifyDiagnostics();

Copy link
Contributor Author

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

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

Added .VerifyDiagnostics() to all 9 CompileAndVerify calls in commit 1c2bd78.

@jcouv
Add VerifyDiagnostics() to CompileAndVerify calls
4dc5da8 
Co-authored-by: jcouv <12466233+jcouv@users.noreply.github.com>
Copilot finished work on behalf of jcouv October 21, 2025 19:06
Copilot AI requested a review from jcouv October 21, 2025 19:06
jcouv
jcouv approved these changes Oct 21, 2025
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (commit 2)

@jcouv jcouv added Area-Compilers Test Test failures in roslyn-CI and removed Area-Infrastructure labels Oct 21, 2025
@jcouv jcouv self-assigned this Oct 21, 2025
AlekseyTs
AlekseyTs reviewed Oct 22, 2025
View reviewed changes
src/Compilers/CSharp/Test/Semantic/Semantics/ForEachTests.cs
try
{
foreach (int x in default(int[]))
{
Copy link
Contributor

@AlekseyTs AlekseyTs Oct 22, 2025

Choose a reason for hiding this comment

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

{

I think it will be useful to observe that the loop didn't execute, consider printing something from the loop, or throw a different exception.

Copy link
Contributor

@AlekseyTs AlekseyTs Oct 23, 2025

Choose a reason for hiding this comment

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

@CyrusNajmabadi Did you see this comment?

Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Oct 23, 2025

Choose a reason for hiding this comment

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

i did. I didn't think it was worthwhile enough, and didn't think the comment was blocking. So i went ahead with merging.

Copy link
Contributor

@AlekseyTs AlekseyTs Oct 23, 2025

Choose a reason for hiding this comment

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

i did. I didn't think it was worthwhile enough, and didn't think the comment was blocking. So i went ahead with merging.

Just making sure you saw it

AlekseyTs
AlekseyTs approved these changes Oct 22, 2025
View reviewed changes
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 3)

@CyrusNajmabadi CyrusNajmabadi merged commit d8fb41b into main Oct 23, 2025
24 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Oct 23, 2025
333fred added a commit to 333fred/roslyn that referenced this pull request Oct 24, 2025
@333fred
Merge remote-tracking branch 'upstream/main' into test-runtime-async
b20ced1 
* upstream/main: (332 commits)
  Cache lambdas in analyzer driver (dotnet#80759)
  Add information for NuGet package version 4.14 (dotnet#80870)
  Add missing search keywords to VB Advanced options page
  Fix IDE0031 false positive when preprocessor directives are used in if statements (dotnet#80878)
  Use core compiler on netfx hosts with toolset package (dotnet#80631)
  Make string concat assert more precise (dotnet#80619)
  Extensions: address some diagnostic quality issues (dotnet#80827)
  Add note on traversal order for bound nodes (dotnet#80872)
  Ensure that locals at the top level of a constructor have the same safe-context as parameters (dotnet#80807)
  Fix handling of SymbolDisplayCompilerInternalOptions.UseArityForGenericTypes option for non-native symbol implementations (dotnet#80826)
  Update src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests.cs
  Add IsValidContainingStatement check to prevent collection initializers in using declarations
  Add back old DocumentSpan constructor (dotnet#80864)
  Add tests verifying pointer types in type parameters require unsafe context (dotnet#80776)
  Add regression test for Interlocked.Exchange with nullable types (dotnet#80796)
  Add regression test for ParseAttributeArgumentList with invalid input (fixes dotnet#8699) (dotnet#80705)
  Add regression test for compiler crash with syntax error in indexer declaration (dotnet#80772)
  Add runtime NullReferenceException validation to foreach null iteration tests (dotnet#80839)
  Update MicrosoftBuildTasksCoreVersionForMetrics to 17.11.48 (dotnet#80812)
  Mark CS4009 error as a "build only" error. (dotnet#80698)
  ...
This was referenced Oct 25, 2025
Closed
Closed
@dotnet-bot dotnet-bot mentioned this pull request Oct 28, 2025
Closed
@dotnet-maestro dotnet-maestro bot mentioned this pull request Oct 29, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi approved these changes

@AlekseyTs AlekseyTs AlekseyTs approved these changes

@jcouv jcouv jcouv approved these changes

Assignees

@CyrusNajmabadi CyrusNajmabadi

@jcouv jcouv

Copilot code review Copilot

Labels

Area-Compilers Test Test failures in roslyn-CI

Projects

None yet

Milestone

Next

Development

Successfully merging this pull request may close these issues.

Consider tests demonstrating a runtime null reference exception when iterating over null.

4 participants

@CyrusNajmabadi @AlekseyTs @jcouv