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 analyzer/fixer for informal IDE brace style with statements. #4647

Closed
wants to merge 17 commits into from
Closed
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
; Please do not edit this file manually, it should only be updated through code fix application.
### New Rules
Rule ID | Category | Severity | Notes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Evangelink Didn't you fix this before (the empty line before the table)? or I'm mis-remembering?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recall doing the fix :)

--------|----------|----------|-------
RS0103 | RoslynDiagnosticsMaintainability | Warning | CSharpBlankLinesBetweenStatementsDiagnosticAnalyzer
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#nullable disable warnings
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

using System;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Roslyn.Diagnostics.Analyzers;

namespace Roslyn.Diagnostics.CSharp.Analyzers.BlankLines
{
[ExportCodeFixProvider(LanguageNames.CSharp)]
public sealed class CSharpBlankLinesBetweenStatementsCodeFixProvider : CodeFixProvider
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
private static readonly SyntaxTrivia s_endOfLine = SyntaxFactory.EndOfLine(Environment.NewLine);

public override ImmutableArray<string> FixableDiagnosticIds
=> ImmutableArray.Create(RoslynDiagnosticIds.BlankLinesBetweenStatementsRuleId);

public override FixAllProvider GetFixAllProvider()
=> WellKnownFixAllProviders.BatchFixer;

public override Task RegisterCodeFixesAsync(CodeFixContext context)
{
var document = context.Document;
var diagnostic = context.Diagnostics.First();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have some code-fixes doing a loop over the Diagnostics and some other doing .First() what is the right way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth doing an anlyzer for this then.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic is correct. If there were multiple diagnostics for the same token, we'd only want to process one of them anyways. If we looked at all diagnostics, we'd have to dedupe. So this has the same effect.

context.RegisterCodeFix(
CodeAction.Create(
RoslynDiagnosticsAnalyzersResources.Add_blank_line_after_block,
c => UpdateDocumentAsync(document, diagnostic, c),
RoslynDiagnosticsAnalyzersResources.Add_blank_line_after_block),
context.Diagnostics);
return Task.CompletedTask;

CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}

private static async Task<Document> UpdateDocumentAsync(Document document, Diagnostic diagnostic, CancellationToken cancellationToken)
{
var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

var closeBraceToken = root.FindToken(diagnostic.Location.SourceSpan.Start);
var nextToken = closeBraceToken.GetNextToken();

var newRoot = root.ReplaceToken(
nextToken,
nextToken.WithLeadingTrivia(nextToken.LeadingTrivia.Insert(0, s_endOfLine)));

return document.WithSyntaxRoot(newRoot);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;
using Analyzer.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Roslyn.Diagnostics.Analyzers;

namespace Roslyn.Diagnostics.CSharp.Analyzers.BlankLines
{
/// <summary>
/// Analyzer that finds code of the form:
/// <code>
/// if (cond)
/// {
/// }
/// NextStatement();
/// </code>
///
/// And requires it to be of the form:
/// <code>
/// if (cond)
/// {
/// }
///
/// NextStatement();
/// </code>
///
/// Specifically, all blocks followed by another statement must have a blank line between them.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpBlankLinesBetweenStatementsDiagnosticAnalyzer : DiagnosticAnalyzer
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(
nameof(RoslynDiagnosticsAnalyzersResources.BlankLinesBetweenStatementsTitle), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(
nameof(RoslynDiagnosticsAnalyzersResources.BlankLinesBetweenStatementsMessage), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));

internal static DiagnosticDescriptor Rule = new(
RoslynDiagnosticIds.BlankLinesBetweenStatementsRuleId,
s_localizableTitle,
s_localizableMessage,
DiagnosticCategory.RoslynDiagnosticsMaintainability,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
helpLinkUri: null,
customTags: WellKnownDiagnosticTags.Telemetry);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
=> ImmutableArray.Create(Rule);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

context.RegisterSyntaxNodeAction(AnalyzeBlock, SyntaxKind.Block);
}

private void AnalyzeBlock(SyntaxNodeAnalysisContext context)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
var block = (BlockSyntax)context.Node;

// Don't examine broken blocks.
var closeBrace = block.CloseBraceToken;
if (closeBrace.IsMissing)
return;
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

// If the close brace itself doesn't have a newline, then ignore this. This is a case of series of
// statements on the same line.
if (!closeBrace.TrailingTrivia.Any())
return;

if (closeBrace.TrailingTrivia.Last().Kind() != SyntaxKind.EndOfLineTrivia)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
return;

// Grab whatever comes after the close brace. If it's not the start of a statement, ignore it.
var nextToken = closeBrace.GetNextToken();
var nextTokenContainingStatement = nextToken.Parent!.FirstAncestorOrSelf<StatementSyntax>();
if (nextTokenContainingStatement == null)
return;

if (nextToken != nextTokenContainingStatement.GetFirstToken())
return;

// There has to be at least a blank line between the end of the block and the start of the next statement.

foreach (var trivia in nextToken.LeadingTrivia)
{
// If there's a blank line between the brace and the next token, we're all set.
if (trivia.Kind() == SyntaxKind.EndOfLineTrivia)
return;

if (trivia.Kind() == SyntaxKind.WhitespaceTrivia)
continue;

// got something that wasn't whitespace. Bail out as we don't want to place any restrictions on this code.
return;
}

context.ReportDiagnostic(Diagnostic.Create(
Rule,
closeBrace.GetLocation()));
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,6 @@ internal static class RoslynDiagnosticIds
public const string WrapStatementsRuleId = "RS0100";
public const string BlankLinesRuleId = "RS0101";
public const string BracePlacementRuleId = "RS0102";
public const string BlankLinesBetweenStatementsRuleId = "RS0103";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -390,4 +390,13 @@
<data name="DoNotCopyValueNoReturnValueFromReferenceMessage" xml:space="preserve">
<value>Cannot return a value from a reference to non-copyable type '{0}'</value>
</data>
<data name="BlankLinesBetweenStatementsMessage" xml:space="preserve">
<value>Blank line required after block statement</value>
</data>
<data name="BlankLinesBetweenStatementsTitle" xml:space="preserve">
<value>Blank line required after block statement</value>
</data>
<data name="Add_blank_line_after_block" xml:space="preserve">
<value>Add blank line after block</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="cs" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="Add_blank_line_after_block">
<source>Add blank line after block</source>
<target state="new">Add blank line after block</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="translated">Odeberte příponu Opt</target>
Expand All @@ -22,6 +27,16 @@
<target state="translated">Nepoužívejte příponu Opt</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsMessage">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsTitle">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="translated">Nepoužívejte několik prázdných řádků.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="de" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="Add_blank_line_after_block">
<source>Add blank line after block</source>
<target state="new">Add blank line after block</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="translated">Suffix "Opt" entfernen</target>
Expand All @@ -22,6 +27,16 @@
<target state="translated">Suffix "Opt" vermeiden</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsMessage">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsTitle">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="translated">Vermeiden Sie mehrere Leerzeilen.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="es" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="Add_blank_line_after_block">
<source>Add blank line after block</source>
<target state="new">Add blank line after block</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="translated">Quitar el sufijo "Opt"</target>
Expand All @@ -22,6 +27,16 @@
<target state="translated">Evitar el sufijo "Opt"</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsMessage">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsTitle">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="translated">Evitar varias líneas en blanco</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="fr" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="Add_blank_line_after_block">
<source>Add blank line after block</source>
<target state="new">Add blank line after block</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="translated">Supprimer le suffixe 'Opt'</target>
Expand All @@ -22,6 +27,16 @@
<target state="translated">Éviter le suffixe 'Opt'</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsMessage">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsTitle">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="translated">Éviter plusieurs lignes vides</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="it" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="Add_blank_line_after_block">
<source>Add blank line after block</source>
<target state="new">Add blank line after block</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="translated">Rimuovere il suffisso 'Opt'</target>
Expand All @@ -22,6 +27,16 @@
<target state="translated">Evitare il suffisso 'Opt'</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsMessage">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsTitle">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="translated">Evitare più righe vuote</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="ja" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="Add_blank_line_after_block">
<source>Add blank line after block</source>
<target state="new">Add blank line after block</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="translated">'Opt' サフィックスの削除</target>
Expand All @@ -22,6 +27,16 @@
<target state="translated">'Opt' サフィックスを使用しない</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsMessage">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsTitle">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="translated">複数の空白行は使用できません</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="ko" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="Add_blank_line_after_block">
<source>Add blank line after block</source>
<target state="new">Add blank line after block</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="translated">'Opt' 접미사 제거</target>
Expand All @@ -22,6 +27,16 @@
<target state="translated">'Opt' 접미사 사용 방지</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsMessage">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesBetweenStatementsTitle">
<source>Blank line required after block statement</source>
<target state="new">Blank line required after block statement</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="translated">여러 빈 줄 방지</target>
Expand Down
Loading