Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,20 @@ protected override bool IsDestructor(IMethodSymbol methodSymbol)

protected override SyntaxNode UpdateArgumentListSyntax(SyntaxNode argumentList, SeparatedSyntaxList<ArgumentSyntax> arguments)
=> ((ArgumentListSyntax)argumentList).WithArguments(arguments);

protected override bool IsAnonymousObjectMemberDeclaratorNameIdentifier(SyntaxNode expression)
{
// Check if this expression is the name identifier in an anonymous object member declarator.
// In C#, the structure is: IdentifierNameSyntax -> NameEqualsSyntax -> AnonymousObjectMemberDeclaratorSyntax
// We want to return true when expression is the identifier on the left side of the '=' in something like 'new { a = value }'
if (expression is IdentifierNameSyntax identifier &&
identifier.Parent is NameEqualsSyntax nameEquals &&
nameEquals.Parent is AnonymousObjectMemberDeclaratorSyntax)
{
// Verify that the identifier is the Name part of NameEqualsSyntax (not some other part)
return nameEquals.Name == identifier;
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1818,4 +1818,59 @@ void M()
}
}
""");

[Fact]
public Task TestNotOnAnonymousObjectMemberName()
=> TestMissingInRegularAndScriptAsync(
"""
class C
{
object M() => new
{
[|a|] = new { },
};
}
""");

[Fact]
public Task TestNotOnAnonymousObjectMemberNameWithValue()
=> TestMissingInRegularAndScriptAsync(
"""
class C
{
object M() => new
{
[|x|] = 5,
};
}
""");

[Fact]
public Task TestNotOnObjectInitializerMemberName()
=> TestMissingInRegularAndScriptAsync(
"""
class Foo { public int X { get; set; } }
class C
{
Foo M() => new Foo
Copy link
Member

Choose a reason for hiding this comment

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

@copilot use 'Goo' instead of 'Foo' in all tests.
use [Fact, WorkItem("...")] for all the tests you added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 0cc1448. Renamed 'Foo' to 'Goo' in all tests and added WorkItem attributes with the issue URL.

{
[|X|] = 5,
};
}
""");

[Fact]
public Task TestNotOnObjectInitializerMemberNameNested()
=> TestMissingInRegularAndScriptAsync(
"""
class Foo { public int X { get; set; } public Foo Inner { get; set; } }
class C
{
Foo M() => new Foo
{
[|X|] = 5,
Inner = new Foo { X = 10 }
};
}
""");
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
protected abstract SyntaxNode UpdateArgumentListSyntax(SyntaxNode argumentList, SeparatedSyntaxList<TArgumentSyntax> arguments);
protected abstract SyntaxNode? GetLocalDeclarationFromDeclarator(SyntaxNode variableDecl);
protected abstract bool IsDestructor(IMethodSymbol methodSymbol);
protected abstract bool IsAnonymousObjectMemberDeclaratorNameIdentifier(SyntaxNode expression);

public sealed override async Task ComputeRefactoringsAsync(CodeRefactoringContext context)
{
Expand All @@ -57,7 +58,7 @@

var generator = SyntaxGenerator.GetGenerator(document);
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
if (!IsValidExpression(expression, syntaxFacts))
if (!IsValidExpression(expression, syntaxFacts, this))
return;

var containingMethod = expression.FirstAncestorOrSelf<SyntaxNode>(node => generator.GetParameterListNode(node) is not null);
Expand Down Expand Up @@ -117,7 +118,7 @@
}
}

private static bool IsValidExpression(SyntaxNode expression, ISyntaxFactsService syntaxFacts)
private bool IsValidExpression(SyntaxNode expression, ISyntaxFactsService syntaxFacts, AbstractIntroduceParameterCodeRefactoringProvider<TExpressionSyntax, TInvocationExpressionSyntax, TObjectCreationExpressionSyntax, TIdentifierNameSyntax, TArgumentSyntax> provider)

Check failure on line 121 in src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs#L121

src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs(121,18): error CA1822: (NETCORE_ENGINEERING_TELEMETRY=Build) Member 'IsValidExpression' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)

Check failure on line 121 in src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs#L121

src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs(121,18): error CA1822: (NETCORE_ENGINEERING_TELEMETRY=Build) Member 'IsValidExpression' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)

Check failure on line 121 in src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs#L121

src/Features/Core/Portable/IntroduceParameter/AbstractIntroduceParameterCodeRefactoringProvider.cs(121,18): error CA1822: (NETCORE_ENGINEERING_TELEMETRY=Build) Member 'IsValidExpression' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
{
// Need to special case for highlighting of method types because they are also "contained" within a method,
// but it does not make sense to introduce a parameter in that case.
Expand All @@ -129,6 +130,16 @@
if (syntaxFacts.IsNameOfAnyMemberAccessExpression(expression))
return false;

// Need to special case for the left-hand side of member initializers in regular objects (e.g., 'X' in 'new Foo { X = ... }')
// because it does not make sense to introduce a parameter for the property/member name itself.
if (syntaxFacts.IsMemberInitializerNamedAssignmentIdentifier(expression, out _))
return false;

// Need to special case for the left-hand side of member initializers in anonymous objects (e.g., 'a' in 'new { a = ... }').
// This checks if the expression is the name identifier in an anonymous object member declarator.
if (provider.IsAnonymousObjectMemberDeclaratorNameIdentifier(expression))
return false;

// Need to special case for expressions that are contained within a parameter or attribute argument
// because it is technically "contained" within a method, but does not make
// sense to introduce.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,29 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.IntroduceParameter
Protected Overrides Function IsDestructor(methodSymbol As IMethodSymbol) As Boolean
Return methodSymbol.Name.Equals(WellKnownMemberNames.DestructorName)
End Function

Protected Overrides Function IsAnonymousObjectMemberDeclaratorNameIdentifier(expression As SyntaxNode) As Boolean
' Check if this expression is the name identifier in an anonymous object member declarator.
' In VB, the structure for anonymous objects with explicit names is: IdentifierNameSyntax -> NamedFieldInitializerSyntax
' We want to return true when expression is the identifier on the left side in something like 'New With { .a = value }'
Dim identifier = TryCast(expression, IdentifierNameSyntax)
If identifier Is Nothing Then
Return False
End If

Dim namedFieldInit = TryCast(identifier.Parent, NamedFieldInitializerSyntax)
If namedFieldInit Is Nothing Then
Return False
End If

' Check if this is part of an anonymous object (not a regular object initializer)
' Anonymous object initializers are inside AnonymousObjectCreationExpressionSyntax
If Not (TypeOf namedFieldInit.Parent Is AnonymousObjectCreationExpressionSyntax) Then
Return False
End If

' Verify that the identifier is the Name part of NamedFieldInitializerSyntax
Return namedFieldInit.Name Is identifier
End Function
End Class
End Namespace
Loading