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

Don't add accessibility modifiers to partial classes in new documents #55713

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
Original file line number Diff line number Diff line change
Expand Up @@ -548,5 +548,38 @@ internal struct S1 { }
LanguageVersion = LanguageVersion.CSharp10,
}.RunAsync();
}

[Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddAccessibilityModifiers)]
[WorkItem(55703, "https://github.com/dotnet/roslyn/issues/55703")]
public async Task TestPartial_WithExistingModifier()
davidwengier marked this conversation as resolved.
Show resolved Hide resolved
{
var source = @"
partial class [|C|]
{
}

public partial class C
{
}
";
var fixedSource = @"
public partial class C
{
}

public partial class C
{
}
";

var test = new VerifyCS.Test
{
TestCode = source,
FixedCode = fixedSource,
LanguageVersion = CodeAnalysis.CSharp.LanguageVersion.Preview,
};

await test.RunAsync();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Notification;
using Microsoft.CodeAnalysis.Test.Utilities.Formatting;
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Formatting
Expand Down Expand Up @@ -171,6 +171,49 @@ internal class C
});
}

[Fact]
[WorkItem(55703, "https://github.com/dotnet/roslyn/issues/55703")]
public async Task TestAccessibilityModifiers_IgnoresPartial()
{
await TestAsync(
testCode: @"using System;

namespace Goo
{
class E
{
}

partial class C
{
}

class D
{
}
}",
expected: @"using System;

namespace Goo
{
internal class E
{
}

partial class C
{
}

internal class D
{
}
}",
options: new[]
{
(CodeStyleOptions2.RequireAccessibilityModifiers, new CodeStyleOption2<AccessibilityModifiersRequired>(AccessibilityModifiersRequired.Always, NotificationOption2.Error))
});
}

[Fact]
public async Task TestUsingDirectivePlacement()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,23 @@ public async Task<Document> FormatNewDocumentAsync(Document document, Document?
if (!service.ShouldUpdateAccessibilityModifier(syntaxFacts, declaration, accessibilityPreferences.Value, out _))
continue;

// Since we format each document as they are added to a project we can't assume we know about all
// of the files that are coming, so we have to opt out of changing partial classes. This especially
// manifests when creating new projects as we format before we have a project at all, so we could get a
// situation like this:
//
// File1.cs:
// partial class C { }
// File2.cs:
// public partial class C { }
//
// When we see File1, we don't know about File2, so would add an internal modifier, which would result in a compile
// error.
var modifiers = syntaxFacts.GetModifiers(declaration);
syntaxFacts.GetAccessibilityAndModifiers(modifiers, out _, out var declarationModifiers, out _);
if (declarationModifiers.IsPartial)
continue;

var type = semanticModel.GetDeclaredSymbol(declaration, cancellationToken);
if (type == null)
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private void ExecuteWhenButtonClicked(object sender, EventArgs e)
}", codeFileActualText);
}

[WpfFact(Skip = "https://github.com/dotnet/roslyn/issues/55703"), Trait(Traits.Feature, Traits.Features.WinForms)]
[WpfFact, Trait(Traits.Feature, Traits.Features.WinForms)]
public void RenameControl()
{
var project = new ProjectUtils.Project(ProjectName);
Expand Down