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

Trigger IRefactorNotify on Workspace.TryApplyChange #47579

Closed
wants to merge 13 commits into from
Closed

Trigger IRefactorNotify on Workspace.TryApplyChange #47579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3ddcc47
Trigger IVSRefactorNotify on Workspace.TryApply changes if a symbol has
ryzngard Sep 10, 2020
9bb9a0b
Trigger IVSRefactorNotify on Workspace.TryApply changes if a symbol has
ryzngard Oct 15, 2020
13b33c7
Post rebase fixups
ryzngard Oct 15, 2020
aba87d3
Merge branch 'issues/47048' of https://github.com/ryzngard/roslyn int…
ryzngard Oct 21, 2020
0dc055a
Merge remote-tracking branch 'upstream/master' into issues/47048
ryzngard Oct 21, 2020
a63ba5f
Fix nullable stuff
ryzngard Oct 21, 2020
c68ffc1
Add threading context to IRefactorNotify extensions
ryzngard Oct 21, 2020
e1cd6a7
Fix messages in VisualStudioWorkspaceImpl.cs
ryzngard Oct 21, 2020
1930061
Fix test code
ryzngard Oct 23, 2020
6648ffd
Merge remote-tracking branch 'upstream/master' into issues/47048
ryzngard Oct 27, 2020
92ee55a
Merge branch 'master' into issues/47048
ryzngard Nov 6, 2020
b5bd964
Remove IRefactorNotifyServices dependency after merge
ryzngard Nov 10, 2020
15e2113
Merge branch 'master' into issues/47048
ryzngard Nov 10, 2020
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
91 changes: 59 additions & 32 deletions src/Analyzers/CSharp/Tests/NamingStyles/NamingStylesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
using Microsoft.CodeAnalysis.CSharp.Diagnostics.NamingStyles;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Editor.UnitTests.Diagnostics.NamingStyles;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities.Utilities;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Test.Utilities;
using Xunit;
using Xunit.Abstractions;
Expand All @@ -32,9 +33,6 @@ public NamingStylesTests(ITestOutputHelper logger)
internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
=> (new CSharpNamingStyleDiagnosticAnalyzer(), new NamingStyleCodeFixProvider());

protected override TestComposition GetComposition()
=> base.GetComposition().AddParts(typeof(TestSymbolRenamedCodeActionOperationFactoryWorkspaceService));

[Fact, Trait(Traits.Feature, Traits.Features.NamingStyle)]
public async Task TestPascalCaseClass_CorrectName()
{
Expand Down Expand Up @@ -1221,11 +1219,7 @@ internal interface
", new TestParameters(options: s_options.InterfaceNamesStartWithI));
}

#if CODE_STYLE
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/42218")]
#else
[Fact]
#endif
[Trait(Traits.Feature, Traits.Features.NamingStyle)]
[WorkItem(16562, "https://github.com/dotnet/roslyn/issues/16562")]
public async Task TestRefactorNotify()
Expand All @@ -1237,21 +1231,30 @@ public async Task TestRefactorNotify()
var (_, action) = await GetCodeActionsAsync(workspace, testParameters);

var previewOperations = await action.GetPreviewOperationsAsync(CancellationToken.None);
Assert.Empty(previewOperations.OfType<TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.Operation>());
Assert.Equal(1, previewOperations.Length);

var commitOperations = await action.GetOperationsAsync(CancellationToken.None);
Assert.Equal(2, commitOperations.Length);
Assert.Equal(1, commitOperations.Length);

var symbolRenamedOperation = commitOperations[0];

var solutionPair = await TestActionAsync(
workspace,
@"public class C { }",
action,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
testParameters);

var symbolRenamedOperation = (TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.Operation)commitOperations[1];
Assert.Equal("c", symbolRenamedOperation._symbol.Name);
Assert.Equal("C", symbolRenamedOperation._newName);
var oldSolution = solutionPair.Item1;
var newSolution = solutionPair.Item2;

await RenameHelpers.AssertRenameAnnotationsAsync(oldSolution, newSolution, RenameHelpers.MakeSymbolPairs("c", "C"));
}

