Skip to content

Commit

Permalink
Merge pull request #1751 from riganti/validation-target-warning
Browse files Browse the repository at this point in the history
control usage validators can produce warning + validate attached properties
  • Loading branch information
tomasherceg authored Jan 12, 2024
2 parents 693317e + 7ae6f1f commit 91fc988
Show file tree
Hide file tree
Showing 8 changed files with 272 additions and 102 deletions.
Original file line number Diff line number Diff line change
@@ -1,18 +1,37 @@
using System.Collections.Generic;
using System.Linq;
using DotVVM.Framework.Binding.Properties;
using DotVVM.Framework.Compilation.Parser.Dothtml.Parser;

namespace DotVVM.Framework.Compilation.Validation
{
/// <summary> Represents an error or a warning reported by a control usage validation method. </summary>
/// <seealso cref="ControlUsageValidatorAttribute"/>
public class ControlUsageError
{
public string ErrorMessage { get; }
/// <summary> The error will be shown on the these syntax nodes. When empty, the start tag is underlined. </summary>
public DothtmlNode[] Nodes { get; }
public ControlUsageError(string message, IEnumerable<DothtmlNode?> nodes)
/// <summary> Error - the page compilation will fail. Warning - the user will only be notified about the reported problem (in the log, for example). Other severities are currently not shown at all. </summary>
public DiagnosticSeverity Severity { get; } = DiagnosticSeverity.Error;
public ControlUsageError(string message, DiagnosticSeverity severity, IEnumerable<DothtmlNode?> nodes)
{
ErrorMessage = message;
Nodes = nodes.Where(n => n != null).ToArray()!;
Severity = severity;
}
public ControlUsageError(string message, IEnumerable<DothtmlNode?> nodes) : this(message, DiagnosticSeverity.Error, nodes) { }
public ControlUsageError(string message, params DothtmlNode?[] nodes) : this(message, DiagnosticSeverity.Error, nodes.AsEnumerable()) { }
public ControlUsageError(string message, DiagnosticSeverity severity, params DothtmlNode?[] nodes) : this(message, severity, nodes.AsEnumerable()) { }

public override string ToString()
{
var core = $"{Severity} {ErrorMessage}";
var someToken = Nodes.SelectMany(n => n.Tokens).FirstOrDefault();
if (someToken == null)
return core;
else
return $"{core} (at {someToken.LineNumber}:{someToken.ColumnNumber})";
}
public ControlUsageError(string message, params DothtmlNode?[] nodes) : this(message, nodes.AsEnumerable()) { }
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
using System;
using System.Collections.Generic;
using System.Linq;
using DotVVM.Framework.Binding.Properties;
using DotVVM.Framework.Compilation.ControlTree.Resolved;
using DotVVM.Framework.Configuration;
using Microsoft.Extensions.DependencyInjection;

namespace DotVVM.Framework.Compilation.Validation
{
Expand All @@ -23,7 +22,19 @@ public override void VisitControl(ResolvedControl control)
{
Errors.Add((control, e));
foreach (var node in e.Nodes)
node.AddError(e.ErrorMessage);
{
switch (e.Severity)
{
case DiagnosticSeverity.Error:
node.AddError(e.ErrorMessage);
break;
case DiagnosticSeverity.Warning:
node.AddWarning(e.ErrorMessage);
break;
default:
break;
}
}
}
base.VisitControl(control);
}
Expand All @@ -33,9 +44,8 @@ public void VisitAndAssert(ResolvedTreeRoot view)
{
if (this.Errors.Any()) throw new Exception("The ControlUsageValidationVisitor has already collected some errors.");
VisitView(view);
if (this.Errors.Any())
if (this.Errors.FirstOrDefault(e => e.err.Severity == DiagnosticSeverity.Error) is { err: {} } controlUsageError)
{
var controlUsageError = this.Errors.First();
var lineNumber =
controlUsageError.control.GetAncestors()
.Select(c => c.DothtmlNode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,24 @@

namespace DotVVM.Framework.Compilation.Validation
{
/// <summary>
/// Call this static method for each compiled control of a matching type.
/// The method should have the signature:
/// <code> <![CDATA[
/// public static IEnumerable< ControlUsageError> Validator(ResolvedControl control)
/// ]]></code>.
/// Optionally, an DotvvmConfiguration parameter may be present on the method.
/// </summary>
[AttributeUsage(AttributeTargets.Method)]
public class ControlUsageValidatorAttribute: Attribute
{
/// <summary> Ignore all validators from the base controls. </summary>
public bool Override { get; set; }
/// <summary>
/// Call this method even on other controls when a property from the declaring class is used.
/// The method will be called once per control.
/// Properties on derived nor base classes do not trigger the validator.
/// </summary>
public bool IncludeAttachedProperties { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using DotVVM.Framework.Binding;
using DotVVM.Framework.Compilation.ControlTree;
using DotVVM.Framework.Compilation.ControlTree.Resolved;
using DotVVM.Framework.Configuration;
using DotVVM.Framework.Controls;
using DotVVM.Framework.Utils;

namespace DotVVM.Framework.Compilation.Validation
{
Expand All @@ -18,7 +20,7 @@ public DefaultControlUsageValidator(DotvvmConfiguration config)
Configuration = config;
}

public static ConcurrentDictionary<Type, MethodInfo[]> cache = new ConcurrentDictionary<Type, MethodInfo[]>();
public static ConcurrentDictionary<Type, (MethodInfo[] controlValidators, MethodInfo[] attachedPropertyValidators)> cache = new();

protected DotvvmConfiguration Configuration { get; }

Expand All @@ -44,6 +46,48 @@ public static IEnumerable<ControlUsageError> ValidateDefaultRules(IAbstractContr
}
}

private IEnumerable<ControlUsageError> CallMethod(MethodInfo method, IAbstractControl control)
{
var par = method.GetParameters();
var args = new object[par.Length];
for (int i = 0; i < par.Length; i++)
{
if (par[i].ParameterType.IsAssignableFrom(control.GetType()))
{
args[i] = control;
}
else if (control.DothtmlNode != null && par[i].ParameterType.IsAssignableFrom(control.DothtmlNode.GetType()))
{
args[i] = control.DothtmlNode;
}
else if (par[i].ParameterType == typeof(DotvvmConfiguration))
{
args[i] = Configuration;
}
else
{
return Enumerable.Empty<ControlUsageError>();
}
}
var r = method.Invoke(null, args);
if (r is null)
{
return Enumerable.Empty<ControlUsageError>();
}
else if (r is IEnumerable<ControlUsageError> errors)
{
return errors;
}
else if (r is IEnumerable<string> stringErrors)
{
return stringErrors.Select(e => new ControlUsageError(e));
}
else
{
throw new Exception($"ControlUsageValidator method '{ReflectionUtils.FormatMethodInfo(method)}' returned an invalid type. Expected IEnumerable<ControlUsageError> or IEnumerable<string>.");
}
}

public IEnumerable<ControlUsageError> Validate(IAbstractControl control)
{
var type = GetControlType(control.Metadata);
Expand All @@ -55,65 +99,59 @@ public IEnumerable<ControlUsageError> Validate(IAbstractControl control)
return result;

var methods = cache.GetOrAdd(type, FindMethods);
foreach (var method in methods)
foreach (var method in methods.controlValidators)
{
var par = method.GetParameters();
var args = new object[par.Length];
for (int i = 0; i < par.Length; i++)
{
if (par[i].ParameterType.IsAssignableFrom(control.GetType()))
{
args[i] = control;
}
else if (control.DothtmlNode != null && par[i].ParameterType.IsAssignableFrom(control.DothtmlNode.GetType()))
{
args[i] = control.DothtmlNode;
}
else if (par[i].ParameterType == typeof(DotvvmConfiguration))
{
args[i] = Configuration;
}
else
{
goto Error; // I think it is better that throw exceptions and catch them
}
}
var r = method.Invoke(null, args);
if (r is IEnumerable<ControlUsageError> errors)
{
result.AddRange(errors);
}
else if (r is IEnumerable<string> stringErrors)
result.AddRange(CallMethod(method, control));
}

var attachedPropertiesTypes = new HashSet<Type>();
foreach (var attachedProperty in control.PropertyNames)
{
if (attachedProperty.DeclaringType.IsAssignableFrom(control.Metadata.Type))
continue; // not an attached property
if (GetPropertyDeclaringType(attachedProperty) is {} declaringType)
attachedPropertiesTypes.Add(declaringType);
}

foreach (var attachedPropertyType in attachedPropertiesTypes)
{
var (_, attachedValidators) = cache.GetOrAdd(attachedPropertyType, FindMethods);
foreach (var method in attachedValidators)
{
result.AddRange(stringErrors.Select(e => new ControlUsageError(e)));
result.AddRange(CallMethod(method, control));
}
continue;
Error:;
}

return result
// add current node to the error, if no control is specified
.Select(e => e.Nodes.Length == 0 ?
new ControlUsageError(e.ErrorMessage, control.DothtmlNode) :
new ControlUsageError(e.ErrorMessage, e.Severity, control.DothtmlNode) :
e);
}

protected virtual MethodInfo[] FindMethods(Type type)
protected virtual (MethodInfo[] controlValidators, MethodInfo[] attachedPropertyValidators) FindMethods(Type type)
{
if (type == typeof(object)) return new MethodInfo[0];
if (type == typeof(object))
return (Array.Empty<MethodInfo>(), Array.Empty<MethodInfo>());

var methods = type.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static)
.Where(m => m.IsDefined(typeof(ControlUsageValidatorAttribute)))
.ToArray();

var attributes = methods.Select(s => s.GetCustomAttribute(typeof(ControlUsageValidatorAttribute))).ToList();
var overrideValidation = attributes.OfType<ControlUsageValidatorAttribute>().Select(s => s.Override).Distinct().ToList();
var attributes = methods.Select(method => (method, attr: method.GetCustomAttribute<ControlUsageValidatorAttribute>().NotNull())).ToList();
var overrideValidation = attributes.Select(s => s.attr.Override).Distinct().ToList();

if (overrideValidation.Count > 1)
throw new Exception($"ControlUsageValidator attributes on '{type.FullName}' are in an inconsistent state. Make sure all attributes have an Override property set to the same value.");

if (overrideValidation.Any() && overrideValidation[0]) return methods;
var attachedValidators = attributes.Where(s => s.attr.IncludeAttachedProperties).Select(m => m.method).ToArray();

if (overrideValidation.Any() && overrideValidation[0])
return (methods, attachedValidators);

var ancestorMethods = FindMethods(type.BaseType!);
return ancestorMethods.Concat(methods).ToArray();
// attached validators are not inherited
return (ancestorMethods.controlValidators.Concat(methods).ToArray(), attachedValidators);
}

protected virtual Type? GetControlType(IControlResolverMetadata metadata)
Expand All @@ -122,6 +160,13 @@ protected virtual MethodInfo[] FindMethods(Type type)
return type?.Type;
}

protected virtual Type? GetPropertyDeclaringType(IPropertyDescriptor property)
{
if (property is DotvvmProperty p)
return p.DeclaringType;
return (property.DeclaringType as ResolvedTypeDescriptor)?.Type;
}

/// <summary> Clear cache when hot reload happens </summary>
internal static void ClearCaches(Type[] types)
{
Expand Down
28 changes: 28 additions & 0 deletions src/Framework/Framework/Controls/Validation.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq.Expressions;
using DotVVM.Framework.Binding;
using DotVVM.Framework.Binding.Expressions;
using DotVVM.Framework.Binding.Properties;
using DotVVM.Framework.Compilation.ControlTree;
using DotVVM.Framework.Compilation.ControlTree.Resolved;
using DotVVM.Framework.Compilation.Parser.Dothtml.Parser;
using DotVVM.Framework.Compilation.Validation;

namespace DotVVM.Framework.Controls
{
Expand All @@ -12,5 +21,24 @@ public class Validation
[AttachedProperty(typeof(object))]
[MarkupOptions(AllowHardCodedValue = false)]
public static DotvvmProperty TargetProperty = DotvvmProperty.Register<object?, Validation>(() => TargetProperty, null, true);


[ControlUsageValidator(IncludeAttachedProperties = true)]
public static IEnumerable<ControlUsageError> ValidateUsage(ResolvedControl control)
{
if (control.GetValue(Validation.TargetProperty) is ResolvedPropertyBinding { Binding: {} targetBinding })
{
if (targetBinding.ResultType.IsPrimitiveTypeDescriptor() &&
targetBinding.Expression is not ConstantExpression // Allow Target={value: 0}, it's an OK hack to supress automatic validation
)
{
yield return new ControlUsageError(
$"Validation.Target should be bound to a complex object instead of '{targetBinding.ResultType!.CSharpName}'. Validation attributes on the specified property are ignored, only the rules inside the target object are validated.",
DiagnosticSeverity.Warning,
(targetBinding.DothtmlNode as DothtmlBindingNode)?.ValueNode ?? targetBinding.DothtmlNode
);
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using DotVVM.Framework.ViewModel;

Expand All @@ -8,6 +9,8 @@ public class ValidationTargetIsCollectionViewModel : DotvvmViewModelBase
{
public List<Customer> Customers { get; set; }

public DateTime Something { get; set; } = DateTime.Now;

public ValidationTargetIsCollectionViewModel()
{
Customers = new List<Customer>()
Expand Down
Loading

0 comments on commit 91fc988

Please sign in to comment.