-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add prefer trailing commas code-style option #54859
base: main
Are you sure you want to change the base?
Changes from 22 commits
1c5303d
0d6b5bd
3679d87
2620913
593f10f
94eb765
6943fb0
200863c
0118f63
f5bda13
04a6ed0
2c46c77
6a6313c
d06994c
3d0270a
acdcbdb
4757f83
b776b61
64972d8
cb0ae2b
c62f326
c79ba3b
effe159
cc0e20a
cce88be
7c4b00d
b317c71
84b1086
6fa0027
23fa534
9c9cb3f
7b7412e
fd8489e
9f16809
499ab63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading; | ||
using Microsoft.CodeAnalysis.CodeStyle; | ||
using Microsoft.CodeAnalysis.CSharp.CodeStyle; | ||
using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
using Microsoft.CodeAnalysis.Diagnostics; | ||
using Roslyn.Utilities; | ||
|
||
namespace Microsoft.CodeAnalysis.CSharp.PreferTrailingComma | ||
{ | ||
[DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
internal sealed class PreferTrailingCommaDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer | ||
{ | ||
public PreferTrailingCommaDiagnosticAnalyzer() : base( | ||
diagnosticId: IDEDiagnosticIds.PreferTrailingCommaDiagnosticId, | ||
enforceOnBuild: EnforceOnBuildValues.PreferTrailingComma, | ||
option: CSharpCodeStyleOptions.PreferTrailingComma, | ||
language: LanguageNames.CSharp, | ||
title: new LocalizableResourceString(nameof(CSharpAnalyzersResources.Missing_trailing_comma), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources))) | ||
{ | ||
} | ||
|
||
public override DiagnosticAnalyzerCategory GetAnalyzerCategory() => DiagnosticAnalyzerCategory.SyntaxTreeWithoutSemanticsAnalysis; | ||
|
||
protected override void InitializeWorker(AnalysisContext context) | ||
{ | ||
// TODO: | ||
// list patterns | ||
// anonymous object creation | ||
// initializer expression | ||
// switch expression | ||
Youssef1313 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
context.RegisterSyntaxNodeAction(AnalyzeSyntaxNode, SyntaxKind.EnumDeclaration, SyntaxKind.PropertyPatternClause); | ||
} | ||
|
||
private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context) | ||
{ | ||
var option = context.GetCSharpAnalyzerOptions().PreferTrailingComma; | ||
if (!option.Value) | ||
return; | ||
|
||
var nodesWithSeparators = GetNodesWithSeparators(context.Node); | ||
if (nodesWithSeparators.Count < 1) | ||
{ | ||
return; | ||
} | ||
|
||
var lastNodeOrSeparator = nodesWithSeparators[^1]; | ||
if (lastNodeOrSeparator.IsToken) | ||
{ | ||
return; | ||
} | ||
|
||
if (lastNodeOrSeparator.IsNode) | ||
{ | ||
var lastNode = lastNodeOrSeparator.AsNode()!; | ||
if (CommaWillBeLastTokenOnLine(lastNode, context.CancellationToken)) | ||
context.ReportDiagnostic(DiagnosticHelper.Create(Descriptor, lastNode.GetLocation(), option.Notification.Severity, additionalLocations: null, properties: null)); | ||
} | ||
} | ||
|
||
private static bool CommaWillBeLastTokenOnLine(SyntaxNode node, CancellationToken cancellationToken) | ||
{ | ||
var lines = node.SyntaxTree.GetText(cancellationToken).Lines; | ||
var lastCurrentToken = node.DescendantTokens().Last(); | ||
var line1 = lines.GetLineFromPosition(lastCurrentToken.Span.End).LineNumber; | ||
var line2 = lines.GetLineFromPosition(lastCurrentToken.GetNextToken().SpanStart).LineNumber; | ||
return line1 != line2; | ||
} | ||
|
||
internal static SyntaxNodeOrTokenList GetNodesWithSeparators(SyntaxNode node) | ||
{ | ||
return node switch | ||
{ | ||
EnumDeclarationSyntax enumDeclaration => enumDeclaration.Members.GetWithSeparators(), | ||
PropertyPatternClauseSyntax propertyPattern => propertyPattern.Subpatterns.GetWithSeparators(), | ||
_ => throw ExceptionUtilities.Unreachable, | ||
}; | ||
} | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
using System.Collections.Immutable; | ||
using System.Composition; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.CodeActions; | ||
using Microsoft.CodeAnalysis.CodeFixes; | ||
using Microsoft.CodeAnalysis.CSharp.Extensions; | ||
using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
using Microsoft.CodeAnalysis.Diagnostics; | ||
using Microsoft.CodeAnalysis.Editing; | ||
using Microsoft.CodeAnalysis.Host.Mef; | ||
using Microsoft.CodeAnalysis.Shared.Extensions; | ||
using Roslyn.Utilities; | ||
|
||
namespace Microsoft.CodeAnalysis.CSharp.PreferTrailingComma | ||
{ | ||
[ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.PreferTrailingComma), Shared] | ||
internal sealed class PreferTrailingCommaCodeFixProvider : SyntaxEditorBasedCodeFixProvider | ||
{ | ||
[ImportingConstructor] | ||
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
public PreferTrailingCommaCodeFixProvider() | ||
{ | ||
} | ||
|
||
public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(IDEDiagnosticIds.PreferTrailingCommaDiagnosticId); | ||
|
||
public override Task RegisterCodeFixesAsync(CodeFixContext context) | ||
{ | ||
foreach (var diagnostic in context.Diagnostics) | ||
RegisterCodeFix(context, "", "", diagnostic); | ||
|
||
return Task.CompletedTask; | ||
} | ||
|
||
protected override Task FixAllAsync(Document document, ImmutableArray<Diagnostic> diagnostics, SyntaxEditor editor, CodeActionOptionsProvider fallbackOptions, CancellationToken cancellationToken) | ||
{ | ||
foreach (var diagnostic in diagnostics) | ||
{ | ||
var node = diagnostic.Location.FindNode(cancellationToken).GetRequiredParent(); | ||
var nodesAndTokens = PreferTrailingCommaDiagnosticAnalyzer.GetNodesWithSeparators(node); | ||
var lastNode = nodesAndTokens[^1]; | ||
nodesAndTokens = nodesAndTokens.ReplaceRange(lastNode, ImmutableArray.Create(lastNode.WithTrailingTrivia(), SyntaxFactory.Token(leading: default, SyntaxKind.CommaToken, trailing: lastNode.GetTrailingTrivia()))); | ||
editor.ReplaceNode(node, GetReplacement(node, nodesAndTokens)); | ||
} | ||
|
||
return Task.CompletedTask; | ||
} | ||
|
||
private static SyntaxNode GetReplacement(SyntaxNode node, SyntaxNodeOrTokenList nodesAndTokens) | ||
{ | ||
return node switch | ||
{ | ||
EnumDeclarationSyntax enumDeclaration => enumDeclaration.WithMembers(SyntaxFactory.SeparatedList<EnumMemberDeclarationSyntax>(nodesAndTokens)), | ||
PropertyPatternClauseSyntax propertyPattern => propertyPattern.WithSubpatterns(SyntaxFactory.SeparatedList<SubpatternSyntax>(nodesAndTokens)), | ||
_ => throw ExceptionUtilities.Unreachable, | ||
}; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢 Every time we add an option it makes me sad.
I've found in the case of this maintainability rule, it also adds to confusion. Some users think this is a readability rule that should be configurable. However, it is not a readability rule, and mistaking the goal here will lead some users to incorrectly configure the rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sharwell Are you referring to the fact that
csharp_style_prefer_trailing_comma = false
will disable the analyzer rather than enforcing a non-trailing comma? Or in other words, you like this code-style option to be used by code generation but not by the analyzer?If that's the case, I definitely agree, but #60584 needs a decision.