DotNetAnalyzers / PropertyChangedAnalyzers

Roslyn analyzers for INotifyPropertyChanged
MIT License
44 stars 4 forks source link

Detect if there is a base class for INPC and if it exists automatically implement it #104

Open RyadaProductions opened 5 years ago

RyadaProductions commented 5 years ago

Create an abstract base class for INPC if it does not exist, or use and implement one if it does exist. This removes clutter and code duplication if you have more than a few ViewModels.

Before:

public class MyViewModel
{
    private string _myBackingField = "";

    public string MyProperty 
    {
        get => _myBackingField;
        set 
        {
            if (_myBackingField == value) return;
            _myBackingField = value;
            OnPropertyChanged();
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) 
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); 
    }
}

after:

public class MyViewModel : BaseViewModel
{
    private string _myBackingField = "";

    public string MyProperty 
    {
        get => _myBackingField;
        set 
        {
            if (_myBackingField == value) return;
            _myBackingField = value;
            OnPropertyChanged();
        }
    }
}

public abstract class BaseViewModel
{
    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) 
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); 
    }
}

Combined with #98 it would look something like this:

public class MyViewModel : BaseViewModel
{
    private string _myBackingField = "";

    public string MyProperty 
    {
        get => _myBackingField;
        set => TrySetAndNotify(ref _myBackingField, value);
    }
}

public abstract class BaseViewModel
{
    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) 
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); 
    }

    protected virtual bool TrySetAndNotify<T>(ref T field, T newValue, [CallerMemberName] string propertyName = "") 
    {
        if (EqualityComparer<T>.Default.Equals(field, newValue)) return false;

        field = newValue;
        OnPropertyChanged(propertyName);
        return true;
    }
}
JohanLarsson commented 5 years ago

We detect if there are references to MVVM frameworks and suggest base classes there.

As it is now we do not walk the current sln looking for candidate base classes.

JohanLarsson commented 5 years ago

Also #98 would enable a code fix for:

Before:

public abstract class BaseViewModel
{
    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}

After:

public abstract class BaseViewModel
{
    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual bool TrySetAndNotify<T>(ref T field, T newValue, [CallerMemberName] string propertyName = "")
    {
        if (EqualityComparer<T>.Default.Equals(field, newValue))
            return false;
        field = newValue;
        OnPropertyChanged(propertyName); 
        return true;
    }

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }
}

And then all subclasses would pick up on the existence of TrySetAndNotify and suggest refactoring to using it.