Browse Source

Added SuperfluousAddOwnerCall

pull/10244/head
Tom Edwards 4 years ago
parent
commit
0b7851fd92
  1. 85
      src/tools/PublicAnalyzers/AvaloniaPropertyAnalyzer.CompileAnalyzer.cs
  2. 21
      src/tools/PublicAnalyzers/AvaloniaPropertyAnalyzer.cs

85
src/tools/PublicAnalyzers/AvaloniaPropertyAnalyzer.CompileAnalyzer.cs

@ -212,7 +212,7 @@ public partial class AvaloniaPropertyAnalyzer
{
var target = fieldInitializations[current];
propertyDescription.AddAssignment(target, new(owner.Type, target.Locations[0])); // This loop handles simple assignment operations, so do NOT change the owner type
propertyDescription.SetAssignment(target, new(owner.Type, target.Locations[0])); // This loop handles simple assignment operations, so do NOT change the owner type
propertyDescriptions[target] = propertyDescription;
fieldInitializations.TryGetValue(target, out current);
@ -358,7 +358,7 @@ public partial class AvaloniaPropertyAnalyzer
description.Name = name;
description.HostType = hostTypeRef;
description.Inherits = inherits;
description.AddAssignment(target, ownerTypeRef);
description.SetAssignment(target, ownerTypeRef);
description.AddOwner(ownerTypeRef);
}
else if (_avaloniaPropertyAddOwnerMethods.Contains(invocation.TargetMethod.OriginalDefinition)) // This is a call to one of the AddOwner methods
@ -373,7 +373,7 @@ public partial class AvaloniaPropertyAnalyzer
return;
}
var description = propertyDescriptions.GetOrAdd(sourceSymbol, s =>
var description = propertyDescriptions[target] = propertyDescriptions.GetOrAdd(sourceSymbol, s =>
{
string inferredName = s.Name;
@ -396,11 +396,16 @@ public partial class AvaloniaPropertyAnalyzer
hostTypeRef = new(_avaloniaObjectType, Location.None); // assume that an attached property applies everywhere until we find its registration
}
return new AvaloniaPropertyDescription(inferredName, propertyType, valueType) { HostType = hostTypeRef };
var result = new AvaloniaPropertyDescription(inferredName, propertyType, valueType) { HostType = hostTypeRef };
// assume that the property is owned by its containing type at the point of assignment, until we find its registration
result.SetAssignment(s, new(s.ContainingType, Location.None));
return result;
});
var ownerTypeRef = TypeReference.FromInvocationTypeParameter(invocation, ownerTypeParam);
description.AddAssignment(target, ownerTypeRef);
description.SetAssignment(target, ownerTypeRef);
description.AddOwner(ownerTypeRef);
}
}
@ -514,7 +519,7 @@ public partial class AvaloniaPropertyAnalyzer
var ownerType = description.AssignedTo[assignmentSymbol];
if (ownerType.Type.TypeKind != TypeKind.Error &&
!IsAvaloniaPropertyType(description.PropertyType, _attachedPropertyType) &&
!IsAvaloniaPropertyType(description.PropertyType, _attachedPropertyType) &&
!SymbolEquals(ownerType.Type, assignmentSymbol.ContainingType))
{
context.ReportDiagnostic(Diagnostic.Create(OwnerDoesNotMatchOuterType, ownerType.Location, ownerType.Type));
@ -565,30 +570,14 @@ public partial class AvaloniaPropertyAnalyzer
if (_allGetSetMethods.Contains(originalMethod))
{
var avaloniaPropertyOperation = invocation.Arguments[0].Value;
var propertyStorageSymbol = GetReferencedFieldOrProperty(avaloniaPropertyOperation);
if (propertyStorageSymbol == null || !_avaloniaPropertyDescriptions.TryGetValue(propertyStorageSymbol, out var propertyDescription))
{
return;
}
TypeReference ownerOrHostType;
if (SymbolEquals(propertyDescription.PropertyType.OriginalDefinition, _attachedPropertyType))
{
ownerOrHostType = propertyDescription.HostType!.Value;
}
else if (!propertyDescription.AssignedTo.TryGetValue(propertyStorageSymbol, out ownerOrHostType))
{
return;
}
if (invocation.Instance is IInstanceReferenceOperation { ReferenceKind: InstanceReferenceKind.ContainingTypeInstance } &&
!DerivesFrom(context.ContainingSymbol.ContainingType, ownerOrHostType.Type))
if (invocation.Instance is IInstanceReferenceOperation { ReferenceKind: InstanceReferenceKind.ContainingTypeInstance } &&
GetReferencedProperty(invocation.Arguments[0]) is { } refProp &&
refProp.description.AssignedTo.TryGetValue(refProp.storageSymbol, out var ownerType) &&
!DerivesFrom(context.ContainingSymbol.ContainingType, ownerType.Type) &&
!DerivesFrom(context.ContainingSymbol.ContainingType, refProp.description.HostType?.Type))
{
context.ReportDiagnostic(Diagnostic.Create(UnexpectedPropertyAccess, invocation.Arguments[0].Syntax.GetLocation(),
propertyStorageSymbol, context.ContainingSymbol.ContainingType));
refProp.storageSymbol, context.ContainingSymbol.ContainingType));
}
}
else if (_allAvaloniaPropertyMethods.Contains(originalMethod))
@ -599,10 +588,32 @@ public partial class AvaloniaPropertyAnalyzer
originalMethod.ToDisplayString(TypeQualifiedName)));
}
if (_ownerTypeParams.TryGetValue(invocation.TargetMethod.OriginalDefinition, out var typeParam) &&
invocation.TargetMethod.TypeArguments[typeParam.Ordinal] is INamedTypeSymbol { IsGenericType: true })
if (_ownerTypeParams.TryGetValue(invocation.TargetMethod.OriginalDefinition, out var typeParam) &&
invocation.TargetMethod.TypeArguments[typeParam.Ordinal] is { } newOwnerType)
{
context.ReportDiagnostic(Diagnostic.Create(PropertyOwnedByGenericType, TypeReference.FromInvocationTypeParameter(invocation, typeParam).Location));
if (newOwnerType is INamedTypeSymbol { IsGenericType: true })
{
context.ReportDiagnostic(Diagnostic.Create(PropertyOwnedByGenericType, TypeReference.FromInvocationTypeParameter(invocation, typeParam).Location));
}
if (_avaloniaPropertyAddOwnerMethods.Contains(originalMethod) && GetReferencedProperty(invocation.Instance!) is { } refProp)
{
var ownerMatches = refProp.description.AssignedTo.Where(kvp => !SymbolEquals(kvp.Key, context.ContainingSymbol) && DerivesFrom(newOwnerType, kvp.Value.Type)).ToArray();
if (ownerMatches.Any())
{
var ownerMatchesExceptBaseTypes = ownerMatches.Where(m => !DerivesFrom(context.ContainingSymbol.ContainingType, m.Key.ContainingType, includeSelf: false)).ToArray();
var routesMessage = ownerMatchesExceptBaseTypes.Length switch
{
0 => "its base type",
1 => ownerMatchesExceptBaseTypes.Single().Key.ToString(),
_ => $"{ownerMatches.Length} routes\n\t{string.Join("\n\t", ownerMatches.Select(kvp => kvp.Key))}"
};
context.ReportDiagnostic(Diagnostic.Create(SuperfluousAddOwnerCall, invocation.Syntax.GetLocation(), ownerMatches.Select(kvp => kvp.Value.Location),
newOwnerType, refProp.storageSymbol, routesMessage));
}
}
}
}
@ -616,6 +627,18 @@ public partial class AvaloniaPropertyAnalyzer
};
}
private (AvaloniaPropertyDescription description, ISymbol storageSymbol)? GetReferencedProperty(IOperation operation)
{
if (GetReferencedFieldOrProperty(operation) is { } storageSymbol && _avaloniaPropertyDescriptions.TryGetValue(storageSymbol, out var result))
{
return (result, storageSymbol);
}
else
{
return null;
}
}
/// <seealso cref="AmbiguousPropertyName"/>
/// <seealso cref="PropertyTypeMismatch"/>
/// <seealso cref="AssociatedAvaloniaProperty"/>

