Skip to content

Commit

Permalink
Fix issue removing unnecessary cast in user defined interpolation han…
Browse files Browse the repository at this point in the history 
…dlers
  • Loading branch information
@CyrusNajmabadi
CyrusNajmabadi committed Sep 25, 2024
1 parent acd83f7 commit 54cf613
Show file tree
Hide file tree
Showing 2 changed files with 213 additions and 28 deletions.
212 changes: 184 additions & 28 deletions src/Analyzers/CSharp/Tests/RemoveUnnecessaryCast/RemoveUnnecessaryCastTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10047,8 +10047,8 @@ public async Task DoNotRemoveForNullWithMultipleMatchingParameterTypes()
using System;
public class TestClass
{
public TestClass(object? value) { }
public TestClass(Func<object?> value) { }
public TestClass(object? value) { }
public TestClass(Func<object?> value) { }

public TestClass Create1() => new ((object?)null);
}
Expand All @@ -10070,7 +10070,7 @@ public async Task DoNotRemoveForNintPointerToVoidPointer()
using System;
public class TestClass
{
unsafe void M(nint** ptr)
unsafe void M(nint** ptr)
{
nint value = (nint)(void*)*ptr;
}
Expand Down Expand Up @@ -12527,39 +12527,39 @@ public async Task DoNotRemoveNecessaryCastWithOverloadedNegationAndImplicitConve

namespace WrongRedundantCastWarning
{
struct Flag
{
public Flag(int value) => this.Value = value;
struct Flag
{
public Flag(int value) => this.Value = value;

public int Value { get; }
public int Value { get; }

// This cast is wrongly reported as redundant
public static FlagSet operator ~(Flag flag) => ~(FlagSet)flag;
}
// This cast is wrongly reported as redundant
public static FlagSet operator ~(Flag flag) => ~(FlagSet)flag;
}

struct FlagSet
{
public FlagSet(int value) => this.Value = value;
struct FlagSet
{
public FlagSet(int value) => this.Value = value;

public int Value { get; }
public int Value { get; }

public static implicit operator FlagSet(Flag flag) => new FlagSet(flag.Value);
public static implicit operator FlagSet(Flag flag) => new FlagSet(flag.Value);

public static FlagSet operator ~(FlagSet flagSet) => new FlagSet(~flagSet.Value);
}
public static FlagSet operator ~(FlagSet flagSet) => new FlagSet(~flagSet.Value);
}

class Program
{
static readonly Flag One = new Flag(1);
static readonly Flag Two = new Flag(2);
class Program
{
static readonly Flag One = new Flag(1);
static readonly Flag Two = new Flag(2);

static void Main(string[] args)
{
var flipped = ~Two;
static void Main(string[] args)
{
var flipped = ~Two;

Console.WriteLine(flipped.Value);
}
}
Console.WriteLine(flipped.Value);
}
}
}
""";
await new VerifyCS.Test
Expand Down Expand Up @@ -12742,7 +12742,7 @@ class C
{
protected sbyte ExtractInt8(object data)
{
return (data is sbyte value) ? value : (sbyte)0;
return (data is sbyte value) ? value : (sbyte)0;
}
}
""";
Expand Down Expand Up @@ -13835,4 +13835,160 @@ void X()
LanguageVersion = LanguageVersion.CSharp12,
}.RunAsync();
}
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75145")]
public async Task UnnecessaryInterpolationCast1()
{
await new VerifyCS.Test
{
TestCode = """
#nullable enable
using System;
public class C
{
public static void Main()
{
string s = $"{[|(object?)|]null}";
Console.WriteLine(s);
}
}
""",
FixedCode = """
#nullable enable
using System;
public class C
{
public static void Main()
{
string s = $"{null}";
Console.WriteLine(s);
}
}
""",
LanguageVersion = LanguageVersion.CSharp12,
ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
}.RunAsync();
}
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75145")]
public async Task UnnecessaryInterpolationCast2()
{
await new VerifyCS.Test
{
TestCode = """
#nullable enable
using System.Runtime.CompilerServices;
public class IssueClass
{
public object? Test()
{
return InterpolateMe($"{[|(object?)|]null}");
}
public static object? InterpolateMe([InterpolatedStringHandlerArgument] ref TestInterpolatedStringHandler handler)
{
return null;
}
[InterpolatedStringHandler]
public ref struct TestInterpolatedStringHandler(int literalLength, int formattedCount)
{
public readonly void AppendLiteral(string s)
{
}
public void AppendFormatted<T>(T value)
{
}
public void AppendFormatted(object? value)
{
}
}
}
""",
FixedCode = """
#nullable enable
using System.Runtime.CompilerServices;
public class IssueClass
{
public object? Test()
{
return InterpolateMe($"{null}");
}
public static object? InterpolateMe([InterpolatedStringHandlerArgument] ref TestInterpolatedStringHandler handler)
{
return null;
}
[InterpolatedStringHandler]
public ref struct TestInterpolatedStringHandler(int literalLength, int formattedCount)
{
public readonly void AppendLiteral(string s)
{
}
public void AppendFormatted<T>(T value)
{
}
public void AppendFormatted(object? value)
{
}
}
}
""",
LanguageVersion = LanguageVersion.CSharp12,
ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
}.RunAsync();
}
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75145")]
public async Task NecessaryInterpolationCast()
{
await new VerifyCS.Test
{
TestCode = """
#nullable enable
using System.Runtime.CompilerServices;
public class IssueClass
{
public object? Test()
{
return InterpolateMe($"{(object?)null}");
}
public static object? InterpolateMe([InterpolatedStringHandlerArgument] ref TestInterpolatedStringHandler handler)
{
return null;
}
[InterpolatedStringHandler]
public ref struct TestInterpolatedStringHandler(int literalLength, int formattedCount)
{
public readonly void AppendLiteral(string s)
{
}
public void AppendFormatted<T>(T value)
{
}
}
}
""",
LanguageVersion = LanguageVersion.CSharp12,
ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
}.RunAsync();
}
}
29 changes: 29 additions & 0 deletions ...SharedUtilitiesAndExtensions/Compiler/CSharp/Simplification/Simplifiers/CastSimplifier.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,35 @@ private static bool IntroducedAmbiguity(
if (newSymbolInfo.Symbol is null)
return true;
}