#if CODE_STYLE
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/42218")]
#else
[Fact]
#endif
[Trait(Traits.Feature, Traits.Features.NamingStyle)]
[WorkItem(38513, "https://github.com/dotnet/roslyn/issues/38513")]
public async Task TestRefactorNotifyInterfaceNamesStartWithI()
Expand All @@ -1263,21 +1266,28 @@ public async Task TestRefactorNotifyInterfaceNamesStartWithI()
var (_, action) = await GetCodeActionsAsync(workspace, testParameters);

var previewOperations = await action.GetPreviewOperationsAsync(CancellationToken.None);
Assert.Empty(previewOperations.OfType<TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.Operation>());
Assert.Equal(1, previewOperations.Length);

var commitOperations = await action.GetOperationsAsync(CancellationToken.None);
Assert.Equal(2, commitOperations.Length);
Assert.Equal(1, commitOperations.Length);

var solutionPair = await TestActionAsync(
workspace,
@"public interface ITest { }",
action,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
testParameters);

var oldSolution = solutionPair.Item1;
var newSolution = solutionPair.Item2;
Comment on lines +1274 to +1285
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth breaking all of this into a helper?


var symbolRenamedOperation = (TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.Operation)commitOperations[1];
Assert.Equal("test", symbolRenamedOperation._symbol.Name);
Assert.Equal("ITest", symbolRenamedOperation._newName);
await RenameHelpers.AssertRenameAnnotationsAsync(oldSolution, newSolution, RenameHelpers.MakeSymbolPairs("test", "ITest"));
}

#if CODE_STYLE
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/42218")]
#else
[Fact]
#endif
[Trait(Traits.Feature, Traits.Features.NamingStyle)]
[WorkItem(38513, "https://github.com/dotnet/roslyn/issues/38513")]
public async Task TestRefactorNotifyTypeParameterNamesStartWithT()
Expand All @@ -1286,20 +1296,37 @@ public async Task TestRefactorNotifyTypeParameterNamesStartWithT()
{
void DoOtherThing<[|arg|]>() { }
}";

var expectedMarkup = @"public class A
{
void DoOtherThing<TArg>() { }
}";

var testParameters = new TestParameters(options: s_options.TypeParameterNamesStartWithT);

using var workspace = CreateWorkspaceFromOptions(markup, testParameters);
var (_, action) = await GetCodeActionsAsync(workspace, testParameters);

var previewOperations = await action.GetPreviewOperationsAsync(CancellationToken.None);
Assert.Empty(previewOperations.OfType<TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.Operation>());
Assert.Equal(1, previewOperations.Length);

var commitOperations = await action.GetOperationsAsync(CancellationToken.None);
Assert.Equal(2, commitOperations.Length);

var symbolRenamedOperation = (TestSymbolRenamedCodeActionOperationFactoryWorkspaceService.Operation)commitOperations[1];
Assert.Equal("arg", symbolRenamedOperation._symbol.Name);
Assert.Equal("TArg", symbolRenamedOperation._newName);
Assert.Equal(1, commitOperations.Length);

var solutionPair = await TestActionAsync(
workspace,
expectedMarkup,
action,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
ImmutableArray<TextSpan>.Empty,
testParameters);

var oldSolution = solutionPair.Item1;
var newSolution = solutionPair.Item2;

await RenameHelpers.AssertRenameAnnotationsAsync(oldSolution, newSolution, RenameHelpers.MakeSymbolPairs("arg", "TArg"));
}

[Fact, Trait(Traits.Feature, Traits.Features.NamingStyle)]
Expand Down
36 changes: 0 additions & 36 deletions src/Analyzers/Core/CodeFixes/NamingStyle/NamingStyleCodeFixProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,10 @@
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

#if !CODE_STYLE // https://github.com/dotnet/roslyn/issues/42218 removing dependency on WorkspaceServices.
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
using Microsoft.CodeAnalysis.CodeActions.WorkspaceServices;
#endif

