Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
49d312c
Add feature to simplify interpolation expressions when possible
CyrusNajmabadi Nov 30, 2019
8cc8c3e
Add VB support
CyrusNajmabadi Nov 30, 2019
ddfdf12
Merge remote-tracking branch 'upstream/master' into simplifyInterpola…
CyrusNajmabadi Dec 6, 2019
55d98a6
Simplify
CyrusNajmabadi Dec 6, 2019
a9eb846
Simplify
CyrusNajmabadi Dec 6, 2019
4aa3bf9
Simplify
CyrusNajmabadi Dec 6, 2019
6209d30
Cleanup
CyrusNajmabadi Dec 7, 2019
c7a9dbb
Cleanup
CyrusNajmabadi Dec 7, 2019
e8c474e
simplify
CyrusNajmabadi Dec 7, 2019
9a9366c
Fix typos
jnm2 Dec 7, 2019
9aa8021
Add initial test
jnm2 Dec 7, 2019
254ae50
Fix NRE
jnm2 Dec 7, 2019
6899307
Add TextSpan Subtract extension methods
jnm2 Dec 7, 2019
0c07263
Show unnecessary spans
jnm2 Dec 7, 2019
02b4007
Check for spans tagged as unnecessary using {|Unnecessary:|}
jnm2 Dec 7, 2019
fca3d9c
Include literal quotes in the unnecessary spans
jnm2 Dec 7, 2019
b444f52
Add more tests
jnm2 Dec 7, 2019
67eab9f
Merge pull request #4 from jnm2/simplifyInterpolation
CyrusNajmabadi Dec 7, 2019
983edc1
Merge remote-tracking branch 'upstream/master' into simplifyInterpola…
CyrusNajmabadi Dec 7, 2019
6eccefd
Merge branch 'simplifyInterpolation' of https://github.com/CyrusNajma…
CyrusNajmabadi Dec 7, 2019
86bd9a3
Simplify
CyrusNajmabadi Dec 7, 2019
3d52fbc
No need for ?.
CyrusNajmabadi Dec 7, 2019
7fe6312
Simplify
CyrusNajmabadi Dec 7, 2019
606e21e
Ensure we have a literal
CyrusNajmabadi Dec 7, 2019
0a4a9a4
Deal with failure
CyrusNajmabadi Dec 7, 2019
2f3b1bf
Restore
CyrusNajmabadi Dec 7, 2019
51fb30f
Format
CyrusNajmabadi Dec 7, 2019
e3e1682
Simplify
CyrusNajmabadi Dec 7, 2019
e0191cc
Fix test naming convention
jnm2 Dec 8, 2019
c6b07dd
Merge pull request #5 from jnm2/simplifyInterpolation
CyrusNajmabadi Dec 8, 2019
8209fa4
Correct tests
jnm2 Dec 8, 2019
af60e75
Finished tests I can think of now
jnm2 Dec 8, 2019
b93006c
Merge pull request #6 from jnm2/simplifyInterpolation
CyrusNajmabadi Dec 8, 2019
2567e99
Merge remote-tracking branch 'upstream/master' into simplifyInterpola…
CyrusNajmabadi Dec 9, 2019
a1d6f8a
test fixup
CyrusNajmabadi Dec 9, 2019
18d16e7
Escape strings properly
CyrusNajmabadi Dec 9, 2019
9255a68
order spans
CyrusNajmabadi Dec 9, 2019
3838baa
fix tests
CyrusNajmabadi Dec 9, 2019
afb7f5b
fix tests
CyrusNajmabadi Dec 9, 2019
aec4047
fix tests
CyrusNajmabadi Dec 9, 2019
ef4f6c5
Merge remote-tracking branch 'upstream/master' into simplifyInterpola…
CyrusNajmabadi Dec 9, 2019
601297e
Merge remote-tracking branch 'upstream/master' into simplifyInterpola…
CyrusNajmabadi Dec 16, 2019
81febf7
Merge remote-tracking branch 'upstream/master' into simplifyInterpola…
CyrusNajmabadi Dec 16, 2019
7d0e777
Merge remote-tracking branch 'upstream/master' into simplifyInterpola…
CyrusNajmabadi Dec 18, 2019
2d29bb3
Add tests
CyrusNajmabadi Dec 18, 2019
bf149af
Simplify
CyrusNajmabadi Dec 18, 2019
a671359
Simplify
CyrusNajmabadi Dec 18, 2019
8971af1
Fix
CyrusNajmabadi Dec 18, 2019
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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,9 @@ public void CSharp_VerifyIDEDiagnosticSeveritiesAreConfigurable()
# IDE0070
dotnet_diagnostic.IDE0070.severity = %value%

