Skip to content
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.

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

Fix issue with 'use explicit cast' and compound assignments #74225

Merged
merged 6 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Simplify
  • Loading branch information
CyrusNajmabadi committed Jul 1, 2024
commit 267a19b0c47916afe0b0ceaddc3b76d2850a64ac
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Simplification;

namespace Microsoft.CodeAnalysis.CSharp.CodeFixes.AddExplicitCast;

Expand All @@ -38,7 +37,7 @@ internal sealed partial class CSharpAddExplicitCastCodeFixProvider()

public override ImmutableArray<string> FixableDiagnosticIds => [CS0266, CS1503];

protected override void GetPartsOfCastOrConversionExpression(ExpressionSyntax expression, out SyntaxNode type, out SyntaxNode castedExpression)
protected override void GetPartsOfCastOrConversionExpression(ExpressionSyntax expression, out SyntaxNode type, out ExpressionSyntax castedExpression)
{
var castExpression = (CastExpressionSyntax)expression;
type = castExpression.Type;
Expand Down Expand Up @@ -94,11 +93,9 @@ protected override bool TryGetTargetTypeInfo(
return !potentialConversionTypes.IsEmpty;
}

protected override bool TryLanguageSpecificFix(
SemanticModel semanticModel, SyntaxNode currentRoot, ExpressionSyntax targetNode, CancellationToken cancellationToken, [NotNullWhen(true)] out SyntaxNode? replacement)
protected override (SyntaxNode finalTarget, SyntaxNode finalReplacement) Cast(
SemanticModel semanticModel, ExpressionSyntax targetNode, ITypeSymbol conversionType)
{
replacement = null;

// The compiler is very ambiguous with assignment expressions `(a += b)`. An error on it may be an error on the
// entire expression or on the RHS of the assignment. Have to reverse engineer what it is doing here.
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the main fix.

if (targetNode is AssignmentExpressionSyntax assignmentExpression &&
Expand All @@ -112,13 +109,11 @@ protected override bool TryLanguageSpecificFix(
var conversion = semanticModel.Compilation.ClassifyConversion(rightType, leftType);
if (conversion.Exists && conversion.IsExplicit)
{
replacement = currentRoot.ReplaceNode(
assignmentExpression.Right,
this.Cast(assignmentExpression.Right, leftType));
return (assignmentExpression.Right, this.Cast(assignmentExpression.Right, leftType));
}
}
}

return replacement != null;
return base.Cast(semanticModel, targetNode, conversionType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

using System;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -15,7 +14,6 @@
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Simplification;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CodeFixes.AddExplicitCast;
Expand All @@ -30,7 +28,7 @@ internal abstract partial class AbstractAddExplicitCastCodeFixProvider<TExpressi
private const int MaximumConversionOptions = 3;

protected abstract TExpressionSyntax Cast(TExpressionSyntax expression, ITypeSymbol type);
protected abstract void GetPartsOfCastOrConversionExpression(TExpressionSyntax expression, out SyntaxNode type, out SyntaxNode castedExpression);
protected abstract void GetPartsOfCastOrConversionExpression(TExpressionSyntax expression, out SyntaxNode type, out TExpressionSyntax castedExpression);
Copy link
Member Author

Choose a reason for hiding this comment

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

stronger type.


/// <summary>
/// Output the current type information of the target node and the conversion type(s) that the target node is
Expand Down Expand Up @@ -91,8 +89,12 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)

actions.Add(CodeAction.Create(
title,
cancellationToken => Task.FromResult(document.WithSyntaxRoot(
ApplyFix(document, semanticModel, root, targetNode, conversionType, cancellationToken))),
cancellationToken =>
{
var (finalTarget, replacement) = ApplyFix(document, semanticModel, targetNode, conversionType, cancellationToken);

return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(finalTarget, replacement)));
},
title));
}

Expand All @@ -101,10 +103,9 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
context.Diagnostics);
}

private SyntaxNode ApplyFix(
private (SyntaxNode finalTarget, SyntaxNode finalReplacement) ApplyFix(
Document document,
SemanticModel semanticModel,
SyntaxNode currentRoot,
TExpressionSyntax targetNode,
ITypeSymbol conversionType,
CancellationToken cancellationToken)
Expand All @@ -126,28 +127,22 @@ private SyntaxNode ApplyFix(
if (firstConversion is { IsImplicit: false, IsReference: true } or { IsIdentity: true } &&
secondConversion is { IsImplicit: false, IsReference: true })
{
return currentRoot.ReplaceNode(
targetNode,
this.Cast((TExpressionSyntax)castedExpression, conversionType)
.WithTriviaFrom(targetNode));
return (targetNode, this.Cast(castedExpression, conversionType).WithTriviaFrom(targetNode));
}
}
}
else if (TryLanguageSpecificFix(semanticModel, currentRoot, targetNode, cancellationToken, out var replacement))
{
return replacement;
}

return currentRoot.ReplaceNode(
targetNode,
this.Cast(targetNode, conversionType));
return Cast(semanticModel, targetNode, conversionType);
}

protected virtual (SyntaxNode finalTarget, SyntaxNode finalReplacement) Cast(SemanticModel semanticModel, TExpressionSyntax targetNode, ITypeSymbol conversionType)
=> (targetNode, this.Cast(targetNode, conversionType));

protected virtual bool TryLanguageSpecificFix(
SemanticModel semanticModel, SyntaxNode currentRoot, TExpressionSyntax targetNode, CancellationToken cancellationToken,
[NotNullWhen(true)] out SyntaxNode? replacement)
out (SyntaxNode finalTarget, SyntaxNode replacement) result)
{
replacement = null;
result = default;
return false;
}

Expand Down Expand Up @@ -223,7 +218,7 @@ await editor.ApplyExpressionLevelSemanticEditsAsync(
if (TryGetTargetTypeInfo(document, semanticModel, root, diagnostics[0].Id, spanNode, cancellationToken, out var potentialConversionTypes) &&
potentialConversionTypes.Length == 1)
{
return ApplyFix(document, semanticModel, root, potentialConversionTypes[0].node, potentialConversionTypes[0].type, cancellationToken);
var (newTarget, newReplacement) = ApplyFix(document, semanticModel, root, potentialConversionTypes[0].node, potentialConversionTypes[0].type, cancellationToken);
}

return root;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ Imports Microsoft.CodeAnalysis.PooledObjects
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax

Namespace Microsoft.CodeAnalysis.VisualBasic.CodeFixes.AddExplicitCast

<ExportCodeFixProvider(LanguageNames.VisualBasic, Name:=PredefinedCodeFixProviderNames.AddExplicitCast), [Shared]>
Partial Friend NotInheritable Class VisualBasicAddExplicitCastCodeFixProvider
Inherits AbstractAddExplicitCastCodeFixProvider(Of ExpressionSyntax)
Expand All @@ -34,7 +33,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeFixes.AddExplicitCast

Public Overrides ReadOnly Property FixableDiagnosticIds As ImmutableArray(Of String) = ImmutableArray.Create(BC30512, BC42016, BC30518, BC30519)

Protected Overrides Sub GetPartsOfCastOrConversionExpression(expression As ExpressionSyntax, ByRef type As SyntaxNode, ByRef castedExpression As SyntaxNode)
Protected Overrides Sub GetPartsOfCastOrConversionExpression(
expression As ExpressionSyntax,
ByRef type As SyntaxNode,
ByRef castedExpression As ExpressionSyntax)
Dim directCastExpression = TryCast(expression, DirectCastExpressionSyntax)
If directCastExpression IsNot Nothing Then
type = directCastExpression.Type
Expand Down