From 9a0113f02abdf03fb3cc4b332648ed4e3ce7a215 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 26 Jan 2023 22:10:04 +0100 Subject: [PATCH 1/5] Add ClassUsingAttributeInsteadOfInheritanceCodeFixer --- ...gAttributeInsteadOfInheritanceCodeFixer.cs | 135 ++++++++++++++++++ ...nceForObservablePropertyFieldCodeFixer.cs} | 4 +- ...ngAttributeInsteadOfInheritanceAnalyzer.cs | 18 ++- .../Diagnostics/DiagnosticDescriptors.cs | 14 +- 4 files changed, 166 insertions(+), 5 deletions(-) create mode 100644 src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs rename src/CommunityToolkit.Mvvm.CodeFixers/{FieldReferenceForObservablePropertyFieldFixer.cs => FieldReferenceForObservablePropertyFieldCodeFixer.cs} (96%) diff --git a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs new file mode 100644 index 000000000..b958b6d85 --- /dev/null +++ b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs @@ -0,0 +1,135 @@ +// 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.Collections.Immutable; +using System.Composition; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using CommunityToolkit.Mvvm.SourceGenerators; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; +using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors; +using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; + +namespace CommunityToolkit.Mvvm.CodeFixers; + +/// +/// A code fixer that automatically updates types using [ObservableObject] or [INotifyPropertyChanged] +/// that have no base type to inherit from ObservableObject instead. +/// +[ExportCodeFixProvider(LanguageNames.CSharp)] +[Shared] +public sealed class ClassUsingAttributeInsteadOfInheritanceCodeFixer : CodeFixProvider +{ + /// + public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create( + InheritFromObservableObjectInsteadOfUsingINotifyPropertyChangedAttributeId, + InheritFromObservableObjectInsteadOfUsingObservableObjectAttributeId); + + /// + public override FixAllProvider? GetFixAllProvider() + { + return WellKnownFixAllProviders.BatchFixer; + } + + /// + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + Diagnostic diagnostic = context.Diagnostics[0]; + TextSpan diagnosticSpan = diagnostic.Location.SourceSpan; + + // Retrieve the property passed by the analyzer + if (diagnostic.Properties[ClassUsingAttributeInsteadOfInheritanceAnalyzer.TypeNameKey] is not string typeName || + diagnostic.Properties[ClassUsingAttributeInsteadOfInheritanceAnalyzer.AttributeTypeNameKey] is not string attributeTypeName) + { + return; + } + + SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + foreach (SyntaxNode syntaxNode in root!.FindNode(diagnosticSpan).DescendantNodesAndSelf()) + { + // Find the first descendant node from the source of the diagnostic that is a class declaration with the target name + if (syntaxNode is ClassDeclarationSyntax { Identifier.Text: string identifierName } classDeclaration && + identifierName == typeName) + { + // Register the code fix to update the class declaration to inherit from ObservableObject instead + context.RegisterCodeFix( + CodeAction.Create( + title: "Inherit from ObservableObject", + createChangedDocument: token => UpdateReference(context.Document, classDeclaration, attributeTypeName, token), + equivalenceKey: "Inherit from ObservableObject"), + diagnostic); + + return; + } + } + } + + /// + /// Applies the code fix to a target class declaration and returns an updated document. + /// + /// The original document being fixed. + /// The to update. + /// The name of the attribute that should be removed. + /// The cancellation token for the operation. + /// An updated document with the applied code fix, and inheriting from ObservableObject. + private static async Task UpdateReference(Document document, ClassDeclarationSyntax classDeclaration, string attributeTypeName, CancellationToken cancellationToken) + { + // Insert ObservableObject always in first position in the base list. The type might have + // some interfaces in the base list, so we just copy them back after ObservableObject. + ClassDeclarationSyntax updatedClassDeclaration = + classDeclaration.WithBaseList(BaseList(SingletonSeparatedList( + (BaseTypeSyntax)SimpleBaseType(IdentifierName("ObservableObject")))) + .AddTypes(classDeclaration.BaseList?.Types.ToArray() ?? Array.Empty())); + + AttributeListSyntax? targetAttributeList = null; + AttributeSyntax? targetAttribute = null; + + // Find the attribute list and attribute to remove + foreach (AttributeListSyntax attributeList in updatedClassDeclaration.AttributeLists) + { + foreach (AttributeSyntax attribute in attributeList.Attributes) + { + if (attribute.Name is IdentifierNameSyntax { Identifier.Text: string identifierName } && + identifierName == attributeTypeName) + { + // We found the attribute to remove and the list to update + targetAttributeList = attributeList; + targetAttribute = attribute; + + break; + } + } + } + + // If we found an attribute to remove, do that + if (targetAttribute is not null) + { + // If the target list has more than one attribute, keep it and just remove the target one + if (targetAttributeList!.Attributes.Count > 1) + { + updatedClassDeclaration = + updatedClassDeclaration.ReplaceNode( + targetAttributeList, + targetAttributeList.RemoveNode(targetAttribute, SyntaxRemoveOptions.KeepNoTrivia)!); + } + else + { + // Otherwise, remove the entire attribute list + updatedClassDeclaration = updatedClassDeclaration.RemoveNode(targetAttributeList, SyntaxRemoveOptions.KeepExteriorTrivia)!; + } + } + + SyntaxNode originalRoot = await classDeclaration.SyntaxTree.GetRootAsync(cancellationToken).ConfigureAwait(false); + SyntaxTree updatedTree = originalRoot.ReplaceNode(classDeclaration, updatedClassDeclaration).SyntaxTree; + + return document.WithSyntaxRoot(await updatedTree.GetRootAsync(cancellationToken).ConfigureAwait(false)); + } +} diff --git a/src/CommunityToolkit.Mvvm.CodeFixers/FieldReferenceForObservablePropertyFieldFixer.cs b/src/CommunityToolkit.Mvvm.CodeFixers/FieldReferenceForObservablePropertyFieldCodeFixer.cs similarity index 96% rename from src/CommunityToolkit.Mvvm.CodeFixers/FieldReferenceForObservablePropertyFieldFixer.cs rename to src/CommunityToolkit.Mvvm.CodeFixers/FieldReferenceForObservablePropertyFieldCodeFixer.cs index 5a3a186db..3788192b3 100644 --- a/src/CommunityToolkit.Mvvm.CodeFixers/FieldReferenceForObservablePropertyFieldFixer.cs +++ b/src/CommunityToolkit.Mvvm.CodeFixers/FieldReferenceForObservablePropertyFieldCodeFixer.cs @@ -7,13 +7,13 @@ using System.Threading; using System.Threading.Tasks; using CommunityToolkit.Mvvm.SourceGenerators; -using CommunityToolkit.Mvvm.SourceGenerators.Diagnostics; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; +using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors; namespace CommunityToolkit.Mvvm.CodeFixers; @@ -25,7 +25,7 @@ namespace CommunityToolkit.Mvvm.CodeFixers; public sealed class FieldReferenceForObservablePropertyFieldCodeFixer : CodeFixProvider { /// - public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(DiagnosticDescriptors.FieldReferenceForObservablePropertyFieldId); + public override ImmutableArray FixableDiagnosticIds { get; } = ImmutableArray.Create(FieldReferenceForObservablePropertyFieldId); /// public override FixAllProvider? GetFixAllProvider() diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/Analyzers/ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/Analyzers/ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs index 6706d9cca..1dbacc7b9 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/Analyzers/ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/Analyzers/ClassUsingAttributeInsteadOfInheritanceAnalyzer.cs @@ -17,6 +17,16 @@ namespace CommunityToolkit.Mvvm.SourceGenerators; [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class ClassUsingAttributeInsteadOfInheritanceAnalyzer : DiagnosticAnalyzer { + /// + /// The key for the name of the target type to update. + /// + internal const string TypeNameKey = "TypeName"; + + /// + /// The key for the name of the attribute that was found and should be removed. + /// + internal const string AttributeTypeNameKey = "AttributeTypeName"; + /// /// The mapping of target attributes that will trigger the analyzer. /// @@ -67,7 +77,13 @@ public override void Initialize(AnalysisContext context) if (classSymbol.BaseType is { SpecialType: SpecialType.System_Object }) { // This type is using the attribute when it could just inherit from ObservableObject, which is preferred - context.ReportDiagnostic(Diagnostic.Create(GeneratorAttributeNamesToDiagnosticsMap[attributeClass.Name], context.Symbol.Locations.FirstOrDefault(), context.Symbol)); + context.ReportDiagnostic(Diagnostic.Create( + GeneratorAttributeNamesToDiagnosticsMap[attributeClass.Name], + context.Symbol.Locations.FirstOrDefault(), + ImmutableDictionary.Create() + .Add(TypeNameKey, classSymbol.Name) + .Add(AttributeTypeNameKey, attributeName), + context.Symbol)); } } } diff --git a/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs b/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs index 05efb3e48..993f959c1 100644 --- a/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs +++ b/src/CommunityToolkit.Mvvm.SourceGenerators/Diagnostics/DiagnosticDescriptors.cs @@ -14,6 +14,16 @@ namespace CommunityToolkit.Mvvm.SourceGenerators.Diagnostics; /// internal static class DiagnosticDescriptors { + /// + /// The diagnostic id for . + /// + public const string InheritFromObservableObjectInsteadOfUsingINotifyPropertyChangedAttributeId = "MVVMTK0032"; + + /// + /// The diagnostic id for . + /// + public const string InheritFromObservableObjectInsteadOfUsingObservableObjectAttributeId = "MVVMTK0033"; + /// /// The diagnostic id for . /// @@ -519,7 +529,7 @@ internal static class DiagnosticDescriptors /// /// public static readonly DiagnosticDescriptor InheritFromObservableObjectInsteadOfUsingINotifyPropertyChangedAttributeWarning = new DiagnosticDescriptor( - id: "MVVMTK0032", + id: InheritFromObservableObjectInsteadOfUsingINotifyPropertyChangedAttributeId, title: "Inherit from ObservableObject instead of using [INotifyPropertyChanged]", messageFormat: "The type {0} is using the [INotifyPropertyChanged] attribute while having no base type, and it should instead inherit from ObservableObject", category: typeof(INotifyPropertyChangedGenerator).FullName, @@ -537,7 +547,7 @@ internal static class DiagnosticDescriptors /// /// public static readonly DiagnosticDescriptor InheritFromObservableObjectInsteadOfUsingObservableObjectAttributeWarning = new DiagnosticDescriptor( - id: "MVVMTK0033", + id: InheritFromObservableObjectInsteadOfUsingObservableObjectAttributeId, title: "Inherit from ObservableObject instead of using [ObservableObject]", messageFormat: "The type {0} is using the [ObservableObject] attribute while having no base type, and it should instead inherit from ObservableObject", category: typeof(ObservableObjectGenerator).FullName, From 3b2226bb2977b0781133ef07149b61b044811711 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Thu, 26 Jan 2023 22:55:02 +0100 Subject: [PATCH 2/5] Add draft of ClassUsingAttributeInsteadOfInheritanceCodeFixer tests --- ...gAttributeInsteadOfInheritanceCodeFixer.cs | 2 +- ...gAttributeInsteadOfInheritanceCodeFixer.cs | 327 ++++++++++++++++++ ...nceForObservablePropertyFieldCodeFixer.cs} | 0 3 files changed, 328 insertions(+), 1 deletion(-) create mode 100644 tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs rename tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/{Test_FieldReferenceForObservablePropertyFieldFixer.cs => Test_FieldReferenceForObservablePropertyFieldCodeFixer.cs} (100%) diff --git a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs index b958b6d85..dd33f35e9 100644 --- a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs +++ b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs @@ -98,7 +98,7 @@ private static async Task UpdateReference(Document document, ClassDecl foreach (AttributeSyntax attribute in attributeList.Attributes) { if (attribute.Name is IdentifierNameSyntax { Identifier.Text: string identifierName } && - identifierName == attributeTypeName) + (identifierName == attributeTypeName || (identifierName + "Attribute") == attributeTypeName)) { // We found the attribute to remove and the list to update targetAttributeList = attributeList; diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs new file mode 100644 index 000000000..17a4a2c09 --- /dev/null +++ b/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs @@ -0,0 +1,327 @@ +// 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.Tasks; +using CommunityToolkit.Mvvm.ComponentModel; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using CSharpCodeFixTest = Microsoft.CodeAnalysis.CSharp.Testing.CSharpCodeFixTest< + CommunityToolkit.Mvvm.SourceGenerators.ClassUsingAttributeInsteadOfInheritanceAnalyzer, + CommunityToolkit.Mvvm.CodeFixers.ClassUsingAttributeInsteadOfInheritanceCodeFixer, + Microsoft.CodeAnalysis.Testing.Verifiers.MSTestVerifier>; +using CSharpCodeFixVerifier = Microsoft.CodeAnalysis.CSharp.Testing.CSharpCodeFixVerifier< + CommunityToolkit.Mvvm.SourceGenerators.ClassUsingAttributeInsteadOfInheritanceAnalyzer, + CommunityToolkit.Mvvm.CodeFixers.ClassUsingAttributeInsteadOfInheritanceCodeFixer, + Microsoft.CodeAnalysis.Testing.Verifiers.MSTestVerifier>; + +namespace CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests; + +[TestClass] +public class ClassUsingAttributeInsteadOfInheritanceCodeFixer +{ + [TestMethod] + [DataRow("INotifyPropertyChanged", "MVVMTK0032")] + [DataRow("ObservableObject", "MVVMTK0033")] + public async Task SingleAttributeList(string attributeTypeName, string diagnosticId) + { + string original = $$""" + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [{{attributeTypeName}}] + class C + { + } + """; + + string @fixed = """ + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + class C : ObservableObject + { + } + """; + + CSharpCodeFixTest test = new() + { + TestCode = original, + FixedCode = @fixed, + ReferenceAssemblies = ReferenceAssemblies.Net.Net60 + }; + + test.TestState.AdditionalReferences.Add(typeof(ObservableObject).Assembly); + test.ExpectedDiagnostics.AddRange(new[] + { + // /0/Test0.cs(5,15): warning : The type C is using the attribute while having no base type, and it should instead inherit from ObservableObject + CSharpCodeFixVerifier.Diagnostic(diagnosticId).WithSpan(5, 7, 5, 8).WithArguments("C") + }); + + await test.RunAsync(); + } + + [TestMethod] + [DataRow("INotifyPropertyChanged", "MVVMTK0032")] + [DataRow("ObservableObject", "MVVMTK0033")] + public async Task SingleAttributeList_WithOtherInterface(string attributeTypeName, string diagnosticId) + { + string original = $$""" + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [{{attributeTypeName}}] + class C : IDisposable + { + public void Dispose() + { + } + } + """; + + string @fixed = """ + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + class C : ObservableObject, IDisposable + { + public void Dispose() + { + } + } + """; + + CSharpCodeFixTest test = new() + { + TestCode = original, + FixedCode = @fixed, + ReferenceAssemblies = ReferenceAssemblies.Net.Net60 + }; + + test.TestState.AdditionalReferences.Add(typeof(ObservableObject).Assembly); + test.ExpectedDiagnostics.AddRange(new[] + { + // /0/Test0.cs(5,15): warning : The type C is using the attribute while having no base type, and it should instead inherit from ObservableObject + CSharpCodeFixVerifier.Diagnostic(diagnosticId).WithSpan(6, 7, 6, 8).WithArguments("C") + }); + + await test.RunAsync(); + } + + [TestMethod] + [DataRow("INotifyPropertyChanged", "MVVMTK0032")] + [DataRow("ObservableObject", "MVVMTK0033")] + public async Task MultipleAttributeLists_OneBeforeTarget(string attributeTypeName, string diagnosticId) + { + string original = $$""" + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [Test] + [{{attributeTypeName}}] + class C + { + } + + class TestAttribute : Attribute + { + } + """; + + string @fixed = """ + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [Test] + class C : ObservableObject + { + } + + class TestAttribute : Attribute + { + } + """; + + CSharpCodeFixTest test = new() + { + TestCode = original, + FixedCode = @fixed, + ReferenceAssemblies = ReferenceAssemblies.Net.Net60 + }; + + test.TestState.AdditionalReferences.Add(typeof(ObservableObject).Assembly); + test.ExpectedDiagnostics.AddRange(new[] + { + // /0/Test0.cs(5,15): warning : The type C is using the attribute while having no base type, and it should instead inherit from ObservableObject + CSharpCodeFixVerifier.Diagnostic(diagnosticId).WithSpan(7, 7, 7, 8).WithArguments("C") + }); + + await test.RunAsync(); + } + + [TestMethod] + [DataRow("INotifyPropertyChanged", "MVVMTK0032")] + [DataRow("ObservableObject", "MVVMTK0033")] + public async Task MultipleAttributeLists_OneAfterTarget(string attributeTypeName, string diagnosticId) + { + string original = $$""" + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [{{attributeTypeName}}] + [Test] + class C + { + } + + class TestAttribute : Attribute + { + } + """; + + string @fixed = """ + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [Test] + class C : ObservableObject + { + } + + class TestAttribute : Attribute + { + } + """; + + CSharpCodeFixTest test = new() + { + TestCode = original, + FixedCode = @fixed, + ReferenceAssemblies = ReferenceAssemblies.Net.Net60 + }; + + test.TestState.AdditionalReferences.Add(typeof(ObservableObject).Assembly); + test.ExpectedDiagnostics.AddRange(new[] + { + // /0/Test0.cs(5,15): warning : The type C is using the attribute while having no base type, and it should instead inherit from ObservableObject + CSharpCodeFixVerifier.Diagnostic(diagnosticId).WithSpan(7, 7, 7, 8).WithArguments("C") + }); + + await test.RunAsync(); + } + + [TestMethod] + [DataRow("INotifyPropertyChanged", "MVVMTK0032")] + [DataRow("ObservableObject", "MVVMTK0033")] + public async Task MultipleAttributeLists_OneBeforeAndOneAfterTarget(string attributeTypeName, string diagnosticId) + { + string original = $$""" + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [Test] + [{{attributeTypeName}}] + [Test] + class C + { + } + + [AttributeUsage(AttributeTargets.Class, AllowMultiple = true)] + class TestAttribute : Attribute + { + } + """; + + string @fixed = """ + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [Test] + [Test] + class C : ObservableObject + { + } + + [AttributeUsage(AttributeTargets.Class, AllowMultiple = true)] + class TestAttribute : Attribute + { + } + """; + + CSharpCodeFixTest test = new() + { + TestCode = original, + FixedCode = @fixed, + ReferenceAssemblies = ReferenceAssemblies.Net.Net60 + }; + + test.TestState.AdditionalReferences.Add(typeof(ObservableObject).Assembly); + test.ExpectedDiagnostics.AddRange(new[] + { + // /0/Test0.cs(5,15): warning : The type C is using the attribute while having no base type, and it should instead inherit from ObservableObject + CSharpCodeFixVerifier.Diagnostic(diagnosticId).WithSpan(8, 7, 8, 8).WithArguments("C") + }); + + await test.RunAsync(); + } + + [TestMethod] + [DataRow("INotifyPropertyChanged", "MVVMTK0032")] + [DataRow("ObservableObject", "MVVMTK0033")] + public async Task MultipleAttributesInAttributeList(string attributeTypeName, string diagnosticId) + { + string original = $$""" + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [Test, {{attributeTypeName}}] + class C + { + } + + class TestAttribute : Attribute + { + } + """; + + string @fixed = """ + using System; + using CommunityToolkit.Mvvm.ComponentModel; + + // This is some trivia + [Test] + class C : ObservableObject + { + } + + class TestAttribute : Attribute + { + } + """; + + CSharpCodeFixTest test = new() + { + TestCode = original, + FixedCode = @fixed, + ReferenceAssemblies = ReferenceAssemblies.Net.Net60 + }; + + test.TestState.AdditionalReferences.Add(typeof(ObservableObject).Assembly); + test.ExpectedDiagnostics.AddRange(new[] + { + // /0/Test0.cs(5,15): warning : The type C is using the attribute while having no base type, and it should instead inherit from ObservableObject + CSharpCodeFixVerifier.Diagnostic(diagnosticId).WithSpan(6, 7, 6, 8).WithArguments("C") + }); + + await test.RunAsync(); + } +} diff --git a/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_FieldReferenceForObservablePropertyFieldFixer.cs b/tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_FieldReferenceForObservablePropertyFieldCodeFixer.cs similarity index 100% rename from tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_FieldReferenceForObservablePropertyFieldFixer.cs rename to tests/CommunityToolkit.Mvvm.SourceGenerators.Roslyn401.UnitTests/Test_FieldReferenceForObservablePropertyFieldCodeFixer.cs From 6cdf53bc9216a5df46aa8b86b5e567c5deb4509f Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Sat, 28 Jan 2023 11:35:19 +0200 Subject: [PATCH 3/5] Use SyntaxGenerator and leave trivia handling up to it --- ...gAttributeInsteadOfInheritanceCodeFixer.cs | 41 ++++--------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs index dd33f35e9..a78376a4f 100644 --- a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs +++ b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Text; using static CommunityToolkit.Mvvm.SourceGenerators.Diagnostics.DiagnosticDescriptors; using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; @@ -63,7 +64,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterCodeFix( CodeAction.Create( title: "Inherit from ObservableObject", - createChangedDocument: token => UpdateReference(context.Document, classDeclaration, attributeTypeName, token), + createChangedDocument: token => UpdateReference(context.Document, root, classDeclaration, attributeTypeName, token), equivalenceKey: "Inherit from ObservableObject"), diagnostic); @@ -76,21 +77,17 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) /// Applies the code fix to a target class declaration and returns an updated document. /// /// The original document being fixed. + /// The original tree root belonging to the current document. /// The to update. /// The name of the attribute that should be removed. /// The cancellation token for the operation. /// An updated document with the applied code fix, and inheriting from ObservableObject. - private static async Task UpdateReference(Document document, ClassDeclarationSyntax classDeclaration, string attributeTypeName, CancellationToken cancellationToken) + private static Task UpdateReference(Document document, SyntaxNode root, ClassDeclarationSyntax classDeclaration, string attributeTypeName, CancellationToken cancellationToken) { // Insert ObservableObject always in first position in the base list. The type might have // some interfaces in the base list, so we just copy them back after ObservableObject. - ClassDeclarationSyntax updatedClassDeclaration = - classDeclaration.WithBaseList(BaseList(SingletonSeparatedList( - (BaseTypeSyntax)SimpleBaseType(IdentifierName("ObservableObject")))) - .AddTypes(classDeclaration.BaseList?.Types.ToArray() ?? Array.Empty())); - - AttributeListSyntax? targetAttributeList = null; - AttributeSyntax? targetAttribute = null; + SyntaxGenerator generator = SyntaxGenerator.GetGenerator(document); + ClassDeclarationSyntax updatedClassDeclaration = (ClassDeclarationSyntax)generator.AddBaseType(classDeclaration, IdentifierName("ObservableObject")); // Find the attribute list and attribute to remove foreach (AttributeListSyntax attributeList in updatedClassDeclaration.AttributeLists) @@ -101,35 +98,13 @@ private static async Task UpdateReference(Document document, ClassDecl (identifierName == attributeTypeName || (identifierName + "Attribute") == attributeTypeName)) { // We found the attribute to remove and the list to update - targetAttributeList = attributeList; - targetAttribute = attribute; + updatedClassDeclaration = (ClassDeclarationSyntax)generator.RemoveNode(updatedClassDeclaration, attribute); break; } } } - // If we found an attribute to remove, do that - if (targetAttribute is not null) - { - // If the target list has more than one attribute, keep it and just remove the target one - if (targetAttributeList!.Attributes.Count > 1) - { - updatedClassDeclaration = - updatedClassDeclaration.ReplaceNode( - targetAttributeList, - targetAttributeList.RemoveNode(targetAttribute, SyntaxRemoveOptions.KeepNoTrivia)!); - } - else - { - // Otherwise, remove the entire attribute list - updatedClassDeclaration = updatedClassDeclaration.RemoveNode(targetAttributeList, SyntaxRemoveOptions.KeepExteriorTrivia)!; - } - } - - SyntaxNode originalRoot = await classDeclaration.SyntaxTree.GetRootAsync(cancellationToken).ConfigureAwait(false); - SyntaxTree updatedTree = originalRoot.ReplaceNode(classDeclaration, updatedClassDeclaration).SyntaxTree; - - return document.WithSyntaxRoot(await updatedTree.GetRootAsync(cancellationToken).ConfigureAwait(false)); + return Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(classDeclaration, updatedClassDeclaration))); } } From e1c9bcf6a8be3d9ac5e6c5b5453f46246cbb8ad5 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Sat, 28 Jan 2023 11:38:56 +0200 Subject: [PATCH 4/5] Remove unused parameter --- .../ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs index a78376a4f..b8ea67be5 100644 --- a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs +++ b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs @@ -64,7 +64,7 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) context.RegisterCodeFix( CodeAction.Create( title: "Inherit from ObservableObject", - createChangedDocument: token => UpdateReference(context.Document, root, classDeclaration, attributeTypeName, token), + createChangedDocument: token => UpdateReference(context.Document, root, classDeclaration, attributeTypeName), equivalenceKey: "Inherit from ObservableObject"), diagnostic); @@ -80,9 +80,8 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) /// The original tree root belonging to the current document. /// The to update. /// The name of the attribute that should be removed. - /// The cancellation token for the operation. /// An updated document with the applied code fix, and inheriting from ObservableObject. - private static Task UpdateReference(Document document, SyntaxNode root, ClassDeclarationSyntax classDeclaration, string attributeTypeName, CancellationToken cancellationToken) + private static Task UpdateReference(Document document, SyntaxNode root, ClassDeclarationSyntax classDeclaration, string attributeTypeName) { // Insert ObservableObject always in first position in the base list. The type might have // some interfaces in the base list, so we just copy them back after ObservableObject. From a421802ef247fadbcd6d34da74ffd344cef76b52 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Sat, 28 Jan 2023 13:01:11 +0100 Subject: [PATCH 5/5] Remove DescendantNodesAndSelf() from code fixer --- ...gAttributeInsteadOfInheritanceCodeFixer.cs | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs index b8ea67be5..9b19fc954 100644 --- a/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs +++ b/src/CommunityToolkit.Mvvm.CodeFixers/ClassUsingAttributeInsteadOfInheritanceCodeFixer.cs @@ -2,11 +2,8 @@ // 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.Collections.Immutable; using System.Composition; -using System.Linq; -using System.Threading; using System.Threading.Tasks; using CommunityToolkit.Mvvm.SourceGenerators; using Microsoft.CodeAnalysis; @@ -54,22 +51,19 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) SyntaxNode? root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); - foreach (SyntaxNode syntaxNode in root!.FindNode(diagnosticSpan).DescendantNodesAndSelf()) + // Get the class declaration from the target diagnostic + if (root!.FindNode(diagnosticSpan) is ClassDeclarationSyntax { Identifier.Text: string identifierName } classDeclaration && + identifierName == typeName) { - // Find the first descendant node from the source of the diagnostic that is a class declaration with the target name - if (syntaxNode is ClassDeclarationSyntax { Identifier.Text: string identifierName } classDeclaration && - identifierName == typeName) - { - // Register the code fix to update the class declaration to inherit from ObservableObject instead - context.RegisterCodeFix( - CodeAction.Create( - title: "Inherit from ObservableObject", - createChangedDocument: token => UpdateReference(context.Document, root, classDeclaration, attributeTypeName), - equivalenceKey: "Inherit from ObservableObject"), - diagnostic); + // Register the code fix to update the class declaration to inherit from ObservableObject instead + context.RegisterCodeFix( + CodeAction.Create( + title: "Inherit from ObservableObject", + createChangedDocument: token => UpdateReference(context.Document, root, classDeclaration, attributeTypeName), + equivalenceKey: "Inherit from ObservableObject"), + diagnostic); - return; - } + return; } }