Skip to content
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

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
1c5303d
Add prefer trailing commas code-style option
Youssef1313 Jul 16, 2021
0d6b5bd
Simplify
Youssef1313 Jul 16, 2021
3679d87
Merge branch 'main' into trailing-comma-code-style
Youssef1313 Jan 18, 2022
2620913
VS and editorconfig
Youssef1313 Jan 18, 2022
593f10f
More fixes
Youssef1313 Jan 18, 2022
94eb765
Update xlf
Youssef1313 Jan 18, 2022
6943fb0
Update src/VisualStudio/CSharp/Impl/Options/Formatting/StyleViewModel.cs
Youssef1313 Jan 18, 2022
200863c
Singular comma
Youssef1313 Feb 5, 2022
0118f63
Merge branch 'main' into trailing-comma-code-style
Youssef1313 Feb 5, 2022
f5bda13
Fix error
Youssef1313 Feb 5, 2022
04a6ed0
Missing commit
Youssef1313 Feb 5, 2022
2c46c77
Merge remote-tracking branch 'upstream/main' into trailing-comma-code…
Youssef1313 Jun 24, 2022
6a6313c
Adapt implementation to recent refactoring
Youssef1313 Jun 24, 2022
d06994c
Delete CSharpCodeGenerationPreferences.cs
Youssef1313 Jun 24, 2022
3d0270a
Update CSharpCodeGenerationOptions.cs
Youssef1313 Jun 24, 2022
acdcbdb
Few fixes
Youssef1313 Jun 24, 2022
4757f83
Merge branch 'trailing-comma-code-style' of https://GitHub.com/Yousse…
Youssef1313 Jun 24, 2022
b776b61
Code fix
Youssef1313 Jun 24, 2022
64972d8
More progress
Youssef1313 Jun 24, 2022
cb0ae2b
Adjust implementation
Youssef1313 Jun 24, 2022
c62f326
Fix incorrectly resolved conflicts
Youssef1313 Jun 24, 2022
c79ba3b
Update ServicesVSResources.ru.xlf
Youssef1313 Jun 24, 2022
effe159
More work.. Nested diagnostics fixall is failing..
Youssef1313 Jun 24, 2022
cc0e20a
Add empty enum test
Youssef1313 Jun 24, 2022
cce88be
Update ServicesVSResources.tr.xlf
Youssef1313 Jun 24, 2022
7c4b00d
Merge branch 'trailing-comma-code-style' of https://GitHub.com/Yousse…
Youssef1313 Jun 24, 2022
b317c71
Update ServicesVSResources.zh-Hans.xlf
Youssef1313 Jun 24, 2022
84b1086
Fix failing tests
Youssef1313 Jun 25, 2022
6fa0027
Support prefer trailing comma in use object initializer
Youssef1313 Jun 25, 2022
23fa534
Support prefer trailing comma in use collection initializer
Youssef1313 Jun 25, 2022
9c9cb3f
Few fixes
Youssef1313 Jun 25, 2022
7b7412e
Fix build
Youssef1313 Jun 25, 2022
fd8489e
Merge remote-tracking branch 'upstream/main' into trailing-comma-code…
Youssef1313 Jul 8, 2022
9f16809
Fix formatting
Youssef1313 Jul 8, 2022
499ab63
Add test
Youssef1313 Jul 8, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
<Compile Include="$(MSBuildThisFileDirectory)NewLines\ConstructorInitializerPlacement\ConstructorInitializerPlacementDiagnosticAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)NewLines\MultipleBlankLines\CSharpMultipleBlankLinesDiagnosticAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)MatchFolderAndNamespace\CSharpMatchFolderAndNamespaceDiagnosticAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)PreferTrailingComma\PreferTrailingCommaDiagnosticAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)RemoveRedundantEquality\CSharpRemoveRedundantEqualityDiagnosticAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)RemoveUnnecessaryDiscardDesignation\CSharpRemoveUnnecessaryDiscardDesignationDiagnosticAnalyzer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)RemoveUnnecessaryLambdaExpression\CSharpRemoveUnnecessaryLambdaExpressionDiagnosticAnalyzer.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,4 +383,7 @@
<data name="Remove_redundant_nullable_directive" xml:space="preserve">
<value>Remove redundant nullable directive</value>
</data>
<data name="Missing_trailing_comma" xml:space="preserve">
<value>Missing trailing comma</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public CSharpAnalyzerOptionsProvider(AnalyzerConfigOptions options, AnalyzerOpti
public CodeStyleOption2<bool> AllowEmbeddedStatementsOnSameLine => GetOption(CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, FallbackSimplifierOptions.AllowEmbeddedStatementsOnSameLine);
public CodeStyleOption2<bool> PreferThrowExpression => GetOption(CSharpCodeStyleOptions.PreferThrowExpression, FallbackSimplifierOptions.PreferThrowExpression);
public CodeStyleOption2<PreferBracesPreference> PreferBraces => GetOption(CSharpCodeStyleOptions.PreferBraces, FallbackSimplifierOptions.PreferBraces);
public CodeStyleOption2<bool> PreferTrailingComma => GetOption(CSharpCodeStyleOptions.PreferTrailingComma, FallbackSimplifierOptions.PreferTrailingComma);

internal CSharpSimplifierOptions GetSimplifierOptions()
=> _options.GetCSharpSimplifierOptions(FallbackSimplifierOptions);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// 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,
Copy link
Member

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.

Copy link
Member Author

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.

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)
{
context.RegisterSyntaxNodeAction(AnalyzeSyntaxNode,
SyntaxKind.EnumDeclaration, SyntaxKind.PropertyPatternClause, SyntaxKind.SwitchExpression,
SyntaxKind.ObjectInitializerExpression, SyntaxKind.CollectionInitializerExpression, SyntaxKind.WithInitializerExpression,
SyntaxKind.ArrayInitializerExpression, SyntaxKind.AnonymousObjectCreationExpression, SyntaxKind.ListPattern);
}

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 nextToken = lastCurrentToken.GetNextToken();
if (nextToken == default)
{
return true;
}

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(),
SwitchExpressionSyntax switchExpression => switchExpression.Arms.GetWithSeparators(),
InitializerExpressionSyntax initializerExpression => initializerExpression.Expressions.GetWithSeparators(),
AnonymousObjectCreationExpressionSyntax anonymousObjectCreation => anonymousObjectCreation.Initializers.GetWithSeparators(),
ListPatternSyntax listPattern => listPattern.Patterns.GetWithSeparators(),
_ => throw ExceptionUtilities.Unreachable,
};
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,10 @@ protected override bool AreCollectionInitializersSupported(Compilation compilati
=> compilation.LanguageVersion() >= LanguageVersion.CSharp3;

protected override ISyntaxFacts GetSyntaxFacts() => CSharpSyntaxFacts.Instance;

protected override bool PreferTrailingComma(SyntaxNodeAnalysisContext context)
{
return context.GetCSharpAnalyzerOptions().PreferTrailingComma.Value;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,10 @@ protected override bool IsValidContainingStatement(StatementSyntax node)
return node is not LocalDeclarationStatementSyntax localDecl ||
localDecl.UsingKeyword == default;
}

protected override bool PreferTrailingComma(SyntaxNodeAnalysisContext context)
{
return context.GetCSharpAnalyzerOptions().PreferTrailingComma.Value;
}
}
}

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.

1 change: 1 addition & 0 deletions src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
<Compile Include="$(MSBuildThisFileDirectory)MakeStatementAsynchronous\CSharpMakeStatementAsynchronousCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)MakeTypeAbstract\CSharpMakeTypeAbstractCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Nullable\CSharpDeclareAsNullableCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)PreferTrailingComma\PreferTrailingCommaCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)RemoveUnnecessaryNullableDirective\CSharpRemoveUnnecessaryNullableDirectiveCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)UnsealClass\CSharpUnsealClassCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)UseInterpolatedVerbatimString\CSharpUseInterpolatedVerbatimStringCodeFixProvider.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,7 @@
<data name="Declare_as_nullable" xml:space="preserve">
<value>Declare as nullable</value>
</data>
<data name="Add_trailing_comma" xml:space="preserve">
<value>Add trailing comma</value>
</data>
</root>
Loading