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

Add accessibility modifier on file creation #51935

Merged
merged 11 commits into from
Jul 30, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -53,55 +53,12 @@ protected sealed override async Task FixAllAsync(
{
var declaration = diagnostic.AdditionalLocations[0].FindNode(cancellationToken);
var declarator = MapToDeclarator(declaration);

var symbol = semanticModel.GetDeclaredSymbol(declarator, cancellationToken);
Contract.ThrowIfNull(symbol);

var preferredAccessibility = GetPreferredAccessibility(symbol);

// Check to see if we need to add or remove
// If there's a modifier, then we need to remove it, otherwise no modifier, add it.
editor.ReplaceNode(
declaration,
(currentDeclaration, _) => UpdateAccessibility(currentDeclaration, preferredAccessibility));
}

return;

SyntaxNode UpdateAccessibility(SyntaxNode declaration, Accessibility preferredAccessibility)
{
var generator = editor.Generator;

// If there was accessibility on the member, then remove it. If there was no accessibility, then add
// the preferred accessibility for this member.
return generator.GetAccessibility(declaration) == Accessibility.NotApplicable
? generator.WithAccessibility(declaration, preferredAccessibility)
: generator.WithAccessibility(declaration, Accessibility.NotApplicable);
AddAccessibilityModifiersHelpers.UpdateDeclaration(editor, symbol, declaration);
}
}

private static Accessibility GetPreferredAccessibility(ISymbol symbol)
{
// If we have an overridden member, then if we're adding an accessibility modifier, use the
// accessibility of the member we're overriding as both should be consistent here.
if (symbol.GetOverriddenMember() is { DeclaredAccessibility: var accessibility })
return accessibility;

// Default abstract members to be protected, and virtual members to be public. They can't be private as
// that's not legal. And these are reasonable default values for them.
if (symbol is IMethodSymbol or IPropertySymbol or IEventSymbol)
{
if (symbol.IsAbstract)
return Accessibility.Protected;

if (symbol.IsVirtual)
return Accessibility.Public;
}

// Otherwise, default to whatever accessibility no-accessibility means for this member;
return symbol.DeclaredAccessibility;
}

