VS 2019 Private member On_PropertyName_Changed is unused

[virzak]

This isn't a bug, but an annoying warning.

Happens since On_PropertyName_Changed is not being referenced at development time. Or is this a question to the roslyn team? here is a similar issue dotnet/roslyn/issues/31581

[ltrzesniewski]

That looks like a ReSharper warning to me, not a Roslyn one (or maybe they show it exactly the same way?)

Anyway, I don't really see what PropertyChanged.Fody could do to prevent this. If that's a ReSharper warning, just add the [UsedImplicitly] attribute if you have JetBrains.Annotations, or [SuppressMessage("ReSharper", "UnusedMember.Local")] if you don't.

[SimonCropp]

Make the member public

[virzak]

@SimonCropp aren't there situations where it is best to keep On_PropertyName_Changed private?

Looks like this issue is happening in plenty other projects and the warning could be disabled programmatically soon dotnet/roslyn/issues/30172

[SimonCropp]

aren't there situations where it is best to keep On_PropertyName_Changed private

not that i know of. this is not a case where u are exposing an API to an external consumer. So encapsulation doesnt really matter. you are exposing an instance to your view to bind to, if u have code that incorrectly uses a viewmodel in other ways, u have bigger problems than a public method.

Also making it public means u can test it and verify the outcome

[SimonCropp]

@virzak BTW did u read this when u created the issue https://github.com/Fody/PropertyChanged/blob/master/.github/ISSUE_TEMPLATE/bug_report.md#you-should-already-be-a-patron ?

[virzak]

I read it when I reported previous issue #247. I see what you're saying.

[SimonCropp]

@virzak i really dont see any way this can be fixed at the fody level. since u have a few workarounds, shall we close this one?

[virzak]

Definitely

[virzak]

It is possible to suppress this warning programmartically using an analyzer.

Not sure if anyone else likes keeping these non-public.
The code is pretty simple.

It would be nice to get this analyzer along with PropertyChanged. If you're interested I can submit a PR.

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using System;
using System.Collections.Immutable;
using System.Linq;
using System.Text.RegularExpressions;

namespace DiagnosticSuppressorTest
{
    [DiagnosticAnalyzer(LanguageNames.CSharp)]
    public class FodyPropertyChangedSuppressor : DiagnosticSuppressor
    {
        public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions { get; } = new[]
        {
            new SuppressionDescriptor("SUPP2953", "IDE0051", "OnChanged suppression")
        }.ToImmutableArray();

        readonly Regex OnChangedPattern = new Regex("^On(.+)Changed$", RegexOptions.Compiled);

        public override void ReportSuppressions(SuppressionAnalysisContext context)
        {
            foreach (var diagnostic in context.ReportedDiagnostics)
            {
                var methodNode = diagnostic.Location.SourceTree.GetRoot(context.CancellationToken).FindNode(diagnostic.Location.SourceSpan);
                if (methodNode == null || !(methodNode is MethodDeclarationSyntax methodDec))
                    continue;

                // Determine if the method matches On_PropertyName_Changed pattern
                var matches = OnChangedPattern.Matches(methodDec.Identifier.Text);
                if (matches.Count == 0 || matches.Count > 1)
                    continue;

                var expectedPropertyName = matches[0].Groups[1].Value;

                var classNode = methodNode.Parent;

                var classModel = context.GetSemanticModel(classNode.SyntaxTree);
                var classDeclaredSymbol = classModel.GetDeclaredSymbol(classNode, context.CancellationToken);

                if (!(classDeclaredSymbol is INamedTypeSymbol classSymbol))
                    continue;

                // Check if parent class implements INotifyPropertyChanged
                bool isINotifyPropertyChanged = false;
                foreach (var inheditedInterface in classSymbol.Interfaces)
                {
                    if (inheditedInterface.Name.Equals("INotifyPropertyChanged", StringComparison.InvariantCulture))
                    {
                        isINotifyPropertyChanged = true;
                        break;
                    }
                }

                // Check if parent class has AddINotifyPropertyChangedInterfaceAttribute
                bool hasAddINotifyPropertyChangedInterface = classSymbol.GetAttributes()
                    .Any(a => a.AttributeClass.Name.Equals("AddINotifyPropertyChangedInterfaceAttribute", StringComparison.InvariantCulture));

                if (!hasAddINotifyPropertyChangedInterface && !isINotifyPropertyChanged)
                    continue;

                var relatedPropertyFound = false;

                // Iterate through all locations (possible partial classes)
                foreach (var loc in classSymbol.Locations)
                {
                    var locnode = loc.SourceTree.GetRoot(context.CancellationToken).FindNode(loc.SourceSpan);
                    
                    SyntaxList<MemberDeclarationSyntax> members;
                    if (locnode is ClassDeclarationSyntax declaringClass)
                    {
                        members = declaringClass.Members;
                    }
                    else if (locnode is StructDeclarationSyntax declaringStruct)
                    {
                        members = declaringStruct.Members;
                    }
                    else
                        continue;

                    // Iterate through member of (partial) class
                    foreach (var member in members)
                    {
                        // Bail out if not a property
                        if (!(member is PropertyDeclarationSyntax property))
                            continue;

                        // Check to see if property name is what's between On and Changed
                        if (property.Identifier.Text.Equals(expectedPropertyName, StringComparison.InvariantCulture))
                        {
                            relatedPropertyFound = true;
                        }
                    }

                    if (relatedPropertyFound)
                        context.ReportSuppression(Suppression.Create(SupportedSuppressions[0], diagnostic));
                }
            }
        }
    }
}

[virzak]

Now available as a NuGet package