From c0b76d10a24d2fcbd56346424e59f66df1a4cfed Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 24 Jul 2025 12:23:42 +0200 Subject: [PATCH 1/3] Fix issue where we weren't properly adding elastic trivia to newly generated members --- ...eAccessorsForAttributeArgumentsAnalyzer.cs | 519 +++++++++++++++ ...fineAccessorsForAttributeArgumentsFixer.cs | 179 ++++++ ...fineAccessorsForAttributeArgumentsTests.cs | 602 ++++++++++++++++++ .../CodeGeneration/CSharpSyntaxGenerator.cs | 27 +- 4 files changed, 1316 insertions(+), 11 deletions(-) create mode 100644 src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsAnalyzer.cs create mode 100644 src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsFixer.cs create mode 100644 src/Features/CSharpTest/NetAnalyzers/DefineAccessorsForAttributeArgumentsTests.cs diff --git a/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsAnalyzer.cs b/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsAnalyzer.cs new file mode 100644 index 0000000000000..a0d951959213f --- /dev/null +++ b/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsAnalyzer.cs @@ -0,0 +1,519 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Reflection; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Roslyn.Utilities; + +namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines +{ + internal static class C + { + public static bool IsAssignableTo( + [NotNullWhen(returnValue: true)] this ITypeSymbol? fromSymbol, + [NotNullWhen(returnValue: true)] ITypeSymbol? toSymbol, + Compilation compilation) + => fromSymbol != null && toSymbol != null && compilation.ClassifyCommonConversion(fromSymbol, toSymbol).IsImplicit; + + public static bool IsLessSevereThan(this ReportDiagnostic current, ReportDiagnostic other) + { + return current switch + { + ReportDiagnostic.Error => false, + + ReportDiagnostic.Warn => + other switch + { + ReportDiagnostic.Error => true, + _ => false + }, + + ReportDiagnostic.Info => + other switch + { + ReportDiagnostic.Error => true, + ReportDiagnostic.Warn => true, + _ => false + }, + + ReportDiagnostic.Hidden => + other switch + { + ReportDiagnostic.Error => true, + ReportDiagnostic.Warn => true, + ReportDiagnostic.Info => true, + _ => false + }, + + ReportDiagnostic.Suppress => true, + + _ => false + }; + } + } + + internal static class DiagnosticExtensions + { + public static Diagnostic CreateDiagnostic( + this SyntaxNode node, + DiagnosticDescriptor rule, + params object[] args) + => node.CreateDiagnostic(rule, properties: null, args); + + public static Diagnostic CreateDiagnostic( + this SyntaxNode node, + DiagnosticDescriptor rule, + ImmutableDictionary? properties, + params object[] args) + => node.CreateDiagnostic(rule, additionalLocations: ImmutableArray.Empty, properties, args); + + public static Diagnostic CreateDiagnostic( + this SyntaxNode node, + DiagnosticDescriptor rule, + ImmutableArray additionalLocations, + ImmutableDictionary? properties, + params object[] args) + => node + .GetLocation() + .CreateDiagnostic( + rule: rule, + additionalLocations: additionalLocations, + properties: properties, + args: args); + + public static Diagnostic CreateDiagnostic( + this IOperation operation, + DiagnosticDescriptor rule, + params object[] args) + => operation.CreateDiagnostic(rule, properties: null, args); + + public static Diagnostic CreateDiagnostic( + this IOperation operation, + DiagnosticDescriptor rule, + ImmutableDictionary? properties, + params object[] args) + { + return operation.Syntax.CreateDiagnostic(rule, properties, args); + } + + public static Diagnostic CreateDiagnostic( + this IOperation operation, + DiagnosticDescriptor rule, + ImmutableArray additionalLocations, + ImmutableDictionary? properties, + params object[] args) + { + return operation.Syntax.CreateDiagnostic(rule, additionalLocations, properties, args); + } + + public static Diagnostic CreateDiagnostic( + this SyntaxToken token, + DiagnosticDescriptor rule, + params object[] args) + { + return token.GetLocation().CreateDiagnostic(rule, args); + } + + public static Diagnostic CreateDiagnostic( + this ISymbol symbol, + DiagnosticDescriptor rule, + params object[] args) + { + return symbol.Locations.CreateDiagnostic(rule, args); + } + + public static Diagnostic CreateDiagnostic( + this ISymbol symbol, + DiagnosticDescriptor rule, + ImmutableDictionary? properties, + params object[] args) + { + return symbol.Locations.CreateDiagnostic(rule, properties, args); + } + + public static Diagnostic CreateDiagnostic( + this Location location, + DiagnosticDescriptor rule, + params object[] args) + => location + .CreateDiagnostic( + rule: rule, + properties: ImmutableDictionary.Empty, + args: args); + + public static Diagnostic CreateDiagnostic( + this Location location, + DiagnosticDescriptor rule, + ImmutableDictionary? properties, + params object[] args) + => location.CreateDiagnostic(rule, ImmutableArray.Empty, properties, args); + + public static Diagnostic CreateDiagnostic( + this Location location, + DiagnosticDescriptor rule, + ImmutableArray additionalLocations, + ImmutableDictionary? properties, + params object[] args) + { + if (!location.IsInSource) + { + location = Location.None; + } + + return Diagnostic.Create( + descriptor: rule, + location: location, + additionalLocations: additionalLocations, + properties: properties, + messageArgs: args); + } + + public static Diagnostic CreateDiagnostic( + this IEnumerable locations, + DiagnosticDescriptor rule, + params object[] args) + { + return locations.CreateDiagnostic(rule, null, args); + } + + public static Diagnostic CreateDiagnostic( + this IEnumerable locations, + DiagnosticDescriptor rule, + ImmutableDictionary? properties, + params object[] args) + { + IEnumerable inSource = locations.Where(l => l.IsInSource); + if (!inSource.Any()) + { + return Diagnostic.Create(rule, null, args); + } + + return Diagnostic.Create(rule, + location: inSource.First(), + additionalLocations: inSource.Skip(1), + properties: properties, + messageArgs: args); + } + + /// + /// TODO: Revert this reflection based workaround once we move to Microsoft.CodeAnalysis version 3.0 + /// + private static readonly PropertyInfo? s_syntaxTreeDiagnosticOptionsProperty = + typeof(SyntaxTree).GetTypeInfo().GetDeclaredProperty("DiagnosticOptions"); + + private static readonly PropertyInfo? s_compilationOptionsSyntaxTreeOptionsProviderProperty = + typeof(CompilationOptions).GetTypeInfo().GetDeclaredProperty("SyntaxTreeOptionsProvider"); + + public static void ReportNoLocationDiagnostic( + this CompilationAnalysisContext context, + DiagnosticDescriptor rule, + params object[] args) + => context.Compilation.ReportNoLocationDiagnostic(rule, context.ReportDiagnostic, properties: null, args); + + public static void ReportNoLocationDiagnostic( + this SyntaxNodeAnalysisContext context, + DiagnosticDescriptor rule, + params object[] args) + => context.Compilation.ReportNoLocationDiagnostic(rule, context.ReportDiagnostic, properties: null, args); + + public static void ReportNoLocationDiagnostic( + this Compilation compilation, + DiagnosticDescriptor rule, + Action addDiagnostic, + ImmutableDictionary? properties, + params object[] args) + { + var effectiveSeverity = GetEffectiveSeverity(); + if (!effectiveSeverity.HasValue) + { + // Disabled rule + return; + } + + if (effectiveSeverity.Value != rule.DefaultSeverity) + { +#pragma warning disable RS0030 // The symbol 'DiagnosticDescriptor.DiagnosticDescriptor.#ctor' is banned in this project: Use 'DiagnosticDescriptorHelper.Create' instead + rule = new DiagnosticDescriptor(rule.Id, rule.Title, rule.MessageFormat, rule.Category, + effectiveSeverity.Value, rule.IsEnabledByDefault, rule.Description, rule.HelpLinkUri, rule.CustomTags.ToArray()); +#pragma warning restore RS0030 + } + + var diagnostic = Diagnostic.Create(rule, Location.None, properties, args); + addDiagnostic(diagnostic); + return; + + DiagnosticSeverity? GetEffectiveSeverity() + { + // Microsoft.CodeAnalysis version >= 3.7 exposes options through 'CompilationOptions.SyntaxTreeOptionsProvider.TryGetDiagnosticValue' + // Microsoft.CodeAnalysis version 3.3 - 3.7 exposes options through 'SyntaxTree.DiagnosticOptions'. This API is deprecated in 3.7. + + var syntaxTreeOptionsProvider = s_compilationOptionsSyntaxTreeOptionsProviderProperty?.GetValue(compilation.Options); + var syntaxTreeOptionsProviderTryGetDiagnosticValueMethod = syntaxTreeOptionsProvider?.GetType().GetRuntimeMethods().FirstOrDefault(m => m.Name == "TryGetDiagnosticValue"); + if (syntaxTreeOptionsProviderTryGetDiagnosticValueMethod == null && s_syntaxTreeDiagnosticOptionsProperty == null) + { + return rule.DefaultSeverity; + } + + ReportDiagnostic? overriddenSeverity = null; + foreach (var tree in compilation.SyntaxTrees) + { + ReportDiagnostic? configuredValue = null; + + // Prefer 'CompilationOptions.SyntaxTreeOptionsProvider', if available. + if (s_compilationOptionsSyntaxTreeOptionsProviderProperty != null) + { + if (syntaxTreeOptionsProviderTryGetDiagnosticValueMethod != null) + { + // public abstract bool TryGetDiagnosticValue(SyntaxTree tree, string diagnosticId, out ReportDiagnostic severity); + // public abstract bool TryGetDiagnosticValue(SyntaxTree tree, string diagnosticId, CancellationToken cancellationToken, out ReportDiagnostic severity); + object?[] parameters; + if (syntaxTreeOptionsProviderTryGetDiagnosticValueMethod.GetParameters().Length == 3) + { + parameters = new object?[] { tree, rule.Id, null }; + } + else + { + parameters = new object?[] { tree, rule.Id, CancellationToken.None, null }; + } + + if (syntaxTreeOptionsProviderTryGetDiagnosticValueMethod.Invoke(syntaxTreeOptionsProvider, parameters) is true && + parameters.Last() is ReportDiagnostic value) + { + configuredValue = value; + } + } + } + else + { + RoslynDebug.Assert(s_syntaxTreeDiagnosticOptionsProperty != null); + var options = (ImmutableDictionary)s_syntaxTreeDiagnosticOptionsProperty.GetValue(tree)!; + if (options.TryGetValue(rule.Id, out var value)) + { + configuredValue = value; + } + } + + if (configuredValue == null) + { + continue; + } + + if (configuredValue == ReportDiagnostic.Suppress) + { + // Any suppression entry always wins. + return null; + } + + if (overriddenSeverity == null) + { + overriddenSeverity = configuredValue; + } + else if (overriddenSeverity.Value.IsLessSevereThan(configuredValue.Value)) + { + // Choose the most severe value for conflicts. + overriddenSeverity = configuredValue; + } + } + + return overriddenSeverity.HasValue ? overriddenSeverity.Value.ToDiagnosticSeverity() : rule.DefaultSeverity; + } + } + } + + + /// + /// CA1019: + /// + /// Cause: + /// In its constructor, an attribute defines arguments that do not have corresponding properties. + /// + [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] + internal sealed class DefineAccessorsForAttributeArgumentsAnalyzer : DiagnosticAnalyzer + { + internal const string RuleId = "CA1019"; + internal const string AddAccessorCase = "AddAccessor"; + internal const string MakePublicCase = "MakePublic"; + internal const string RemoveSetterCase = "RemoveSetter"; + + private static readonly LocalizableString s_localizableTitle = "DefineAccessorsForAttributeArgumentsTitle"; + + internal static readonly DiagnosticDescriptor DefaultRule = new DiagnosticDescriptor( + RuleId, + s_localizableTitle, + "DefineAccessorsForAttributeArgumentsTitle", + "DiagnosticCategory.Design", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: null); + + internal static readonly DiagnosticDescriptor IncreaseVisibilityRule = new DiagnosticDescriptor( + RuleId, + s_localizableTitle, + "DefineAccessorsForAttributeArgumentsTitle", + "DiagnosticCategory.Design", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: null); + + internal static readonly DiagnosticDescriptor RemoveSetterRule = new DiagnosticDescriptor( + RuleId, + s_localizableTitle, + "DefineAccessorsForAttributeArgumentsMessageRemoveSetter", + "DiagnosticCategory.Design", + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: null); + + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(DefaultRule, IncreaseVisibilityRule, RemoveSetterRule); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(compilationContext => + { + INamedTypeSymbol? attributeType = compilationContext.Compilation.GetTypeByMetadataName("System.Attribute"); + if (attributeType == null) + { + return; + } + + compilationContext.RegisterSymbolAction(context => + { + AnalyzeSymbol((INamedTypeSymbol)context.Symbol, attributeType, context.Compilation, context.ReportDiagnostic); + }, + SymbolKind.NamedType); + }); + } + + private static void AnalyzeSymbol(INamedTypeSymbol symbol, INamedTypeSymbol attributeType, Compilation compilation, Action addDiagnostic) + { + if (symbol != null && symbol.GetBaseTypesAndThis().Contains(attributeType) && symbol.DeclaredAccessibility != Accessibility.Private) + { + IEnumerable parametersToCheck = GetAllPublicConstructorParameters(symbol); + if (parametersToCheck.Any()) + { + IDictionary propertiesMap = GetAllPropertiesInTypeChain(symbol); + AnalyzeParameters(compilation, parametersToCheck, propertiesMap, symbol, addDiagnostic); + } + } + } + + private static IEnumerable GetAllPublicConstructorParameters(INamedTypeSymbol attributeType) + { + // FxCop compatibility: + // Only examine parameters of public constructors. Can't use protected + // constructors to define an attribute so this rule only applies to + // public constructors. + IEnumerable instanceConstructorsToCheck = attributeType.InstanceConstructors.Where(c => c.DeclaredAccessibility == Accessibility.Public); + + if (instanceConstructorsToCheck.Any()) + { + var uniqueParamNames = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (IMethodSymbol constructor in instanceConstructorsToCheck) + { + foreach (IParameterSymbol parameter in constructor.Parameters) + { + if (uniqueParamNames.Add(parameter.Name)) + { + yield return parameter; + } + } + } + } + } + + private static IDictionary GetAllPropertiesInTypeChain(INamedTypeSymbol attributeType) + { + var propertiesMap = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (INamedTypeSymbol currentType in attributeType.GetBaseTypesAndThis()) + { + foreach (IPropertySymbol property in currentType.GetMembers().Where(m => m.Kind == SymbolKind.Property).Cast()) + { + if (!propertiesMap.ContainsKey(property.Name)) + { + propertiesMap.Add(property.Name, property); + } + } + } + + return propertiesMap; + } + + private static void AnalyzeParameters(Compilation compilation, IEnumerable parameters, IDictionary propertiesMap, INamedTypeSymbol attributeType, Action addDiagnostic) + { + foreach (IParameterSymbol parameter in parameters) + { + if (parameter.Type.Kind != SymbolKind.ErrorType) + { + if (!propertiesMap.TryGetValue(parameter.Name, out IPropertySymbol property) || + !parameter.Type.IsAssignableTo(property.Type, compilation)) + { + // Add a public read-only property accessor for positional argument '{0}' of attribute '{1}'. + addDiagnostic(GetDefaultDiagnostic(parameter, attributeType)); + } + else + { + if (property.GetMethod == null) + { + // Add a public read-only property accessor for positional argument '{0}' of attribute '{1}'. + addDiagnostic(GetDefaultDiagnostic(parameter, attributeType)); + } + else if (property.DeclaredAccessibility != Accessibility.Public || + property.GetMethod.DeclaredAccessibility != Accessibility.Public) + { + if (!property.ContainingType.Equals(attributeType)) + { + // A non-public getter exists in one of the base types. + // However, we cannot be sure if the user can modify the base type (it could be from a third party library). + // So generate the default diagnostic instead of increase visibility here. + + // Add a public read-only property accessor for positional argument '{0}' of attribute '{1}'. + addDiagnostic(GetDefaultDiagnostic(parameter, attributeType)); + } + else + { + // If '{0}' is the property accessor for positional argument '{1}', make it public. + addDiagnostic(GetIncreaseVisibilityDiagnostic(parameter, property)); + } + } + + if (property.SetMethod != null && + property.SetMethod.DeclaredAccessibility == Accessibility.Public && + Equals(property.ContainingType, attributeType)) + { + // Remove the property setter from '{0}' or reduce its accessibility because it corresponds to positional argument '{1}'. + addDiagnostic(GetRemoveSetterDiagnostic(parameter, property)); + } + } + } + } + } + + private static Diagnostic GetDefaultDiagnostic(IParameterSymbol parameter, INamedTypeSymbol attributeType) + { + // Add a public read-only property accessor for positional argument '{0}' of attribute '{1}'. + return parameter.Locations.CreateDiagnostic(DefaultRule, new Dictionary { { "case", AddAccessorCase } }.ToImmutableDictionary(), parameter.Name, attributeType.Name); + } + + private static Diagnostic GetIncreaseVisibilityDiagnostic(IParameterSymbol parameter, IPropertySymbol property) + { + // If '{0}' is the property accessor for positional argument '{1}', make it public. + return property.GetMethod!.Locations.CreateDiagnostic(IncreaseVisibilityRule, new Dictionary { { "case", MakePublicCase } }.ToImmutableDictionary(), property.Name, parameter.Name); + } + + private static Diagnostic GetRemoveSetterDiagnostic(IParameterSymbol parameter, IPropertySymbol property) + { + // Remove the property setter from '{0}' or reduce its accessibility because it corresponds to positional argument '{1}'. + return property.SetMethod!.Locations.CreateDiagnostic(RemoveSetterRule, new Dictionary { { "case", RemoveSetterCase } }.ToImmutableDictionary(), property.Name, parameter.Name); + } + } +} diff --git a/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsFixer.cs b/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsFixer.cs new file mode 100644 index 0000000000000..fe8a790b7596f --- /dev/null +++ b/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsFixer.cs @@ -0,0 +1,179 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Globalization; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Shared.Extensions; + +namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines +{ + static class Extensions + { + public static IEnumerable DefaultMethodBody( + this SyntaxGenerator generator, Compilation compilation) + { + yield return DefaultMethodStatement(generator, compilation); + } + + public static SyntaxNode DefaultMethodStatement(this SyntaxGenerator generator, Compilation compilation) + { + return generator.ThrowStatement(generator.ObjectCreationExpression( + generator.TypeExpression( + compilation.GetTypeByMetadataName("System.NotImplementedException")))); + } + } + + [ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared] + internal sealed class DefineAccessorsForAttributeArgumentsFixer : CodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(DefineAccessorsForAttributeArgumentsAnalyzer.RuleId); + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + SyntaxGenerator generator = SyntaxGenerator.GetGenerator(context.Document); + SyntaxNode root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + SyntaxNode node = root.FindNode(context.Span); + + foreach (var diagnostic in context.Diagnostics) + { + if (diagnostic.Properties.TryGetValue("case", out var fixCase)) + { + string title; + switch (fixCase) + { + case DefineAccessorsForAttributeArgumentsAnalyzer.AddAccessorCase: + SyntaxNode parameter = generator.GetDeclaration(node, DeclarationKind.Parameter); + if (parameter != null) + { + title = "MicrosoftCodeQualityAnalyzersResources.CreatePropertyAccessorForParameter"; + context.RegisterCodeFix(CodeAction.Create(title, + async ct => await AddAccessorAsync(context.Document, parameter, ct).ConfigureAwait(false), + equivalenceKey: title), + diagnostic); + } + + return; + + case DefineAccessorsForAttributeArgumentsAnalyzer.MakePublicCase: + SyntaxNode property = generator.GetDeclaration(node, DeclarationKind.Property); + if (property != null) + { + title = "MicrosoftCodeQualityAnalyzersResources.MakeGetterPublic"; + context.RegisterCodeFix(CodeAction.Create(title, + async ct => await MakePublicAsync(context.Document, node, property, ct).ConfigureAwait(false), + equivalenceKey: title), + diagnostic); + } + + return; + + case DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterCase: + title = "MicrosoftCodeQualityAnalyzersResources.MakeSetterNonPublic"; + context.RegisterCodeFix(CodeAction.Create(title, + async ct => await RemoveSetterAsync(context.Document, node, ct).ConfigureAwait(false), + equivalenceKey: title), + diagnostic); + return; + + default: + return; + } + } + } + } + + private static async Task AddAccessorAsync(Document document, SyntaxNode parameter, CancellationToken cancellationToken) + { + SymbolEditor symbolEditor = SymbolEditor.Create(document); + SemanticModel model = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + + if (model.GetDeclaredSymbol(parameter, cancellationToken) is not IParameterSymbol parameterSymbol) + { + return document; + } + + // Make the first character uppercase since we are generating a property. + string propName = char.ToUpper(parameterSymbol.Name[0], CultureInfo.InvariantCulture).ToString() + parameterSymbol.Name[1..]; + + INamedTypeSymbol typeSymbol = parameterSymbol.ContainingType; + ISymbol? propertySymbol = typeSymbol.GetMembers(propName).FirstOrDefault(m => m.Kind == SymbolKind.Property); + + // Add a new property + if (propertySymbol == null) + { + await symbolEditor.EditOneDeclarationAsync(typeSymbol, + parameter.GetLocation(), // edit the partial declaration that has this parameter symbol. + (editor, typeDeclaration) => + { + SyntaxNode newProperty = editor.Generator.PropertyDeclaration(propName, + editor.Generator.TypeExpression(parameterSymbol.Type), + Accessibility.Public, + DeclarationModifiers.ReadOnly); + newProperty = editor.Generator.WithGetAccessorStatements(newProperty, null); + editor.AddMember(typeDeclaration, newProperty); + }, + cancellationToken).ConfigureAwait(false); + } + else + { + await symbolEditor.EditOneDeclarationAsync(propertySymbol, + (editor, propertyDeclaration) => + { + editor.SetGetAccessorStatements(propertyDeclaration, editor.Generator.DefaultMethodBody(model.Compilation)); + editor.SetModifiers(propertyDeclaration, editor.Generator.GetModifiers(propertyDeclaration) - DeclarationModifiers.WriteOnly); + }, + cancellationToken).ConfigureAwait(false); + } + + return symbolEditor.GetChangedDocuments().First(); + } + + private static async Task MakePublicAsync(Document document, SyntaxNode getMethod, SyntaxNode property, CancellationToken cancellationToken) + { + // Clear the accessibility on the getter. + DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + editor.SetAccessibility(getMethod, Accessibility.NotApplicable); + + // If the containing property is not public, make it so + Accessibility propertyAccessibility = editor.Generator.GetAccessibility(property); + if (propertyAccessibility != Accessibility.Public) + { + editor.SetAccessibility(property, Accessibility.Public); + + // Having just made the property public, if it has a setter with no Accessibility set, then we've just made the setter public. + // Instead restore the setter's original accessibility so that we don't fire a violation with the generated code. + SyntaxNode setter = editor.Generator.GetAccessor(property, DeclarationKind.SetAccessor); + if (setter != null) + { + Accessibility setterAccessibility = editor.Generator.GetAccessibility(setter); + if (setterAccessibility == Accessibility.NotApplicable) + { + editor.SetAccessibility(setter, propertyAccessibility); + } + } + } + + return editor.GetChangedDocument(); + } + + private static async Task RemoveSetterAsync(Document document, SyntaxNode setMethod, CancellationToken cancellationToken) + { + DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); + editor.SetAccessibility(setMethod, Accessibility.Internal); + return editor.GetChangedDocument(); + } + + public override FixAllProvider GetFixAllProvider() + { + return WellKnownFixAllProviders.BatchFixer; + } + } +} diff --git a/src/Features/CSharpTest/NetAnalyzers/DefineAccessorsForAttributeArgumentsTests.cs b/src/Features/CSharpTest/NetAnalyzers/DefineAccessorsForAttributeArgumentsTests.cs new file mode 100644 index 0000000000000..0788b5606c0c6 --- /dev/null +++ b/src/Features/CSharpTest/NetAnalyzers/DefineAccessorsForAttributeArgumentsTests.cs @@ -0,0 +1,602 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions; +using Xunit; + +namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.UnitTests +{ + using VerifyCS = CSharpCodeFixVerifier< + Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.DefineAccessorsForAttributeArgumentsAnalyzer, + Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.DefineAccessorsForAttributeArgumentsFixer>; + using VerifyVB = VisualBasicCodeFixVerifier< + Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.DefineAccessorsForAttributeArgumentsAnalyzer, + Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.DefineAccessorsForAttributeArgumentsFixer>; + + public partial class DefineAccessorsForAttributeArgumentsTests + { + [Fact] + public async Task CSharp_CA1019_AddAccessorAsync() + { + await VerifyCS.VerifyCodeFixAsync(@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class NoAccessorTestAttribute : Attribute +{ + private string m_name; + + public NoAccessorTestAttribute(string name) + { + m_name = name; + } +}", + VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.DefaultRule).WithSpan(9, 43, 9, 47).WithArguments("name", "NoAccessorTestAttribute"), +@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class NoAccessorTestAttribute : Attribute +{ + private string m_name; + + public NoAccessorTestAttribute(string name) + { + m_name = name; + } + + public string Name { get; } +}"); + } + + [Fact] + public async Task CSharp_CA1019_AddAccessor1Async() + { + await new VerifyCS.Test + { + TestState = + { + Sources = + { + @" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class SetterOnlyTestAttribute : Attribute +{ + private string m_name; + + public SetterOnlyTestAttribute(string name) + { + m_name = name; + } + + public string Name + { + set { m_name = value; } + } +}", + }, + ExpectedDiagnostics = + { + VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.DefaultRule).WithSpan(9, 43, 9, 47).WithArguments("name", "SetterOnlyTestAttribute"), + VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterRule).WithSpan(16, 9, 16, 12).WithArguments("Name", "name"), + }, + }, + FixedState = + { + Sources = + { + @" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class SetterOnlyTestAttribute : Attribute +{ + private string m_name; + + public SetterOnlyTestAttribute(string name) + { + m_name = name; + } + + public string Name + { + internal set { m_name = value; } + + get + { + throw new NotImplementedException(); + } + } +}", + }, + }, + NumberOfFixAllIterations = 2, + }.RunAsync(); + } + + [Fact] + public async Task CSharp_CA1019_MakeGetterPublicAsync() + { + await VerifyCS.VerifyCodeFixAsync(@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class InternalGetterTestAttribute : Attribute +{ + private string m_name; + + public InternalGetterTestAttribute(string name) + { + m_name = name; + } + + internal string Name + { + get { return m_name; } + set { m_name = value; } + } +}", + VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(16, 9, 16, 12).WithArguments("Name", "name"), +@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class InternalGetterTestAttribute : Attribute +{ + private string m_name; + + public InternalGetterTestAttribute(string name) + { + m_name = name; + } + + public string Name + { + get { return m_name; } + + internal set { m_name = value; } + } +}"); + } + + [Fact] + public async Task CSharp_CA1019_MakeGetterPublic2Async() + { + await VerifyCS.VerifyCodeFixAsync(@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class InternalGetterTestAttribute : Attribute +{ + private string m_name; + + public InternalGetterTestAttribute(string name) + { + m_name = name; + } + + internal string Name + { + get { return m_name; } + set { m_name = value; } + } +}", + VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(16, 9, 16, 12).WithArguments("Name", "name"), +@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class InternalGetterTestAttribute : Attribute +{ + private string m_name; + + public InternalGetterTestAttribute(string name) + { + m_name = name; + } + + public string Name + { + get { return m_name; } + + internal set { m_name = value; } + } +}"); + } + + [Fact] + public async Task CSharp_CA1019_MakeGetterPublic3Async() + { + await VerifyCS.VerifyCodeFixAsync(@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class InternalGetterTestAttribute : Attribute +{ + private string m_name; + + public InternalGetterTestAttribute(string name) + { + m_name = name; + } + + internal string Name + { + get { return m_name; } + } +}", + VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(16, 9, 16, 12).WithArguments("Name", "name"), +@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class InternalGetterTestAttribute : Attribute +{ + private string m_name; + + public InternalGetterTestAttribute(string name) + { + m_name = name; + } + + public string Name + { + get { return m_name; } + } +}"); + } + + [Fact] + public async Task CSharp_CA1019_MakeSetterInternalAsync() + { + await VerifyCS.VerifyCodeFixAsync(@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class PublicSetterTestAttribute : Attribute +{ + private string m_name; + + public PublicSetterTestAttribute(string name) + { + m_name = name; + } + + public string Name + { + get { return m_name; } + set { m_name = value; } + } +}", + VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterRule).WithSpan(17, 9, 17, 12).WithArguments("Name", "name"), +@" +using System; + +[AttributeUsage(AttributeTargets.All)] +public sealed class PublicSetterTestAttribute : Attribute +{ + private string m_name; + + public PublicSetterTestAttribute(string name) + { + m_name = name; + } + + public string Name + { + get { return m_name; } + + internal set { m_name = value; } + } +}"); + } + + [Fact] + public async Task VisualBasic_CA1019_AddAccessorAsync() + { + await VerifyVB.VerifyCodeFixAsync(@" +Imports System + + _ +Public NotInheritable Class NoAccessorTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub +End Class", + VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.DefaultRule).WithSpan(9, 20, 9, 24).WithArguments("name", "NoAccessorTestAttribute"), +@" +Imports System + + _ +Public NotInheritable Class NoAccessorTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Public ReadOnly Property Name As String + Get + End Get + End Property +End Class"); + } + + [Fact] + public async Task VisualBasic_CA1019_AddAccessor2Async() + { + await new VerifyVB.Test + { + TestState = + { + Sources = + { + @" +Imports System + + _ +Public NotInheritable Class SetterOnlyTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Friend WriteOnly Property Name() As String + Set + m_name = value + End Set + End Property +End Class", + }, + ExpectedDiagnostics = + { + VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.DefaultRule).WithSpan(9, 20, 9, 24).WithArguments("name", "SetterOnlyTestAttribute"), + }, + }, + FixedState = + { + Sources = + { + @" +Imports System + + _ +Public NotInheritable Class SetterOnlyTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Public Property Name() As String + Friend Set + m_name = value + End Set + Get + Throw New NotImplementedException() + End Get + End Property +End Class", + }, + }, + NumberOfIncrementalIterations = 2, + NumberOfFixAllIterations = 2, + }.RunAsync(); + } + + [Fact] + public async Task VisualBasic_CA1019_MakeGetterPublicAsync() + { + await new VerifyVB.Test + { + TestState = + { + Sources = + { + @" +Imports System + + _ +Public NotInheritable Class InternalGetterTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Public Property Name() As String + Friend Get + Return m_name + End Get + Set + m_name = value + End Set + End Property +End Class", + }, + ExpectedDiagnostics = + { + VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(14, 16, 14, 19).WithArguments("Name", "name"), + VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterRule).WithSpan(17, 9, 17, 12).WithArguments("Name", "name"), + }, + }, + FixedState = + { + Sources = + { + @" +Imports System + + _ +Public NotInheritable Class InternalGetterTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Public Property Name() As String + Get + Return m_name + End Get + Friend Set + m_name = value + End Set + End Property +End Class", + }, + }, + NumberOfFixAllIterations = 2, + }.RunAsync(); + } + + [Fact] + public async Task VisualBasic_CA1019_MakeGetterPublic2Async() + { + await VerifyVB.VerifyCodeFixAsync(@" +Imports System + + _ +Public NotInheritable Class InternalGetterTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Friend Property Name() As String + Get + Return m_name + End Get + Set + m_name = value + End Set + End Property +End Class", + VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(14, 9, 14, 12).WithArguments("Name", "name"), +@" +Imports System + + _ +Public NotInheritable Class InternalGetterTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Public Property Name() As String + Get + Return m_name + End Get + Friend Set + m_name = value + End Set + End Property +End Class"); + } + + [Fact] + public async Task VisualBasic_CA1019_MakeGetterPublic3Async() + { + await VerifyVB.VerifyCodeFixAsync(@" +Imports System + + _ +Public NotInheritable Class InternalGetterTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Friend ReadOnly Property Name() As String + Get + Return m_name + End Get + End Property +End Class", + VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(14, 9, 14, 12).WithArguments("Name", "name"), +@" +Imports System + + _ +Public NotInheritable Class InternalGetterTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Public ReadOnly Property Name() As String + Get + Return m_name + End Get + End Property +End Class"); + } + + [Fact] + public async Task VisualBasic_CA1019_MakeSetterInternalAsync() + { + await VerifyVB.VerifyCodeFixAsync(@" +Imports System + + _ +Public NotInheritable Class PublicSetterTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Public Property Name() As String + Get + Return m_name + End Get + Set + m_name = value + End Set + End Property +End Class", + VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterRule).WithSpan(17, 9, 17, 12).WithArguments("Name", "name"), +@" +Imports System + + _ +Public NotInheritable Class PublicSetterTestAttribute + Inherits Attribute + Private m_name As String + + Public Sub New(name As String) + m_name = name + End Sub + + Public Property Name() As String + Get + Return m_name + End Get + Friend Set + m_name = value + End Set + End Property +End Class"); + } + } +} diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs index cf03d43c8ed4e..7ad89b49d2666 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs @@ -236,9 +236,7 @@ private protected override SyntaxNode MethodDeclaration( return SyntaxFactory.MethodDeclaration( attributeLists: default, - // Pass `withLeadingElasticMarker: true` to ensure method will space itself properly within the members it - // is added to. - modifiers: AsModifierList(accessibility, modifiers, SyntaxKind.MethodDeclaration, withLeadingElasticMarker: true), + modifiers: AsModifierList(accessibility, modifiers, SyntaxKind.MethodDeclaration), returnType: returnType != null ? (TypeSyntax)returnType : SyntaxFactory.PredefinedType(VoidKeyword), explicitInterfaceSpecifier: null, identifier: name.ToIdentifierToken(), @@ -430,12 +428,13 @@ public override SyntaxNode GetAccessorDeclaration(Accessibility accessibility, I private protected override SyntaxNode SetAccessorDeclaration(Accessibility accessibility, bool isInitOnly, IEnumerable? statements) => AccessorDeclaration(isInitOnly ? SyntaxKind.InitAccessorDeclaration : SyntaxKind.SetAccessorDeclaration, accessibility, statements); - private static SyntaxNode AccessorDeclaration( + private static AccessorDeclarationSyntax AccessorDeclaration( SyntaxKind kind, Accessibility accessibility, IEnumerable? statements) { - var accessor = SyntaxFactory.AccessorDeclaration(kind); - accessor = accessor.WithModifiers( - AsModifierList(accessibility, DeclarationModifiers.None, SyntaxKind.PropertyDeclaration)); + var accessor = SyntaxFactory + .AccessorDeclaration(kind) + .WithModifiers( + AsModifierList(accessibility, DeclarationModifiers.None, SyntaxKind.PropertyDeclaration)); accessor = statements == null ? accessor.WithSemicolonToken(SemicolonToken) @@ -1437,7 +1436,10 @@ public override SyntaxNode WithAccessibility(SyntaxNode declaration, Accessibili modifiers = modifiers.WithIsStatic(false); } - var newTokens = Merge(tokens, AsModifierList(accessibility, modifiers)); + // We're updating the modifiers for something. We don't want to add elastic trivia in that case as + // we don't want the act of adding/removing/modifying modifiers to change the formatting of the parent + // construct. + var newTokens = Merge(tokens, AsModifierList(accessibility, modifiers, withLeadingElasticMarker: false)); return SetModifierTokens(d, newTokens); }); } @@ -1647,7 +1649,10 @@ private SyntaxNode WithModifiersInternal(SyntaxNode declaration, DeclarationModi } } - var newTokens = Merge(tokens, AsModifierList(accessibility, modifiers)); + // We're updating the modifiers for something. We don't want to add elastic trivia in that case as + // we don't want the act of adding/removing/modifying modifiers to change the formatting of the parent + // construct. + var newTokens = Merge(tokens, AsModifierList(accessibility, modifiers, withLeadingElasticMarker: false)); return SetModifierTokens(d, newTokens); }); } @@ -1674,13 +1679,13 @@ private static SyntaxTokenList AsModifierList( Accessibility accessibility, DeclarationModifiers modifiers, SyntaxKind kind, - bool withLeadingElasticMarker = false) + bool withLeadingElasticMarker = true) => AsModifierList(accessibility, GetAllowedModifiers(kind) & modifiers, withLeadingElasticMarker); private static SyntaxTokenList AsModifierList( Accessibility accessibility, DeclarationModifiers modifiers, - bool withLeadingElasticMarker = false) + bool withLeadingElasticMarker = true) { using var _ = ArrayBuilder.GetInstance(out var list); From 210d31a1969cb772b7632ff256c7134aa94be0ed Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 24 Jul 2025 12:24:51 +0200 Subject: [PATCH 2/3] Fix behavior --- ...eAccessorsForAttributeArgumentsAnalyzer.cs | 519 --------------- ...fineAccessorsForAttributeArgumentsFixer.cs | 179 ------ ...fineAccessorsForAttributeArgumentsTests.cs | 602 ------------------ 3 files changed, 1300 deletions(-) delete mode 100644 src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsAnalyzer.cs delete mode 100644 src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsFixer.cs delete mode 100644 src/Features/CSharpTest/NetAnalyzers/DefineAccessorsForAttributeArgumentsTests.cs diff --git a/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsAnalyzer.cs b/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsAnalyzer.cs deleted file mode 100644 index a0d951959213f..0000000000000 --- a/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsAnalyzer.cs +++ /dev/null @@ -1,519 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Reflection; -using System.Threading; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.Shared.Extensions; -using Roslyn.Utilities; - -namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines -{ - internal static class C - { - public static bool IsAssignableTo( - [NotNullWhen(returnValue: true)] this ITypeSymbol? fromSymbol, - [NotNullWhen(returnValue: true)] ITypeSymbol? toSymbol, - Compilation compilation) - => fromSymbol != null && toSymbol != null && compilation.ClassifyCommonConversion(fromSymbol, toSymbol).IsImplicit; - - public static bool IsLessSevereThan(this ReportDiagnostic current, ReportDiagnostic other) - { - return current switch - { - ReportDiagnostic.Error => false, - - ReportDiagnostic.Warn => - other switch - { - ReportDiagnostic.Error => true, - _ => false - }, - - ReportDiagnostic.Info => - other switch - { - ReportDiagnostic.Error => true, - ReportDiagnostic.Warn => true, - _ => false - }, - - ReportDiagnostic.Hidden => - other switch - { - ReportDiagnostic.Error => true, - ReportDiagnostic.Warn => true, - ReportDiagnostic.Info => true, - _ => false - }, - - ReportDiagnostic.Suppress => true, - - _ => false - }; - } - } - - internal static class DiagnosticExtensions - { - public static Diagnostic CreateDiagnostic( - this SyntaxNode node, - DiagnosticDescriptor rule, - params object[] args) - => node.CreateDiagnostic(rule, properties: null, args); - - public static Diagnostic CreateDiagnostic( - this SyntaxNode node, - DiagnosticDescriptor rule, - ImmutableDictionary? properties, - params object[] args) - => node.CreateDiagnostic(rule, additionalLocations: ImmutableArray.Empty, properties, args); - - public static Diagnostic CreateDiagnostic( - this SyntaxNode node, - DiagnosticDescriptor rule, - ImmutableArray additionalLocations, - ImmutableDictionary? properties, - params object[] args) - => node - .GetLocation() - .CreateDiagnostic( - rule: rule, - additionalLocations: additionalLocations, - properties: properties, - args: args); - - public static Diagnostic CreateDiagnostic( - this IOperation operation, - DiagnosticDescriptor rule, - params object[] args) - => operation.CreateDiagnostic(rule, properties: null, args); - - public static Diagnostic CreateDiagnostic( - this IOperation operation, - DiagnosticDescriptor rule, - ImmutableDictionary? properties, - params object[] args) - { - return operation.Syntax.CreateDiagnostic(rule, properties, args); - } - - public static Diagnostic CreateDiagnostic( - this IOperation operation, - DiagnosticDescriptor rule, - ImmutableArray additionalLocations, - ImmutableDictionary? properties, - params object[] args) - { - return operation.Syntax.CreateDiagnostic(rule, additionalLocations, properties, args); - } - - public static Diagnostic CreateDiagnostic( - this SyntaxToken token, - DiagnosticDescriptor rule, - params object[] args) - { - return token.GetLocation().CreateDiagnostic(rule, args); - } - - public static Diagnostic CreateDiagnostic( - this ISymbol symbol, - DiagnosticDescriptor rule, - params object[] args) - { - return symbol.Locations.CreateDiagnostic(rule, args); - } - - public static Diagnostic CreateDiagnostic( - this ISymbol symbol, - DiagnosticDescriptor rule, - ImmutableDictionary? properties, - params object[] args) - { - return symbol.Locations.CreateDiagnostic(rule, properties, args); - } - - public static Diagnostic CreateDiagnostic( - this Location location, - DiagnosticDescriptor rule, - params object[] args) - => location - .CreateDiagnostic( - rule: rule, - properties: ImmutableDictionary.Empty, - args: args); - - public static Diagnostic CreateDiagnostic( - this Location location, - DiagnosticDescriptor rule, - ImmutableDictionary? properties, - params object[] args) - => location.CreateDiagnostic(rule, ImmutableArray.Empty, properties, args); - - public static Diagnostic CreateDiagnostic( - this Location location, - DiagnosticDescriptor rule, - ImmutableArray additionalLocations, - ImmutableDictionary? properties, - params object[] args) - { - if (!location.IsInSource) - { - location = Location.None; - } - - return Diagnostic.Create( - descriptor: rule, - location: location, - additionalLocations: additionalLocations, - properties: properties, - messageArgs: args); - } - - public static Diagnostic CreateDiagnostic( - this IEnumerable locations, - DiagnosticDescriptor rule, - params object[] args) - { - return locations.CreateDiagnostic(rule, null, args); - } - - public static Diagnostic CreateDiagnostic( - this IEnumerable locations, - DiagnosticDescriptor rule, - ImmutableDictionary? properties, - params object[] args) - { - IEnumerable inSource = locations.Where(l => l.IsInSource); - if (!inSource.Any()) - { - return Diagnostic.Create(rule, null, args); - } - - return Diagnostic.Create(rule, - location: inSource.First(), - additionalLocations: inSource.Skip(1), - properties: properties, - messageArgs: args); - } - - /// - /// TODO: Revert this reflection based workaround once we move to Microsoft.CodeAnalysis version 3.0 - /// - private static readonly PropertyInfo? s_syntaxTreeDiagnosticOptionsProperty = - typeof(SyntaxTree).GetTypeInfo().GetDeclaredProperty("DiagnosticOptions"); - - private static readonly PropertyInfo? s_compilationOptionsSyntaxTreeOptionsProviderProperty = - typeof(CompilationOptions).GetTypeInfo().GetDeclaredProperty("SyntaxTreeOptionsProvider"); - - public static void ReportNoLocationDiagnostic( - this CompilationAnalysisContext context, - DiagnosticDescriptor rule, - params object[] args) - => context.Compilation.ReportNoLocationDiagnostic(rule, context.ReportDiagnostic, properties: null, args); - - public static void ReportNoLocationDiagnostic( - this SyntaxNodeAnalysisContext context, - DiagnosticDescriptor rule, - params object[] args) - => context.Compilation.ReportNoLocationDiagnostic(rule, context.ReportDiagnostic, properties: null, args); - - public static void ReportNoLocationDiagnostic( - this Compilation compilation, - DiagnosticDescriptor rule, - Action addDiagnostic, - ImmutableDictionary? properties, - params object[] args) - { - var effectiveSeverity = GetEffectiveSeverity(); - if (!effectiveSeverity.HasValue) - { - // Disabled rule - return; - } - - if (effectiveSeverity.Value != rule.DefaultSeverity) - { -#pragma warning disable RS0030 // The symbol 'DiagnosticDescriptor.DiagnosticDescriptor.#ctor' is banned in this project: Use 'DiagnosticDescriptorHelper.Create' instead - rule = new DiagnosticDescriptor(rule.Id, rule.Title, rule.MessageFormat, rule.Category, - effectiveSeverity.Value, rule.IsEnabledByDefault, rule.Description, rule.HelpLinkUri, rule.CustomTags.ToArray()); -#pragma warning restore RS0030 - } - - var diagnostic = Diagnostic.Create(rule, Location.None, properties, args); - addDiagnostic(diagnostic); - return; - - DiagnosticSeverity? GetEffectiveSeverity() - { - // Microsoft.CodeAnalysis version >= 3.7 exposes options through 'CompilationOptions.SyntaxTreeOptionsProvider.TryGetDiagnosticValue' - // Microsoft.CodeAnalysis version 3.3 - 3.7 exposes options through 'SyntaxTree.DiagnosticOptions'. This API is deprecated in 3.7. - - var syntaxTreeOptionsProvider = s_compilationOptionsSyntaxTreeOptionsProviderProperty?.GetValue(compilation.Options); - var syntaxTreeOptionsProviderTryGetDiagnosticValueMethod = syntaxTreeOptionsProvider?.GetType().GetRuntimeMethods().FirstOrDefault(m => m.Name == "TryGetDiagnosticValue"); - if (syntaxTreeOptionsProviderTryGetDiagnosticValueMethod == null && s_syntaxTreeDiagnosticOptionsProperty == null) - { - return rule.DefaultSeverity; - } - - ReportDiagnostic? overriddenSeverity = null; - foreach (var tree in compilation.SyntaxTrees) - { - ReportDiagnostic? configuredValue = null; - - // Prefer 'CompilationOptions.SyntaxTreeOptionsProvider', if available. - if (s_compilationOptionsSyntaxTreeOptionsProviderProperty != null) - { - if (syntaxTreeOptionsProviderTryGetDiagnosticValueMethod != null) - { - // public abstract bool TryGetDiagnosticValue(SyntaxTree tree, string diagnosticId, out ReportDiagnostic severity); - // public abstract bool TryGetDiagnosticValue(SyntaxTree tree, string diagnosticId, CancellationToken cancellationToken, out ReportDiagnostic severity); - object?[] parameters; - if (syntaxTreeOptionsProviderTryGetDiagnosticValueMethod.GetParameters().Length == 3) - { - parameters = new object?[] { tree, rule.Id, null }; - } - else - { - parameters = new object?[] { tree, rule.Id, CancellationToken.None, null }; - } - - if (syntaxTreeOptionsProviderTryGetDiagnosticValueMethod.Invoke(syntaxTreeOptionsProvider, parameters) is true && - parameters.Last() is ReportDiagnostic value) - { - configuredValue = value; - } - } - } - else - { - RoslynDebug.Assert(s_syntaxTreeDiagnosticOptionsProperty != null); - var options = (ImmutableDictionary)s_syntaxTreeDiagnosticOptionsProperty.GetValue(tree)!; - if (options.TryGetValue(rule.Id, out var value)) - { - configuredValue = value; - } - } - - if (configuredValue == null) - { - continue; - } - - if (configuredValue == ReportDiagnostic.Suppress) - { - // Any suppression entry always wins. - return null; - } - - if (overriddenSeverity == null) - { - overriddenSeverity = configuredValue; - } - else if (overriddenSeverity.Value.IsLessSevereThan(configuredValue.Value)) - { - // Choose the most severe value for conflicts. - overriddenSeverity = configuredValue; - } - } - - return overriddenSeverity.HasValue ? overriddenSeverity.Value.ToDiagnosticSeverity() : rule.DefaultSeverity; - } - } - } - - - /// - /// CA1019: - /// - /// Cause: - /// In its constructor, an attribute defines arguments that do not have corresponding properties. - /// - [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)] - internal sealed class DefineAccessorsForAttributeArgumentsAnalyzer : DiagnosticAnalyzer - { - internal const string RuleId = "CA1019"; - internal const string AddAccessorCase = "AddAccessor"; - internal const string MakePublicCase = "MakePublic"; - internal const string RemoveSetterCase = "RemoveSetter"; - - private static readonly LocalizableString s_localizableTitle = "DefineAccessorsForAttributeArgumentsTitle"; - - internal static readonly DiagnosticDescriptor DefaultRule = new DiagnosticDescriptor( - RuleId, - s_localizableTitle, - "DefineAccessorsForAttributeArgumentsTitle", - "DiagnosticCategory.Design", - DiagnosticSeverity.Warning, - isEnabledByDefault: true, - description: null); - - internal static readonly DiagnosticDescriptor IncreaseVisibilityRule = new DiagnosticDescriptor( - RuleId, - s_localizableTitle, - "DefineAccessorsForAttributeArgumentsTitle", - "DiagnosticCategory.Design", - DiagnosticSeverity.Warning, - isEnabledByDefault: true, - description: null); - - internal static readonly DiagnosticDescriptor RemoveSetterRule = new DiagnosticDescriptor( - RuleId, - s_localizableTitle, - "DefineAccessorsForAttributeArgumentsMessageRemoveSetter", - "DiagnosticCategory.Design", - DiagnosticSeverity.Warning, - isEnabledByDefault: true, - description: null); - - public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(DefaultRule, IncreaseVisibilityRule, RemoveSetterRule); - - public override void Initialize(AnalysisContext context) - { - context.EnableConcurrentExecution(); - context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); - - context.RegisterCompilationStartAction(compilationContext => - { - INamedTypeSymbol? attributeType = compilationContext.Compilation.GetTypeByMetadataName("System.Attribute"); - if (attributeType == null) - { - return; - } - - compilationContext.RegisterSymbolAction(context => - { - AnalyzeSymbol((INamedTypeSymbol)context.Symbol, attributeType, context.Compilation, context.ReportDiagnostic); - }, - SymbolKind.NamedType); - }); - } - - private static void AnalyzeSymbol(INamedTypeSymbol symbol, INamedTypeSymbol attributeType, Compilation compilation, Action addDiagnostic) - { - if (symbol != null && symbol.GetBaseTypesAndThis().Contains(attributeType) && symbol.DeclaredAccessibility != Accessibility.Private) - { - IEnumerable parametersToCheck = GetAllPublicConstructorParameters(symbol); - if (parametersToCheck.Any()) - { - IDictionary propertiesMap = GetAllPropertiesInTypeChain(symbol); - AnalyzeParameters(compilation, parametersToCheck, propertiesMap, symbol, addDiagnostic); - } - } - } - - private static IEnumerable GetAllPublicConstructorParameters(INamedTypeSymbol attributeType) - { - // FxCop compatibility: - // Only examine parameters of public constructors. Can't use protected - // constructors to define an attribute so this rule only applies to - // public constructors. - IEnumerable instanceConstructorsToCheck = attributeType.InstanceConstructors.Where(c => c.DeclaredAccessibility == Accessibility.Public); - - if (instanceConstructorsToCheck.Any()) - { - var uniqueParamNames = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (IMethodSymbol constructor in instanceConstructorsToCheck) - { - foreach (IParameterSymbol parameter in constructor.Parameters) - { - if (uniqueParamNames.Add(parameter.Name)) - { - yield return parameter; - } - } - } - } - } - - private static IDictionary GetAllPropertiesInTypeChain(INamedTypeSymbol attributeType) - { - var propertiesMap = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (INamedTypeSymbol currentType in attributeType.GetBaseTypesAndThis()) - { - foreach (IPropertySymbol property in currentType.GetMembers().Where(m => m.Kind == SymbolKind.Property).Cast()) - { - if (!propertiesMap.ContainsKey(property.Name)) - { - propertiesMap.Add(property.Name, property); - } - } - } - - return propertiesMap; - } - - private static void AnalyzeParameters(Compilation compilation, IEnumerable parameters, IDictionary propertiesMap, INamedTypeSymbol attributeType, Action addDiagnostic) - { - foreach (IParameterSymbol parameter in parameters) - { - if (parameter.Type.Kind != SymbolKind.ErrorType) - { - if (!propertiesMap.TryGetValue(parameter.Name, out IPropertySymbol property) || - !parameter.Type.IsAssignableTo(property.Type, compilation)) - { - // Add a public read-only property accessor for positional argument '{0}' of attribute '{1}'. - addDiagnostic(GetDefaultDiagnostic(parameter, attributeType)); - } - else - { - if (property.GetMethod == null) - { - // Add a public read-only property accessor for positional argument '{0}' of attribute '{1}'. - addDiagnostic(GetDefaultDiagnostic(parameter, attributeType)); - } - else if (property.DeclaredAccessibility != Accessibility.Public || - property.GetMethod.DeclaredAccessibility != Accessibility.Public) - { - if (!property.ContainingType.Equals(attributeType)) - { - // A non-public getter exists in one of the base types. - // However, we cannot be sure if the user can modify the base type (it could be from a third party library). - // So generate the default diagnostic instead of increase visibility here. - - // Add a public read-only property accessor for positional argument '{0}' of attribute '{1}'. - addDiagnostic(GetDefaultDiagnostic(parameter, attributeType)); - } - else - { - // If '{0}' is the property accessor for positional argument '{1}', make it public. - addDiagnostic(GetIncreaseVisibilityDiagnostic(parameter, property)); - } - } - - if (property.SetMethod != null && - property.SetMethod.DeclaredAccessibility == Accessibility.Public && - Equals(property.ContainingType, attributeType)) - { - // Remove the property setter from '{0}' or reduce its accessibility because it corresponds to positional argument '{1}'. - addDiagnostic(GetRemoveSetterDiagnostic(parameter, property)); - } - } - } - } - } - - private static Diagnostic GetDefaultDiagnostic(IParameterSymbol parameter, INamedTypeSymbol attributeType) - { - // Add a public read-only property accessor for positional argument '{0}' of attribute '{1}'. - return parameter.Locations.CreateDiagnostic(DefaultRule, new Dictionary { { "case", AddAccessorCase } }.ToImmutableDictionary(), parameter.Name, attributeType.Name); - } - - private static Diagnostic GetIncreaseVisibilityDiagnostic(IParameterSymbol parameter, IPropertySymbol property) - { - // If '{0}' is the property accessor for positional argument '{1}', make it public. - return property.GetMethod!.Locations.CreateDiagnostic(IncreaseVisibilityRule, new Dictionary { { "case", MakePublicCase } }.ToImmutableDictionary(), property.Name, parameter.Name); - } - - private static Diagnostic GetRemoveSetterDiagnostic(IParameterSymbol parameter, IPropertySymbol property) - { - // Remove the property setter from '{0}' or reduce its accessibility because it corresponds to positional argument '{1}'. - return property.SetMethod!.Locations.CreateDiagnostic(RemoveSetterRule, new Dictionary { { "case", RemoveSetterCase } }.ToImmutableDictionary(), property.Name, parameter.Name); - } - } -} diff --git a/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsFixer.cs b/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsFixer.cs deleted file mode 100644 index fe8a790b7596f..0000000000000 --- a/src/Features/CSharp/Portable/NetAnalyzers/DefineAccessorsForAttributeArgumentsFixer.cs +++ /dev/null @@ -1,179 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. - -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Composition; -using System.Globalization; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CodeActions; -using Microsoft.CodeAnalysis.CodeFixes; -using Microsoft.CodeAnalysis.Editing; -using Microsoft.CodeAnalysis.Shared.Extensions; - -namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines -{ - static class Extensions - { - public static IEnumerable DefaultMethodBody( - this SyntaxGenerator generator, Compilation compilation) - { - yield return DefaultMethodStatement(generator, compilation); - } - - public static SyntaxNode DefaultMethodStatement(this SyntaxGenerator generator, Compilation compilation) - { - return generator.ThrowStatement(generator.ObjectCreationExpression( - generator.TypeExpression( - compilation.GetTypeByMetadataName("System.NotImplementedException")))); - } - } - - [ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared] - internal sealed class DefineAccessorsForAttributeArgumentsFixer : CodeFixProvider - { - public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(DefineAccessorsForAttributeArgumentsAnalyzer.RuleId); - - public override async Task RegisterCodeFixesAsync(CodeFixContext context) - { - SyntaxGenerator generator = SyntaxGenerator.GetGenerator(context.Document); - SyntaxNode root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - SyntaxNode node = root.FindNode(context.Span); - - foreach (var diagnostic in context.Diagnostics) - { - if (diagnostic.Properties.TryGetValue("case", out var fixCase)) - { - string title; - switch (fixCase) - { - case DefineAccessorsForAttributeArgumentsAnalyzer.AddAccessorCase: - SyntaxNode parameter = generator.GetDeclaration(node, DeclarationKind.Parameter); - if (parameter != null) - { - title = "MicrosoftCodeQualityAnalyzersResources.CreatePropertyAccessorForParameter"; - context.RegisterCodeFix(CodeAction.Create(title, - async ct => await AddAccessorAsync(context.Document, parameter, ct).ConfigureAwait(false), - equivalenceKey: title), - diagnostic); - } - - return; - - case DefineAccessorsForAttributeArgumentsAnalyzer.MakePublicCase: - SyntaxNode property = generator.GetDeclaration(node, DeclarationKind.Property); - if (property != null) - { - title = "MicrosoftCodeQualityAnalyzersResources.MakeGetterPublic"; - context.RegisterCodeFix(CodeAction.Create(title, - async ct => await MakePublicAsync(context.Document, node, property, ct).ConfigureAwait(false), - equivalenceKey: title), - diagnostic); - } - - return; - - case DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterCase: - title = "MicrosoftCodeQualityAnalyzersResources.MakeSetterNonPublic"; - context.RegisterCodeFix(CodeAction.Create(title, - async ct => await RemoveSetterAsync(context.Document, node, ct).ConfigureAwait(false), - equivalenceKey: title), - diagnostic); - return; - - default: - return; - } - } - } - } - - private static async Task AddAccessorAsync(Document document, SyntaxNode parameter, CancellationToken cancellationToken) - { - SymbolEditor symbolEditor = SymbolEditor.Create(document); - SemanticModel model = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - - if (model.GetDeclaredSymbol(parameter, cancellationToken) is not IParameterSymbol parameterSymbol) - { - return document; - } - - // Make the first character uppercase since we are generating a property. - string propName = char.ToUpper(parameterSymbol.Name[0], CultureInfo.InvariantCulture).ToString() + parameterSymbol.Name[1..]; - - INamedTypeSymbol typeSymbol = parameterSymbol.ContainingType; - ISymbol? propertySymbol = typeSymbol.GetMembers(propName).FirstOrDefault(m => m.Kind == SymbolKind.Property); - - // Add a new property - if (propertySymbol == null) - { - await symbolEditor.EditOneDeclarationAsync(typeSymbol, - parameter.GetLocation(), // edit the partial declaration that has this parameter symbol. - (editor, typeDeclaration) => - { - SyntaxNode newProperty = editor.Generator.PropertyDeclaration(propName, - editor.Generator.TypeExpression(parameterSymbol.Type), - Accessibility.Public, - DeclarationModifiers.ReadOnly); - newProperty = editor.Generator.WithGetAccessorStatements(newProperty, null); - editor.AddMember(typeDeclaration, newProperty); - }, - cancellationToken).ConfigureAwait(false); - } - else - { - await symbolEditor.EditOneDeclarationAsync(propertySymbol, - (editor, propertyDeclaration) => - { - editor.SetGetAccessorStatements(propertyDeclaration, editor.Generator.DefaultMethodBody(model.Compilation)); - editor.SetModifiers(propertyDeclaration, editor.Generator.GetModifiers(propertyDeclaration) - DeclarationModifiers.WriteOnly); - }, - cancellationToken).ConfigureAwait(false); - } - - return symbolEditor.GetChangedDocuments().First(); - } - - private static async Task MakePublicAsync(Document document, SyntaxNode getMethod, SyntaxNode property, CancellationToken cancellationToken) - { - // Clear the accessibility on the getter. - DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - editor.SetAccessibility(getMethod, Accessibility.NotApplicable); - - // If the containing property is not public, make it so - Accessibility propertyAccessibility = editor.Generator.GetAccessibility(property); - if (propertyAccessibility != Accessibility.Public) - { - editor.SetAccessibility(property, Accessibility.Public); - - // Having just made the property public, if it has a setter with no Accessibility set, then we've just made the setter public. - // Instead restore the setter's original accessibility so that we don't fire a violation with the generated code. - SyntaxNode setter = editor.Generator.GetAccessor(property, DeclarationKind.SetAccessor); - if (setter != null) - { - Accessibility setterAccessibility = editor.Generator.GetAccessibility(setter); - if (setterAccessibility == Accessibility.NotApplicable) - { - editor.SetAccessibility(setter, propertyAccessibility); - } - } - } - - return editor.GetChangedDocument(); - } - - private static async Task RemoveSetterAsync(Document document, SyntaxNode setMethod, CancellationToken cancellationToken) - { - DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - editor.SetAccessibility(setMethod, Accessibility.Internal); - return editor.GetChangedDocument(); - } - - public override FixAllProvider GetFixAllProvider() - { - return WellKnownFixAllProviders.BatchFixer; - } - } -} diff --git a/src/Features/CSharpTest/NetAnalyzers/DefineAccessorsForAttributeArgumentsTests.cs b/src/Features/CSharpTest/NetAnalyzers/DefineAccessorsForAttributeArgumentsTests.cs deleted file mode 100644 index 0788b5606c0c6..0000000000000 --- a/src/Features/CSharpTest/NetAnalyzers/DefineAccessorsForAttributeArgumentsTests.cs +++ /dev/null @@ -1,602 +0,0 @@ -// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information. - -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions; -using Xunit; - -namespace Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.UnitTests -{ - using VerifyCS = CSharpCodeFixVerifier< - Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.DefineAccessorsForAttributeArgumentsAnalyzer, - Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.DefineAccessorsForAttributeArgumentsFixer>; - using VerifyVB = VisualBasicCodeFixVerifier< - Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.DefineAccessorsForAttributeArgumentsAnalyzer, - Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.DefineAccessorsForAttributeArgumentsFixer>; - - public partial class DefineAccessorsForAttributeArgumentsTests - { - [Fact] - public async Task CSharp_CA1019_AddAccessorAsync() - { - await VerifyCS.VerifyCodeFixAsync(@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class NoAccessorTestAttribute : Attribute -{ - private string m_name; - - public NoAccessorTestAttribute(string name) - { - m_name = name; - } -}", - VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.DefaultRule).WithSpan(9, 43, 9, 47).WithArguments("name", "NoAccessorTestAttribute"), -@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class NoAccessorTestAttribute : Attribute -{ - private string m_name; - - public NoAccessorTestAttribute(string name) - { - m_name = name; - } - - public string Name { get; } -}"); - } - - [Fact] - public async Task CSharp_CA1019_AddAccessor1Async() - { - await new VerifyCS.Test - { - TestState = - { - Sources = - { - @" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class SetterOnlyTestAttribute : Attribute -{ - private string m_name; - - public SetterOnlyTestAttribute(string name) - { - m_name = name; - } - - public string Name - { - set { m_name = value; } - } -}", - }, - ExpectedDiagnostics = - { - VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.DefaultRule).WithSpan(9, 43, 9, 47).WithArguments("name", "SetterOnlyTestAttribute"), - VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterRule).WithSpan(16, 9, 16, 12).WithArguments("Name", "name"), - }, - }, - FixedState = - { - Sources = - { - @" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class SetterOnlyTestAttribute : Attribute -{ - private string m_name; - - public SetterOnlyTestAttribute(string name) - { - m_name = name; - } - - public string Name - { - internal set { m_name = value; } - - get - { - throw new NotImplementedException(); - } - } -}", - }, - }, - NumberOfFixAllIterations = 2, - }.RunAsync(); - } - - [Fact] - public async Task CSharp_CA1019_MakeGetterPublicAsync() - { - await VerifyCS.VerifyCodeFixAsync(@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class InternalGetterTestAttribute : Attribute -{ - private string m_name; - - public InternalGetterTestAttribute(string name) - { - m_name = name; - } - - internal string Name - { - get { return m_name; } - set { m_name = value; } - } -}", - VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(16, 9, 16, 12).WithArguments("Name", "name"), -@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class InternalGetterTestAttribute : Attribute -{ - private string m_name; - - public InternalGetterTestAttribute(string name) - { - m_name = name; - } - - public string Name - { - get { return m_name; } - - internal set { m_name = value; } - } -}"); - } - - [Fact] - public async Task CSharp_CA1019_MakeGetterPublic2Async() - { - await VerifyCS.VerifyCodeFixAsync(@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class InternalGetterTestAttribute : Attribute -{ - private string m_name; - - public InternalGetterTestAttribute(string name) - { - m_name = name; - } - - internal string Name - { - get { return m_name; } - set { m_name = value; } - } -}", - VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(16, 9, 16, 12).WithArguments("Name", "name"), -@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class InternalGetterTestAttribute : Attribute -{ - private string m_name; - - public InternalGetterTestAttribute(string name) - { - m_name = name; - } - - public string Name - { - get { return m_name; } - - internal set { m_name = value; } - } -}"); - } - - [Fact] - public async Task CSharp_CA1019_MakeGetterPublic3Async() - { - await VerifyCS.VerifyCodeFixAsync(@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class InternalGetterTestAttribute : Attribute -{ - private string m_name; - - public InternalGetterTestAttribute(string name) - { - m_name = name; - } - - internal string Name - { - get { return m_name; } - } -}", - VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(16, 9, 16, 12).WithArguments("Name", "name"), -@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class InternalGetterTestAttribute : Attribute -{ - private string m_name; - - public InternalGetterTestAttribute(string name) - { - m_name = name; - } - - public string Name - { - get { return m_name; } - } -}"); - } - - [Fact] - public async Task CSharp_CA1019_MakeSetterInternalAsync() - { - await VerifyCS.VerifyCodeFixAsync(@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class PublicSetterTestAttribute : Attribute -{ - private string m_name; - - public PublicSetterTestAttribute(string name) - { - m_name = name; - } - - public string Name - { - get { return m_name; } - set { m_name = value; } - } -}", - VerifyCS.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterRule).WithSpan(17, 9, 17, 12).WithArguments("Name", "name"), -@" -using System; - -[AttributeUsage(AttributeTargets.All)] -public sealed class PublicSetterTestAttribute : Attribute -{ - private string m_name; - - public PublicSetterTestAttribute(string name) - { - m_name = name; - } - - public string Name - { - get { return m_name; } - - internal set { m_name = value; } - } -}"); - } - - [Fact] - public async Task VisualBasic_CA1019_AddAccessorAsync() - { - await VerifyVB.VerifyCodeFixAsync(@" -Imports System - - _ -Public NotInheritable Class NoAccessorTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub -End Class", - VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.DefaultRule).WithSpan(9, 20, 9, 24).WithArguments("name", "NoAccessorTestAttribute"), -@" -Imports System - - _ -Public NotInheritable Class NoAccessorTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Public ReadOnly Property Name As String - Get - End Get - End Property -End Class"); - } - - [Fact] - public async Task VisualBasic_CA1019_AddAccessor2Async() - { - await new VerifyVB.Test - { - TestState = - { - Sources = - { - @" -Imports System - - _ -Public NotInheritable Class SetterOnlyTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Friend WriteOnly Property Name() As String - Set - m_name = value - End Set - End Property -End Class", - }, - ExpectedDiagnostics = - { - VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.DefaultRule).WithSpan(9, 20, 9, 24).WithArguments("name", "SetterOnlyTestAttribute"), - }, - }, - FixedState = - { - Sources = - { - @" -Imports System - - _ -Public NotInheritable Class SetterOnlyTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Public Property Name() As String - Friend Set - m_name = value - End Set - Get - Throw New NotImplementedException() - End Get - End Property -End Class", - }, - }, - NumberOfIncrementalIterations = 2, - NumberOfFixAllIterations = 2, - }.RunAsync(); - } - - [Fact] - public async Task VisualBasic_CA1019_MakeGetterPublicAsync() - { - await new VerifyVB.Test - { - TestState = - { - Sources = - { - @" -Imports System - - _ -Public NotInheritable Class InternalGetterTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Public Property Name() As String - Friend Get - Return m_name - End Get - Set - m_name = value - End Set - End Property -End Class", - }, - ExpectedDiagnostics = - { - VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(14, 16, 14, 19).WithArguments("Name", "name"), - VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterRule).WithSpan(17, 9, 17, 12).WithArguments("Name", "name"), - }, - }, - FixedState = - { - Sources = - { - @" -Imports System - - _ -Public NotInheritable Class InternalGetterTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Public Property Name() As String - Get - Return m_name - End Get - Friend Set - m_name = value - End Set - End Property -End Class", - }, - }, - NumberOfFixAllIterations = 2, - }.RunAsync(); - } - - [Fact] - public async Task VisualBasic_CA1019_MakeGetterPublic2Async() - { - await VerifyVB.VerifyCodeFixAsync(@" -Imports System - - _ -Public NotInheritable Class InternalGetterTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Friend Property Name() As String - Get - Return m_name - End Get - Set - m_name = value - End Set - End Property -End Class", - VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(14, 9, 14, 12).WithArguments("Name", "name"), -@" -Imports System - - _ -Public NotInheritable Class InternalGetterTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Public Property Name() As String - Get - Return m_name - End Get - Friend Set - m_name = value - End Set - End Property -End Class"); - } - - [Fact] - public async Task VisualBasic_CA1019_MakeGetterPublic3Async() - { - await VerifyVB.VerifyCodeFixAsync(@" -Imports System - - _ -Public NotInheritable Class InternalGetterTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Friend ReadOnly Property Name() As String - Get - Return m_name - End Get - End Property -End Class", - VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.IncreaseVisibilityRule).WithSpan(14, 9, 14, 12).WithArguments("Name", "name"), -@" -Imports System - - _ -Public NotInheritable Class InternalGetterTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Public ReadOnly Property Name() As String - Get - Return m_name - End Get - End Property -End Class"); - } - - [Fact] - public async Task VisualBasic_CA1019_MakeSetterInternalAsync() - { - await VerifyVB.VerifyCodeFixAsync(@" -Imports System - - _ -Public NotInheritable Class PublicSetterTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Public Property Name() As String - Get - Return m_name - End Get - Set - m_name = value - End Set - End Property -End Class", - VerifyVB.Diagnostic(DefineAccessorsForAttributeArgumentsAnalyzer.RemoveSetterRule).WithSpan(17, 9, 17, 12).WithArguments("Name", "name"), -@" -Imports System - - _ -Public NotInheritable Class PublicSetterTestAttribute - Inherits Attribute - Private m_name As String - - Public Sub New(name As String) - m_name = name - End Sub - - Public Property Name() As String - Get - Return m_name - End Get - Friend Set - m_name = value - End Set - End Property -End Class"); - } - } -} From 6e73c9f1841d892866cc15ac805b6f55867c728d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 24 Jul 2025 12:33:27 +0200 Subject: [PATCH 3/3] Add test --- .../CodeGeneration/SyntaxGeneratorTests.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Workspaces/CSharpTest/CodeGeneration/SyntaxGeneratorTests.cs b/src/Workspaces/CSharpTest/CodeGeneration/SyntaxGeneratorTests.cs index 0e054d2e13945..353a97993edc2 100644 --- a/src/Workspaces/CSharpTest/CodeGeneration/SyntaxGeneratorTests.cs +++ b/src/Workspaces/CSharpTest/CodeGeneration/SyntaxGeneratorTests.cs @@ -2916,6 +2916,30 @@ interface i """); } + [Fact] + public void TestMultipleMembersIntoClass() + { + using var workspace = new AdhocWorkspace(); + var node = Generator.AddMembers(Generator.ClassDeclaration("C"), + [ + Generator.MethodDeclaration("M1", returnType: Generator.TypeExpression(SpecialType.System_Void), accessibility: Accessibility.Public), + Generator.PropertyDeclaration("P1", Generator.TypeExpression(SpecialType.System_Int32), accessibility: Accessibility.Public) + ]); + + AssertEx.EqualOrDiff( + """ + class C + { + public void M1() + { + } + + public int P1 { get; set; } + } + """, + Formatter.Format(node, workspace).ToFullString()); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/65932")] public void TestAddExpressionBodyMembersToInterface() {