Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,16 @@ protected override bool IsValidExpressionUnderExpressionStatement(ExpressionSynt
// ;
var isNullConditionalInvocationExpression = IsNullConditionalInvocationExpression(expressionNode);

return expressionNode.IsKind(SyntaxKind.InvocationExpression)
Copy link
Member Author

Choose a reason for hiding this comment

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

top check was redundant with check in the last clause.

|| isNullConditionalInvocationExpression
|| expressionNode is AssignmentExpressionSyntax
|| expressionNode.Kind()
is SyntaxKind.InvocationExpression
or SyntaxKind.ObjectCreationExpression
or SyntaxKind.PreIncrementExpression
or SyntaxKind.PreDecrementExpression
or SyntaxKind.PostIncrementExpression
or SyntaxKind.PostDecrementExpression
or SyntaxKind.AwaitExpression;
return isNullConditionalInvocationExpression
|| expressionNode is AssignmentExpressionSyntax
|| expressionNode.Kind()
is SyntaxKind.InvocationExpression
or SyntaxKind.ObjectCreationExpression
or SyntaxKind.PreIncrementExpression
or SyntaxKind.PreDecrementExpression
or SyntaxKind.PostIncrementExpression
or SyntaxKind.PostDecrementExpression
or SyntaxKind.AwaitExpression;
}