if (currentOld is InterpolatedStringExpressionSyntax && currentNew is InterpolatedStringExpressionSyntax)
{
// In the case of interpolations, we need to dive into the operation level to determine if the meaning
// of the the interpolation stayed the same in the case of interpolation handlers.
if (originalSemanticModel.GetOperation(currentOld, cancellationToken) is not IInterpolatedStringOperation oldInterpolationOperation)
return true;

if (rewrittenSemanticModel.GetOperation(currentNew, cancellationToken) is not IInterpolatedStringOperation newInterpolationOperation)
return true;

if (oldInterpolationOperation.Parts.Length != newInterpolationOperation.Parts.Length)
return true;

for (int i = 0, n = oldInterpolationOperation.Parts.Length; i < n; i++)
{
var oldInterpolationPart = oldInterpolationOperation.Parts[i];
var newInterpolationPart = newInterpolationOperation.Parts[i];
if (oldInterpolationPart.Kind != newInterpolationPart.Kind)
return true;

// If we were calling some interpolation AppendFormatted helper, and now we're not, we introduced a problem.
if (oldInterpolationPart is IInterpolatedStringAppendOperation { AppendCall: not IInvalidOperation } &&
newInterpolationPart is IInterpolatedStringAppendOperation { AppendCall: IInvalidOperation })
{
return true;
}
}
}
}

return false;
Expand Down

0 comments on commit 54cf613

Please sign in to comment.