Skip to content

Commit f262c13

Browse files
sharwellsharwell
authored
Merge pull request #3278 from sharwell/Fix3277
Support implicit object creation expressions in SA1129 code fix
2 parents c8ce94b + 46d2e37 commit f262c13

File tree

6 files changed

+154
-26
lines changed

6 files changed

+154
-26
lines changed

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1129CodeFixProvider.cs‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ namespace StyleCop.Analyzers.ReadabilityRules
1515
using Microsoft.CodeAnalysis.CodeFixes;
1616
using Microsoft.CodeAnalysis.CSharp;
1717
using Microsoft.CodeAnalysis.CSharp.Syntax;
18+
using Microsoft.CodeAnalysis.Editing;
1819
using StyleCop.Analyzers.Helpers;
20+
using StyleCop.Analyzers.Lightup;
1921

2022
/// <summary>
2123
/// Implements a code fix for <see cref="SA1129DoNotUseDefaultValueTypeConstructor"/>.
@@ -54,17 +56,19 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
5456
var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
5557

5658
var newExpression = syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
57-
var newSyntaxRoot = syntaxRoot.ReplaceNode(newExpression, GetReplacementNode(newExpression, semanticModel, cancellationToken));
59+
var newSyntaxRoot = syntaxRoot.ReplaceNode(newExpression, GetReplacementNode(document.Project, newExpression, semanticModel, cancellationToken));
5860

5961
return document.WithSyntaxRoot(newSyntaxRoot);
6062
}
6163

62-
private static SyntaxNode GetReplacementNode(SyntaxNode node, SemanticModel semanticModel, CancellationToken cancellationToken)
64+
private static SyntaxNode GetReplacementNode(Project project, SyntaxNode node, SemanticModel semanticModel, CancellationToken cancellationToken)
6365
{
64-
var newExpression = (ObjectCreationExpressionSyntax)node;
66+
var newExpression = (BaseObjectCreationExpressionSyntaxWrapper)node;
6567

66-
var symbolInfo = semanticModel.GetSymbolInfo(newExpression.Type, cancellationToken);
67-
var namedTypeSymbol = symbolInfo.Symbol as INamedTypeSymbol;
68+
var symbolInfo = semanticModel.GetSymbolInfo(newExpression, cancellationToken);
69+
var namedTypeSymbol = (symbolInfo.Symbol as IMethodSymbol)?.ContainingType;
70+
71+
var type = GetOrCreateTypeSyntax(project, newExpression, namedTypeSymbol);
6872

6973
SyntaxNode replacement;
7074

@@ -75,7 +79,7 @@ private static SyntaxNode GetReplacementNode(SyntaxNode node, SemanticModel sema
7579
{
7680
if (IsDefaultParameterValue(newExpression))
7781
{
78-
replacement = SyntaxFactory.DefaultExpression(newExpression.Type);
82+
replacement = SyntaxFactory.DefaultExpression(type);
7983
}
8084
else
8185
{
@@ -98,21 +102,34 @@ private static SyntaxNode GetReplacementNode(SyntaxNode node, SemanticModel sema
98102
fieldName = nameof(Guid.Empty);
99103
}
100104

101-
replacement = ConstructMemberAccessSyntax(newExpression.Type, fieldName);
105+
replacement = ConstructMemberAccessSyntax(type, fieldName);
102106
}
103107
}
104108
else if (IsEnumWithDefaultMember(namedTypeSymbol, out string memberName))
105109
{
106-
replacement = ConstructMemberAccessSyntax(newExpression.Type, memberName);
110+
replacement = ConstructMemberAccessSyntax(type, memberName);
107111
}
108112
else
109113
{
110-
replacement = SyntaxFactory.DefaultExpression(newExpression.Type);
114+
replacement = SyntaxFactory.DefaultExpression(type);
111115
}
112116

113117
return replacement
114-
.WithLeadingTrivia(newExpression.GetLeadingTrivia())
115-
.WithTrailingTrivia(newExpression.GetTrailingTrivia());
118+
.WithLeadingTrivia(newExpression.SyntaxNode.GetLeadingTrivia())
119+
.WithTrailingTrivia(newExpression.SyntaxNode.GetTrailingTrivia());
120+
}
121+
122+
private static TypeSyntax GetOrCreateTypeSyntax(Project project, BaseObjectCreationExpressionSyntaxWrapper baseObjectCreationExpression, INamedTypeSymbol constructedType)
123+
{
124+
if (baseObjectCreationExpression.SyntaxNode is ObjectCreationExpressionSyntax objectCreationExpressionSyntax)
125+
{
126+
return objectCreationExpressionSyntax.Type;
127+
}
128+
else
129+
{
130+
SyntaxGenerator generator = SyntaxGenerator.GetGenerator(project);
131+
return (TypeSyntax)generator.TypeExpression(constructedType);
132+
}
116133
}
117134

118135
/// <summary>
@@ -146,9 +163,9 @@ private static bool IsType<T>(INamedTypeSymbol namedTypeSymbol)
146163
return true;
147164
}
148165