# IDE0071
dotnet_style_prefer_simplified_interpolation = true:suggestion

# IDE0072
dotnet_diagnostic.IDE0072.severity = %value%

Expand Down Expand Up @@ -487,6 +490,9 @@ public void VisualBasic_VerifyIDEDiagnosticSeveritiesAreConfigurable()
# IDE0070
dotnet_diagnostic.IDE0070.severity = %value%

# IDE0071
dotnet_style_prefer_simplified_interpolation = true:suggestion

# IDE1006
dotnet_diagnostic.IDE1006.severity = %value%

Expand Down Expand Up @@ -835,6 +841,9 @@ No editorconfig based code style option
# IDE0070
No editorconfig based code style option

# IDE0071, PreferSimplifiedInterpolation
dotnet_style_prefer_simplified_interpolation = true:suggestion

# IDE0072
No editorconfig based code style option

Expand Down Expand Up @@ -1002,6 +1011,9 @@ No editorconfig based code style option
# IDE0070
No editorconfig based code style option

# IDE0071, PreferSimplifiedInterpolation
dotnet_style_prefer_simplified_interpolation = true:suggestion

# IDE1006
No editorconfig based code style option

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,16 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes.Suppression;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.Editor.Implementation.Preview;
using Microsoft.CodeAnalysis.Editor.UnitTests.Extensions;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Remote;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Text;
Expand All @@ -37,6 +36,7 @@ public struct TestParameters
internal readonly int index;
internal readonly CodeActionPriority? priority;
internal readonly bool retainNonFixableDiagnostics;
internal readonly bool includeDiagnosticsOutsideSelection;
internal readonly string title;

internal TestParameters(
Expand All @@ -47,6 +47,7 @@ internal TestParameters(
int index = 0,
CodeActionPriority? priority = null,
bool retainNonFixableDiagnostics = false,
bool includeDiagnosticsOutsideSelection = false,
string title = null)
{
this.parseOptions = parseOptions;
Expand All @@ -56,23 +57,27 @@ internal TestParameters(
this.index = index;
this.priority = priority;
this.retainNonFixableDiagnostics = retainNonFixableDiagnostics;
this.includeDiagnosticsOutsideSelection = includeDiagnosticsOutsideSelection;
this.title = title;
}

public TestParameters WithParseOptions(ParseOptions parseOptions)
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title);
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);

public TestParameters WithOptions(IDictionary<OptionKey, object> options)
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title);
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);

public TestParameters WithFixProviderData(object fixProviderData)
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title);
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);

public TestParameters WithIndex(int index)
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title);
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);

public TestParameters WithRetainNonFixableDiagnostics(bool retainNonFixableDiagnostics)
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, title: title, retainNonFixableDiagnostics: retainNonFixableDiagnostics);
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);

public TestParameters WithIncludeDiagnosticsOutsideSelection(bool includeDiagnosticsOutsideSelection)
=> new TestParameters(parseOptions, compilationOptions, options, fixProviderData, index, priority, retainNonFixableDiagnostics, includeDiagnosticsOutsideSelection, title);
}