21
src/tools/PublicAnalyzers/AvaloniaPropertyAnalyzer.cs

@ -93,6 +93,16 @@ public partial class AvaloniaPropertyAnalyzer : DiagnosticAnalyzer
"not the control itself. Constructor parameters and assignments within UserControl or TopLevel types are exempt from this diagnostic.",
InappropriateReadWriteTag);
private static readonly DiagnosticDescriptor SuperfluousAddOwnerCall = new(
"AVP1013",
"Do not add superfluous AvaloniaProperty owners",
"Superfluous owner: {0} is already an owner of {1} via {2}",
Category,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
"Ownership of an AvaloniaProperty is inherited along the type hierarchy. There is no need for a derived type to assert ownership over a base type's properties. This diagnostic can be a symptom of an incorrect property owner elsewhere.",
InappropriateReadWriteTag);
private static readonly DiagnosticDescriptor DuplicatePropertyName = new(
"AVP1020",
"AvaloniaProperty names should be unique within each class",
@ -175,6 +185,7 @@ public partial class AvaloniaPropertyAnalyzer : DiagnosticAnalyzer
OwnerDoesNotMatchOuterType,
UnexpectedPropertyAccess,
SettingOwnStyledPropertyValue,
SuperfluousAddOwnerCall,
DuplicatePropertyName,
AmbiguousPropertyName,
PropertyNameMismatch,
@ -206,10 +217,15 @@ public partial class AvaloniaPropertyAnalyzer : DiagnosticAnalyzer
return propertyTypes.Any(t => SymbolEquals(type, t));
}
private static bool DerivesFrom(ITypeSymbol? type, ITypeSymbol? baseType)
private static bool DerivesFrom(ITypeSymbol? type, ITypeSymbol? baseType, bool includeSelf = true)
{
if (baseType != null)
{
if (!includeSelf)
{
type = type?.BaseType;
}
while (type != null)
{
if (SymbolEquals(type, baseType))
@ -266,6 +282,7 @@ public partial class AvaloniaPropertyAnalyzer : DiagnosticAnalyzer
{
IFieldReferenceOperation fieldRef => fieldRef.Field,
IPropertyReferenceOperation propertyRef => propertyRef.Property,
IArgumentOperation argument => GetReferencedFieldOrProperty(argument.Value),
_ => null,
};
@ -349,7 +366,7 @@ public partial class AvaloniaPropertyAnalyzer : DiagnosticAnalyzer
public void AddPropertyWrapper(IPropertySymbol property) => (_propertyWrappers ?? throw new InvalidOperationException(SealedError)).Add(property);
public void AddAssignment(ISymbol assignmentTarget, TypeReference ownerType) => (_assignedTo ?? throw new InvalidOperationException(SealedError)).TryAdd(assignmentTarget, ownerType);
public void SetAssignment(ISymbol assignmentTarget, TypeReference ownerType) => (_assignedTo ?? throw new InvalidOperationException(SealedError))[assignmentTarget] = ownerType;
public AvaloniaPropertyDescription Seal()
{

Loading…
Cancel
Save