Skip to content

Commit

Permalink
Improved error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
Nikolay Pyanikov authored and NikolayPianikov committed Sep 2, 2022
1 parent 43aac5e commit 5d9f386
Show file tree
Hide file tree
Showing 12 changed files with 125 additions and 86 deletions.
29 changes: 20 additions & 9 deletions Pure.DI.Tests/Integration/ErrorsAndWarningsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,16 @@ public interface IService {}
public class Service: IService { public Service(IDependency dependency) {} }
public class CompositionRoot
public class Abc
{
public readonly IService Value;
internal CompositionRoot(IService value) {}
internal Abc(IService value) {}
}
public class CompositionRoot
{
public readonly Abc Value;
internal CompositionRoot(Abc value) {}
}
internal static partial class Composer
Expand All @@ -81,7 +87,7 @@ static Composer()
}".Run(out var generatedCode, new RunOptions {Statements = string.Empty});

// Then
output.Any(i => i.Contains(Diagnostics.Error.CannotResolve)).ShouldBeTrue(generatedCode);
output.Any(i => i.Contains(Diagnostics.Error.CannotResolve) && i.Contains("constructor Sample.Dependency.Dependency(Sample.IDeepDependency) argument IDeepDependency deepDependency")).ShouldBeTrue(generatedCode);
}

[Fact]
Expand Down Expand Up @@ -144,15 +150,20 @@ namespace Sample
using Pure.DI;
using static Pure.DI.Lifetime;
public class MyClass
public class MyClass1
{
public MyClass(MyClass value) { }
public MyClass(MyClass2 value) { }
}
public class MyClass2
{
public MyClass(MyClass1 value) { }
}
public class CompositionRoot
{
public readonly MyClass Value;
internal CompositionRoot(MyClass value) { }
public readonly MyClass1 Value;
internal CompositionRoot(MyClass1 value) { }
}
internal static partial class Composer
Expand All @@ -164,10 +175,10 @@ static Composer()
.Bind<CompositionRoot>().To<CompositionRoot>();
}
}
}".Run(out var generatedCode);
}".Run(out var generatedCode, new RunOptions {Statements = string.Empty});

// Then
output.Any(i => i.Contains(Diagnostics.Error.CircularDependency)).ShouldBeTrue(generatedCode);
output.Any(i => i.Contains(Diagnostics.Error.CircularDependency) && i.Contains("resolving constructor Sample.MyClass2.MyClass2(Sample.MyClass1) argument MyClass1 value")).ShouldBeTrue(generatedCode);
}

