Skip to content

Commit

Permalink
Merge pull request #49303 from CyrusNajmabadi/useCompoundFix
Browse files Browse the repository at this point in the history
Don't offer use-compound-assignment with implicit object creation.
  • Loading branch information
CyrusNajmabadi authored Nov 12, 2020
2 parents dff9639 + 0e03a26 commit 782c293
Show file tree
Hide file tree
Showing 12 changed files with 193 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// 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.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
Expand Down Expand Up @@ -790,6 +788,44 @@ InsertionPoint Up()
}");
}

[WorkItem(49294, "https://github.com/dotnet/roslyn/issues/49294")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseCompoundAssignment)]
public async Task TestNotOnImplicitObjectInitializer()
{
await TestMissingAsync(
@"
struct InsertionPoint
{
int level;
InsertionPoint Up()
{
return new()
{
level [||]= level - 1,
};
}
}");
}

[WorkItem(49294, "https://github.com/dotnet/roslyn/issues/49294")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseCompoundAssignment)]
public async Task TestNotOnRecord()
{
await TestMissingAsync(
@"
record InsertionPoint(int level)
{
InsertionPoint Up()
{
return this with
{
level [||]= level - 1,
};
}
}");
}

[WorkItem(38137, "https://github.com/dotnet/roslyn/issues/38137")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseCompoundAssignment)]
public async Task TestParenthesizedExpression()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,10 @@ private void AnalyzeAssignment(SyntaxNodeAnalysisContext context)
}

// Don't offer if this is `x = x + 1` inside an obj initializer like:
// `new Point { x = x + 1 }`
if (_syntaxFacts.IsObjectInitializerNamedAssignmentIdentifier(assignmentLeft))
// `new Point { x = x + 1 }` or
// `new () { x = x + 1 }` or
// `p with { x = x + 1 }`
if (_syntaxFacts.IsMemberInitializerNamedAssignmentIdentifier(assignmentLeft))
{
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// 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.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeRefactorings;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
Expand Down Expand Up @@ -2415,7 +2413,7 @@ int Goo

private async Task TestWithAllCodeStyleOff(
string initialMarkup, string expectedMarkup,
ParseOptions parseOptions = null, int index = 0)
ParseOptions? parseOptions = null, int index = 0)
{
await TestAsync(
initialMarkup, expectedMarkup, parseOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// 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.Threading.Tasks;
using Microsoft.CodeAnalysis.CodeRefactorings;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
Expand Down Expand Up @@ -1889,6 +1887,94 @@ void Main()
}");
}

[WorkItem(45171, "https://github.com/dotnet/roslyn/issues/45171")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsReplacePropertyWithMethods)]
public async Task TestReferenceInImplicitObjectInitializer()
{
await TestInRegularAndScriptAsync(
@"public class Tweet
{
public string [||]Tweet { get; }
}
class C
{
void Main()
{
var t = new Tweet();
Tweet t1 = new()
{
Tweet = t.Tweet
};
}
}",
@"public class Tweet
{
private readonly string tweet;
public string GetTweet()
{
return tweet;
}
}
class C
{
void Main()
{
var t = new Tweet();
Tweet t1 = new()
{
{|Conflict:Tweet|} = t.GetTweet()
};
}
}");
}

[WorkItem(45171, "https://github.com/dotnet/roslyn/issues/45171")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsReplacePropertyWithMethods)]
public async Task TestReferenceInWithInitializer()
{
await TestInRegularAndScriptAsync(
@"public class Tweet
{
public string [||]Tweet { get; }
}
class C
{
void Main()
{
var t = new Tweet();
var t1 = t with
{
Tweet = t.Tweet
};
}
}",
@"public class Tweet
{
private readonly string tweet;
public string GetTweet()
{
return tweet;
}
}
class C
{
void Main()
{
var t = new Tweet();
var t1 = t with
{
{|Conflict:Tweet|} = t.GetTweet()
};
}
}");
}

private OptionsCollection PreferExpressionBodiedMethods =>
new OptionsCollection(GetLanguage()) { { CSharpCodeStyleOptions.PreferExpressionBodiedMethods, CSharpCodeStyleOptions.WhenPossibleWithSuggestionEnforcement } };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// 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.Collections.Generic;
using System.Collections.Immutable;
using System.Threading.Tasks;
Expand Down Expand Up @@ -38,7 +36,7 @@ public GenerateVariableTests(ITestOutputHelper logger)
{
}

internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
internal override (DiagnosticAnalyzer?, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
=> (null, new CSharpGenerateVariableCodeFixProvider());

private readonly CodeStyleOption2<bool> onWithInfo = new CodeStyleOption2<bool>(true, NotificationOption2.Suggestion);
Expand Down Expand Up @@ -3515,6 +3513,36 @@ void goo()
}");
}

