Skip to content

Commit 4711fbc

Browse files
author
fabien.menager
committed
Guard against insupported syntax
1 parent ae40b47 commit 4711fbc

4 files changed

Lines changed: 117 additions & 2 deletions

File tree

src/EntityFrameworkCore.Projectables.CodeFixes/FactoryMethodTransformationHelper.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,9 +347,11 @@ private static SyntaxNode BuildRootWithConstructor(
347347
var initializer = creation.Initializer!;
348348

349349
// Only support simple object-initializer assignments (Prop = value). If there are
350-
// other initializer forms (e.g., collection initializers), bail out to avoid
350+
// other initializer forms (e.g., collection initializers) or assignments whose RHS
351+
// is a nested initializer (e.g. Items = { 1, 2 }), bail out to avoid
351352
// producing a constructor that does not preserve behavior.
352-
if (initializer.Expressions.Any(e => e is not AssignmentExpressionSyntax))
353+
if (initializer.Expressions.Any(
354+
e => e is not AssignmentExpressionSyntax { Right: not InitializerExpressionSyntax }))
353355
{
354356
return root;
355357
}

src/EntityFrameworkCore.Projectables.CodeFixes/SyntaxHelpers.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,17 @@ static internal bool TryGetFactoryMethodPattern(
5454
return false;
5555
}
5656

57+
// Only pure simple-assignment initializers (Prop = value) — no bare collection
58+
// elements (which are not AssignmentExpressionSyntax) and no nested initializer
59+
// assignments (Items = { 1, 2 }) whose RHS is an InitializerExpressionSyntax.
60+
// Converting such entries to statements would produce invalid C#, so we must not
61+
// offer the refactoring at all for these patterns.
62+
if (creation.Initializer.Expressions.Any(
63+
e => e is not AssignmentExpressionSyntax { Right: not InitializerExpressionSyntax }))
64+
{
65+
return false;
66+
}
67+
5768
// Only allow static factory methods, to keep the code fix simpler
5869
if (!method.Modifiers.Any(m => m.IsKind(SyntaxKind.StaticKeyword)))
5970
{

tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeFixProviderTests.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,4 +130,56 @@ class MyObj {
130130

131131
Assert.Empty(actions);
132132
}
133+
134+
/// <summary>
135+
/// <c>Items = { 1, 2 }</c> in an object initializer is an
136+
/// <see cref="Microsoft.CodeAnalysis.CSharp.Syntax.AssignmentExpressionSyntax"/>
137+
/// whose RHS is an <see cref="Microsoft.CodeAnalysis.CSharp.Syntax.InitializerExpressionSyntax"/>.
138+
/// Converting it to a statement produces invalid C# (<c>Items = { 1, 2 };</c>),
139+
/// so the code fix must not be offered for this pattern.
140+
/// </summary>
141+
[Fact]
142+
public async Task NoCodeFix_WhenInitializerHasNestedCollectionInitializer()
143+
{
144+
var actions = await GetCodeFixActionsAsync(
145+
@"
146+
using System.Collections.Generic;
147+
namespace Foo {
148+
class MyObj {
149+
public List<int> Items { get; set; }
150+
[Projectable]
151+
public static MyObj Create() => new MyObj { Items = { 1, 2 } };
152+
}
153+
}",
154+
"EFP0012",
155+
FactoryMethodToCtorSources.FirstMethodIdentifierSpan,
156+
_provider);
157+
158+
Assert.Empty(actions);
159+
}
160+
161+
/// <summary>
162+
/// A mixed initializer that combines a simple assignment with a nested collection
163+
/// initializer (<c>Items = { 1, 2 }</c>) must also be rejected.
164+
/// </summary>
165+
[Fact]
166+
public async Task NoCodeFix_WhenMixedSimpleAndNestedCollectionInitializer()
167+
{
168+
var actions = await GetCodeFixActionsAsync(
169+
@"
170+
using System.Collections.Generic;
171+
namespace Foo {
172+
class MyObj {
173+
public int Value { get; set; }
174+
public List<int> Items { get; set; }
175+
[Projectable]
176+
public static MyObj Create(int v) => new MyObj { Value = v, Items = { 1, 2 } };
177+
}
178+
}",
179+
"EFP0012",
180+
FactoryMethodToCtorSources.FirstMethodIdentifierSpan,
181+
_provider);
182+
183+
Assert.Empty(actions);
184+
}
133185
}

tests/EntityFrameworkCore.Projectables.CodeFixes.Tests/FactoryMethodToCtorCodeRefProviderTests.cs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,56 @@ class MyObj {
205205
Assert.Empty(actions);
206206
}
207207

208+
/// <summary>
209+
/// <c>Items = { 1, 2 }</c> in an object initializer is an
210+
/// <see cref="Microsoft.CodeAnalysis.CSharp.Syntax.AssignmentExpressionSyntax"/>
211+
/// whose RHS is an <see cref="Microsoft.CodeAnalysis.CSharp.Syntax.InitializerExpressionSyntax"/>.
212+
/// Converting it to a statement produces invalid C# (<c>Items = { 1, 2 };</c>),
213+
/// so the refactoring must not be offered for this pattern.
214+
/// </summary>
215+
[Fact]
216+
public async Task NoRefactoring_WhenInitializerHasNestedCollectionInitializer()
217+
{
218+
var actions = await GetRefactoringActionsAsync(
219+
@"
220+
using System.Collections.Generic;
221+
namespace Foo {
222+
class MyObj {
223+
public List<int> Items { get; set; }
224+
[Projectable]
225+
public static MyObj Create() => new MyObj { Items = { 1, 2 } };
226+
}
227+
}",
228+
FactoryMethodToCtorSources.FirstMethodIdentifierSpan,
229+
_provider);
230+
231+
Assert.Empty(actions);
232+
}
233+
234+
/// <summary>
235+
/// A mixed initializer that combines a simple assignment with a nested collection
236+
/// initializer (<c>Items = { 1, 2 }</c>) must also be rejected.
237+
/// </summary>
238+
[Fact]
239+
public async Task NoRefactoring_WhenMixedSimpleAndNestedCollectionInitializer()
240+
{
241+
var actions = await GetRefactoringActionsAsync(
242+
@"
243+
using System.Collections.Generic;
244+
namespace Foo {
245+
class MyObj {
246+
public int Value { get; set; }
247+
public List<int> Items { get; set; }
248+
[Projectable]
249+
public static MyObj Create(int v) => new MyObj { Value = v, Items = { 1, 2 } };
250+
}
251+
}",
252+
FactoryMethodToCtorSources.FirstMethodIdentifierSpan,
253+
_provider);
254+
255+
Assert.Empty(actions);
256+
}
257+
208258
/// <summary>
209259
/// Regression: <c>new Other.MyObj { }</c> has a qualified type name that cannot be
210260
/// confirmed as the containing type without a semantic model. The pattern must reject

0 commit comments

Comments
 (0)