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.

Sign up for GitHub

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

Jump to bottom

Fix that SA1134 Fix All maybe non-deterministic #2853

Merged
merged 2 commits into from
Dec 19, 2023
Merged

Fix that SA1134 Fix All maybe non-deterministic #2853

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df70004
Fix that SA1134 Fix All maybe non-deterministic
pdelvo Jan 7, 2019
5431f37
Merge remote-tracking branch 'DotNetAnalyzers/master' into SA1134NonD…
sharwell Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 39 additions & 6 deletions StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1134CodeFixProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace StyleCop.Analyzers.ReadabilityRules
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using StyleCop.Analyzers.Helpers;
using StyleCop.Analyzers.Settings.ObjectModel;

/// <summary>
/// Implements a code fix for <see cref="SA1134AttributesMustNotShareLine"/>.
Expand All @@ -31,7 +32,7 @@ internal class SA1134CodeFixProvider : CodeFixProvider
/// <inheritdoc/>
public override FixAllProvider GetFixAllProvider()
{
return CustomFixAllProviders.BatchFixer;
Copy link
Member

Choose a reason for hiding this comment

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

❓ Any idea what's making this not deterministic? We originally added the custom batch fixer to address non-deterministic behavior in the one provided by Roslyn.

Copy link
Member Author

Choose a reason for hiding this comment

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

No. I had the test fail exactly once in the IDE. I have not been able to reproduce it since.

return FixAll.Instance;
}

/// <inheritdoc/>
Expand All @@ -58,16 +59,25 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
{
var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, syntaxRoot.SyntaxTree, cancellationToken);
var tokensToReplace = new Dictionary<SyntaxToken, SyntaxToken>();

AddTokensToReplaceToMap(tokensToReplace, syntaxRoot, diagnostic, settings);

var newSyntaxRoot = syntaxRoot.ReplaceTokens(tokensToReplace.Keys, (original, rewritten) => tokensToReplace[original]);
var newDocument = document.WithSyntaxRoot(newSyntaxRoot.WithoutFormatting());

return newDocument;
}

private static void AddTokensToReplaceToMap(Dictionary<SyntaxToken, SyntaxToken> tokensToReplace, SyntaxNode syntaxRoot, Diagnostic diagnostic, StyleCopSettings settings)
{
var attributeListSyntax = (AttributeListSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan);

// use the containing type to determine the indentation level, anything else is less reliable.
var containingType = attributeListSyntax.Parent?.Parent;
var indentationSteps = (containingType != null) ? IndentationHelper.GetIndentationSteps(settings.Indentation, containingType) + 1 : 0;
var indentationTrivia = IndentationHelper.GenerateWhitespaceTrivia(settings.Indentation, indentationSteps);

var tokensToReplace = new Dictionary<SyntaxToken, SyntaxToken>();

if (diagnostic.Properties.ContainsKey(SA1134AttributesMustNotShareLine.FixWithNewLineBeforeKey))
{
var token = attributeListSyntax.OpenBracketToken;
Expand All @@ -89,11 +99,34 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
var newLeadingTrivia = nextToken.LeadingTrivia.Insert(0, indentationTrivia);
tokensToReplace[nextToken] = nextToken.WithLeadingTrivia(newLeadingTrivia);
}
}

var newSyntaxRoot = syntaxRoot.ReplaceTokens(tokensToReplace.Keys, (original, rewritten) => tokensToReplace[original]);
var newDocument = document.WithSyntaxRoot(newSyntaxRoot.WithoutFormatting());
private class FixAll : DocumentBasedFixAllProvider
{
public static FixAllProvider Instance { get; } = new FixAll();

return newDocument;
protected override string CodeActionTitle => ReadabilityResources.SA1134CodeFix;

protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fixAllContext, Document document, ImmutableArray<Diagnostic> diagnostics)
{
if (diagnostics.IsEmpty)
{
return null;
}

var syntaxRoot = await document.GetSyntaxRootAsync(fixAllContext.CancellationToken).ConfigureAwait(false);
var settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, syntaxRoot.SyntaxTree, fixAllContext.CancellationToken);
var tokensToReplace = new Dictionary<SyntaxToken, SyntaxToken>();

foreach (var diagnostic in diagnostics)
{
AddTokensToReplaceToMap(tokensToReplace, syntaxRoot, diagnostic, settings);
}

var newSyntaxRoot = syntaxRoot.ReplaceTokens(tokensToReplace.Keys, (original, rewritten) => tokensToReplace[original]);

return newSyntaxRoot;
}
}
}
}