-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Port roslyn-analyer style rules to experimental rules in roslyn. #50358
Merged
CyrusNajmabadi
merged 40 commits into
dotnet:master
from
CyrusNajmabadi:experimentalFormatting
Feb 10, 2021
Merged
Changes from 20 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
8aacc22
Port 'wrap embedded statement' to Roslyn
CyrusNajmabadi ff4a8ff
Add options
CyrusNajmabadi 3e5867a
Switch
CyrusNajmabadi fe54d8e
Move
CyrusNajmabadi d59d427
Fix
CyrusNajmabadi 4d916ae
Port 'consecutive brace placement' to Roslyn
CyrusNajmabadi 5af2e88
Add UI
CyrusNajmabadi 2a41e0b
Port C# side of 'multiple blank lines' to Roslyn
CyrusNajmabadi baf628a
Add VB tests
CyrusNajmabadi 4f855da
Refine
CyrusNajmabadi 6152063
Fix
CyrusNajmabadi 35431ed
Fix
CyrusNajmabadi 64f10e3
Merge impls
CyrusNajmabadi 027622e
Revert
CyrusNajmabadi afddafb
Use EnforceOnBuildValues
CyrusNajmabadi 896066d
Port consecutive statement analyzer
CyrusNajmabadi 6f698eb
Add VBside
CyrusNajmabadi 10f5ad2
Add UI
CyrusNajmabadi ff8fcd5
VBtoo
CyrusNajmabadi c6cb66a
Add test
CyrusNajmabadi 7d46c47
Invert values
CyrusNajmabadi 5cfcadd
Fixup tests
CyrusNajmabadi 6f10224
Fixup tests
CyrusNajmabadi 186145b
Simplify
CyrusNajmabadi 5b50c71
Add loop statement
CyrusNajmabadi 1369ba6
Merge remote-tracking branch 'upstream/master' into experimentalForma…
CyrusNajmabadi 1003442
Add fixer to put : on the same line as his/base
CyrusNajmabadi 00e3545
Add UI
CyrusNajmabadi 2999f4f
Merge remote-tracking branch 'upstream/master' into experimentalForma…
CyrusNajmabadi 11c0c4e
Fixup tests
CyrusNajmabadi 697aacb
Merge branch 'master' into experimentalFormatting
CyrusNajmabadi c154f74
Formatting
CyrusNajmabadi e7970d4
Merge remote-tracking branch 'upstream/master' into experimentalForma…
CyrusNajmabadi e3f3192
Use syntax facts
CyrusNajmabadi 77646aa
Merge remote-tracking branch 'upstream/master' into experimentalForma…
CyrusNajmabadi 2a81547
Update IDEDiagnosticIDConfigurationTests.cs
CyrusNajmabadi b70814c
Merge remote-tracking branch 'upstream/master' into experimentalForma…
CyrusNajmabadi 00e925d
Fix tests
CyrusNajmabadi a31df02
Merge branch 'experimentalFormatting' of https://github.com/CyrusNajm…
CyrusNajmabadi 7ab8b99
Fix tests
CyrusNajmabadi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
139 changes: 139 additions & 0 deletions
139
...s/NewLines/ConsecutiveBracePlacement/CSharpConsecutiveBracePlacementDiagnosticAnalyzer.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
// 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.Linq; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.CodeStyle; | ||
using Microsoft.CodeAnalysis.CSharp.CodeStyle; | ||
using Microsoft.CodeAnalysis.Diagnostics; | ||
using Microsoft.CodeAnalysis.PooledObjects; | ||
using Microsoft.CodeAnalysis.Text; | ||
|
||
namespace Microsoft.CodeAnalysis.CSharp.NewLines.ConsecutiveBracePlacement | ||
{ | ||
[DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
internal sealed class CSharpConsecutiveBracePlacementDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer | ||
{ | ||
public CSharpConsecutiveBracePlacementDiagnosticAnalyzer() | ||
: base(IDEDiagnosticIds.ConsecutiveBracePlacementDiagnosticId, | ||
EnforceOnBuildValues.ConsecutiveBracePlacement, | ||
CSharpCodeStyleOptions.DisallowBlankLinesBetweenConsecutiveBraces, | ||
LanguageNames.CSharp, | ||
new LocalizableResourceString( | ||
nameof(CSharpAnalyzersResources.Consecutive_braces_must_not_have_a_blank_between_them), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources))) | ||
{ | ||
} | ||
|
||
public override DiagnosticAnalyzerCategory GetAnalyzerCategory() | ||
=> DiagnosticAnalyzerCategory.SyntaxTreeWithoutSemanticsAnalysis; | ||
|
||
protected override void InitializeWorker(AnalysisContext context) | ||
=> context.RegisterSyntaxTreeAction(AnalyzeTree); | ||
|
||
private void AnalyzeTree(SyntaxTreeAnalysisContext context) | ||
{ | ||
var option = context.GetOption(CSharpCodeStyleOptions.DisallowBlankLinesBetweenConsecutiveBraces); | ||
if (!option.Value) | ||
return; | ||
|
||
using var _ = ArrayBuilder<SyntaxNode>.GetInstance(out var stack); | ||
Recurse(context, option.Notification.Severity, stack); | ||
} | ||
|
||
private void Recurse(SyntaxTreeAnalysisContext context, ReportDiagnostic severity, ArrayBuilder<SyntaxNode> stack) | ||
{ | ||
var tree = context.Tree; | ||
var cancellationToken = context.CancellationToken; | ||
|
||
var root = tree.GetRoot(cancellationToken); | ||
var text = tree.GetText(cancellationToken); | ||
|
||
stack.Add(root); | ||
while (stack.Count > 0) | ||
{ | ||
cancellationToken.ThrowIfCancellationRequested(); | ||
|
||
var current = stack.Last(); | ||
stack.RemoveLast(); | ||
|
||
// Don't bother analyzing nodes that have syntax errors in them. | ||
if (current.ContainsDiagnostics && current.GetDiagnostics().Any(d => d.Severity == DiagnosticSeverity.Error)) | ||
continue; | ||
|
||
foreach (var child in current.ChildNodesAndTokens()) | ||
{ | ||
if (child.IsNode) | ||
stack.Add(child.AsNode()!); | ||
else if (child.IsToken) | ||
ProcessToken(context, severity, text, child.AsToken()); | ||
} | ||
} | ||
} | ||
|
||
private void ProcessToken(SyntaxTreeAnalysisContext context, ReportDiagnostic severity, SourceText text, SyntaxToken token) | ||
{ | ||
if (!HasExcessBlankLinesAfter(text, token, out var secondBrace, out _)) | ||
return; | ||
|
||
context.ReportDiagnostic(DiagnosticHelper.Create( | ||
this.Descriptor, | ||
secondBrace.GetLocation(), | ||
severity, | ||
additionalLocations: null, | ||
properties: null)); | ||
} | ||
|
||
public static bool HasExcessBlankLinesAfter( | ||
SourceText text, SyntaxToken token, | ||
out SyntaxToken secondBrace, | ||
out SyntaxTrivia endOfLineTrivia) | ||
{ | ||
secondBrace = default; | ||
endOfLineTrivia = default; | ||
if (!token.IsKind(SyntaxKind.CloseBraceToken)) | ||
return false; | ||
|
||
var nextToken = token.GetNextToken(); | ||
if (!nextToken.IsKind(SyntaxKind.CloseBraceToken)) | ||
return false; | ||
|
||
var firstBrace = token; | ||
secondBrace = nextToken; | ||
|
||
// two } tokens. They need to be on the same line, or if they are not on subsequent lines, then there needs | ||
// to be more than whitespace between them. | ||
var lines = text.Lines; | ||
var firstBraceLine = lines.GetLineFromPosition(firstBrace.SpanStart).LineNumber; | ||
var secondBraceLine = lines.GetLineFromPosition(secondBrace.SpanStart).LineNumber; | ||
|
||
var lineCount = secondBraceLine - firstBraceLine; | ||
|
||
// if they're both on the same line, or one line apart, then there's no problem. | ||
if (lineCount <= 1) | ||
return false; | ||
|
||
// they're multiple lines apart. This i not ok if those lines are all whitespace. | ||
for (var currentLine = firstBraceLine + 1; currentLine < secondBraceLine; currentLine++) | ||
{ | ||
if (!IsAllWhitespace(lines[currentLine])) | ||
return false; | ||
} | ||
|
||
endOfLineTrivia = secondBrace.LeadingTrivia.Last(t => t.IsKind(SyntaxKind.EndOfLineTrivia)); | ||
return endOfLineTrivia != default; | ||
} | ||
|
||
private static bool IsAllWhitespace(TextLine textLine) | ||
{ | ||
var text = textLine.Text!; | ||
for (var i = textLine.Start; i < textLine.End; i++) | ||
{ | ||
if (!SyntaxFacts.IsWhitespace(text[i])) | ||
return false; | ||
} | ||
|
||
return true; | ||
} | ||
} | ||
} |
46 changes: 46 additions & 0 deletions
46
...es/ConsecutiveStatementPlacement/CSharpConsecutiveStatementPlacementDiagnosticAnalyzer.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// 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 Microsoft.CodeAnalysis.CSharp.Syntax; | ||
using Microsoft.CodeAnalysis.Diagnostics; | ||
using Microsoft.CodeAnalysis.NewLines.ConsecutiveStatementPlacement; | ||
|
||
namespace Microsoft.CodeAnalysis.CSharp.NewLines.ConsecutiveStatementPlacement | ||
{ | ||
/// <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)] | ||
internal sealed class CSharpConsecutiveStatementPlacementDiagnosticAnalyzer : AbstractConsecutiveStatementPlacementDiagnosticAnalyzer<StatementSyntax> | ||
{ | ||
protected override bool IsEndOfLine(SyntaxTrivia trivia) | ||
=> trivia.Kind() == SyntaxKind.EndOfLineTrivia; | ||
|
||
protected override bool IsWhitespace(SyntaxTrivia trivia) | ||
=> trivia.Kind() == SyntaxKind.WhitespaceTrivia; | ||
|
||
protected override bool IsBlockStatement(SyntaxNode node) | ||
=> node is BlockSyntax || node is SwitchStatementSyntax; | ||
|
||
protected override Location GetDiagnosticLocation(SyntaxNode block) | ||
=> block.GetLastToken().GetLocation(); | ||
} | ||
} |
16 changes: 16 additions & 0 deletions
16
...Sharp/Analyzers/NewLines/MultipleBlankLines/CSharpMultipleBlankLinesDiagnosticAnalyzer.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// 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 Microsoft.CodeAnalysis.Diagnostics; | ||
using Microsoft.CodeAnalysis.NewLines.MultipleBlankLines; | ||
|
||
namespace Microsoft.CodeAnalysis.CSharp.NewLines.MultipleBlankLines | ||
{ | ||
[DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
internal sealed class CSharpMultipleBlankLinesDiagnosticAnalyzer : AbstractMultipleBlankLinesDiagnosticAnalyzer | ||
{ | ||
protected override bool IsEndOfLine(SyntaxTrivia trivia) | ||
=> trivia.IsKind(SyntaxKind.EndOfLineTrivia); | ||
} | ||
} |
135 changes: 135 additions & 0 deletions
135
...Analyzers/NewLines/WrapEmbeddedStatement/CSharpWrapEmbeddedStatementDiagnosticAnalyzer.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
// 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.Collections.Immutable; | ||
using System.Linq; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.CodeStyle; | ||
using Microsoft.CodeAnalysis.CSharp.CodeStyle; | ||
using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
using Microsoft.CodeAnalysis.Diagnostics; | ||
|
||
namespace Microsoft.CodeAnalysis.CSharp.NewLines.WrapEmbeddedStatement | ||
{ | ||
[DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
internal sealed class CSharpWrapEmbeddedStatementDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer | ||
{ | ||
public CSharpWrapEmbeddedStatementDiagnosticAnalyzer() | ||
: base(IDEDiagnosticIds.WrapEmbeddedStatementDiagnosticId, | ||
EnforceOnBuildValues.WrapEmbeddedStatement, | ||
CSharpCodeStyleOptions.DisallowEmbeddedStatementsOnSameLine, | ||
LanguageNames.CSharp, | ||
new LocalizableResourceString( | ||
nameof(CSharpAnalyzersResources.Embedded_statements_must_be_on_their_own_line), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources))) | ||
{ | ||
} | ||
|
||
public override DiagnosticAnalyzerCategory GetAnalyzerCategory() | ||
=> DiagnosticAnalyzerCategory.SyntaxTreeWithoutSemanticsAnalysis; | ||
|
||
protected override void InitializeWorker(AnalysisContext context) | ||
=> context.RegisterSyntaxTreeAction(AnalyzeTree); | ||
|
||
private void AnalyzeTree(SyntaxTreeAnalysisContext context) | ||
{ | ||
var option = context.GetOption(CSharpCodeStyleOptions.DisallowEmbeddedStatementsOnSameLine); | ||
if (!option.Value) | ||
return; | ||
|
||
Recurse(context, option.Notification.Severity, context.Tree.GetRoot(context.CancellationToken)); | ||
} | ||
|
||
private void Recurse(SyntaxTreeAnalysisContext context, ReportDiagnostic severity, SyntaxNode node) | ||
{ | ||
context.CancellationToken.ThrowIfCancellationRequested(); | ||
|
||
// Don't bother analyzing nodes that have syntax errors in them. | ||
if (node.ContainsDiagnostics) | ||
return; | ||
|
||
// Report on the topmost statement that has an issue. No need to recurse further at that point. Note: the | ||
// fixer will fix up all statements, but we don't want to clutter things with lots of diagnostics on the | ||
// same line. | ||
if (node is StatementSyntax statement && | ||
CheckStatementSyntax(context, severity, statement)) | ||
{ | ||
return; | ||
} | ||
|
||
foreach (var child in node.ChildNodesAndTokens()) | ||
{ | ||
if (child.IsNode) | ||
Recurse(context, severity, child.AsNode()!); | ||
} | ||
} | ||
|
||
private bool CheckStatementSyntax(SyntaxTreeAnalysisContext context, ReportDiagnostic severity, StatementSyntax statement) | ||
{ | ||
if (!StatementNeedsWrapping(statement)) | ||
return false; | ||
|
||
var additionalLocations = ImmutableArray.Create(statement.GetLocation()); | ||
context.ReportDiagnostic(DiagnosticHelper.Create( | ||
this.Descriptor, | ||
statement.GetFirstToken().GetLocation(), | ||
severity, | ||
additionalLocations, | ||
properties: null)); | ||
return true; | ||
} | ||
|
||
public static bool StatementNeedsWrapping(StatementSyntax statement) | ||
{ | ||
// Statement has to be parented by another statement (or an else-clause) to count. | ||
var parent = statement.Parent; | ||
var parentIsElseClause = parent.IsKind(SyntaxKind.ElseClause); | ||
|
||
if (!(parent is StatementSyntax || parentIsElseClause)) | ||
return false; | ||
|
||
// `else if` is always allowed. | ||
if (statement.IsKind(SyntaxKind.IfStatement) && parentIsElseClause) | ||
return false; | ||
|
||
var statementStartToken = statement.GetFirstToken(); | ||
|
||
// we have to have a newline between the start of this statement and the previous statement. | ||
if (ContainsEndOfLineBetween(statementStartToken.GetPreviousToken(), statementStartToken)) | ||
return false; | ||
|
||
// Looks like a statement that might need wrapping. However, we do suppress wrapping for a few well known | ||
// acceptable cases. | ||
|
||
if (parent.IsKind(SyntaxKind.Block)) | ||
{ | ||
// Blocks can be on a single line if parented by a member/accessor/lambda. | ||
// And if they only contain a single statement at most within them. | ||
var blockParent = parent.Parent; | ||
if (blockParent is MemberDeclarationSyntax or | ||
AccessorDeclarationSyntax or | ||
AnonymousFunctionExpressionSyntax) | ||
{ | ||
if (parent.DescendantNodes().OfType<StatementSyntax>().Count() <= 1) | ||
return false; | ||
} | ||
} | ||
|
||
return true; | ||
} | ||
|
||
public static bool ContainsEndOfLineBetween(SyntaxToken previous, SyntaxToken next) | ||
=> ContainsEndOfLine(previous.TrailingTrivia) || ContainsEndOfLine(next.LeadingTrivia); | ||
|
||
private static bool ContainsEndOfLine(SyntaxTriviaList triviaList) | ||
{ | ||
foreach (var trivia in triviaList) | ||
{ | ||
if (trivia.IsKind(SyntaxKind.EndOfLineTrivia)) | ||
return true; | ||
} | ||
|
||
return false; | ||
} | ||
} | ||
} |
10 changes: 10 additions & 0 deletions
10
src/Analyzers/CSharp/Analyzers/xlf/CSharpAnalyzersResources.cs.xlf
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
nit:
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.
thanks!