protected abstract string GetLanguage();
Expand Down Expand Up @@ -373,17 +378,35 @@ private async Task TestAsync(
CodeActionPriority? priority,
TestParameters parameters)
{
MarkupTestFile.GetSpans(
initialMarkup.NormalizeLineEndings(),
out var initialMarkupWithoutSpans, out IDictionary<string, ImmutableArray<TextSpan>> initialSpanMap);

const string UnnecessaryMarkupKey = "Unnecessary";
var unnecessarySpans = initialSpanMap.GetOrAdd(UnnecessaryMarkupKey, _ => ImmutableArray<TextSpan>.Empty);

MarkupTestFile.GetSpans(
expectedMarkup.NormalizeLineEndings(),
out var expected, out IDictionary<string, ImmutableArray<TextSpan>> spanMap);
out var expected, out IDictionary<string, ImmutableArray<TextSpan>> expectedSpanMap);

var conflictSpans = spanMap.GetOrAdd("Conflict", _ => ImmutableArray<TextSpan>.Empty);
var renameSpans = spanMap.GetOrAdd("Rename", _ => ImmutableArray<TextSpan>.Empty);
var warningSpans = spanMap.GetOrAdd("Warning", _ => ImmutableArray<TextSpan>.Empty);
var navigationSpans = spanMap.GetOrAdd("Navigation", _ => ImmutableArray<TextSpan>.Empty);
var conflictSpans = expectedSpanMap.GetOrAdd("Conflict", _ => ImmutableArray<TextSpan>.Empty);
var renameSpans = expectedSpanMap.GetOrAdd("Rename", _ => ImmutableArray<TextSpan>.Empty);
var warningSpans = expectedSpanMap.GetOrAdd("Warning", _ => ImmutableArray<TextSpan>.Empty);
var navigationSpans = expectedSpanMap.GetOrAdd("Navigation", _ => ImmutableArray<TextSpan>.Empty);

