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

Do not apply Fix All operations to generated code #33515

Merged
merged 1 commit into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions src/CodeStyle/Core/CodeFixes/FixAllContextHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,13 @@ public static async Task<ImmutableDictionary<Document, ImmutableArray<Diagnostic
}

return await GetDocumentDiagnosticsToFixAsync(
allDiagnostics, projectsToFix, fixAllContext.CancellationToken).ConfigureAwait(false);
allDiagnostics, projectsToFix, isGeneratedCode, fixAllContext.CancellationToken).ConfigureAwait(false);
}

private static async Task<ImmutableDictionary<Document, ImmutableArray<Diagnostic>>> GetDocumentDiagnosticsToFixAsync(
ImmutableArray<Diagnostic> diagnostics,
ImmutableArray<Project> projects,
Func<Document, CancellationToken, bool> isGeneratedCode,
sharwell marked this conversation as resolved.
Show resolved Hide resolved
CancellationToken cancellationToken)
{
var treeToDocumentMap = await GetTreeToDocumentMapAsync(projects, cancellationToken).ConfigureAwait(false);
Expand All @@ -89,8 +90,11 @@ private static async Task<ImmutableDictionary<Document, ImmutableArray<Diagnosti
{
cancellationToken.ThrowIfCancellationRequested();
var document = documentAndDiagnostics.Key;
var diagnosticsForDocument = documentAndDiagnostics.ToImmutableArray();
builder.Add(document, diagnosticsForDocument);
if (!isGeneratedCode(document, cancellationToken))
{
var diagnosticsForDocument = documentAndDiagnostics.ToImmutableArray();
builder.Add(document, diagnosticsForDocument);
}
}

return builder.ToImmutable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,146 @@ public int Y2
Assert.Equal(expectedText, VisualStudio.Editor.GetText());
}

[CriticalWpfTheory]
[InlineData(FixAllScope.Project)]
[InlineData(FixAllScope.Solution)]
[Trait(Traits.Feature, Traits.Features.CodeActionsFixAllOccurrences)]
[WorkItem(33507, "https://github.com/dotnet/roslyn/issues/33507")]
public void FixAllOccurrencesIgnoresGeneratedCode(FixAllScope scope)
{
var markup = @"
using System;
using $$System.Threading;

class C
{
public IntPtr X1 { get; set; }
}";
var expectedText = @"
using System;

class C
{
public IntPtr X1 { get; set; }
}";
var generatedSourceMarkup = @"// <auto-generated/>
using System;
using $$System.Threading;

class D
{
public IntPtr X1 { get; set; }
}";
var expectedGeneratedSource = @"// <auto-generated/>
using System;

class D
{
public IntPtr X1 { get; set; }
}";

MarkupTestFile.GetPosition(generatedSourceMarkup, out var generatedSource, out int generatedSourcePosition);

VisualStudio.SolutionExplorer.AddFile(new ProjectUtils.Project(ProjectName), "D.cs", generatedSource, open: false);

// Switch to the main document we'll be editing
VisualStudio.SolutionExplorer.OpenFile(new ProjectUtils.Project(ProjectName), "Class1.cs");

// Verify that applying a Fix All operation does not change generated files.
// This is a regression test for correctness with respect to the design.
SetUpEditor(markup);
VisualStudio.WaitForApplicationIdle(CancellationToken.None);
VisualStudio.Editor.InvokeCodeActionList();
VisualStudio.Editor.Verify.CodeAction(
"Remove Unnecessary Usings",
applyFix: true,
fixAllScope: scope);

Assert.Equal(expectedText, VisualStudio.Editor.GetText());

VisualStudio.SolutionExplorer.OpenFile(new ProjectUtils.Project(ProjectName), "D.cs");
Assert.Equal(generatedSource, VisualStudio.Editor.GetText());

// Verify that a Fix All in Document in the generated file still does nothing.
// ⚠ This is a statement of the current behavior, and not a claim regarding correctness of the design.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this worthy of filing a tracking design issue or do we want to wait on customer feedback on this known broken behavior before spending any cycles on it?

Copy link
Member

Choose a reason for hiding this comment

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

And how hard is this to just fix? 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this worthy of filing a tracking design issue or do we want to wait on customer feedback on this known broken behavior before spending any cycles on it?

#2753

// The current behavior is observable; any change to this behavior should be part of an intentional design
// change.
VisualStudio.Editor.MoveCaret(generatedSourcePosition);
VisualStudio.Editor.InvokeCodeActionList();
VisualStudio.Editor.Verify.CodeAction(
"Remove Unnecessary Usings",
applyFix: true,
fixAllScope: FixAllScope.Document);

Assert.Equal(generatedSource, VisualStudio.Editor.GetText());

// Verify that the code action can still be applied manually from within the generated file.
// This is a regression test for correctness with respect to the design.
VisualStudio.Editor.MoveCaret(generatedSourcePosition);
VisualStudio.Editor.InvokeCodeActionList();
VisualStudio.Editor.Verify.CodeAction(
"Remove Unnecessary Usings",
applyFix: true,
fixAllScope: null);

Assert.Equal(expectedGeneratedSource, VisualStudio.Editor.GetText());
}

[CriticalWpfTheory]
[InlineData(FixAllScope.Project)]
[InlineData(FixAllScope.Solution)]
[Trait(Traits.Feature, Traits.Features.CodeActionsFixAllOccurrences)]
[WorkItem(33507, "https://github.com/dotnet/roslyn/issues/33507")]
public void FixAllOccurrencesTriggeredFromGeneratedCode(FixAllScope scope)
{
var markup = @"// <auto-generated/>
using System;
using $$System.Threading;

class C
{
public IntPtr X1 { get; set; }
}";
var secondFile = @"
using System;
using System.Threading;

class D
{
public IntPtr X1 { get; set; }
}";
var expectedSecondFile = @"
using System;

class D
{
public IntPtr X1 { get; set; }
}";

VisualStudio.SolutionExplorer.AddFile(new ProjectUtils.Project(ProjectName), "D.cs", secondFile, open: false);

// Switch to the main document we'll be editing
VisualStudio.SolutionExplorer.OpenFile(new ProjectUtils.Project(ProjectName), "Class1.cs");

// Verify that applying a Fix All operation does not change generated file, but does change other files.
// ⚠ This is a statement of the current behavior, and not a claim regarding correctness of the design.
// The current behavior is observable; any change to this behavior should be part of an intentional design
// change.
MarkupTestFile.GetPosition(markup, out var expectedText, out int _);
SetUpEditor(markup);
VisualStudio.WaitForApplicationIdle(CancellationToken.None);
VisualStudio.Editor.InvokeCodeActionList();
VisualStudio.Editor.Verify.CodeAction(
"Remove Unnecessary Usings",
applyFix: true,
fixAllScope: scope);

Assert.Equal(expectedText, VisualStudio.Editor.GetText());

VisualStudio.SolutionExplorer.OpenFile(new ProjectUtils.Project(ProjectName), "D.cs");
Assert.Equal(expectedSecondFile, VisualStudio.Editor.GetText());
}

[WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsGenerateMethod)]
public void ClassificationInPreviewPane()
{
Expand Down