protected override bool CanBeReplacedByThrowExpression(SyntaxNode syntaxNode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,29 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System;
using System.Collections.Generic;
using System.Composition;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.LanguageService;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.IntroduceVariable;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Shared.Extensions;

namespace Microsoft.CodeAnalysis.CSharp.IntroduceVariable;

[ExportLanguageService(typeof(IIntroduceVariableService), LanguageNames.CSharp), Shared]
internal sealed partial class CSharpIntroduceVariableService :
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
internal sealed partial class CSharpIntroduceVariableService() :
AbstractIntroduceVariableService<CSharpIntroduceVariableService, ExpressionSyntax, TypeSyntax, TypeDeclarationSyntax, QueryExpressionSyntax, NameSyntax>
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public CSharpIntroduceVariableService()
{
}
protected override ISyntaxFacts SyntaxFacts => CSharpSyntaxFacts.Instance;

protected override bool IsInNonFirstQueryClause(ExpressionSyntax expression)
{
Expand All @@ -45,10 +43,9 @@ protected override bool IsInNonFirstQueryClause(ExpressionSyntax expression)
}

protected override bool IsInFieldInitializer(ExpressionSyntax expression)
{
return expression.GetAncestorOrThis<VariableDeclaratorSyntax>()
.GetAncestorOrThis<FieldDeclarationSyntax>() != null;
}
=> expression
.GetAncestorOrThis<VariableDeclaratorSyntax>()
.GetAncestorOrThis<FieldDeclarationSyntax>() != null;

protected override bool IsInParameterInitializer(ExpressionSyntax expression)
=> expression.GetAncestorOrThis<EqualsValueClauseSyntax>().IsParentKind(SyntaxKind.Parameter);
Expand All @@ -62,7 +59,7 @@ protected override bool IsInAutoPropertyInitializer(ExpressionSyntax expression)
protected override bool IsInExpressionBodiedMember(ExpressionSyntax expression)
{
// walk up until we find a nearest enclosing block or arrow expression.
for (SyntaxNode node = expression; node != null; node = node.Parent)
for (SyntaxNode? node = expression; node != null; node = node.Parent)
{
// If we are in an expression bodied member and if the expression has a block body, then,
// act as if we're in a block context and not in an expression body context at all.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9535,4 +9535,37 @@ void M()
}
""");
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78680")]
public async Task FixReferenceInMemberAccessNameInTopLevel()
{
await TestAsync(
"""
C c = null;
C d = null;
Console.WriteLine([|c.c|]);
Console.WriteLine(d.c.c);

class C
{
public C c;
}
""",
"""
C c = null;
C d = null;

C {|Rename:c1|} = c.c;

Console.WriteLine(c1);
Console.WriteLine(d.c.c);

class C
{
public C c;
}
""",
parseOptions: null,
index: 1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Simplification;
Expand All @@ -32,6 +31,8 @@ internal abstract partial class AbstractIntroduceVariableService<TService, TExpr
where TQueryExpressionSyntax : TExpressionSyntax
where TNameSyntax : TTypeSyntax
{
protected abstract ISyntaxFacts SyntaxFacts { get; }

protected abstract bool IsInNonFirstQueryClause(TExpressionSyntax expression);
protected abstract bool IsInFieldInitializer(TExpressionSyntax expression);
protected abstract bool IsInParameterInitializer(TExpressionSyntax expression);
Expand Down Expand Up @@ -319,54 +320,28 @@ private bool NodeMatchesExpression(
{
cancellationToken.ThrowIfCancellationRequested();
if (nodeInCurrent == expressionInOriginal)
{
return true;
}

if (allOccurrences && CanReplace(nodeInCurrent))
var syntaxFacts = this.SyntaxFacts;
if (syntaxFacts.IsNameOfAnyMemberAccessExpression(nodeInCurrent) ||
syntaxFacts.IsMemberInitializerNamedAssignmentIdentifier(nodeInCurrent))
{
// Original expression and current node being semantically equivalent isn't enough when the original expression
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 comment was redundant. SemanticEquivalence.AreEquivalent already has the same comment within it.

// is a member access via instance reference (either implicit or explicit), the check only ensures that the expression
// and current node are both backed by the same member symbol. So in this case, in addition to SemanticEquivalence check,
// we also check if expression and current node are both instance member access.
//
// For example, even though the first `c` binds to a field and we are introducing a local for it,
// we don't want other references to that field to be replaced as well (i.e. the second `c` in the expression).
//
// class C
// {
// C c;
// void Test()
// {
// var x = [|c|].c;
// }
// }

if (SemanticEquivalence.AreEquivalent(originalSemanticModel, currentSemanticModel, expressionInOriginal, nodeInCurrent))
{
var originalOperation = originalSemanticModel.GetOperation(expressionInOriginal, cancellationToken);
if (IsInstanceMemberReference(originalOperation))
{
var currentOperation = currentSemanticModel.GetOperation(nodeInCurrent, cancellationToken);
return IsInstanceMemberReference(currentOperation);
}
return false;
}

// If the original expression is within a static local function, further checks are unnecessary since our scope has already been narrowed down to within the local function.
// If the original expression is not within a static local function, we need to further check whether the expression we're comparing against is within a static local
// function. If so, the expression is not a valid match since we cannot refer to instance variables from within static local functions.
if (!IsExpressionInStaticLocalFunction(expressionInOriginal))
{
return !IsExpressionInStaticLocalFunction(nodeInCurrent);
}
if (allOccurrences && CanReplace(nodeInCurrent) &&
SemanticEquivalence.AreEquivalent(originalSemanticModel, currentSemanticModel, expressionInOriginal, nodeInCurrent))
{
// If the original expression is within a static local function, further checks are unnecessary since our scope has already been narrowed down to within the local function.
// If the original expression is not within a static local function, we need to further check whether the expression we're comparing against is within a static local
// function. If so, the expression is not a valid match since we cannot refer to instance variables from within static local functions.
if (!IsExpressionInStaticLocalFunction(expressionInOriginal))
return !IsExpressionInStaticLocalFunction(nodeInCurrent);

return true;
}
return true;
}

return false;

static bool IsInstanceMemberReference(IOperation operation)
=> operation is IMemberReferenceOperation { Instance.Kind: OperationKind.InstanceReference };
}

protected TNode Rewrite<TNode>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Imports Microsoft.CodeAnalysis.Host.Mef
Imports Microsoft.CodeAnalysis.Text
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
Imports System.Composition
Imports Microsoft.CodeAnalysis.LanguageService
Imports Microsoft.CodeAnalysis.VisualBasic.LanguageService

Namespace Microsoft.CodeAnalysis.VisualBasic.IntroduceVariable
<ExportLanguageService(GetType(IIntroduceVariableService), LanguageNames.VisualBasic), [Shared]>
Expand All @@ -20,6 +22,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.IntroduceVariable
Public Sub New()
End Sub

Protected Overrides ReadOnly Property SyntaxFacts As ISyntaxFacts = VisualBasicSyntaxFacts.Instance

Protected Overrides Function GetContainingExecutableBlocks(expression As ExpressionSyntax) As IEnumerable(Of SyntaxNode)
Return expression.GetContainingExecutableBlocks()
End Function
Expand Down