using (var workspace = CreateWorkspaceFromOptions(initialMarkup, parameters))
{
// Ideally this check would always run, but there are several hundred tests that would need to be
// updated with {|Unnecessary:|} spans.
if (unnecessarySpans.Any())
{
var allDiagnostics = await GetDiagnosticsWorkerAsync(workspace, parameters
.WithRetainNonFixableDiagnostics(true)
.WithIncludeDiagnosticsOutsideSelection(true));

TestDiagnosticTags(allDiagnostics, unnecessarySpans, WellKnownDiagnosticTags.Unnecessary, UnnecessaryMarkupKey, initialMarkupWithoutSpans);
}

var (_, action) = await GetCodeActionsAsync(workspace, parameters);
await TestActionAsync(
workspace, expected, action,
Expand All @@ -392,6 +415,85 @@ await TestActionAsync(
}
}

private static void TestDiagnosticTags(
ImmutableArray<Diagnostic> diagnostics,
ImmutableArray<TextSpan> expectedSpans,
string diagnosticTag,
string markupKey,
string initialMarkupWithoutSpans)
{
var diagnosticsWithTag = diagnostics
.Where(d => d.Descriptor.CustomTags.Contains(diagnosticTag))
.OrderBy(s => s.Location.SourceSpan.Start)
.ToImmutableArray();

if (expectedSpans.Length != diagnosticsWithTag.Length)
{
AssertEx.Fail(BuildFailureMessage(expectedSpans, diagnosticTag, markupKey, initialMarkupWithoutSpans, diagnosticsWithTag));
}

for (var i = 0; i < expectedSpans.Length; i++)
{
var actual = diagnosticsWithTag[i].Location.SourceSpan;
var expected = expectedSpans[i];
Assert.Equal(expected, actual);
}
}

private static string BuildFailureMessage(
ImmutableArray<TextSpan> expectedSpans,
string diagnosticTag,
string markupKey,
string initialMarkupWithoutSpans,
ImmutableArray<Diagnostic> diagnosticsWithTag)
{
var message = $"Expected {expectedSpans.Length} diagnostic spans with custom tag '{diagnosticTag}', but there were {diagnosticsWithTag.Length}.";

if (expectedSpans.Length == 0)
{
message += $" If a diagnostic span tagged '{diagnosticTag}' is expected, surround the span in the test markup with the following syntax: {{|Unnecessary:...}}";

var segments = new List<(int originalStringIndex, string segment)>();

foreach (var diagnostic in diagnosticsWithTag)
{
var documentOffset = initialMarkupWithoutSpans.IndexOf(diagnosticsWithTag.First().Location.SourceTree.ToString());
if (documentOffset == -1) continue;

segments.Add((documentOffset + diagnostic.Location.SourceSpan.Start, "{|" + markupKey + ":"));
segments.Add((documentOffset + diagnostic.Location.SourceSpan.End, "|}"));
}

if (segments.Any())
{
message += Environment.NewLine
+ "Example:" + Environment.NewLine
+ Environment.NewLine
+ InsertSegments(initialMarkupWithoutSpans, segments);
}
}

return message;
}

private static string InsertSegments(string originalString, IEnumerable<(int originalStringIndex, string segment)> segments)
{
var builder = new StringBuilder();

var positionInOriginalString = 0;

foreach (var (originalStringIndex, segment) in segments.OrderBy(s => s.originalStringIndex))
{
builder.Append(originalString, positionInOriginalString, originalStringIndex - positionInOriginalString);
builder.Append(segment);

positionInOriginalString = originalStringIndex;
}

builder.Append(originalString, positionInOriginalString, originalString.Length - positionInOriginalString);
return builder.ToString();
}

internal async Task<Tuple<Solution, Solution>> TestActionAsync(
TestWorkspace workspace, string expected,
CodeAction action,
Expand Down Expand Up @@ -659,7 +761,7 @@ internal static IDictionary<OptionKey, object> OptionsSet(
/// <summary>
/// Tests all the code actions for the given <paramref name="input"/> string. Each code
/// action must produce the corresponding output in the <paramref name="outputs"/> array.
///
///
/// Will throw if there are more outputs than code actions or more code actions than outputs.
/// </summary>
protected Task TestAllInRegularAndScriptAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ internal async override Task<IEnumerable<Diagnostic>> GetDiagnosticsAsync(
}

var testDriver = new TestDiagnosticAnalyzerDriver(document.Project, provider);
var diagnostics = (await testDriver.GetAllDiagnosticsAsync(document, span)).ToImmutableArray();
var filterSpan = parameters.includeDiagnosticsOutsideSelection ? (TextSpan?)null : span;
var diagnostics = (await testDriver.GetAllDiagnosticsAsync(document, filterSpan)).ToImmutableArray();
AssertNoAnalyzerExceptionDiagnostics(diagnostics);

var fixer = providerAndFixer.Item2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private TestDiagnosticAnalyzerService CreateDiagnosticAnalyzerService(Project pr
private async Task<IEnumerable<Diagnostic>> GetDiagnosticsAsync(
Project project,
Document document,
TextSpan span,
TextSpan? filterSpan,
bool getDocumentDiagnostics,
bool getProjectDiagnostics)
{
Expand All @@ -66,7 +66,12 @@ private async Task<IEnumerable<Diagnostic>> GetDiagnosticsAsync(
if (getDocumentDiagnostics)
{
var dxs = await _diagnosticAnalyzerService.GetDiagnosticsAsync(project.Solution, project.Id, document.Id, _includeSuppressedDiagnostics);
documentDiagnostics = await CodeAnalysis.Diagnostics.Extensions.ToDiagnosticsAsync(dxs.Where(d => d.HasTextSpan && d.GetTextSpan().IntersectsWith(span)), project, CancellationToken.None);
documentDiagnostics = await CodeAnalysis.Diagnostics.Extensions.ToDiagnosticsAsync(
filterSpan is null
? dxs.Where(d => d.HasTextSpan)
: dxs.Where(d => d.HasTextSpan && d.GetTextSpan().IntersectsWith(filterSpan.Value)),
project,
CancellationToken.None);
}

if (getProjectDiagnostics)
Expand All @@ -86,9 +91,9 @@ private async Task<IEnumerable<Diagnostic>> GetDiagnosticsAsync(
return allDiagnostics;
}

public Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Document document, TextSpan span)
public Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Document document, TextSpan? filterSpan)
{
return GetDiagnosticsAsync(document.Project, document, span, getDocumentDiagnostics: true, getProjectDiagnostics: true);
return GetDiagnosticsAsync(document.Project, document, filterSpan, getDocumentDiagnostics: true, getProjectDiagnostics: true);
}

public async Task<IEnumerable<Diagnostic>> GetAllDiagnosticsAsync(Project project)
Expand Down Expand Up @@ -125,7 +130,7 @@ public Task<IEnumerable<Diagnostic>> GetDocumentDiagnosticsAsync(Document docume

public Task<IEnumerable<Diagnostic>> GetProjectDiagnosticsAsync(Project project)
{
return GetDiagnosticsAsync(project, document: null, span: default, getDocumentDiagnostics: false, getProjectDiagnostics: true);
return GetDiagnosticsAsync(project, document: null, filterSpan: default, getDocumentDiagnostics: false, getProjectDiagnostics: true);
}

private async Task SynchronizeGlobalAssetToRemoteHostIfNeededAsync(Workspace workspace)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// 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.Composition;
using System.Text;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.SimplifyInterpolation;

namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation
{
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
internal class CSharpSimplifyInterpolationCodeFixProvider : AbstractSimplifyInterpolationCodeFixProvider<
InterpolationSyntax, ExpressionSyntax, InterpolationAlignmentClauseSyntax,
InterpolationFormatClauseSyntax, InterpolatedStringExpressionSyntax>
{
protected override InterpolationSyntax WithExpression(InterpolationSyntax interpolation, ExpressionSyntax expression)
=> interpolation.WithExpression(expression);

protected override InterpolationSyntax WithAlignmentClause(InterpolationSyntax interpolation, InterpolationAlignmentClauseSyntax alignmentClause)
=> interpolation.WithAlignmentClause(alignmentClause);

protected override InterpolationSyntax WithFormatClause(InterpolationSyntax interpolation, InterpolationFormatClauseSyntax formatClause)
=> interpolation.WithFormatClause(formatClause);

protected override string Escape(InterpolatedStringExpressionSyntax interpolatedString, string formatString)
{
var result = new StringBuilder();
if (interpolatedString.StringStartToken.Kind() == SyntaxKind.InterpolatedVerbatimStringStartToken)
{
foreach (var c in formatString)
{
// in a verbatim string, the only char we have to escape is the double-quote char
if (c == '"')
{
result.Append(c);
}

result.Append(c);
}
}
else
{
// In a normal string we have to escape quotes and we have to escape an
// escape character itself.
foreach (var c in formatString)
{
if (c == '"' || c == '\\')
{
result.Append('\\');
}

result.Append(c);
}
}

return result.ToString();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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 Microsoft.CodeAnalysis.CSharp.EmbeddedLanguages.VirtualChars;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.EmbeddedLanguages.VirtualChars;
using Microsoft.CodeAnalysis.SimplifyInterpolation;

namespace Microsoft.CodeAnalysis.CSharp.SimplifyInterpolation
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal class CSharpSimplifyInterpolationDiagnosticAnalyzer : AbstractSimplifyInterpolationDiagnosticAnalyzer<
InterpolationSyntax, ExpressionSyntax>
{
protected override IVirtualCharService GetVirtualCharService()
=> CSharpVirtualCharService.Instance;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ internal static class IDEDiagnosticIds

public const string UseSystemHashCode = "IDE0070";

public const string SimplifyInterpolationId = "IDE0071";

public const string PopulateSwitchExpressionDiagnosticId = "IDE0072";

// Analyzer error Ids
Expand Down
Loading