[WorkItem(49294, "https://github.com/dotnet/roslyn/issues/49294")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateVariable)]
public async Task TestPropertyInWithInitializer()
{
await TestInRegularAndScriptAsync(
@"record Goo
{
}
class Bar
{
void goo(Goo g)
{
var c = g with { [|Gibberish|] = 24 };
}
}",
@"record Goo
{
public int Gibberish { get; internal set; }
}
class Bar
{
void goo(Goo g)
{
var c = g with { Gibberish = 24 };
}
}");
}

[WorkItem(13166, "https://github.com/dotnet/roslyn/issues/13166")]
[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateVariable)]
public async Task TestPropertyOnNestedObjectInitializer()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private static void TryDetermineTypeToGenerateInWorker(
return;
}

if (syntaxFacts.IsObjectInitializerNamedAssignmentIdentifier(
if (syntaxFacts.IsMemberInitializerNamedAssignmentIdentifier(
expression, out var initializedObject))
{
typeToGenerateIn = semanticModel.GetTypeInfo(initializedObject, cancellationToken).Type as INamedTypeSymbol;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ private bool TryInitializeSimpleName(
IsInConstructor = DetermineIsInConstructor(semanticDocument, simpleName);
IsInMemberContext =
simpleName != SimpleNameOrMemberAccessExpressionOpt ||
syntaxFacts.IsObjectInitializerNamedAssignmentIdentifier(SimpleNameOrMemberAccessExpressionOpt);
syntaxFacts.IsMemberInitializerNamedAssignmentIdentifier(SimpleNameOrMemberAccessExpressionOpt);

ContainingMethod = semanticModel.GetEnclosingSymbol<IMethodSymbol>(IdentifierToken.SpanStart, cancellationToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ private static async Task ReplaceReferencesAsync(
editor.ReplaceNode(parent, parent.WithAdditionalAnnotations(
ConflictAnnotation.Create(FeaturesResources.Property_referenced_implicitly)));
}
else if (syntaxFacts.IsObjectInitializerNamedAssignmentIdentifier(parent))
else if (syntaxFacts.IsMemberInitializerNamedAssignmentIdentifier(parent))
{
editor.ReplaceNode(parent, parent.WithAdditionalAnnotations(
ConflictAnnotation.Create(FeaturesResources.Property_reference_cannot_be_updated)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,27 +667,35 @@ public bool IsNameOfSubpattern([NotNullWhen(true)] SyntaxNode? node)
public bool IsPropertyPatternClause(SyntaxNode node)
=> node.Kind() == SyntaxKind.PropertyPatternClause;

public bool IsObjectInitializerNamedAssignmentIdentifier([NotNullWhen(true)] SyntaxNode? node)
=> IsObjectInitializerNamedAssignmentIdentifier(node, out _);
public bool IsMemberInitializerNamedAssignmentIdentifier([NotNullWhen(true)] SyntaxNode? node)
=> IsMemberInitializerNamedAssignmentIdentifier(node, out _);

public bool IsObjectInitializerNamedAssignmentIdentifier(
public bool IsMemberInitializerNamedAssignmentIdentifier(
[NotNullWhen(true)] SyntaxNode? node, [NotNullWhen(true)] out SyntaxNode? initializedInstance)
{
initializedInstance = null;
if (node is IdentifierNameSyntax identifier &&
identifier.IsLeftSideOfAssignExpression() &&
identifier.Parent.IsParentKind(SyntaxKind.ObjectInitializerExpression))
identifier.IsLeftSideOfAssignExpression())
{
var objectInitializer = identifier.Parent.Parent;
if (objectInitializer.IsParentKind(SyntaxKind.ObjectCreationExpression))
if (identifier.Parent.IsParentKind(SyntaxKind.WithInitializerExpression))
{
initializedInstance = objectInitializer.Parent!;
var withInitializer = identifier.Parent.GetRequiredParent();
initializedInstance = withInitializer.GetRequiredParent();
return true;
}
else if (objectInitializer.IsParentKind(SyntaxKind.SimpleAssignmentExpression, out AssignmentExpressionSyntax? assignment))
else if (identifier.Parent.IsParentKind(SyntaxKind.ObjectInitializerExpression))
{
initializedInstance = assignment.Left;
return true;
var objectInitializer = identifier.Parent.GetRequiredParent();
if (objectInitializer.Parent is BaseObjectCreationExpressionSyntax)
{
initializedInstance = objectInitializer.Parent;
return true;
}
else if (objectInitializer.IsParentKind(SyntaxKind.SimpleAssignmentExpression, out AssignmentExpressionSyntax? assignment))
{
initializedInstance = assignment.Left;
return true;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ namespace Microsoft.CodeAnalysis.Shared.Extensions
{
internal static class SyntaxNodeExtensions
{
public static SyntaxNode GetRequiredParent(this SyntaxNode node)
=> node.Parent ?? throw new InvalidOperationException("Node's parent was null");

public static IEnumerable<SyntaxNodeOrToken> DepthFirstTraversal(this SyntaxNode node)
=> SyntaxNodeOrTokenExtensions.DepthFirstTraversal(node);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ void GetPartsOfTupleExpression<TArgumentSyntax>(SyntaxNode node,
SyntaxList<SyntaxNode> GetAttributeLists(SyntaxNode? node);

bool IsAttributeNamedArgumentIdentifier([NotNullWhen(true)] SyntaxNode? node);
bool IsObjectInitializerNamedAssignmentIdentifier([NotNullWhen(true)] SyntaxNode? node);
bool IsObjectInitializerNamedAssignmentIdentifier([NotNullWhen(true)] SyntaxNode? node, [NotNullWhen(true)] out SyntaxNode? initializedInstance);
bool IsMemberInitializerNamedAssignmentIdentifier([NotNullWhen(true)] SyntaxNode? node);
bool IsMemberInitializerNamedAssignmentIdentifier([NotNullWhen(true)] SyntaxNode? node, [NotNullWhen(true)] out SyntaxNode? initializedInstance);

bool IsDirective([NotNullWhen(true)] SyntaxNode? node);
bool IsStatement([NotNullWhen(true)] SyntaxNode? node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -685,14 +685,14 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageServices
Return node.FindTokenOnRightOfPosition(position, includeSkipped, includeDirectives, includeDocumentationComments)
End Function

Public Function IsObjectInitializerNamedAssignmentIdentifier(node As SyntaxNode) As Boolean Implements ISyntaxFacts.IsObjectInitializerNamedAssignmentIdentifier
Public Function IsMemberInitializerNamedAssignmentIdentifier(node As SyntaxNode) As Boolean Implements ISyntaxFacts.IsMemberInitializerNamedAssignmentIdentifier
Dim unused As SyntaxNode = Nothing
Return IsObjectInitializerNamedAssignmentIdentifier(node, unused)
Return IsMemberInitializerNamedAssignmentIdentifier(node, unused)
End Function

Public Function IsObjectInitializerNamedAssignmentIdentifier(
Public Function IsMemberInitializerNamedAssignmentIdentifier(
node As SyntaxNode,
ByRef initializedInstance As SyntaxNode) As Boolean Implements ISyntaxFacts.IsObjectInitializerNamedAssignmentIdentifier
ByRef initializedInstance As SyntaxNode) As Boolean Implements ISyntaxFacts.IsMemberInitializerNamedAssignmentIdentifier

Dim identifier = TryCast(node, IdentifierNameSyntax)
If identifier?.IsChildNode(Of NamedFieldInitializerSyntax)(Function(n) n.Name) Then
Expand Down

0 comments on commit 782c293

Please sign in to comment.