149-
private static bool IsDefaultParameterValue(ObjectCreationExpressionSyntax expression)
166+
private static bool IsDefaultParameterValue(BaseObjectCreationExpressionSyntaxWrapper expression)
150167
{
151-
if (expression.Parent.Parent is ParameterSyntax parameterSyntax)
168+
if (expression.SyntaxNode.Parent.Parent is ParameterSyntax parameterSyntax)
152169
{
153170
return parameterSyntax.Parent.Parent is BaseMethodDeclarationSyntax;
154171
}
@@ -221,7 +238,7 @@ protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fi
221238

222239
var nodes = diagnostics.Select(diagnostic => syntaxRoot.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true));
223240

224-
return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => GetReplacementNode(rewrittenNode, semanticModel, fixAllContext.CancellationToken));
241+
return syntaxRoot.ReplaceNodes(nodes, (originalNode, rewrittenNode) => GetReplacementNode(document.Project, rewrittenNode, semanticModel, fixAllContext.CancellationToken));
225242
}
226243
}
227244
}

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeGeneration/SyntaxLightupGenerator.cs‎

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,6 @@ private void GenerateSyntaxWrapperHelper(in GeneratorExecutionContext context, I
861861
SyntaxFactory.IdentifierName("Type"),
862862
})))))))))));
863863