namespace Microsoft.CodeAnalysis.CodeFixes.NamingStyles
{
#if !CODE_STYLE // https://github.com/dotnet/roslyn/issues/42218 tracks enabling this fixer in CodeStyle layer.
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic,
Name = PredefinedCodeFixProviderNames.ApplyNamingStyle), Shared]
#endif
internal class NamingStyleCodeFixProvider : CodeFixProvider
{
[ImportingConstructor]
Expand Down Expand Up @@ -88,11 +82,6 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
context.RegisterCodeFix(
new FixNameCodeAction(
#if !CODE_STYLE
document.Project.Solution,
symbol,
fixedName,
#endif
string.Format(CodeFixesResources.Fix_Name_Violation_colon_0, fixedName),
c => FixAsync(document, symbol, fixedName, c),
equivalenceKey: nameof(NamingStyleCodeFixProvider)),
Expand All @@ -111,31 +100,15 @@ await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false),

private class FixNameCodeAction : CodeAction
{
#if !CODE_STYLE
private readonly Solution _startingSolution;
private readonly ISymbol _symbol;
private readonly string _newName;
#endif

private readonly string _title;
private readonly Func<CancellationToken, Task<Solution>> _createChangedSolutionAsync;
private readonly string _equivalenceKey;

public FixNameCodeAction(
#if !CODE_STYLE
Solution startingSolution,
ISymbol symbol,
string newName,
#endif
string title,
Func<CancellationToken, Task<Solution>> createChangedSolutionAsync,
string equivalenceKey)
{
#if !CODE_STYLE
_startingSolution = startingSolution;
_symbol = symbol;
_newName = newName;
#endif
_title = title;
_createChangedSolutionAsync = createChangedSolutionAsync;
_equivalenceKey = equivalenceKey;
Expand All @@ -151,16 +124,7 @@ protected override async Task<IEnumerable<CodeActionOperation>> ComputeOperation
var newSolution = await _createChangedSolutionAsync(cancellationToken).ConfigureAwait(false);
var codeAction = new ApplyChangesOperation(newSolution);

#if CODE_STYLE // https://github.com/dotnet/roslyn/issues/42218 tracks removing this conditional code.
return SpecializedCollections.SingletonEnumerable(codeAction);
#else
var factory = _startingSolution.Workspace.Services.GetRequiredService<ISymbolRenamedCodeActionOperationFactoryWorkspaceService>();
return new CodeActionOperation[]
{
codeAction,
factory.CreateSymbolRenamedOperation(_symbol, _newName, _startingSolution, newSolution)
}.AsEnumerable();
#endif
}

public override string Title => _title;
Expand Down
4 changes: 0 additions & 4 deletions src/Analyzers/VisualBasic/Tests/NamingStyles/NamingStylesTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Diagnostics.Naming
Return (New VisualBasicNamingStyleDiagnosticAnalyzer(), New NamingStyleCodeFixProvider())
End Function

Protected Overrides Function GetComposition() As TestComposition
Copy link
Member

Choose a reason for hiding this comment

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

[this is a comment to the file itself.]

Did we want some of the tests that exist on the C# side of things here as well?

Return MyBase.GetComposition().AddParts(GetType(TestSymbolRenamedCodeActionOperationFactoryWorkspaceService))
End Function

' TODO: everything else apart from locals

<Fact, Trait(Traits.Feature, Traits.Features.NamingStyle)>
Expand Down
4 changes: 1 addition & 3 deletions src/EditorFeatures/CSharpTest/MoveToNamespace/MoveToNamespaceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ public class MoveToNamespaceTests : AbstractMoveToNamespaceTests
{
private static readonly TestComposition s_compositionWithoutOptions = FeaturesTestCompositions.Features
.AddExcludedPartTypes(typeof(IDiagnosticUpdateSourceRegistrationService))
.AddParts(
typeof(MockDiagnosticUpdateSourceRegistrationService),
typeof(TestSymbolRenamedCodeActionOperationFactoryWorkspaceService));
.AddParts(typeof(MockDiagnosticUpdateSourceRegistrationService));

private static readonly TestComposition s_composition = s_compositionWithoutOptions.AddParts(
typeof(TestMoveToNamespaceOptionsService));
Expand Down
Loading