Skip to content

Commit

Permalink
Support foreach over parameters in DoNotCopyValue
Browse files Browse the repository at this point in the history
  • Loading branch information
sharwell committed Oct 27, 2020
1 parent a235f3f commit 653580d
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 6 deletions.
36 changes: 30 additions & 6 deletions src/Roslyn.Diagnostics.Analyzers/Core/DoNotCopyValue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.FlowAnalysis;
using Microsoft.CodeAnalysis.Operations;
Expand Down Expand Up @@ -367,7 +369,7 @@ bool IsSupportedConversion()
switch (Acquire(operation.Operand))
{
case RefKind.None:
case RefKind.Ref when operation.Conversion.IsIdentity:
case RefKind.Ref or RefKind.RefReadOnly when operation.Conversion.IsIdentity:
return true;

default:
Expand Down Expand Up @@ -547,14 +549,36 @@ public override void VisitForEachLoop(IForEachLoopOperation operation)

var instance = operation.Collection;
var instance2 = (operation.Collection as IConversionOperation)?.Operand;
if (Acquire(operation.Collection) != RefKind.Ref)

switch (Acquire(operation.Collection))
{
instance = null;
instance2 = null;
case RefKind.Ref:
// No special requirements
break;

case RefKind.RefReadOnly when operation.Syntax is CommonForEachStatementSyntax syntax && operation.SemanticModel.GetForEachStatementInfo(syntax).GetEnumeratorMethod is { IsReadOnly: true }:
// Requirement of readonly GetEnumerator is met
break;

default:
instance = null;
instance2 = null;
break;
}
else if (Acquire(instance2) != RefKind.Ref)

switch (Acquire(instance2))
{
instance2 = null;
case RefKind.Ref:
// No special requirements
break;

case RefKind.RefReadOnly when operation.Syntax is CommonForEachStatementSyntax syntax && operation.SemanticModel.GetForEachStatementInfo(syntax).GetEnumeratorMethod is { IsReadOnly: true }:
// Requirement of readonly GetEnumerator is met
break;

default:
instance2 = null;
break;
}

using var releaser = TryAddForVisit(_handledOperations, instance, out _);
Expand Down
53 changes: 53 additions & 0 deletions src/Roslyn.Diagnostics.Analyzers/UnitTests/DoNotCopyValueTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#nullable enable

using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Testing;
using Xunit;
using VerifyCS = Test.Utilities.CSharpSecurityCodeFixVerifier<
Roslyn.Diagnostics.Analyzers.DoNotCopyValue,
Expand Down Expand Up @@ -685,6 +686,58 @@ internal sealed class NonCopyableAttribute : System.Attribute { }
}.RunAsync();
}

[Theory]
[CombinatorialData]
public async Task AllowCustomForeachEnumeratorParameterReference(
[CombinatorialValues("", "ref", "in")] string parameterModifiers,
[CombinatorialValues("", "readonly")] string getEnumeratorModifiers)
{
var source = $@"
using System.Runtime.InteropServices;
class C
{{
void Method({parameterModifiers} CannotCopy cannotCopy)
{{
foreach (var obj in {{|#0:cannotCopy|}})
{{
}}
}}
}}
[NonCopyable]
struct CannotCopy
{{
public {getEnumeratorModifiers} Enumerator GetEnumerator() => throw null;
public struct Enumerator
{{
public object Current => throw null;
public bool MoveNext() => throw null;
}}
}}
internal sealed class NonCopyableAttribute : System.Attribute {{ }}
";

var expected = (parameterModifiers, getEnumeratorModifiers) switch
{
// /0/Test0.cs(8,29): warning RS0042: Unsupported use of non-copyable type 'CannotCopy' in 'ParameterReference' operation
("in", "") => new[] { VerifyCS.Diagnostic(DoNotCopyValue.UnsupportedUseRule).WithLocation(0).WithArguments("CannotCopy", "ParameterReference") },

_ => DiagnosticResult.EmptyDiagnosticResults,
};

var test = new VerifyCS.Test
{
TestCode = source,
LanguageVersion = Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp8,
};

test.ExpectedDiagnostics.AddRange(expected);
await test.RunAsync();
}

[Fact]
public async Task AllowCustomForeachEnumeratorDisposableObject()
{
Expand Down

0 comments on commit 653580d

Please sign in to comment.