[Fact]
Expand Down
75 changes: 40 additions & 35 deletions Pure.DI/Core/AutowiringObjectBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ public Optional<ExpressionSyntax> TryBuild(IBuildStrategy buildStrategy, Depende
var stopwatch = new Stopwatch();
stopwatch.Start();
var ctorInfos = (
from ctor in
dependency.Implementation.Type is INamedTypeSymbol implementationType
? implementationType.Constructors
: Enumerable.Empty<IMethodSymbol>()
where !_buildContext.IsCancellationRequested
where ctor.DeclaredAccessibility is Accessibility.Internal or Accessibility.Public or Accessibility.Friend
let order = GetOrder(ctor) ?? int.MaxValue
let parameters = ResolveMethodParameters(_buildContext.TypeResolver, buildStrategy, dependency, ctor, true).ToArray()
from ctor in
dependency.Implementation.Type is INamedTypeSymbol implementationType
? implementationType.Constructors
: Enumerable.Empty<IMethodSymbol>()
where !_buildContext.IsCancellationRequested
where ctor.DeclaredAccessibility is Accessibility.Internal or Accessibility.Public or Accessibility.Friend
let order = GetOrder(ctor) ?? int.MaxValue
let parameters = ResolveMethodParameters($"constructor {ctor} argument", _buildContext.TypeResolver, buildStrategy, dependency, ctor, true).ToArray()
let description = new StringBuilder(ctor.ToString())
let resolvedParams = parameters.Where(i => i.Expression.HasValue).ToArray()
let isResolved = parameters.Length == resolvedParams.Length
Expand All @@ -68,19 +68,18 @@ where ctor.DeclaredAccessibility is Accessibility.Internal or Accessibility.Publ
if (ctorInfo == default || _buildContext.IsCancellationRequested)
{
var errors = (
from ctorResolve in ctorInfos
from paramResolve in ctorResolve.parameters
let parameterDescription = paramResolve.Expression.HasValue ? string.Empty : $"{paramResolve.Target.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)} {paramResolve.Target.Name}"
where !string.IsNullOrWhiteSpace(parameterDescription)
select new CodeError(parameterDescription, paramResolve.Target.Locations.ToArray()))
from ctorResolve in ctorInfos
from paramResolve in ctorResolve.parameters
let parameterDescription = paramResolve.Expression.HasValue ? string.Empty : $"constructor {ctorResolve.ctor} argument {GetParameterDisplayName(paramResolve.Target)}"
where !string.IsNullOrWhiteSpace(parameterDescription)
select new [] {new CodeError(dependency, Diagnostics.Error.CannotResolve, parameterDescription, paramResolve.Target.Locations.ToArray())}.Concat(paramResolve.Expression.Errors)
)
.SelectMany(i => i)
.DefaultIfEmpty(new CodeError(dependency, Diagnostics.Error.CannotResolve, "Cannot found a constructor", dependency.Implementation.Type.Locations.ToArray()))
.ToArray();

var reasons = string.Join("; ", errors.Select(i => i.Description));
_log.Trace(() => new[]
{
$"[{stopwatch.ElapsedMilliseconds}] Cannot found a constructor for {dependency}: {reasons}."
});

_log.Trace(() => new[] { $"[{stopwatch.ElapsedMilliseconds}] Cannot find a constructor for {dependency}: {reasons}." });
_tracer.Save();
return Optional<ExpressionSyntax>.CreateEmpty(errors);
}
Expand Down Expand Up @@ -121,7 +120,7 @@ orderby order
{
case IMethodSymbol method:
var arguments =
from parameter in ResolveMethodParameters(_buildContext.TypeResolver, buildStrategy, dependency, method, false)
from parameter in ResolveMethodParameters($"method {member} argument", _buildContext.TypeResolver, buildStrategy, dependency, method, false)
where parameter.Expression.HasValue
select SyntaxFactory.Argument(parameter.Expression.Value);

Expand All @@ -139,7 +138,7 @@ where parameter.Expression.HasValue
case IFieldSymbol filed:
if (!filed.IsReadOnly && !filed.IsStatic && !filed.IsConst)
{
var fieldValue = ResolveInstance(filed, dependency, filed.Type, _buildContext.TypeResolver, buildStrategy, filed.Locations, false);
var fieldValue = ResolveInstance("instance field", filed, dependency, filed.Type, _buildContext.TypeResolver, buildStrategy, filed.Locations, false);
if (fieldValue.Expression.HasValue)
{
var assignmentExpression = SyntaxFactory.AssignmentExpression(
Expand All @@ -160,7 +159,7 @@ where parameter.Expression.HasValue
case IPropertySymbol property:
if (property.SetMethod != null && !property.IsReadOnly && !property.IsStatic)
{
var propertyValue = ResolveInstance(property, dependency, property.Type, _buildContext.TypeResolver, buildStrategy, property.Locations, false);
var propertyValue = ResolveInstance("instance property", property, dependency, property.Type, _buildContext.TypeResolver, buildStrategy, property.Locations, false);
if (propertyValue.Expression.HasValue)
{
var assignmentExpression = SyntaxFactory.AssignmentExpression(
Expand Down Expand Up @@ -206,19 +205,12 @@ where parameter.Expression.HasValue

return objectCreationExpression;
}

private static string CreateDescription(ISymbol ctor, IEnumerable<ResolveResult> results)
{
var unresolvedParameters = results
.Select(i => i.Expression.HasValue ? string.Empty : $"\"{i.Target.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)} {i.Target.Name}\"")
.Where(i => !string.IsNullOrWhiteSpace(i))
.ToArray();
var reason = string.Join(", ", unresolvedParameters);
var suf = unresolvedParameters.Length > 1 ? "s" : "";
return unresolvedParameters.Length > 0 ? $"the constructor {ctor.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)} has unresolved parameter{suf} {reason}" : string.Empty;
}

private static string GetParameterDisplayName(ISymbol parameter) =>
$"{parameter.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat)} {parameter.Name}";

private ResolveResult ResolveInstance(
string parameterTypeDescription,
ISymbol target,
Dependency targetDependency,
ITypeSymbol defaultType,
Expand Down Expand Up @@ -271,7 +263,20 @@ private ResolveResult ResolveInstance(

if (dependency.IsResolved)
{
return new ResolveResult(target, buildStrategy.TryBuild(dependency, resolvingType), ResolveType.Resolved, DefaultType.ResolvedValue);
try
{
return new ResolveResult(target, buildStrategy.TryBuild(dependency, resolvingType), ResolveType.Resolved, DefaultType.ResolvedValue);
}
catch (BuildException buildException)
{
if (!buildException.Dependency.Equals(targetDependency))
{
throw;
}

var parameterName = GetParameterDisplayName(target);
throw new BuildException(targetDependency, buildException.Id, $"{buildException.Message} when resolving {parameterTypeDescription} {parameterName}", target.Locations.ToArray());
}
}

if (probe)
Expand Down Expand Up @@ -310,9 +315,9 @@ private ResolveResult ResolveInstance(
}
}

private IEnumerable<ResolveResult> ResolveMethodParameters(ITypeResolver typeResolver, IBuildStrategy buildStrategy, Dependency dependency, IMethodSymbol method, bool probe) =>
private IEnumerable<ResolveResult> ResolveMethodParameters(string parameterTypeDescription, ITypeResolver typeResolver, IBuildStrategy buildStrategy, Dependency dependency, IMethodSymbol method, bool probe) =>
from parameter in method.Parameters
select ResolveInstance(parameter, dependency, parameter.Type, typeResolver, buildStrategy, parameter.Locations, probe);
select ResolveInstance(parameterTypeDescription, parameter, dependency, parameter.Type, typeResolver, buildStrategy, parameter.Locations, probe);

private SemanticType? GetDependencyType(ISymbol type, SemanticModel semanticModel) =>
(
Expand Down
7 changes: 5 additions & 2 deletions Pure.DI/Core/BuildException.cs
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
namespace Pure.DI.Core;

public class BuildException : Exception
internal class BuildException : Exception
{
public BuildException(string id, string message, params Location[] location)
public BuildException(Dependency dependency, string id, string message, params Location[] location)
: base(message)
{
Dependency = dependency;
Id = id;
Locations = location;
}

public Dependency Dependency { get; }

public string Id { get; }

public Location[] Locations { get; }
Expand Down
30 changes: 18 additions & 12 deletions Pure.DI/Core/BuildStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace Pure.DI.Core;
// ReSharper disable once ClassNeverInstantiated.Global
internal class BuildStrategy : IBuildStrategy
{
private const string Separator = "---";
private readonly IDiagnostic _diagnostic;
private readonly ITracer _tracer;
private readonly ILog<BuildStrategy> _log;
Expand Down Expand Up @@ -48,20 +49,25 @@ public Optional<ExpressionSyntax> TryBuildInternal(Dependency dependency, Semant
var objectBuildExpression = dependency.ObjectBuilder.TryBuild(this, dependency);
if (!objectBuildExpression.HasValue)
{
var path = _tracer.Paths
.Where(i => i.Length > 1)
.OrderBy(i => i.Length)
.LastOrDefault()
?.Reverse()
.Select(i => $"[{i}]")
.ToArray()
?? Array.Empty<string>();

var chain = string.Join("---", path);
var errors = objectBuildExpression.Errors;
if (chain.Length > 0)
if (errors.Any())
{
errors = objectBuildExpression.Errors.Select(i => new CodeError($"{i.Description}; the chain is {chain}", i.Locations)).ToArray();
var path = _tracer.Paths
.Where(i => i.Length > 1)
.OrderBy(i => i.Length)
.LastOrDefault()
?.Reverse()
.ToArray() ?? Array.Empty<Dependency>();

if (path.Any())
{
var chain = string.Join(Separator, path);
var last = path.Last();
errors = objectBuildExpression.Errors
.Where(error => error.Dependency.Equals(last))
.Select(error => new CodeError(error.Dependency, error.Id, error.Description.Contains(Separator) ? error.Description : $"{error.Description}, {chain}", error.Locations))
.ToArray();
}
}

return new Optional<ExpressionSyntax>(objectBuildExpression.Value, objectBuildExpression.HasValue, errors);
Expand Down
22 changes: 10 additions & 12 deletions Pure.DI/Core/CannotResolveExceptionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,14 @@ public CannotResolveExceptionFactory(IDiagnostic diagnostic) =>

public HandledException Create(IBindingMetadata binding, ExpressionSyntax? tag, CodeError[] errors)
{
var tagName = tag != default ? $"({tag})" : string.Empty;
var baseErrorMessage = $"Cannot resolve {binding.Implementation?.ToString() ?? binding.Factory?.ToString() ?? binding.ToString()}{tagName}";
errors = errors.Select(error =>
{
var errorMessage = new StringBuilder($"{baseErrorMessage}: {error.Description}.");
var newLocations = error.Locations.Concat(new[] { binding.Location }).Where(i => i != default).Select(i => i!).Distinct().ToArray();
return new CodeError(errorMessage.ToString(), newLocations);
}).ToArray();

_diagnostic.Error(Diagnostics.Error.CannotResolve, errors);
return new HandledException($"{baseErrorMessage}.");
_diagnostic.Error(
errors.Select(error =>
{
var newLocations = error.Locations.Concat(new[] { binding.Location }).Where(i => i != default).Select(i => i!).Distinct().ToArray();
return new CodeError(error.Dependency, error.Id, $"Cannot resolve {error.Dependency} {error.Description}", newLocations);
})
);

return new HandledException("Cannot resolve");
}
}
}
6 changes: 5 additions & 1 deletion Pure.DI/Core/CodeError.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
namespace Pure.DI.Core;

internal readonly record struct CodeError(string Description, params Location[] Locations)
internal readonly record struct CodeError(Dependency Dependency, string Id, string Description, params Location[] Locations)
{
public Dependency Dependency { get; } = Dependency;

public string Id { get; } = Id;

public string Description { get; } = Description;

public Location[] Locations { get; } = Locations;
Expand Down
15 changes: 12 additions & 3 deletions Pure.DI/Core/CompilationDiagnostic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,24 @@ internal class CompilationDiagnostic : IDiagnostic

public IExecutionContext? Context { get; set; }

public void Error(string id, params CodeError[] errors)
public void Error(IEnumerable<CodeError> errors)
{
foreach (var error in errors)
var curErrors = errors.ToList();
foreach (var error in curErrors)
{
Error(id, error.Description, error.Locations);
ErrorInternal(error.Id, error.Description, error.Locations);
}

throw new HandledException(string.Join(", ", curErrors.Select(i => i.Description)));
}

public void Error(string id, string message, params Location[] locations)
{
ErrorInternal(id, message, locations);
throw new HandledException(message);
}

private void ErrorInternal(string id, string message, params Location[] locations)
{
try
{
Expand Down
9 changes: 5 additions & 4 deletions Pure.DI/Core/DefaultDiagnostic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ public DefaultDiagnostic(IStdOut stdOut, IStdErr stdErr)
_stdErr = stdErr;
}

public void Error(string id, params CodeError[] errors)
public void Error(IEnumerable<CodeError> errors)
{
foreach (var error in errors)
var curErrors = errors.ToList();
foreach (var error in curErrors)
{
_stdErr.WriteErrorLine($"Error {id}: {error.Description}{GetLine(error.Locations)}");
_stdErr.WriteErrorLine($"Error {error.Id}: {error.Description}{GetLine(error.Locations)}");
}

throw new HandledException(string.Join(", ", errors.Select(i => i.Description)));
throw new HandledException(string.Join(", ", curErrors.Select(i => i.Description)));
}

public void Error(string id, string message, params Location[] locations)
Expand Down
2 changes: 1 addition & 1 deletion Pure.DI/Core/FactoryWrapperStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public ExpressionSyntax Build(SemanticType resolvingType, Dependency dependency,
var factoryType = baseFactoryType?.Construct(resolvingType.Type);
if (factoryType == null)
{
throw _cannotResolveExceptionFactory.Create(dependency.Binding, dependency.Tag, new CodeError[] { new($"cannot construct a factory of the type \"{resolvingType.Type}\"")});
throw _cannotResolveExceptionFactory.Create(dependency.Binding, dependency.Tag, new CodeError[] { new(dependency,Diagnostics.Error.CannotResolve, $"Cannot construct a factory of the type \"{resolvingType.Type}\"")});
}

if (dependency.Implementation.Type is INamedTypeSymbol { IsGenericType: true } namedTypeSymbol)
Expand Down
2 changes: 1 addition & 1 deletion Pure.DI/Core/IDiagnostic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

internal interface IDiagnostic
{
void Error(string id, params CodeError[] errors);
void Error(IEnumerable<CodeError> errors);

void Error(string id, string message, params Location[] locations);

Expand Down
10 changes: 6 additions & 4 deletions Pure.DI/Core/Optional.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ namespace Pure.DI.Core;
internal readonly record struct Optional<T>(T Value, bool HasValue = true, params CodeError[] Errors)
where T: class
{
private readonly CodeError[] _errors = Errors;

public T Value { get; } = Value;

public bool HasValue { get; } = HasValue;
public CodeError[] Errors { get; } = Errors;

public CodeError[] Errors => _errors ?? Array.Empty<CodeError>();

public static implicit operator Optional<T>(T value) => new(value);

public static Optional<T> CreateEmpty(params CodeError[] Errors) =>
new(default!, false, Errors);
public static Optional<T> CreateEmpty(params CodeError[] errors) =>
new(default!, false, errors);
}
Loading

0 comments on commit 5d9f386

Please sign in to comment.