Skip to content

Commit 729d215

Browse files
authored
Merge pull request #1443 from riganti/attribute-uppercase-warning
Compile time warning for uppercase atrributes (i.e. mistyped properties)
2 parents 4de462d + eb708c7 commit 729d215

14 files changed

+203
-27
lines changed

src/Framework/Framework/Binding/DotvvmCapabilityProperty.cs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -285,14 +285,11 @@ internal static IControlAttributeDescriptor InitializeArgument(ICustomAttributeP
285285
propertyName,
286286
unwrappedType,
287287
attributeProvider,
288-
boxedDefaultValue
288+
boxedDefaultValue,
289+
declaringCapability: declaringCapability
289290
);
291+
propertyGroup.AddUsedInCapability(declaringCapability);
290292

291-
if (declaringCapability is object)
292-
{
293-
propertyGroup.OwningCapability = declaringCapability;
294-
propertyGroup.UsedInCapabilities = propertyGroup.UsedInCapabilities.Add(declaringCapability);
295-
}
296293
return propertyGroup;
297294
}
298295
// Control Capability

src/Framework/Framework/Binding/DotvvmProperty.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ public string FullName
105105
public DotvvmCapabilityProperty? OwningCapability { get; internal set; }
106106
/// <summary> The capabilities which use this property. </summary>
107107
public ImmutableArray<DotvvmCapabilityProperty> UsedInCapabilities { get; internal set; } = ImmutableArray<DotvvmCapabilityProperty>.Empty;
108+
IPropertyDescriptor? IControlAttributeDescriptor.OwningCapability => OwningCapability;
109+
IEnumerable<IPropertyDescriptor> IControlAttributeDescriptor.UsedInCapabilities => UsedInCapabilities;
108110

109111

110112
internal void AddUsedInCapability(DotvvmCapabilityProperty? p)

src/Framework/Framework/Binding/GroupedDotvvmProperty.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@
88