864-
bool first = true;
865864
foreach (var node in wrapperTypes.OrderBy(node => node.Name, StringComparer.OrdinalIgnoreCase))
866865
{
867866
if (node.WrapperName is null)
@@ -924,8 +923,67 @@ private void GenerateSyntaxWrapperHelper(in GeneratorExecutionContext context, I
924923
continue;
925924
}
926925

926+
if (node.Name == nameof(BaseObjectCreationExpressionSyntax))
927+
{
928+
// Prior to C# 9, ObjectCreationExpressionSyntax was the base type for all object creation
929+
// statements. If the BaseObjectCreationExpressionSyntax type isn't found at runtime, we fall back
930+
// to using this type instead.
931+
//
932+
// var objectCreationExpressionSyntaxType = csharpCodeAnalysisAssembly.GetType(BaseObjectCreationExpressionSyntaxWrapper.WrappedTypeName)
933+
// ?? csharpCodeAnalysisAssembly.GetType(BaseObjectCreationExpressionSyntaxWrapper.FallbackWrappedTypeName);
934+
LocalDeclarationStatementSyntax localStatement =
935+
SyntaxFactory.LocalDeclarationStatement(SyntaxFactory.VariableDeclaration(
936+
type: SyntaxFactory.IdentifierName("var"),
937+
variables: SyntaxFactory.SingletonSeparatedList(SyntaxFactory.VariableDeclarator(
938+
identifier: SyntaxFactory.Identifier("objectCreationExpressionSyntaxType"),
939+
argumentList: null,
940+
initializer: SyntaxFactory.EqualsValueClause(
941+
SyntaxFactory.BinaryExpression(
942+
SyntaxKind.CoalesceExpression,
943+
left: SyntaxFactory.InvocationExpression(
944+
expression: SyntaxFactory.MemberAccessExpression(
945+
SyntaxKind.SimpleMemberAccessExpression,
946+
expression: SyntaxFactory.IdentifierName("csharpCodeAnalysisAssembly"),
947+
name: SyntaxFactory.IdentifierName("GetType")),
948+
argumentList: SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList(SyntaxFactory.Argument(
949+
SyntaxFactory.MemberAccessExpression(
950+
SyntaxKind.SimpleMemberAccessExpression,
951+
expression: SyntaxFactory.IdentifierName(node.WrapperName),
952+
name: SyntaxFactory.IdentifierName("WrappedTypeName")))))),
953+
right: SyntaxFactory.InvocationExpression(
954+
expression: SyntaxFactory.MemberAccessExpression(
955+
SyntaxKind.SimpleMemberAccessExpression,
956+
expression: SyntaxFactory.IdentifierName("csharpCodeAnalysisAssembly"),
957+
name: SyntaxFactory.IdentifierName("GetType")),
958+
argumentList: SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList(SyntaxFactory.Argument(
959+
SyntaxFactory.MemberAccessExpression(
960+
SyntaxKind.SimpleMemberAccessExpression,
961+
expression: SyntaxFactory.IdentifierName(node.WrapperName),
962+
name: SyntaxFactory.IdentifierName("FallbackWrappedTypeName"))))))))))));
963+
964+
// This is the first line of the statements that initialize 'builder', so start it with a blank line
965+
staticCtorStatements = staticCtorStatements.Add(localStatement.WithLeadingBlankLine());
966+
967+
// builder.Add(typeof(BaseObjectCreationExpressionSyntaxWrapper), objectCreationExpressionSyntaxType);
968+
staticCtorStatements = staticCtorStatements.Add(SyntaxFactory.ExpressionStatement(
969+
SyntaxFactory.InvocationExpression(
970+
expression: SyntaxFactory.MemberAccessExpression(
971+
SyntaxKind.SimpleMemberAccessExpression,
972+
expression: SyntaxFactory.IdentifierName("builder"),
973+
name: SyntaxFactory.IdentifierName("Add")),
974+
argumentList: SyntaxFactory.ArgumentList(
975+
SyntaxFactory.SeparatedList(
976+
new[]
977+
{
978+
SyntaxFactory.Argument(SyntaxFactory.TypeOfExpression(SyntaxFactory.IdentifierName(node.WrapperName))),
979+
SyntaxFactory.Argument(SyntaxFactory.IdentifierName("objectCreationExpressionSyntaxType")),
980+
})))));
981+
982+
continue;
983+
}
984+
927985
// builder.Add(typeof(ConstantPatternSyntaxWrapper), csharpCodeAnalysisAssembly.GetType(ConstantPatternSyntaxWrapper.WrappedTypeName));
928-
ExpressionStatementSyntax statement = SyntaxFactory.ExpressionStatement(
986+
staticCtorStatements = staticCtorStatements.Add(SyntaxFactory.ExpressionStatement(
929987
SyntaxFactory.InvocationExpression(
930988
expression: SyntaxFactory.MemberAccessExpression(
931989
SyntaxKind.SimpleMemberAccessExpression,
@@ -947,15 +1005,7 @@ private void GenerateSyntaxWrapperHelper(in GeneratorExecutionContext context, I
9471005
SyntaxKind.SimpleMemberAccessExpression,
9481006
expression: SyntaxFactory.IdentifierName(node.WrapperName),
9491007
name: SyntaxFactory.IdentifierName("WrappedTypeName"))))))),
950-
}))));
951-
952-
if (first)
953-
{
954-
statement = statement.WithLeadingBlankLine();
955-
first = false;
956-
}
957-
958-
staticCtorStatements = staticCtorStatements.Add(statement);
1008+
})))));
9591009
}
9601010

9611011
// WrappedTypes = builder.ToImmutable();

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp9/ReadabilityRules/SA1129CSharp9UnitTests.cs‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,46 @@
33

