Skip to content

Commit

Permalink
Allow 'ref' and 'in' parameters to be null-checked (#58938)
Browse files Browse the repository at this point in the history
  • Loading branch information
RikkiGibson authored Jan 19, 2022
1 parent 259a9e2 commit 0ada814
Show file tree
Hide file tree
Showing 20 changed files with 172 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ bool ParameterCanUseNullChecking([NotNullWhen(true)] IParameterSymbol? parameter
if (parameter is null)
return false;

if (parameter.RefKind != RefKind.None)
if (parameter.RefKind == RefKind.Out)
return false;

if (parameter.Type.IsValueType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,17 +579,43 @@ public C(string s!!)
[Theory, Trait(Traits.Feature, Traits.Features.CodeActionsUseIsNullCheck)]
[InlineData("ref")]
[InlineData("in")]
[InlineData("out")]
public async Task TestRefParameter(string refKind)
{
// https://github.com/dotnet/roslyn/issues/58699
// When the implementation changes to permit ref/in parameters, we should also change the fixer.
var testCode = @"
await new VerifyCS.Test()
{
TestCode = @"
using System;
class C
{
public C(" + refKind + @" string s)
{
[|if (s is null)
throw new ArgumentNullException(nameof(s));|]
}
}",
FixedCode = @"
using System;
class C
{
public C(" + refKind + @" string s!!)
{
}
}",
LanguageVersion = LanguageVersionExtensions.CSharpNext
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseIsNullCheck)]
public async Task TestOutParameter()
{
var testCode = @"
using System;
class C
{
public C(out string s)
{
if (s is null)
throw new ArgumentNullException(nameof(s));
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6184,8 +6184,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_NullCheckedHasDefaultNull_Title" xml:space="preserve">
<value>Parameter is null-checked but is null by default.</value>
</data>
<data name="ERR_NullCheckingOnByRefParameter" xml:space="preserve">
<value>By-reference parameter '{0}' cannot be null-checked.</value>
<data name="ERR_NullCheckingOnOutParameter" xml:space="preserve">
<value>'out' parameter '{0}' cannot be null-checked.</value>
</data>
<data name="WRN_NullCheckingOnNullableType" xml:space="preserve">
<value>Nullable type '{0}' is null-checked and will throw if null.</value>
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2024,7 +2024,7 @@ internal enum ErrorCode
ERR_MustNullCheckInImplementation = 8991,
ERR_NonNullableValueTypeIsNullChecked = 8992,
WRN_NullCheckedHasDefaultNull = 8993,
ERR_NullCheckingOnByRefParameter = 8994,
ERR_NullCheckingOnOutParameter = 8994,
WRN_NullCheckingOnNullableType = 8995,

#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ private static ImmutableArray<TParameterSymbol> MakeParameters<TParameterSyntax,
CheckParameterModifiers(parameterSyntax, diagnostics, parsingFunctionPointer);

var refKind = GetModifiers(parameterSyntax.Modifiers, out SyntaxToken refnessKeyword, out SyntaxToken paramsKeyword, out SyntaxToken thisKeyword);
if (refKind != RefKind.None && parameterSyntax is ParameterSyntax { ExclamationExclamationToken: var exExToken, Identifier: var identifier } && exExToken.Kind() != SyntaxKind.None)
if (refKind == RefKind.Out && parameterSyntax is ParameterSyntax { ExclamationExclamationToken: var exExToken, Identifier: var identifier } && exExToken.Kind() != SyntaxKind.None)
{
diagnostics.Add(ErrorCode.ERR_NullCheckingOnByRefParameter, exExToken.GetLocation(), identifier.ValueText);
diagnostics.Add(ErrorCode.ERR_NullCheckingOnOutParameter, exExToken.GetLocation(), identifier.ValueText);
}
if (thisKeyword.Kind() != SyntaxKind.None && !allowThis)
{
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -2352,5 +2352,98 @@ .locals init (C.<M>d__0 V_0)
}
");
}

[Fact]
public void TestNullCheckedRefString()
{
var source = @"
using System;

class C
{
public static void Main()
{
var s = ""1"";
Console.Write(s);
M(ref s);
Console.Write(s);
s = null;
try
{
M(ref s!);
Console.Write(0);
}
catch (ArgumentNullException)
{
Console.Write(3);
}
}

public static void M(ref string x!!)
{
x = ""2"";
}
}";
var verifier = CompileAndVerify(source, parseOptions: TestOptions.RegularPreview, expectedOutput: "123");
verifier.VerifyDiagnostics();

verifier.VerifyIL("C.M", @"
{
// Code size 20 (0x14)
.maxstack 2
IL_0000: ldarg.0
IL_0001: ldind.ref
IL_0002: ldstr ""x""
IL_0007: call ""ThrowIfNull""
IL_000c: ldarg.0
IL_000d: ldstr ""2""
IL_0012: stind.ref
IL_0013: ret
}");
}

[Fact]
public void TestNullCheckedInString()
{
var source = @"
using System;

class C
{
public static void Main()
{
var s = ""1"";
Console.Write(s);
M(in s);
s = null;
try
{
M(in s!);
Console.Write(0);
}
catch (ArgumentNullException)
{
Console.Write(2);
}
}

public static void M(in string x!!)
{
}
}";
var verifier = CompileAndVerify(source, parseOptions: TestOptions.RegularPreview, expectedOutput: "12");
verifier.VerifyDiagnostics();

verifier.VerifyIL("C.M", @"
{
// Code size 13 (0xd)
.maxstack 2
IL_0000: ldarg.0
IL_0001: ldind.ref
IL_0002: ldstr ""x""
IL_0007: call ""ThrowIfNull""
IL_000c: ret
}");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -680,9 +680,9 @@ public void M(out string x!!)
}";
var compilation = CreateCompilation(source, parseOptions: TestOptions.RegularPreview);
compilation.VerifyDiagnostics(
// (5,31): error CS8720: By-reference parameter 'x' cannot be null-checked.
// (5,31): error CS8994: 'out' parameter 'x' cannot be null-checked.
// public void M(out string x!!)
Diagnostic(ErrorCode.ERR_NullCheckingOnByRefParameter, "!!").WithArguments("x").WithLocation(5, 31));
Diagnostic(ErrorCode.ERR_NullCheckingOnOutParameter, "!!").WithArguments("x").WithLocation(5, 31));
}

[Fact]
Expand All @@ -698,10 +698,7 @@ public void M(ref string x!!)
}
}";
var compilation = CreateCompilation(source, parseOptions: TestOptions.RegularPreview);
compilation.VerifyDiagnostics(
// (5,31): error CS8720: By-reference parameter 'x' cannot be null-checked.
// public void M(ref string x!!)
Diagnostic(ErrorCode.ERR_NullCheckingOnByRefParameter, "!!").WithArguments("x").WithLocation(5, 31));
compilation.VerifyDiagnostics();
}

[Fact]
Expand All @@ -714,10 +711,7 @@ public static void Main() { }
public void M(in string x!!) { }
}";
var compilation = CreateCompilation(source, parseOptions: TestOptions.RegularPreview);
compilation.VerifyDiagnostics(
// (5,30): error CS8720: By-reference parameter 'x' cannot be null-checked.
// public void M(in string x!!) { }
Diagnostic(ErrorCode.ERR_NullCheckingOnByRefParameter, "!!").WithArguments("x").WithLocation(5, 30));
compilation.VerifyDiagnostics();
}

[Fact]
Expand Down

0 comments on commit 0ada814

Please sign in to comment.