99
namespace DotVVM.Framework.Binding
1010
{
11-
public sealed class GroupedDotvvmProperty : DotvvmProperty
11+
public sealed class GroupedDotvvmProperty : DotvvmProperty, IGroupedPropertyDescriptor
1212
{
1313
public DotvvmPropertyGroup PropertyGroup { get; }
1414

1515
public string GroupMemberName { get; }
1616

17+
IPropertyGroupDescriptor IGroupedPropertyDescriptor.PropertyGroup => PropertyGroup;
18+
1719
public GroupedDotvvmProperty(string groupMemberName, DotvvmPropertyGroup propertyGroup)
1820
{
1921
this.GroupMemberName = groupMemberName;
@@ -30,7 +32,9 @@ public static GroupedDotvvmProperty Create(DotvvmPropertyGroup group, string nam
3032
DefaultValue = group.DefaultValue,
3133
IsValueInherited = false,
3234
Name = propname,
33-
ObsoleteAttribute = group.ObsoleteAttribute
35+
ObsoleteAttribute = group.ObsoleteAttribute,
36+
OwningCapability = group.OwningCapability,
37+
UsedInCapabilities = group.UsedInCapabilities
3438
};
3539

3640
DotvvmProperty.InitializeProperty(prop, group.AttributeProvider);

src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,8 @@ private void ProcessAttribute(IPropertyDescriptor property, DothtmlAttributeNode
315315
return;
316316
}
317317

318+
ValidateAttribute(property, control, attribute);
319+
318320
// set the property
319321
if (attribute.ValueNode == null)
320322
{
@@ -345,15 +347,79 @@ private void ProcessAttribute(IPropertyDescriptor property, DothtmlAttributeNode
345347
else
346348
{
347349
// hard-coded value in markup
348-
if (!property.MarkupOptions.AllowHardCodedValue)
350+
var textValue = (DothtmlValueTextNode)attribute.ValueNode;
351+
352+
try
349353
{
350-
attribute.ValueNode.AddError($"The property '{ property.FullName }' cannot contain hard coded value.");
354+
// ConvertValue may fail, we don't want to crash the compiler in that case.
355+
var value = ConvertValue(WebUtility.HtmlDecode(textValue.Text), property.PropertyType);
356+
var propertyValue = treeBuilder.BuildPropertyValue(property, value, attribute);
357+
358+
if (!treeBuilder.AddProperty(control, propertyValue, out var error)) attribute.AddError(error);
351359
}
360+
catch (Exception e)
361+
{
362+
textValue.AddError($"The value '{textValue.Text}' could not be converted to {property.PropertyType.FullName}: {e.Message}");
363+
}
364+
}
365+
}
352366

353-
var textValue = (DothtmlValueTextNode)attribute.ValueNode;
354-
var value = ConvertValue(WebUtility.HtmlDecode(textValue.Text), property.PropertyType);
355-
var propertyValue = treeBuilder.BuildPropertyValue(property, value, attribute);
356-
if (!treeBuilder.AddProperty(control, propertyValue, out var error)) attribute.AddError(error);
367+
private void ValidateAttribute(IPropertyDescriptor property, IAbstractControl control, DothtmlAttributeNode attribute)
368+
{
369+
if (!property.MarkupOptions.AllowHardCodedValue && attribute.ValueNode is not DothtmlValueBindingNode)
370+
{
371+
var err = $"The property {property.FullName} cannot contain hard coded value.";
372+
if (attribute.ValueNode is not null)
373+
attribute.ValueNode.AddError(err);
374+
else
375+
attribute.AddError(err);
376+
return;
377+
}
378+
379+
if (!property.MarkupOptions.AllowHardCodedValue &&
380+
attribute.ValueNode is DothtmlValueBindingNode resourceBinding &&
381+
treatBindingAsHardCodedValue.Contains(resourceBinding.BindingNode.Name))
382+
{
383+
// TODO: next major version - make this error
384+
var err = $"The property {property.FullName} cannot contain hardcoded value nor resource bindings. This will be an error in the next major version.";
385+
resourceBinding.AddWarning(err);
386+
return;
387+
}
388+
389+
if (!property.MarkupOptions.AllowBinding &&
390+
attribute.ValueNode is DothtmlValueBindingNode binding &&
391+
!treatBindingAsHardCodedValue.Contains(binding.BindingNode.Name))
392+
{
393+
// stupid edge case, precompiled controls currently don't allow resource bindings, so it's better to not suggest it in the error message
394+
var allowsResourceBinding =
395+
control.Metadata.PrecompilationMode <= ControlPrecompilationMode.IfPossibleAndIgnoreExceptions;
396+
var resourceBindingHelp =
397+
allowsResourceBinding && property.PropertyType.IsPrimitiveTypeDescriptor() ? " You can use a resource binding instead - the resource will evaluate the expression server-side." : "";
398+
var err = $"The property {property.FullName} cannot contain {binding.BindingNode.Name} binding." + resourceBindingHelp;
399+
attribute.AddError(err);
400+
return;
401+
}
402+
403+
// validate that html attributes names look valid
404+
if (property is IGroupedPropertyDescriptor groupedProperty &&
405+
property.UsedInCapabilities.Any(c => c.PropertyType.IsEqualTo(typeof(HtmlCapability))))
406+
{
407+
var pGroup = groupedProperty.PropertyGroup;
408+
var name = groupedProperty.GroupMemberName;
409+
if (pGroup.Name.EndsWith("Attributes") && name.ToLowerInvariant() != name)
410+
{
411+
// properties with at most two typos
412+
var similarNameProperties =
413+
control.Metadata.AllProperties
414+
.Where(p => StringSimilarity.DamerauLevenshteinDistance(p.Name.ToLowerInvariant(), name.ToLowerInvariant()) <= 2)
415+
.Select(p => p.Name)
416+
.ToArray();
417+
var similarPropertyHelp =
418+
similarNameProperties.Any() ? $" Did you mean {string.Join(", ", similarNameProperties)}, or another DotVVM property?" : " Did you intent to use a DotVVM property instead?";
419+
attribute.AttributeNameNode.AddWarning(
420+
$"HTML attribute name '{name}' should not contain uppercase letters." + similarPropertyHelp
421+
);
422+
}
357423
}
358424
}
359425

src/Framework/Framework/Compilation/ControlTree/DotvvmPropertyGroup.cs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,13 @@ public class DotvvmPropertyGroup : IPropertyGroupDescriptor
4141
private ConcurrentDictionary<string, GroupedDotvvmProperty> generatedProperties = new();
4242

4343
/// <summary> The capability which declared this property. When the property is declared by an capability, it can only be used by this capability. </summary>
44-
public DotvvmCapabilityProperty? OwningCapability { get; internal set; }
44+
public DotvvmCapabilityProperty? OwningCapability { get; }
45+
IPropertyDescriptor? IControlAttributeDescriptor.OwningCapability => OwningCapability;
4546
/// <summary> The capabilities which use this property. </summary>
46-
public ImmutableArray<DotvvmCapabilityProperty> UsedInCapabilities { get; internal set; } = ImmutableArray<DotvvmCapabilityProperty>.Empty;
47+
public ImmutableArray<DotvvmCapabilityProperty> UsedInCapabilities { get; private set; } = ImmutableArray<DotvvmCapabilityProperty>.Empty;
48+
IEnumerable<IPropertyDescriptor> IControlAttributeDescriptor.UsedInCapabilities => UsedInCapabilities;
4749

48-
internal DotvvmPropertyGroup(PrefixArray prefixes, Type valueType, Type declaringType, FieldInfo? descriptorField, ICustomAttributeProvider attributeProvider, string name, object? defaultValue)
50+
internal DotvvmPropertyGroup(PrefixArray prefixes, Type valueType, Type declaringType, FieldInfo? descriptorField, ICustomAttributeProvider attributeProvider, string name, object? defaultValue, DotvvmCapabilityProperty? owningCapability = null)
4951
{
5052
this.DescriptorField = descriptorField;
5153
this.DeclaringType = declaringType;
@@ -58,6 +60,7 @@ internal DotvvmPropertyGroup(PrefixArray prefixes, Type valueType, Type declarin
5860
{
5961
ValueMerger = (IAttributeValueMerger?)Activator.CreateInstance(MarkupOptions.AttributeValueMerger);
6062
}
63+
this.OwningCapability = owningCapability;
6164
}
6265

6366
private static (MarkupOptionsAttribute, DataContextChangeAttribute[], DataContextStackManipulationAttribute?, ObsoleteAttribute?)
@@ -72,6 +75,16 @@ private static (MarkupOptionsAttribute, DataContextChangeAttribute[], DataContex
7275
return (markupOptions, dataContextChange.ToArray(), dataContextManipulation, obsoleteAttribute);
7376
}
7477

78+
internal void AddUsedInCapability(DotvvmCapabilityProperty? p)
79+
{
80+
if (p is object)
81+
lock(this)
82+
{
83+
if (!UsedInCapabilities.Contains(p))
84+
UsedInCapabilities = UsedInCapabilities.Add(p);
85+
}
86+
}
87+
7588
IPropertyDescriptor IPropertyGroupDescriptor.GetDotvvmProperty(string name) => GetDotvvmProperty(name);
7689
public GroupedDotvvmProperty GetDotvvmProperty(string name)
7790
{
@@ -109,12 +122,12 @@ internal static FieldInfo FindDescriptorField(Type declaringType, string name)
109122
return field;
110123
}
111124

112-
public static DotvvmPropertyGroup Register(Type declaringType, PrefixArray prefixes, string name, Type valueType, ICustomAttributeProvider attributeProvider, object? defaultValue)
125+
public static DotvvmPropertyGroup Register(Type declaringType, PrefixArray prefixes, string name, Type valueType, ICustomAttributeProvider attributeProvider, object? defaultValue, DotvvmCapabilityProperty? declaringCapability = null)
113126
{
114127
return descriptorDictionary.GetOrAdd((declaringType, name), fullName => {
115128
// the field is optional, here
116129
var field = declaringType.GetField(name + "GroupDescriptor", BindingFlags.Public | BindingFlags.Static);
117-
return new DotvvmPropertyGroup(prefixes, valueType, declaringType, field, attributeProvider, name, defaultValue);
130+
return new DotvvmPropertyGroup(prefixes, valueType, declaringType, field, attributeProvider, name, defaultValue, declaringCapability);
118131
});
119132
}
120133

src/Framework/Framework/Compilation/ControlTree/IControlResolverMetadata.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Collections.Generic;
22
using System.Diagnostics.CodeAnalysis;
33
using DotVVM.Framework.Binding;
4+
using DotVVM.Framework.Controls;
45

56
namespace DotVVM.Framework.Compilation.ControlTree
67
{
@@ -29,6 +30,8 @@ public interface IControlResolverMetadata
2930
/// </summary>
3031
IReadOnlyList<PropertyGroupMatcher> PropertyGroups { get; }
3132

33+
ControlPrecompilationMode PrecompilationMode { get; }
34+
3235
DataContextChangeAttribute[] DataContextChangeAttributes { get; }
3336
DataContextStackManipulationAttribute? DataContextManipulationAttribute { get; }
3437
}

src/Framework/Framework/Compilation/ControlTree/IPropertyDescriptor.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,10 @@ public interface IPropertyDescriptor: IControlAttributeDescriptor
88
bool IsBindingProperty { get; }
99
string FullName { get; }
1010
}
11+
12+
public interface IGroupedPropertyDescriptor: IPropertyDescriptor
13+
{
14+
string GroupMemberName { get; }
15+
IPropertyGroupDescriptor PropertyGroup { get; }
16+
}
1117
}

src/Framework/Framework/Compilation/ControlTree/ITypeDescriptor.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using DotVVM.Framework.Controls;
23

34
namespace DotVVM.Framework.Compilation.ControlTree
@@ -19,6 +20,7 @@ public interface ITypeDescriptor
1920
ControlMarkupOptionsAttribute? GetControlMarkupOptionsAttribute();
2021

2122
bool IsEqualTo(ITypeDescriptor other);
23+
bool IsEqualTo(Type other);
2224

2325
ITypeDescriptor? TryGetArrayElementOrIEnumerableType();
2426

src/Framework/Framework/Compilation/ControlTree/Resolved/ResolvedTypeDescriptor.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,25 @@ public bool IsAssignableFrom(ITypeDescriptor typeDescriptor)
4444
return Type.GetCustomAttribute<ControlMarkupOptionsAttribute>();
4545
}
4646

47+
public bool IsEqualTo(Type other)
48+
{
49+
return this.Type == other;
50+
}
4751
public bool IsEqualTo(ITypeDescriptor other)
4852
{
53+
if (other is ResolvedTypeDescriptor { Type: var otherType })
54+
return this.IsEqualTo(otherType);
4955
return Name == other.Name && Namespace == other.Namespace && Assembly == other.Assembly;
5056
}
5157

58+
public override bool Equals(object? obj) =>
59+
obj is null ? false :
60+
obj is ResolvedTypeDescriptor { Type: var otherType } ? IsEqualTo(otherType) :
61+
obj is Type otherType2 ? IsEqualTo(otherType2) :
62+
obj is ITypeDescriptor typeD ? IsEqualTo(typeD) :
63+
false;
64+
public override int GetHashCode() => Type.GetHashCode();
65+
5266
public ITypeDescriptor? TryGetArrayElementOrIEnumerableType()
5367
{
5468
// handle array

src/Framework/Framework/Compilation/IControlAttributeDescriptor.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,8 @@ public interface IControlAttributeDescriptor
1919
ObsoleteAttribute? ObsoleteAttribute { get; }
2020
ITypeDescriptor DeclaringType { get; }
2121
ITypeDescriptor PropertyType { get; }
22+
23+
IPropertyDescriptor? OwningCapability { get; }
24+
IEnumerable<IPropertyDescriptor> UsedInCapabilities { get; }
2225
}
2326
}

0 commit comments

Comments
 (0)