Skip to content

Commit 173996a

Browse files
jnm2JohanLarsson
authored andcommitted
Fix false positives where the same syntax has a different meaning
1 parent 0b60dd9 commit 173996a

5 files changed

Lines changed: 117 additions & 7 deletions

File tree

PropertyChangedAnalyzers.Test/INPC002MutablePublicPropertyShouldNotify/Valid.cs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,5 +1069,71 @@ protected bool Set<T>(ref T location, T value, [CallerMemberName] string propert
10691069

10701070
RoslynAssert.Valid(Analyzer, code);
10711071
}
1072+
1073+
[Test]
1074+
public static void SameExpressionWithDifferentMeaning()
1075+
{
1076+
var code = @"
1077+
namespace ValidCode
1078+
{
1079+
using System;
1080+
using System.ComponentModel;
1081+
using System.Runtime.CompilerServices;
1082+
1083+
record DifferentClass
1084+
{
1085+
public int p;
1086+
}
1087+
1088+
public class SomeViewModel : INotifyPropertyChanged
1089+
{
1090+
private int p;
1091+
public int P
1092+
{
1093+
get => p;
1094+
set
1095+
{
1096+
if (!Set(ref p, value)) return;
1097+
1098+
_ = new DifferentClass { p = value };
1099+
_ = new DifferentClass() with { p = value };
1100+
1101+
{
1102+
int p;
1103+
p = value;
1104+
}
1105+
1106+
_ = new Action<int>(p =>
1107+
{
1108+
p = value;
1109+
});
1110+
1111+
LocalFunction(42);
1112+
void LocalFunction(int p)
1113+
{
1114+
p = value;
1115+
}
1116+
}
1117+
}
1118+
1119+
public event PropertyChangedEventHandler? PropertyChanged;
1120+
1121+
protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
1122+
{
1123+
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
1124+
}
1125+
1126+
protected bool Set<T>(ref T location, T value, [CallerMemberName] string propertyName = null)
1127+
{
1128+
if (RuntimeHelpers.Equals(location, value)) return false;
1129+
location = value;
1130+
OnPropertyChanged(propertyName);
1131+
return true;
1132+
}
1133+
}
1134+
}";
1135+
1136+
RoslynAssert.Valid(Analyzer, code);
1137+
}
10721138
}
10731139
}

PropertyChangedAnalyzers/CodeFixes/MakePropertyNotifyFix.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ async Task TrySet(DocumentEditor editor, CancellationToken cancellationToken)
6969
_ = editor.FormatNode(propertyDeclaration!);
7070
}
7171
}
72-
else if (FindSimpleAssignment(propertyDeclaration) is { } assignment)
72+
else if (FindSimpleAssignment(propertyDeclaration, semanticModel, context.CancellationToken) is { } assignment)
7373
{
7474
context.RegisterCodeFix(
7575
trySetMethod.DisplaySignature(),
@@ -127,7 +127,7 @@ async Task NotifyWhenValueChangesAsync(DocumentEditor editor, CancellationToken
127127
_ = editor.FormatNode(propertyDeclaration!);
128128
}
129129
}
130-
else if (FindSimpleAssignment(propertyDeclaration) is { } assignment)
130+
else if (FindSimpleAssignment(propertyDeclaration, semanticModel, context.CancellationToken) is { } assignment)
131131
{
132132
context.RegisterCodeFix(
133133
NotifyWhenValueChanges,
@@ -198,11 +198,14 @@ async Task NotifyAsync(DocumentEditor editor, CancellationToken cancellationToke
198198
}
199199
}
200200

201-
private static AssignmentExpressionSyntax? FindSimpleAssignment(PropertyDeclarationSyntax property)
201+
private static AssignmentExpressionSyntax? FindSimpleAssignment(
202+
PropertyDeclarationSyntax property,
203+
SemanticModel semanticModel,
204+
CancellationToken cancellationToken)
202205
{
203206
return property.TryGetSetter(out var setter) &&
204207
IsSimple(setter)
205-
? Setter.AssignsValueToBackingField(setter)
208+
? Setter.AssignsValueToBackingField(setter, semanticModel, cancellationToken)
206209
: null;
207210

208211
static bool IsSimple(AccessorDeclarationSyntax localSetter)

PropertyChangedAnalyzers/Helpers/Property.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ property.ContainingType is null ||
108108

109109
if (declaration.TryGetSetter(out var setter))
110110
{
111-
if (Setter.AssignsValueToBackingField(setter) is { } assignment)
111+
if (Setter.AssignsValueToBackingField(setter, semanticModel, cancellationToken) is { } assignment)
112112
{
113113
return PropertyChanged.InvokesPropertyChangedFor(assignment, property, semanticModel, cancellationToken) == AnalysisResult.No;
114114
}

PropertyChangedAnalyzers/Helpers/Setter.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,10 @@ IdentifierNameSyntax identifierName
108108
};
109109
}
110110

111-
internal static AssignmentExpressionSyntax? AssignsValueToBackingField(AccessorDeclarationSyntax setter)
111+
internal static AssignmentExpressionSyntax? AssignsValueToBackingField(
112+
AccessorDeclarationSyntax setter,
113+
SemanticModel semanticModel,
114+
CancellationToken cancellationToken)
112115
{
113116
if (setter.FirstAncestor<PropertyDeclarationSyntax>() is not { } property || !property.TrySingleReturned(out var backingExpression))
114117
{
@@ -120,7 +123,10 @@ IdentifierNameSyntax identifierName
120123
foreach (var assignment in walker.Assignments)
121124
{
122125
if (assignment is { Right: IdentifierNameSyntax { Identifier: { ValueText: "value" } } } &&
123-
MemberPath.Equals(backingExpression, assignment.Left))
126+
MemberPath.Equals(backingExpression, assignment.Left) &&
127+
SymbolEqualityComparer.Default.Equals(
128+
semanticModel.GetSymbolSafe(backingExpression, cancellationToken),
129+
semanticModel.GetSymbolSafe(assignment.Left, cancellationToken)))
124130
{
125131
return assignment;
126132
}

ValidCode/NotMutations.cs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
namespace ValidCode
22
{
3+
using System;
34
using System.ComponentModel;
45
using System.Runtime.CompilerServices;
56

@@ -40,5 +41,39 @@ public int SamePropertyNameOnUnrelatedInstance
4041
Settings.Default.SamePropertyNameOnUnrelatedInstance = value;
4142
}
4243
}
44+
45+
record DifferentClass
46+
{
47+
public int sameExpressionWithDifferentMeaning;
48+
}
49+
50+
private int sameExpressionWithDifferentMeaning;
51+
public int SameExpressionWithDifferentMeaning
52+
{
53+
get => sameExpressionWithDifferentMeaning;
54+
set
55+
{
56+
if (!Set(ref sameExpressionWithDifferentMeaning, value)) return;
57+
58+
_ = new DifferentClass { sameExpressionWithDifferentMeaning = value };
59+
_ = new DifferentClass() with { sameExpressionWithDifferentMeaning = value };
60+
61+
{
62+
int sameExpressionWithDifferentMeaning;
63+
sameExpressionWithDifferentMeaning = value;
64+
}
65+
66+
_ = new Action<int>(sameExpressionWithDifferentMeaning =>
67+
{
68+
sameExpressionWithDifferentMeaning = value;
69+
});
70+
71+
LocalFunction(42);
72+
void LocalFunction(int sameExpressionWithDifferentMeaning)
73+
{
74+
sameExpressionWithDifferentMeaning = value;
75+
}
76+
}
77+
}
4378
}
4479
}

0 commit comments

Comments
 (0)