private class MyCodeAction : CustomCodeActions.DocumentChangeAction
{
#if CODE_STYLE // 'CodeActionPriority' is not a public API, hence not supported in CodeStyle layer.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Threading;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.AddAccessibilityModifiers
{
internal static class AddAccessibilityModifiersHelpers
{
public static void UpdateDeclaration(
SyntaxEditor editor, ISymbol symbol, SyntaxNode declaration)
{
Contract.ThrowIfNull(symbol);

var preferredAccessibility = GetPreferredAccessibility(symbol);

// Check to see if we need to add or remove
// If there's a modifier, then we need to remove it, otherwise no modifier, add it.
editor.ReplaceNode(
declaration,
(currentDeclaration, _) => UpdateAccessibility(currentDeclaration, preferredAccessibility));

return;

SyntaxNode UpdateAccessibility(SyntaxNode declaration, Accessibility preferredAccessibility)
{
var generator = editor.Generator;

// If there was accessibility on the member, then remove it. If there was no accessibility, then add
// the preferred accessibility for this member.
return generator.GetAccessibility(declaration) == Accessibility.NotApplicable
? generator.WithAccessibility(declaration, preferredAccessibility)
: generator.WithAccessibility(declaration, Accessibility.NotApplicable);
}
}

private static Accessibility GetPreferredAccessibility(ISymbol symbol)
{
// If we have an overridden member, then if we're adding an accessibility modifier, use the
// accessibility of the member we're overriding as both should be consistent here.
if (symbol.GetOverriddenMember() is { DeclaredAccessibility: var accessibility })
return accessibility;

// Default abstract members to be protected, and virtual members to be public. They can't be private as
// that's not legal. And these are reasonable default values for them.
if (symbol is IMethodSymbol or IPropertySymbol or IEventSymbol)
{
if (symbol.IsAbstract)
return Accessibility.Protected;

if (symbol.IsVirtual)
return Accessibility.Public;
}

// Otherwise, default to whatever accessibility no-accessibility means for this member;
return symbol.DeclaredAccessibility;
}
}
}
1 change: 1 addition & 0 deletions src/Analyzers/Core/CodeFixes/CodeFixes.projitems
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
</ItemGroup>
<ItemGroup>
<Compile Include="$(MSBuildThisFileDirectory)AddAccessibilityModifiers\AbstractAddAccessibilityModifiersCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)AddAccessibilityModifiers\AddAccessibilityModifiersHelpers.cs" />
<Compile Include="$(MSBuildThisFileDirectory)AddRequiredParentheses\AddRequiredParenthesesCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ConvertAnonymousTypeToTuple\AbstractConvertAnonymousTypeToTupleCodeFixProvider.cs" />
<Compile Include="$(MSBuildThisFileDirectory)ConvertTypeOfToNameOf\AbstractConvertTypeOfToNameOfCodeFixProvider.cs" />
Expand Down
63 changes: 46 additions & 17 deletions src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,26 @@

using System;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Runtime.Remoting.Contexts;
using System.Runtime.Versioning;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.AddAccessibilityModifiers;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.FileHeaders;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.ComponentModelHost;
using Microsoft.VisualStudio.Designer.Interfaces;
using Microsoft.VisualStudio.Editor;
using Microsoft.VisualStudio.LanguageServices.Implementation.ProjectSystem;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;
using Microsoft.VisualStudio.TextManager.Interop;
using Microsoft.VisualStudio.Utilities;
Expand Down Expand Up @@ -115,7 +118,7 @@ public int CreateEditorInstance(

// We must create the WinForms designer here
var loaderName = GetWinFormsLoaderName(vsHierarchy);
var designerService = (IVSMDDesignerService)_oleServiceProvider.QueryService<SVSMDDesignerService>();
var designerService = (IVSMDDesignerService)Microsoft.VisualStudio.Shell.PackageUtilities.QueryService<SVSMDDesignerService>(_oleServiceProvider);
var designerLoader = (IVSMDDesignerLoader)designerService.CreateDesignerLoader(loaderName);
if (designerLoader is null)
{
Expand Down Expand Up @@ -293,6 +296,11 @@ protected virtual Task<Document> OrganizeUsingsCreatedFromTemplateAsync(Document
=> Formatter.OrganizeImportsAsync(document, cancellationToken);

private void FormatDocumentCreatedFromTemplate(IVsHierarchy hierarchy, uint itemid, string filePath, CancellationToken cancellationToken)
{
Microsoft.VisualStudio.Shell.ThreadHelper.JoinableTaskFactory.Run(() => FormatDocumentCreatedFromTemplateAsync(hierarchy, itemid, filePath, cancellationToken));
}

private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy, uint itemid, string filePath, CancellationToken cancellationToken)
{
// A file has been created on disk which the user added from the "Add Item" dialog. We need
// to include this in a workspace to figure out the right options it should be formatted with.
Expand Down Expand Up @@ -327,36 +335,39 @@ private void FormatDocumentCreatedFromTemplate(IVsHierarchy hierarchy, uint item
var forkedSolution = solution.AddDocument(DocumentInfo.Create(documentId, filePath, loader: new FileTextLoader(filePath, defaultEncoding: null), filePath: filePath));
var addedDocument = forkedSolution.GetDocument(documentId)!;

var rootToFormat = addedDocument.GetSyntaxRootSynchronously(cancellationToken);
Contract.ThrowIfNull(rootToFormat);
var documentOptions = ThreadHelper.JoinableTaskFactory.Run(() => addedDocument.GetOptionsAsync(cancellationToken));
var rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var documentOptions = await addedDocument.GetOptionsAsync(cancellationToken).ConfigureAwait(false);

// Add access modifier
if (documentOptions.GetOption(CodeStyleOptions2.RequireAccessibilityModifiers, addedDocument.Project.Language).Value != AccessibilityModifiersRequired.Never)
{
addedDocument = await AddAccessibilityModifiersAsync(addedDocument, cancellationToken).ConfigureAwait(false);
rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
}

// Apply file header preferences
var fileHeaderTemplate = documentOptions.GetOption(CodeStyleOptions2.FileHeaderTemplate);
if (!string.IsNullOrEmpty(fileHeaderTemplate))
{
var documentWithFileHeader = ThreadHelper.JoinableTaskFactory.Run(() =>
{
var newLineText = documentOptions.GetOption(FormattingOptions.NewLine, rootToFormat.Language);
var newLineTrivia = SyntaxGeneratorInternal.EndOfLine(newLineText);
return AbstractFileHeaderCodeFixProvider.GetTransformedSyntaxRootAsync(
var newLineText = documentOptions.GetOption(FormattingOptions.NewLine, rootToFormat.Language);
var newLineTrivia = SyntaxGeneratorInternal.EndOfLine(newLineText);
var rootWithFileHeader = await AbstractFileHeaderCodeFixProvider.GetTransformedSyntaxRootAsync(
SyntaxGenerator.SyntaxFacts,
FileHeaderHelper,
newLineTrivia,
addedDocument,
cancellationToken);
});
cancellationToken).ConfigureAwait(false);

addedDocument = addedDocument.WithSyntaxRoot(documentWithFileHeader);
rootToFormat = documentWithFileHeader;
addedDocument = addedDocument.WithSyntaxRoot(rootWithFileHeader);
rootToFormat = rootWithFileHeader;
}

// Organize using directives
addedDocument = ThreadHelper.JoinableTaskFactory.Run(() => OrganizeUsingsCreatedFromTemplateAsync(addedDocument, cancellationToken));
rootToFormat = ThreadHelper.JoinableTaskFactory.Run(() => addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).AsTask());
addedDocument = await OrganizeUsingsCreatedFromTemplateAsync(addedDocument, cancellationToken).ConfigureAwait(false);
Copy link
Member

@sharwell sharwell Jul 28, 2021

Choose a reason for hiding this comment

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

📝 These can all be ConfigureAwait(true) to improve efficiency (the main thread is the caller and available to process continuations without the potential latency of the thread pool)

rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);

// Format document
var unformattedText = addedDocument.GetTextSynchronously(cancellationToken);
var unformattedText = await addedDocument.GetTextAsync(cancellationToken).ConfigureAwait(false);
var formattedRoot = Formatter.Format(rootToFormat, workspace, documentOptions, cancellationToken);
var formattedText = formattedRoot.GetText(unformattedText.Encoding, unformattedText.ChecksumAlgorithm);

Expand Down Expand Up @@ -384,5 +395,23 @@ private void FormatDocumentCreatedFromTemplate(IVsHierarchy hierarchy, uint item
formattedText.Write(textWriter, cancellationToken: CancellationToken.None);
});
}

private static async Task<Document> AddAccessibilityModifiersAsync(Document document, CancellationToken cancellationToken)
{
var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
var typeDeclarations = root.DescendantNodes().Where(node => syntaxFacts.IsTypeDeclaration(node));
var editor = new SyntaxEditor(root, document.Project.Solution.Workspace);

foreach (var declaration in typeDeclarations)
{
var type = semanticModel.GetDeclaredSymbol(declaration, cancellationToken);
if (type != null)
AddAccessibilityModifiersHelpers.UpdateDeclaration(editor, type, declaration);
}

return document.WithSyntaxRoot(editor.GetChangedRoot());
}
}
}