From e4c90bb7d675ceedaeee9ee33e1d97d5d4bf60e6 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Wed, 17 Mar 2021 15:31:09 +0000 Subject: [PATCH 1/8] Add accessibility modifier on file creation --- ...ddAccessibilityModifiersCodeFixProvider.cs | 53 +++++++++++++------ .../Implementation/AbstractEditorFactory.cs | 16 ++++-- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs index f3b9f136e8c32..3ed27071d9db1 100644 --- a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; @@ -43,28 +44,34 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) return Task.CompletedTask; } - protected sealed override async Task FixAllAsync( - Document document, ImmutableArray diagnostics, - SyntaxEditor editor, CancellationToken cancellationToken) + // Intended for use by AbstractEditorFactory to add accessibility when creating a new file. + internal static async Task GetTransformedSyntaxRootAsync(Document document, CancellationToken cancellationToken) { + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - - foreach (var diagnostic in diagnostics) + var syntaxFacts = document.GetRequiredLanguageService(); + var typeDeclarations = root.DescendantNodes().Where(node => syntaxFacts.IsTypeDeclaration(node)); + var editor = new SyntaxEditor(root, SyntaxGenerator.GetGenerator(document)); + foreach (var declaration in typeDeclarations) { - var declaration = diagnostic.AdditionalLocations[0].FindNode(cancellationToken); - var declarator = MapToDeclarator(declaration); + UpdateDeclaration(declaration, editor, semanticModel, cancellationToken); + } + + return editor.GetChangedRoot(); + } - var symbol = semanticModel.GetDeclaredSymbol(declarator, cancellationToken); - Contract.ThrowIfNull(symbol); + private static void UpdateDeclaration(SyntaxNode declaration, SyntaxEditor editor, SemanticModel semanticModel, CancellationToken cancellationToken) + { + var symbol = semanticModel.GetDeclaredSymbol(declaration, cancellationToken); + Contract.ThrowIfNull(symbol); - var preferredAccessibility = GetPreferredAccessibility(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)); - } + // 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; @@ -80,6 +87,20 @@ SyntaxNode UpdateAccessibility(SyntaxNode declaration, Accessibility preferredAc } } + protected sealed override async Task FixAllAsync( + Document document, ImmutableArray diagnostics, + SyntaxEditor editor, CancellationToken cancellationToken) + { + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + + foreach (var diagnostic in diagnostics) + { + var declaration = diagnostic.AdditionalLocations[0].FindNode(cancellationToken); + var declarator = MapToDeclarator(declaration); + UpdateDeclaration(declarator, editor, semanticModel, cancellationToken); + } + } + private static Accessibility GetPreferredAccessibility(ISymbol symbol) { // If we have an overridden member, then if we're adding an accessibility modifier, use the diff --git a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs index 79afe2a0db2a6..c4b0158da5872 100644 --- a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs +++ b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs @@ -9,6 +9,7 @@ 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.Editor.Host; @@ -331,11 +332,20 @@ private void FormatDocumentCreatedFromTemplate(IVsHierarchy hierarchy, uint item Contract.ThrowIfNull(rootToFormat); var documentOptions = ThreadHelper.JoinableTaskFactory.Run(() => addedDocument.GetOptionsAsync(cancellationToken)); + // Add access modifier + var rootWithAccessModifier = ThreadHelper.JoinableTaskFactory.Run(() => + { + return AbstractAddAccessibilityModifiersCodeFixProvider.GetTransformedSyntaxRootAsync(addedDocument, cancellationToken); + }); + + addedDocument = addedDocument.WithSyntaxRoot(rootWithAccessModifier); + rootToFormat = rootWithAccessModifier; + // Apply file header preferences var fileHeaderTemplate = documentOptions.GetOption(CodeStyleOptions2.FileHeaderTemplate); if (!string.IsNullOrEmpty(fileHeaderTemplate)) { - var documentWithFileHeader = ThreadHelper.JoinableTaskFactory.Run(() => + var rootWithFileHeader = ThreadHelper.JoinableTaskFactory.Run(() => { var newLineText = documentOptions.GetOption(FormattingOptions.NewLine, rootToFormat.Language); var newLineTrivia = SyntaxGeneratorInternal.EndOfLine(newLineText); @@ -347,8 +357,8 @@ private void FormatDocumentCreatedFromTemplate(IVsHierarchy hierarchy, uint item cancellationToken); }); - addedDocument = addedDocument.WithSyntaxRoot(documentWithFileHeader); - rootToFormat = documentWithFileHeader; + addedDocument = addedDocument.WithSyntaxRoot(rootWithFileHeader); + rootToFormat = rootWithFileHeader; } // Organize using directives From a358ab7e09565f99d5fb3c2521d782c1443df55b Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Wed, 17 Mar 2021 17:52:01 +0200 Subject: [PATCH 2/8] Update AbstractAddAccessibilityModifiersCodeFixProvider.cs --- .../AbstractAddAccessibilityModifiersCodeFixProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs index 3ed27071d9db1..685794a4c188c 100644 --- a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs @@ -51,7 +51,7 @@ internal static async Task GetTransformedSyntaxRootAsync(Document do var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); var syntaxFacts = document.GetRequiredLanguageService(); var typeDeclarations = root.DescendantNodes().Where(node => syntaxFacts.IsTypeDeclaration(node)); - var editor = new SyntaxEditor(root, SyntaxGenerator.GetGenerator(document)); + var editor = new SyntaxEditor(root, document.Project.Solution.Workspace); foreach (var declaration in typeDeclarations) { UpdateDeclaration(declaration, editor, semanticModel, cancellationToken); From 1f97af2ef218b23d333751446ba3931408766fd6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 28 Jul 2021 10:43:06 -0700 Subject: [PATCH 3/8] Move code. --- ...ddAccessibilityModifiersCodeFixProvider.cs | 69 +------------------ .../AddAccessibilityModifiersHelpers.cs | 65 +++++++++++++++++ .../Core/CodeFixes/CodeFixes.projitems | 1 + .../Implementation/AbstractEditorFactory.cs | 54 ++++++++++----- 4 files changed, 103 insertions(+), 86 deletions(-) create mode 100644 src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AddAccessibilityModifiersHelpers.cs diff --git a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs index 685794a4c188c..0e63e0df3b731 100644 --- a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs @@ -11,7 +11,6 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; -using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; @@ -44,49 +43,6 @@ public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) return Task.CompletedTask; } - // Intended for use by AbstractEditorFactory to add accessibility when creating a new file. - internal static async Task GetTransformedSyntaxRootAsync(Document document, CancellationToken cancellationToken) - { - var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var syntaxFacts = document.GetRequiredLanguageService(); - var typeDeclarations = root.DescendantNodes().Where(node => syntaxFacts.IsTypeDeclaration(node)); - var editor = new SyntaxEditor(root, document.Project.Solution.Workspace); - foreach (var declaration in typeDeclarations) - { - UpdateDeclaration(declaration, editor, semanticModel, cancellationToken); - } - - return editor.GetChangedRoot(); - } - - private static void UpdateDeclaration(SyntaxNode declaration, SyntaxEditor editor, SemanticModel semanticModel, CancellationToken cancellationToken) - { - var symbol = semanticModel.GetDeclaredSymbol(declaration, 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); - } - } - protected sealed override async Task FixAllAsync( Document document, ImmutableArray diagnostics, SyntaxEditor editor, CancellationToken cancellationToken) @@ -97,30 +53,9 @@ protected sealed override async Task FixAllAsync( { var declaration = diagnostic.AdditionalLocations[0].FindNode(cancellationToken); var declarator = MapToDeclarator(declaration); - UpdateDeclaration(declarator, editor, semanticModel, cancellationToken); - } - } - - 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; + AddAccessibilityModifiersHelpers.UpdateDeclaration( + semanticModel, editor, declarator, cancellationToken); } - - // Otherwise, default to whatever accessibility no-accessibility means for this member; - return symbol.DeclaredAccessibility; } private class MyCodeAction : CustomCodeActions.DocumentChangeAction diff --git a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AddAccessibilityModifiersHelpers.cs b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AddAccessibilityModifiersHelpers.cs new file mode 100644 index 0000000000000..e63a85ed0b0eb --- /dev/null +++ b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AddAccessibilityModifiersHelpers.cs @@ -0,0 +1,65 @@ +// 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( + SemanticModel semanticModel, SyntaxEditor editor, + SyntaxNode declaration, CancellationToken cancellationToken) + { + var symbol = semanticModel.GetDeclaredSymbol(declaration, 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); + } + } + + 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; + } + } +} diff --git a/src/Analyzers/Core/CodeFixes/CodeFixes.projitems b/src/Analyzers/Core/CodeFixes/CodeFixes.projitems index 888d7dd7d7bee..6a54b139c8462 100644 --- a/src/Analyzers/Core/CodeFixes/CodeFixes.projitems +++ b/src/Analyzers/Core/CodeFixes/CodeFixes.projitems @@ -14,6 +14,7 @@ + diff --git a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs index ddda68f1a74c2..3c31e6ba251d5 100644 --- a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs +++ b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs @@ -4,7 +4,9 @@ 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; @@ -14,6 +16,7 @@ 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; @@ -21,7 +24,6 @@ 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; @@ -294,6 +296,11 @@ protected virtual Task 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. @@ -328,45 +335,40 @@ 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); + var rootToFormat = await addedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); Contract.ThrowIfNull(rootToFormat); - var documentOptions = ThreadHelper.JoinableTaskFactory.Run(() => addedDocument.GetOptionsAsync(cancellationToken)); + var documentOptions = await addedDocument.GetOptionsAsync(cancellationToken).ConfigureAwait(false); // Add access modifier - var rootWithAccessModifier = ThreadHelper.JoinableTaskFactory.Run(() => + if (documentOptions.GetOption(CodeStyleOptions2.RequireAccessibilityModifiers, addedDocument.Project.Language).Value != AccessibilityModifiersRequired.Never) { - return AbstractAddAccessibilityModifiersCodeFixProvider.GetTransformedSyntaxRootAsync(addedDocument, cancellationToken); - }); - - addedDocument = addedDocument.WithSyntaxRoot(rootWithAccessModifier); - rootToFormat = rootWithAccessModifier; + addedDocument = await AddAccessibilityModifiersAsync(addedDocument, cancellationToken).ConfigureAwait(false); + rootToFormat = await addedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + } // Apply file header preferences var fileHeaderTemplate = documentOptions.GetOption(CodeStyleOptions2.FileHeaderTemplate); if (!string.IsNullOrEmpty(fileHeaderTemplate)) { - var rootWithFileHeader = 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(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); + 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); @@ -394,5 +396,19 @@ private void FormatDocumentCreatedFromTemplate(IVsHierarchy hierarchy, uint item formattedText.Write(textWriter, cancellationToken: CancellationToken.None); }); } + + private static async Task 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(); + var typeDeclarations = root.DescendantNodes().Where(node => syntaxFacts.IsTypeDeclaration(node)); + var editor = new SyntaxEditor(root, document.Project.Solution.Workspace); + + foreach (var declaration in typeDeclarations) + AddAccessibilityModifiersHelpers.UpdateDeclaration(semanticModel, editor, declaration, cancellationToken); + + return document.WithSyntaxRoot(editor.GetChangedRoot()); + } } } From 6a80471f204fc51dd5412cc62cfafcd76527aff2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 28 Jul 2021 11:02:15 -0700 Subject: [PATCH 4/8] fix --- .../Core/Def/Implementation/AbstractEditorFactory.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs index 3c31e6ba251d5..37be8921f417a 100644 --- a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs +++ b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs @@ -118,7 +118,7 @@ public int CreateEditorInstance( // We must create the WinForms designer here var loaderName = GetWinFormsLoaderName(vsHierarchy); - var designerService = (IVSMDDesignerService)_oleServiceProvider.QueryService(); + var designerService = (IVSMDDesignerService)Microsoft.VisualStudio.Shell.PackageUtilities.QueryService(_oleServiceProvider); var designerLoader = (IVSMDDesignerLoader)designerService.CreateDesignerLoader(loaderName); if (designerLoader is null) { @@ -335,15 +335,14 @@ private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy var forkedSolution = solution.AddDocument(DocumentInfo.Create(documentId, filePath, loader: new FileTextLoader(filePath, defaultEncoding: null), filePath: filePath)); var addedDocument = forkedSolution.GetDocument(documentId)!; - var rootToFormat = await addedDocument.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - Contract.ThrowIfNull(rootToFormat); + 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.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); } // Apply file header preferences From 9af41e4a06abaf9340225fd56981b106301fa0bc Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 28 Jul 2021 12:50:28 -0700 Subject: [PATCH 5/8] Fix --- .../AbstractAddAccessibilityModifiersCodeFixProvider.cs | 5 +++-- .../AddAccessibilityModifiersHelpers.cs | 4 +--- .../Core/Def/Implementation/AbstractEditorFactory.cs | 6 +++++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs index 0e63e0df3b731..653b69703b24c 100644 --- a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs +++ b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersCodeFixProvider.cs @@ -53,8 +53,9 @@ protected sealed override async Task FixAllAsync( { var declaration = diagnostic.AdditionalLocations[0].FindNode(cancellationToken); var declarator = MapToDeclarator(declaration); - AddAccessibilityModifiersHelpers.UpdateDeclaration( - semanticModel, editor, declarator, cancellationToken); + var symbol = semanticModel.GetDeclaredSymbol(declarator, cancellationToken); + Contract.ThrowIfNull(symbol); + AddAccessibilityModifiersHelpers.UpdateDeclaration(editor, symbol, declaration); } } diff --git a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AddAccessibilityModifiersHelpers.cs b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AddAccessibilityModifiersHelpers.cs index e63a85ed0b0eb..30e3f839b4d06 100644 --- a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AddAccessibilityModifiersHelpers.cs +++ b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/AddAccessibilityModifiersHelpers.cs @@ -12,10 +12,8 @@ namespace Microsoft.CodeAnalysis.AddAccessibilityModifiers internal static class AddAccessibilityModifiersHelpers { public static void UpdateDeclaration( - SemanticModel semanticModel, SyntaxEditor editor, - SyntaxNode declaration, CancellationToken cancellationToken) + SyntaxEditor editor, ISymbol symbol, SyntaxNode declaration) { - var symbol = semanticModel.GetDeclaredSymbol(declaration, cancellationToken); Contract.ThrowIfNull(symbol); var preferredAccessibility = GetPreferredAccessibility(symbol); diff --git a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs index 37be8921f417a..794ed0c6dda92 100644 --- a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs +++ b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs @@ -405,7 +405,11 @@ private static async Task AddAccessibilityModifiersAsync(Document docu var editor = new SyntaxEditor(root, document.Project.Solution.Workspace); foreach (var declaration in typeDeclarations) - AddAccessibilityModifiersHelpers.UpdateDeclaration(semanticModel, editor, declaration, cancellationToken); + { + var type = semanticModel.GetDeclaredSymbol(declaration, cancellationToken); + if (type != null) + AddAccessibilityModifiersHelpers.UpdateDeclaration(editor, type, declaration); + } return document.WithSyntaxRoot(editor.GetChangedRoot()); } From 9f796a627f680e396386399d21f34eade9e8ac44 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 28 Jul 2021 15:18:23 -0700 Subject: [PATCH 6/8] introduce service --- .../CSharpAddAccessibilityModifiers.cs | 90 +++++++++++++++++++ ...ccessibilityModifiersDiagnosticAnalyzer.cs | 70 +-------------- .../Analyzers/CSharpAnalyzers.projitems | 3 +- .../CSharpAddAccessibilityModifiersService.cs | 21 +++++ .../CodeFixes/CSharpCodeFixes.projitems | 1 + .../AbstractAddAccessibilityModifiers.cs | 30 +++++++ ...ccessibilityModifiersDiagnosticAnalyzer.cs | 4 - .../IAddAccessibilityModifiersService.cs | 18 ++++ .../Core/Analyzers/Analyzers.projitems | 2 + .../IAddAccessibilityModifiersService.cs | 12 +++ .../Core/CodeFixes/CodeFixes.projitems | 1 + .../VisualBasicAddAccessibilityModifiers.vb | 83 +++++++++++++++++ ...ccessibilityModifiersDiagnosticAnalyzer.vb | 55 +----------- .../Analyzers/VisualBasicAnalyzers.projitems | 1 + ...alBasicAddAccessibilityModifiersService.vb | 20 +++++ .../CodeFixes/VisualBasicCodeFixes.projitems | 1 + .../Implementation/AbstractEditorFactory.cs | 19 ++-- 17 files changed, 299 insertions(+), 132 deletions(-) create mode 100644 src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiers.cs create mode 100644 src/Analyzers/CSharp/CodeFixes/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersService.cs create mode 100644 src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/AbstractAddAccessibilityModifiers.cs create mode 100644 src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/IAddAccessibilityModifiersService.cs create mode 100644 src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/IAddAccessibilityModifiersService.cs create mode 100644 src/Analyzers/VisualBasic/Analyzers/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiers.vb create mode 100644 src/Analyzers/VisualBasic/CodeFixes/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiersService.vb diff --git a/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiers.cs b/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiers.cs new file mode 100644 index 0000000000000..62361d757fa24 --- /dev/null +++ b/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiers.cs @@ -0,0 +1,90 @@ +// 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 Microsoft.CodeAnalysis.AddAccessibilityModifiers; +using Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.CSharp.Extensions; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.LanguageServices; +using Microsoft.CodeAnalysis.Shared.Extensions; + +namespace Microsoft.CodeAnalysis.CSharp.AddAccessibilityModifiers +{ + internal class CSharpAddAccessibilityModifiers : AbstractAddAccessibilityModifiers + { + public static readonly CSharpAddAccessibilityModifiers Instance = new(); + + protected CSharpAddAccessibilityModifiers() + { + } + + public override bool ShouldUpdateAccessibilityModifier( + ISyntaxFacts syntaxFacts, + MemberDeclarationSyntax member, + AccessibilityModifiersRequired option, + out SyntaxToken name) + { + // Have to have a name to report the issue on. + name = member.GetNameToken(); + if (name.Kind() == SyntaxKind.None) + return false; + + // Certain members never have accessibility. Don't bother reporting on them. + if (!syntaxFacts.CanHaveAccessibility(member)) + return false; + + // This analyzer bases all of its decisions on the accessibility + var accessibility = syntaxFacts.GetAccessibility(member); + + // Omit will flag any accessibility values that exist and are default + // The other options will remove or ignore accessibility + var isOmit = option == AccessibilityModifiersRequired.OmitIfDefault; + + if (isOmit) + { + if (accessibility == Accessibility.NotApplicable) + return false; + + var parentKind = member.GetRequiredParent().Kind(); + switch (parentKind) + { + // Check for default modifiers in namespace and outside of namespace + case SyntaxKind.CompilationUnit: + case SyntaxKind.FileScopedNamespaceDeclaration: + case SyntaxKind.NamespaceDeclaration: + { + // Default is internal + if (accessibility != Accessibility.Internal) + return false; + } + + break; + + case SyntaxKind.ClassDeclaration: + case SyntaxKind.RecordDeclaration: + case SyntaxKind.StructDeclaration: + case SyntaxKind.RecordStructDeclaration: + { + // Inside a type, default is private + if (accessibility != Accessibility.Private) + return false; + } + + break; + + default: + return false; // Unknown parent kind, don't do anything + } + } + else + { + // Mode is always, so we have to flag missing modifiers + if (accessibility != Accessibility.NotApplicable) + return false; + } + + return true; + } + } +} diff --git a/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersDiagnosticAnalyzer.cs index 54c27c7ce7d1a..9bcbb1ae5ac4b 100644 --- a/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersDiagnosticAnalyzer.cs @@ -62,76 +62,8 @@ private void ProcessMemberDeclaration( } #endif - // Have to have a name to report the issue on. - var name = member.GetNameToken(); - if (name.Kind() == SyntaxKind.None) - { - return; - } - - // Certain members never have accessibility. Don't bother reporting on them. - if (!SyntaxFacts.CanHaveAccessibility(member)) - { + if (!CSharpAddAccessibilityModifiers.Instance.ShouldUpdateAccessibilityModifier(SyntaxFacts, member, option.Value, out var name)) return; - } - - // This analyzer bases all of its decisions on the accessibility - var accessibility = SyntaxFacts.GetAccessibility(member); - - // Omit will flag any accessibility values that exist and are default - // The other options will remove or ignore accessibility - var isOmit = option.Value == AccessibilityModifiersRequired.OmitIfDefault; - - if (isOmit) - { - if (accessibility == Accessibility.NotApplicable) - { - return; - } - - var parentKind = member.GetRequiredParent().Kind(); - switch (parentKind) - { - // Check for default modifiers in namespace and outside of namespace - case SyntaxKind.CompilationUnit: - case SyntaxKind.FileScopedNamespaceDeclaration: - case SyntaxKind.NamespaceDeclaration: - { - // Default is internal - if (accessibility != Accessibility.Internal) - { - return; - } - } - - break; - - case SyntaxKind.ClassDeclaration: - case SyntaxKind.RecordDeclaration: - case SyntaxKind.StructDeclaration: - case SyntaxKind.RecordStructDeclaration: - { - // Inside a type, default is private - if (accessibility != Accessibility.Private) - { - return; - } - } - - break; - - default: - return; // Unknown parent kind, don't do anything - } - } - else - { - // Mode is always, so we have to flag missing modifiers - if (accessibility != Accessibility.NotApplicable) - { - return; - } - } // Have an issue to flag, either add or remove. Report issue to user. var additionalLocations = ImmutableArray.Create(member.GetLocation()); diff --git a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems index e9940207cf13d..ac1a5e472fb1f 100644 --- a/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems +++ b/src/Analyzers/CSharp/Analyzers/CSharpAnalyzers.projitems @@ -15,6 +15,7 @@ + @@ -119,4 +120,4 @@ - + \ No newline at end of file diff --git a/src/Analyzers/CSharp/CodeFixes/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersService.cs b/src/Analyzers/CSharp/CodeFixes/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersService.cs new file mode 100644 index 0000000000000..65fd2072bd996 --- /dev/null +++ b/src/Analyzers/CSharp/CodeFixes/AddAccessibilityModifiers/CSharpAddAccessibilityModifiersService.cs @@ -0,0 +1,21 @@ +// 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; +using System.Composition; +using Microsoft.CodeAnalysis.AddAccessibilityModifiers; +using Microsoft.CodeAnalysis.Host.Mef; + +namespace Microsoft.CodeAnalysis.CSharp.AddAccessibilityModifiers +{ + [ExportLanguageService(typeof(IAddAccessibilityModifiersService), LanguageNames.CSharp), Shared] + internal class CSharpAddAccessibilityModifiersService : CSharpAddAccessibilityModifiers, IAddAccessibilityModifiersService + { + [ImportingConstructor] + [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] + public CSharpAddAccessibilityModifiersService() + { + } + } +} diff --git a/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems b/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems index d9a2923277073..5f80d36063302 100644 --- a/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems +++ b/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems @@ -14,6 +14,7 @@ + diff --git a/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/AbstractAddAccessibilityModifiers.cs b/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/AbstractAddAccessibilityModifiers.cs new file mode 100644 index 0000000000000..cc359b8d33a48 --- /dev/null +++ b/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/AbstractAddAccessibilityModifiers.cs @@ -0,0 +1,30 @@ +// 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 Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.LanguageServices; + +namespace Microsoft.CodeAnalysis.AddAccessibilityModifiers +{ + internal abstract class AbstractAddAccessibilityModifiers : IAddAccessibilityModifiers + where TMemberDeclarationSyntax : SyntaxNode + { + public bool ShouldUpdateAccessibilityModifier( + ISyntaxFacts syntaxFacts, + SyntaxNode member, + AccessibilityModifiersRequired option, + out SyntaxToken name) + { + name = default; + return member is TMemberDeclarationSyntax memberDecl && + ShouldUpdateAccessibilityModifier(syntaxFacts, memberDecl, option, out name); + } + + public abstract bool ShouldUpdateAccessibilityModifier( + ISyntaxFacts syntaxFacts, + TMemberDeclarationSyntax member, + AccessibilityModifiersRequired option, + out SyntaxToken name); + } +} diff --git a/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersDiagnosticAnalyzer.cs index f7b2d1cbfd35f..462188d6dcd5e 100644 --- a/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/AbstractAddAccessibilityModifiersDiagnosticAnalyzer.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.Diagnostics; @@ -36,9 +34,7 @@ private void AnalyzeSyntaxTree(SyntaxTreeAnalysisContext context) var language = syntaxTree.Options.Language; var option = context.GetOption(CodeStyleOptions2.RequireAccessibilityModifiers, language); if (option.Value == AccessibilityModifiersRequired.Never) - { return; - } ProcessCompilationUnit(context, option, (TCompilationUnitSyntax)syntaxTree.GetRoot(cancellationToken)); } diff --git a/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/IAddAccessibilityModifiersService.cs b/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/IAddAccessibilityModifiersService.cs new file mode 100644 index 0000000000000..da5e86dfb43dc --- /dev/null +++ b/src/Analyzers/Core/Analyzers/AddAccessibilityModifiers/IAddAccessibilityModifiersService.cs @@ -0,0 +1,18 @@ +// 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 Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.LanguageServices; + +namespace Microsoft.CodeAnalysis.AddAccessibilityModifiers +{ + internal interface IAddAccessibilityModifiers + { + bool ShouldUpdateAccessibilityModifier( + ISyntaxFacts syntaxFacts, + SyntaxNode member, + AccessibilityModifiersRequired option, + out SyntaxToken name); + } +} diff --git a/src/Analyzers/Core/Analyzers/Analyzers.projitems b/src/Analyzers/Core/Analyzers/Analyzers.projitems index 010bb8993e949..987454bd75641 100644 --- a/src/Analyzers/Core/Analyzers/Analyzers.projitems +++ b/src/Analyzers/Core/Analyzers/Analyzers.projitems @@ -17,7 +17,9 @@ + + diff --git a/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/IAddAccessibilityModifiersService.cs b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/IAddAccessibilityModifiersService.cs new file mode 100644 index 0000000000000..93c6f7ff23281 --- /dev/null +++ b/src/Analyzers/Core/CodeFixes/AddAccessibilityModifiers/IAddAccessibilityModifiersService.cs @@ -0,0 +1,12 @@ +// 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 Microsoft.CodeAnalysis.Host; + +namespace Microsoft.CodeAnalysis.AddAccessibilityModifiers +{ + internal interface IAddAccessibilityModifiersService : IAddAccessibilityModifiers, ILanguageService + { + } +} diff --git a/src/Analyzers/Core/CodeFixes/CodeFixes.projitems b/src/Analyzers/Core/CodeFixes/CodeFixes.projitems index 6a54b139c8462..293e9c1f4196a 100644 --- a/src/Analyzers/Core/CodeFixes/CodeFixes.projitems +++ b/src/Analyzers/Core/CodeFixes/CodeFixes.projitems @@ -15,6 +15,7 @@ + diff --git a/src/Analyzers/VisualBasic/Analyzers/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiers.vb b/src/Analyzers/VisualBasic/Analyzers/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiers.vb new file mode 100644 index 0000000000000..2a7986d02aee6 --- /dev/null +++ b/src/Analyzers/VisualBasic/Analyzers/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiers.vb @@ -0,0 +1,83 @@ +' 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. + +Imports Microsoft.CodeAnalysis.AddAccessibilityModifiers +Imports Microsoft.CodeAnalysis.CodeStyle +Imports Microsoft.CodeAnalysis.VisualBasic.Syntax + +Namespace Microsoft.CodeAnalysis.VisualBasic.AddAccessibilityModifiers + Friend Class VisualBasicAddAccessibilityModifiers + Inherits AbstractAddAccessibilityModifiers(Of StatementSyntax) + + Public Shared ReadOnly Instance As New VisualBasicAddAccessibilityModifiers() + + Protected Sub New() + End Sub + + Public Overrides Function ShouldUpdateAccessibilityModifier( + syntaxFacts As CodeAnalysis.LanguageServices.ISyntaxFacts, + member As StatementSyntax, + [option] As AccessibilityModifiersRequired, + ByRef name As SyntaxToken) As Boolean + + ' Have to have a name to report the issue on. + name = member.GetNameToken() + If name.Kind() = SyntaxKind.None Then + Return False + End If + + ' Certain members never have accessibility. Don't bother reporting on them. + If Not syntaxFacts.CanHaveAccessibility(member) Then + Return False + End If + + ' This analyzer bases all of its decisions on the accessibility + Dim Accessibility = syntaxFacts.GetAccessibility(member) + + ' Omit will flag any accesibility values that exist and are default + ' The other options will remove or ignore accessibility + Dim isOmit = [option] = AccessibilityModifiersRequired.OmitIfDefault + + If isOmit Then + If Accessibility = Accessibility.NotApplicable Then + ' Accessibility modifier already missing. nothing we need to do. + Return False + End If + + If Not MatchesDefaultAccessibility(Accessibility, member) Then + ' Explicit accessibility was different than the default accessibility. + ' We have to keep this here. + Return False + End If + + Else ' Require all, flag missing modidifers + If Accessibility <> Accessibility.NotApplicable Then + Return False + End If + End If + + Return True + End Function + + Private Shared Function MatchesDefaultAccessibility(accessibility As Accessibility, member As StatementSyntax) As Boolean + ' Top level items in a namespace or file + If member.IsParentKind(SyntaxKind.CompilationUnit) OrElse + member.IsParentKind(SyntaxKind.NamespaceBlock) Then + ' default is Friend + Return accessibility = Accessibility.Friend + End If + + ' default for const and field in a class is private + If member.IsParentKind(SyntaxKind.ClassBlock) OrElse + member.IsParentKind(SyntaxKind.ModuleBlock) Then + If member.IsKind(SyntaxKind.FieldDeclaration) Then + Return accessibility = Accessibility.Private + End If + End If + + ' Everything else has a default of public + Return accessibility = Accessibility.Public + End Function + End Class +End Namespace diff --git a/src/Analyzers/VisualBasic/Analyzers/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiersDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiersDiagnosticAnalyzer.vb index 16bad2c13f1e7..7ecda4c89eef7 100644 --- a/src/Analyzers/VisualBasic/Analyzers/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiersDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiersDiagnosticAnalyzer.vb @@ -47,42 +47,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.AddAccessibilityModifiers ProcessMembers(context, [option], typeBlock.Members) End If - ' Have to have a name to report the issue on. - Dim name = member.GetNameToken() - If name.Kind() = SyntaxKind.None Then + Dim name As SyntaxToken = Nothing + If Not VisualBasicAddAccessibilityModifiers.Instance.ShouldUpdateAccessibilityModifier(SyntaxFacts, member, [option].Value, name) Then Return End If - ' Certain members never have accessibility. Don't bother reporting on them. - If Not SyntaxFacts.CanHaveAccessibility(member) Then - Return - End If - - ' This analyzer bases all of its decisions on the accessibility - Dim Accessibility = SyntaxFacts.GetAccessibility(member) - - ' Omit will flag any accesibility values that exist and are default - ' The other options will remove or ignore accessibility - Dim isOmit = [option].Value = AccessibilityModifiersRequired.OmitIfDefault - - If isOmit Then - If Accessibility = Accessibility.NotApplicable Then - ' Accessibility modifier already missing. nothing we need to do. - Return - End If - - If Not MatchesDefaultAccessibility(Accessibility, member) Then - ' Explicit accessibility was different than the default accessibility. - ' We have to keep this here. - Return - End If - - Else ' Require all, flag missing modidifers - If Accessibility <> Accessibility.NotApplicable Then - Return - End If - End If - ' Have an issue to flag, either add or remove. Report issue to user. Dim additionalLocations = ImmutableArray.Create(member.GetLocation()) context.ReportDiagnostic(DiagnosticHelper.Create( @@ -92,25 +61,5 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.AddAccessibilityModifiers additionalLocations:=additionalLocations, properties:=Nothing)) End Sub - - Private Shared Function MatchesDefaultAccessibility(accessibility As Accessibility, member As StatementSyntax) As Boolean - ' Top level items in a namespace or file - If member.IsParentKind(SyntaxKind.CompilationUnit) OrElse - member.IsParentKind(SyntaxKind.NamespaceBlock) Then - ' default is Friend - Return accessibility = Accessibility.Friend - End If - - ' default for const and field in a class is private - If member.IsParentKind(SyntaxKind.ClassBlock) OrElse - member.IsParentKind(SyntaxKind.ModuleBlock) Then - If member.IsKind(SyntaxKind.FieldDeclaration) Then - Return accessibility = Accessibility.Private - End If - End If - - ' Everything else has a default of public - Return accessibility = Accessibility.Public - End Function End Class End Namespace diff --git a/src/Analyzers/VisualBasic/Analyzers/VisualBasicAnalyzers.projitems b/src/Analyzers/VisualBasic/Analyzers/VisualBasicAnalyzers.projitems index acbdb7df3bd8a..3ced98ebbf8e0 100644 --- a/src/Analyzers/VisualBasic/Analyzers/VisualBasicAnalyzers.projitems +++ b/src/Analyzers/VisualBasic/Analyzers/VisualBasicAnalyzers.projitems @@ -13,6 +13,7 @@ + diff --git a/src/Analyzers/VisualBasic/CodeFixes/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiersService.vb b/src/Analyzers/VisualBasic/CodeFixes/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiersService.vb new file mode 100644 index 0000000000000..3993b1e957f2f --- /dev/null +++ b/src/Analyzers/VisualBasic/CodeFixes/AddAccessibilityModifiers/VisualBasicAddAccessibilityModifiersService.vb @@ -0,0 +1,20 @@ +' 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. + +Imports System.Composition +Imports Microsoft.CodeAnalysis.AddAccessibilityModifiers +Imports Microsoft.CodeAnalysis.Host.Mef + +Namespace Microsoft.CodeAnalysis.VisualBasic.AddAccessibilityModifiers + + Friend Class VisualBasicAddAccessibilityModifiersService + Inherits VisualBasicAddAccessibilityModifiers + Implements IAddAccessibilityModifiersService + + + + Public Sub New() + End Sub + End Class +End Namespace diff --git a/src/Analyzers/VisualBasic/CodeFixes/VisualBasicCodeFixes.projitems b/src/Analyzers/VisualBasic/CodeFixes/VisualBasicCodeFixes.projitems index 23087c51f179c..ba29babacf791 100644 --- a/src/Analyzers/VisualBasic/CodeFixes/VisualBasicCodeFixes.projitems +++ b/src/Analyzers/VisualBasic/CodeFixes/VisualBasicCodeFixes.projitems @@ -14,6 +14,7 @@ + diff --git a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs index 794ed0c6dda92..274af4675db07 100644 --- a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs +++ b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs @@ -339,9 +339,10 @@ private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy var documentOptions = await addedDocument.GetOptionsAsync(cancellationToken).ConfigureAwait(false); // Add access modifier - if (documentOptions.GetOption(CodeStyleOptions2.RequireAccessibilityModifiers, addedDocument.Project.Language).Value != AccessibilityModifiersRequired.Never) + var accessibilityPreferences = documentOptions.GetOption(CodeStyleOptions2.RequireAccessibilityModifiers, addedDocument.Project.Language); + if (accessibilityPreferences.Value != AccessibilityModifiersRequired.Never) { - addedDocument = await AddAccessibilityModifiersAsync(addedDocument, cancellationToken).ConfigureAwait(false); + addedDocument = await AddAccessibilityModifiersAsync(addedDocument, accessibilityPreferences.Value, cancellationToken).ConfigureAwait(false); rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); } @@ -396,7 +397,8 @@ private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy }); } - private static async Task AddAccessibilityModifiersAsync(Document document, CancellationToken cancellationToken) + private static async Task AddAccessibilityModifiersAsync( + Document document, AccessibilityModifiersRequired option, CancellationToken cancellationToken) { var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); @@ -404,11 +406,18 @@ private static async Task AddAccessibilityModifiersAsync(Document docu var typeDeclarations = root.DescendantNodes().Where(node => syntaxFacts.IsTypeDeclaration(node)); var editor = new SyntaxEditor(root, document.Project.Solution.Workspace); + var service = document.GetRequiredLanguageService(); + foreach (var declaration in typeDeclarations) { + if (!service.ShouldUpdateAccessibilityModifier(syntaxFacts, declaration, option, out _)) + continue; + var type = semanticModel.GetDeclaredSymbol(declaration, cancellationToken); - if (type != null) - AddAccessibilityModifiersHelpers.UpdateDeclaration(editor, type, declaration); + if (type == null) + continue; + + AddAccessibilityModifiersHelpers.UpdateDeclaration(editor, type, declaration); } return document.WithSyntaxRoot(editor.GetChangedRoot()); From fda6aeaf6023878c5d6a62324fb3e1fbdb41fff7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 29 Jul 2021 15:01:47 -0700 Subject: [PATCH 7/8] move down --- .../Def/Implementation/AbstractEditorFactory.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs index 274af4675db07..4b5ee323efa40 100644 --- a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs +++ b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs @@ -338,14 +338,6 @@ private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy var rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var documentOptions = await addedDocument.GetOptionsAsync(cancellationToken).ConfigureAwait(false); - // Add access modifier - var accessibilityPreferences = documentOptions.GetOption(CodeStyleOptions2.RequireAccessibilityModifiers, addedDocument.Project.Language); - if (accessibilityPreferences.Value != AccessibilityModifiersRequired.Never) - { - addedDocument = await AddAccessibilityModifiersAsync(addedDocument, accessibilityPreferences.Value, cancellationToken).ConfigureAwait(false); - rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - } - // Apply file header preferences var fileHeaderTemplate = documentOptions.GetOption(CodeStyleOptions2.FileHeaderTemplate); if (!string.IsNullOrEmpty(fileHeaderTemplate)) @@ -367,6 +359,15 @@ private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy addedDocument = await OrganizeUsingsCreatedFromTemplateAsync(addedDocument, cancellationToken).ConfigureAwait(false); rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + // Add access modifier + var accessibilityPreferences = documentOptions.GetOption(CodeStyleOptions2.RequireAccessibilityModifiers, addedDocument.Project.Language); + if (addedDocument.Project.Language == LanguageNames.CSharp && + accessibilityPreferences.Value != AccessibilityModifiersRequired.Never) + { + addedDocument = await AddAccessibilityModifiersAsync(addedDocument, accessibilityPreferences.Value, cancellationToken).ConfigureAwait(false); + rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + } + // Format document var unformattedText = await addedDocument.GetTextAsync(cancellationToken).ConfigureAwait(false); var formattedRoot = Formatter.Format(rootToFormat, workspace, documentOptions, cancellationToken); From f8f5ab4cbedfabf9229965cfa1026a38ccbc8f4d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 29 Jul 2021 17:15:38 -0700 Subject: [PATCH 8/8] Use COnfigureAwait(true) --- .../Implementation/AbstractEditorFactory.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs index 4b5ee323efa40..a6596c0f94da2 100644 --- a/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs +++ b/src/VisualStudio/Core/Def/Implementation/AbstractEditorFactory.cs @@ -335,8 +335,8 @@ private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy var forkedSolution = solution.AddDocument(DocumentInfo.Create(documentId, filePath, loader: new FileTextLoader(filePath, defaultEncoding: null), filePath: filePath)); var addedDocument = forkedSolution.GetDocument(documentId)!; - var rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var documentOptions = await addedDocument.GetOptionsAsync(cancellationToken).ConfigureAwait(false); + var rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(true); + var documentOptions = await addedDocument.GetOptionsAsync(cancellationToken).ConfigureAwait(true); // Apply file header preferences var fileHeaderTemplate = documentOptions.GetOption(CodeStyleOptions2.FileHeaderTemplate); @@ -349,27 +349,27 @@ private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy FileHeaderHelper, newLineTrivia, addedDocument, - cancellationToken).ConfigureAwait(false); + cancellationToken).ConfigureAwait(true); addedDocument = addedDocument.WithSyntaxRoot(rootWithFileHeader); rootToFormat = rootWithFileHeader; } // Organize using directives - addedDocument = await OrganizeUsingsCreatedFromTemplateAsync(addedDocument, cancellationToken).ConfigureAwait(false); - rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + addedDocument = await OrganizeUsingsCreatedFromTemplateAsync(addedDocument, cancellationToken).ConfigureAwait(true); + rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(true); // Add access modifier var accessibilityPreferences = documentOptions.GetOption(CodeStyleOptions2.RequireAccessibilityModifiers, addedDocument.Project.Language); if (addedDocument.Project.Language == LanguageNames.CSharp && accessibilityPreferences.Value != AccessibilityModifiersRequired.Never) { - addedDocument = await AddAccessibilityModifiersAsync(addedDocument, accessibilityPreferences.Value, cancellationToken).ConfigureAwait(false); - rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + addedDocument = await AddAccessibilityModifiersAsync(addedDocument, accessibilityPreferences.Value, cancellationToken).ConfigureAwait(true); + rootToFormat = await addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(true); } // Format document - var unformattedText = await addedDocument.GetTextAsync(cancellationToken).ConfigureAwait(false); + var unformattedText = await addedDocument.GetTextAsync(cancellationToken).ConfigureAwait(true); var formattedRoot = Formatter.Format(rootToFormat, workspace, documentOptions, cancellationToken); var formattedText = formattedRoot.GetText(unformattedText.Encoding, unformattedText.ChecksumAlgorithm); @@ -401,8 +401,8 @@ private async Task FormatDocumentCreatedFromTemplateAsync(IVsHierarchy hierarchy private static async Task AddAccessibilityModifiersAsync( Document document, AccessibilityModifiersRequired option, CancellationToken cancellationToken) { - var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(true); + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(true); var syntaxFacts = document.GetRequiredLanguageService(); var typeDeclarations = root.DescendantNodes().Where(node => syntaxFacts.IsTypeDeclaration(node)); var editor = new SyntaxEditor(root, document.Project.Solution.Workspace);