44
namespace StyleCop.Analyzers.Test.CSharp9.ReadabilityRules
55
{
6+
using System.Threading;
7+
using System.Threading.Tasks;
8+
using Microsoft.CodeAnalysis.Testing;
69
using StyleCop.Analyzers.Test.CSharp8.ReadabilityRules;
10+
using Xunit;
11+
using static StyleCop.Analyzers.Test.Verifiers.StyleCopCodeFixVerifier<
12+
StyleCop.Analyzers.ReadabilityRules.SA1129DoNotUseDefaultValueTypeConstructor,
13+
StyleCop.Analyzers.ReadabilityRules.SA1129CodeFixProvider>;
714

815
public class SA1129CSharp9UnitTests : SA1129CSharp8UnitTests
916
{
17+
/// <summary>
18+
/// Verifies that target type new expressions for value types will generate diagnostics.
19+
/// </summary>
20+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
21+
[Fact]
22+
[WorkItem(3277, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3277")]
23+
public async Task VerifyValueTypeWithTargetTypeNewAsync()
24+
{
25+
var testCode = @"struct S
26+
{
27+
internal static S F()
28+
{
29+
S s = [|new()|];
30+
return s;
31+
}
32+
}
33+
";
34+
35+
var fixedTestCode = @"struct S
36+
{
37+
internal static S F()
38+
{
39+
S s = default(S);
40+
return s;
41+
}
42+
}
43+
";
44+
45+
await VerifyCSharpFixAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
46+
}
1047
}
1148
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/Lightup/SyntaxWrapperHelperTests.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ public void VerifyThatWrapperClassIsPresent(Type wrapperType)
4444
Assert.False(LightupHelpers.SupportsCSharp7);
4545
Assert.Same(typeof(ForEachStatementSyntax), SyntaxWrapperHelper.GetWrappedType(wrapperType));
4646
}
47+
else if (wrapperType == typeof(BaseObjectCreationExpressionSyntaxWrapper))
48+
{
49+
// Special case for C# 6-8 analysis compatibility
50+
Assert.False(LightupHelpers.SupportsCSharp9);
51+
Assert.Same(typeof(ObjectCreationExpressionSyntax), SyntaxWrapperHelper.GetWrappedType(wrapperType));
52+
}
4753
else
4854
{
4955
Assert.Null(SyntaxWrapperHelper.GetWrappedType(wrapperType));

‎StyleCop.Analyzers/StyleCop.Analyzers/Lightup/.generated/StyleCop.Analyzers.CodeGeneration/StyleCop.Analyzers.CodeGeneration.SyntaxLightupGenerator/SyntaxWrapperHelper.g.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ static SyntaxWrapperHelper()
1818
var csharpCodeAnalysisAssembly = typeof(CSharpSyntaxNode).GetTypeInfo().Assembly;
1919
var builder = ImmutableDictionary.CreateBuilder<Type, Type>();
2020

21-
builder.Add(typeof(BaseObjectCreationExpressionSyntaxWrapper), csharpCodeAnalysisAssembly.GetType(BaseObjectCreationExpressionSyntaxWrapper.WrappedTypeName));
21+
var objectCreationExpressionSyntaxType = csharpCodeAnalysisAssembly.GetType(BaseObjectCreationExpressionSyntaxWrapper.WrappedTypeName) ?? csharpCodeAnalysisAssembly.GetType(BaseObjectCreationExpressionSyntaxWrapper.FallbackWrappedTypeName);
22+
builder.Add(typeof(BaseObjectCreationExpressionSyntaxWrapper), objectCreationExpressionSyntaxType);
2223
builder.Add(typeof(BaseParameterSyntaxWrapper), csharpCodeAnalysisAssembly.GetType(BaseParameterSyntaxWrapper.WrappedTypeName));
2324
builder.Add(typeof(BinaryPatternSyntaxWrapper), csharpCodeAnalysisAssembly.GetType(BinaryPatternSyntaxWrapper.WrappedTypeName));
2425
builder.Add(typeof(CasePatternSwitchLabelSyntaxWrapper), csharpCodeAnalysisAssembly.GetType(CasePatternSwitchLabelSyntaxWrapper.WrappedTypeName));

‎StyleCop.Analyzers/StyleCop.Analyzers/Lightup/BaseObjectCreationExpressionSyntaxWrapper.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the MIT License. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.Lightup
5+
{
6+
using Microsoft.CodeAnalysis.CSharp.Syntax;
7+
8+
internal partial struct BaseObjectCreationExpressionSyntaxWrapper : ISyntaxWrapper<ExpressionSyntax>
9+
{
10+
internal const string FallbackWrappedTypeName = "Microsoft.CodeAnalysis.CSharp.Syntax.ObjectCreationExpressionSyntax";
11+
12+
public static implicit operator BaseObjectCreationExpressionSyntaxWrapper(ObjectCreationExpressionSyntax node)
13+
{
14+
return new BaseObjectCreationExpressionSyntaxWrapper(node);
15+
}
16+
}
17+
}

0 commit comments

Comments
 (0)