Fill out test plan for auto-default structs#60688
Conversation
|
Bleh, the fix for sequence points effectively means I need to change a lot of test baselines. Going to draft for now. |
Is the question how to write an "upgrade project" test with multiple projects? It looks like there are examples such as |
| IL_0008: ret | ||
| } | ||
| "); | ||
| } |
|
|
||
| comp = CreateCompilation(source); | ||
| comp.VerifyDiagnostics(); | ||
| } |
I poked at this a bit more but I wasn't able to get the warning wave diagnostic to show up. Not sure why. It does work in manual testing. [Fact]
public async Task UpgradeProjectForStructAutoDefaultError_4()
{
await TestLanguageVersionUpgradedAsync(
@"
<Workspace>
<Project Language=""C#"" AssemblyName=""Assembly1"" CommonReferences=""true"" LanguageVersion=""10"">
<Document FilePath=""Derived.cs"">
public struct S1
{
private object objectField;
}
</Document>
</Project>
<Project Language=""C#"" AssemblyName=""Assembly2"" CommonReferences=""true"" LanguageVersion=""10"">
<ProjectReference>Assembly1</ProjectReference>
<Document FilePath=""Base.cs"">
public struct S2
{
public S1 structField;
public [|S2|](bool unused) { }
}
</Document>
</Project>
</Workspace>
",
expected: LanguageVersion.Preview,
new CSharpParseOptions(LanguageVersion.CSharp10),
// also added as a parameter to 'TestLanguageVersionUpgradedAsync' and threaded through
new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, warningLevel: 9999)); |
| Diagnostics.Add( | ||
| ErrorCode.ERR_UseDefViolationThisUnsupportedVersion, | ||
| node.Location, | ||
| thisParameter.Name, |
There was a problem hiding this comment.
nit: I wonder if our diagnostic infrastructure should/could have caught that, the same way that it verifies that all parts are "printable".
There was a problem hiding this comment.
Perhaps the DiagnosticInfo constructor should verify args.Length - 1 matches the largest index in the message.
|
|
||
| [Theory] | ||
| [InlineData(LanguageVersion.CSharp10)] | ||
| [InlineData(LanguageVersion.Latest)] |
There was a problem hiding this comment.
nit: probably should be Preview or Next, we don't really use Latest much
jcouv
left a comment
There was a problem hiding this comment.
LGTM Thanks (iteration 7). Left a couple of comments for nits
|
@dotnet/roslyn-ide for review of IDE-specific changes |
sharwell
left a comment
There was a problem hiding this comment.
IDE side looks fine. Would be good to add remaining tests if possible.
| }", | ||
| expected: LanguageVersion.Preview, | ||
| new CSharpParseOptions(LanguageVersion.CSharp10)); | ||
| } |
There was a problem hiding this comment.
💭 It looks like there are 6 new error cases added, but only 3 tested.
Closes #60167 (test plan)
I'm not sure how to test the LangVersion warnings here, as it requires having a struct with private reference type fields in another assembly, but I did test it manually. langversion-warning
I manually verified in the debugger that changing an auto-defaulted field's value, then dragging to the
{of the constructor, causes default to get written to the field again. auto-default-debugger