From bccb5e689862fe8f33dd03f2ee561e393218b66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Tue, 16 Jan 2024 21:34:47 +0100 Subject: [PATCH 1/3] Refactor compilation diagnostics, add them to compilation status page This is a large commit of various interdependent changes with the simple goal of adding warnings to the compilation status page. * Non-critical errors from tokens are now reported as warnings. Before, they were ignored completely. * Added IDiagnosticsCompilationTracer interface - it allows watching the compilation process, for example observing changes made by various visitors or in this case getting the warnings when the compilation otherwise succeeds * Added DotvvmCompilationDiagnostic and DotvvmCompilationSourceLocation records - it designed to be a generic class for warnings and errors from the DotHTML compilation. It contains references to the syntax and resolved trees, so it should not be stored long-term, but provides all the possible detail about each reported error/warning. * BindingCompilationException is refactored to use the DotvvmCompilationDiagnostics and support any number of them. - plus, IDotvvmException compatibility is added - the exception will present as the "first" / primary error for better compatibility * ErrorCheckingVisitor is now smarter about binding errors, the BindingToken ranges are mapped into the DothtmlToken so we can underline only the affected part of the binding. This is somewhat important for displaying diagnostics of multiline bindings in the compilation page. Multiple errors can be collected from the error. * ViewCompilationService now collects the warnings and errors from the tracer and DotvvmCompilationException * And finally, the compilation displays up to 8 errors and warnings encountered during the page compilation under each row. Warnings tab was also added which shows only the views with at least one warning/error --- .../ControlTree/ControlTreeResolverBase.cs | 9 +- .../DefaultControlBuilderFactory.cs | 12 +- .../DiagnosticsCompilationTracer.cs | 97 +++++++++ .../Framework/Compilation/DotHtmlFileInfo.cs | 37 ++++ .../DotvvmCompilationDiagnostic.cs | 189 ++++++++++++++++++ .../Compilation/DotvvmCompilationException.cs | 130 +++++++++--- .../Compilation/DotvvmLocationInfo.cs | 1 + .../DotvvmViewCompilationService.cs | 96 ++++++++- .../Compilation/ErrorCheckingVisitor.cs | 152 ++++++++++++-- .../Parser/Binding/Tokenizer/BindingToken.cs | 11 + .../Compilation/Static/CompilationReport.cs | 71 ------- .../Static/DefaultCompilationReportLogger.cs | 6 +- .../Static/ICompilationReportLogger.cs | 2 +- .../Compilation/Static/StaticViewCompiler.cs | 22 +- .../ControlUsageValidationVisitor.cs | 49 +++-- .../ViewCompiler/DefaultViewCompiler.cs | 165 +++++++++------ .../DotvvmCompilationPageConfiguration.cs | 3 + .../Controls/DotvvmBindableObjectHelper.cs | 10 +- .../Framework/Controls/GridViewColumn.cs | 5 +- .../DotVVMServiceCollectionExtensions.cs | 3 + .../CompilationDiagnostic.dotcontrol | 14 ++ .../CompilationDiagnosticRows.dotcontrol | 26 +++ .../Diagnostics/CompilationPage.dothtml | 71 +++++-- .../Diagnostics/CompilationPageViewModel.cs | 3 + .../Framework/DotVVM.Framework.csproj | 2 + .../ErrorPages/DotvvmMarkupErrorSection.cs | 18 +- .../Hosting/ErrorPages/ErrorFormatter.cs | 61 +++++- .../Hosting/ErrorPages/ErrorPageTemplate.cs | 19 +- .../ReflectionAssemblyJsonConverter.cs | 18 ++ .../Resources/Styles/DotVVM.Internal.css | 47 ++++- .../Utils/ReferenceEqualityComparer.cs | 12 ++ src/Samples/Common/DotvvmStartup.cs | 1 + ...ing_ItemValueBinding_Complex_Error.dothtml | 2 +- .../Views/Errors/InvalidViewModel.dothtml | 2 +- .../NotAllowedHardCodedPropertyValue.dothtml | 2 +- .../LambdaExpressions.dothtml | 2 +- src/Samples/Tests/Tests/ErrorsTests.cs | 12 +- src/Tests/ControlTests/GridViewTests.cs | 2 +- .../ViewModulesServerSideTests.cs | 4 +- ...rTests.AuthView_InvalidWrapperTagUsage.txt | 4 +- ...sts.HtmlLiteral_InvalidWrapperTagUsage.txt | 4 +- src/Tests/Runtime/DefaultViewCompilerTests.cs | 4 +- src/Tests/Runtime/DotvvmControlErrorsTests.cs | 4 +- 43 files changed, 1096 insertions(+), 308 deletions(-) create mode 100644 src/Framework/Framework/Compilation/DiagnosticsCompilationTracer.cs create mode 100644 src/Framework/Framework/Compilation/DotvvmCompilationDiagnostic.cs delete mode 100644 src/Framework/Framework/Compilation/Static/CompilationReport.cs create mode 100644 src/Framework/Framework/Diagnostics/CompilationDiagnostic.dotcontrol create mode 100644 src/Framework/Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol create mode 100644 src/Framework/Framework/Utils/ReferenceEqualityComparer.cs diff --git a/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs b/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs index bd1eae2fca..6c4c325b50 100644 --- a/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs +++ b/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs @@ -180,11 +180,12 @@ protected virtual void ResolveRootContent(DothtmlRootNode root, IAbstractControl } catch (DotvvmCompilationException ex) { - if (ex.Tokens == null) + if (ex.Tokens is null) { - ex.Tokens = node.Tokens; - ex.ColumnNumber = node.Tokens.First().ColumnNumber; - ex.LineNumber = node.Tokens.First().LineNumber; + var oldLoc = ex.CompilationError.Location; + ex.CompilationError = ex.CompilationError with { + Location = new(oldLoc.FileName, oldLoc.MarkupFile, node.Tokens) + }; } if (!LogError(ex, node)) throw; diff --git a/src/Framework/Framework/Compilation/DefaultControlBuilderFactory.cs b/src/Framework/Framework/Compilation/DefaultControlBuilderFactory.cs index c184f5a10d..e0f43b53dc 100644 --- a/src/Framework/Framework/Compilation/DefaultControlBuilderFactory.cs +++ b/src/Framework/Framework/Compilation/DefaultControlBuilderFactory.cs @@ -106,16 +106,12 @@ public DefaultControlBuilderFactory(DotvvmConfiguration configuration, IMarkupFi var compilationService = configuration.ServiceProvider.GetService(); void editCompilationException(DotvvmCompilationException ex) { - if (ex.FileName == null) + var fileName = ex.FileName ?? file.FullPath; + if (!Path.IsPathRooted(fileName) && Path.IsPathRooted(file.FullPath)) { - ex.FileName = file.FullPath; - } - else if (!Path.IsPathRooted(ex.FileName)) - { - ex.FileName = Path.Combine( - file.FullPath.Remove(file.FullPath.Length - file.FileName.Length), - ex.FileName); + fileName = Path.Combine(file.FullPath.Remove(file.FullPath.Length - file.FileName.Length), fileName); } + ex.SetFile(fileName, file); } try { diff --git a/src/Framework/Framework/Compilation/DiagnosticsCompilationTracer.cs b/src/Framework/Framework/Compilation/DiagnosticsCompilationTracer.cs new file mode 100644 index 0000000000..01adcbe972 --- /dev/null +++ b/src/Framework/Framework/Compilation/DiagnosticsCompilationTracer.cs @@ -0,0 +1,97 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using DotVVM.Framework.Compilation.ControlTree.Resolved; +using DotVVM.Framework.Compilation.Parser.Dothtml.Parser; +using DotVVM.Framework.Compilation.Parser.Dothtml.Tokenizer; +using DotVVM.Framework.Compilation.ViewCompiler; + +namespace DotVVM.Framework.Compilation +{ + + public interface IDiagnosticsCompilationTracer + { + Handle CompilationStarted(string file, string sourceCode); + abstract class Handle + { + public virtual void Parsed(List tokens, DothtmlRootNode syntaxTree) { } + public virtual void Resolved(ResolvedTreeRoot tree, ControlBuilderDescriptor descriptor) { } + public virtual void AfterVisitor(ResolvedControlTreeVisitor visitor, ResolvedTreeRoot tree) { } + public virtual void CompilationDiagnostic(DotvvmCompilationDiagnostic diagnostic, string? contextLine) { } + public virtual void Failed(Exception exception) { } + } + sealed class NopHandle: Handle + { + private NopHandle() { } + public static readonly NopHandle Instance = new NopHandle(); + } + } + + public sealed class CompositeDiagnosticsCompilationTracer : IDiagnosticsCompilationTracer + { + readonly IDiagnosticsCompilationTracer[] tracers; + + public CompositeDiagnosticsCompilationTracer(IEnumerable tracers) + { + this.tracers = tracers.ToArray(); + } + + public IDiagnosticsCompilationTracer.Handle CompilationStarted(string file, string sourceCode) + { + var handles = this.tracers + .Select(t => t.CompilationStarted(file, sourceCode)) + .Where(t => t != IDiagnosticsCompilationTracer.NopHandle.Instance) + .ToArray(); + + + return handles.Length switch { + 0 => IDiagnosticsCompilationTracer.NopHandle.Instance, + 1 => handles[0], + _ => new Handle(handles) + }; + } + + sealed class Handle : IDiagnosticsCompilationTracer.Handle, IDisposable + { + private IDiagnosticsCompilationTracer.Handle[] handles; + + public Handle(IDiagnosticsCompilationTracer.Handle[] handles) + { + this.handles = handles; + } + + public override void AfterVisitor(ResolvedControlTreeVisitor visitor, ResolvedTreeRoot tree) + { + foreach (var h in handles) + h.AfterVisitor(visitor, tree); + } + public override void CompilationDiagnostic(DotvvmCompilationDiagnostic warning, string? contextLine) + { + foreach (var h in handles) + h.CompilationDiagnostic(warning, contextLine); + } + + + public override void Failed(Exception exception) + { + foreach (var h in handles) + h.Failed(exception); + } + public override void Parsed(List tokens, DothtmlRootNode syntaxTree) + { + foreach (var h in handles) + h.Parsed(tokens, syntaxTree); + } + public override void Resolved(ResolvedTreeRoot tree, ControlBuilderDescriptor descriptor) + { + foreach (var h in handles) + h.Resolved(tree, descriptor); + } + public void Dispose() + { + foreach (var h in handles) + (h as IDisposable)?.Dispose(); + } + } + } +} diff --git a/src/Framework/Framework/Compilation/DotHtmlFileInfo.cs b/src/Framework/Framework/Compilation/DotHtmlFileInfo.cs index 6a76358030..c9dee547c3 100644 --- a/src/Framework/Framework/Compilation/DotHtmlFileInfo.cs +++ b/src/Framework/Framework/Compilation/DotHtmlFileInfo.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using DotVVM.Framework.Binding.Properties; namespace DotVVM.Framework.Compilation { @@ -9,6 +10,9 @@ public sealed class DotHtmlFileInfo public CompilationState Status { get; internal set; } public string? Exception { get; internal set; } + public ImmutableArray Errors { get; internal set; } = ImmutableArray.Empty; + public ImmutableArray Warnings { get; internal set; } = ImmutableArray.Empty; + /// Gets or sets the virtual path to the view. public string VirtualPath { get; } @@ -46,5 +50,38 @@ private static bool IsDothtmlFile(string virtualPath) virtualPath.IndexOf(".dotlayout", StringComparison.OrdinalIgnoreCase) > -1 ); } + + public sealed record CompilationDiagnosticViewModel( + DiagnosticSeverity Severity, + string Message, + string? FileName, + int? LineNumber, + int? ColumnNumber, + string? SourceLine, + int? HighlightLength + ) + { + public string? SourceLine { get; set; } = SourceLine; + public string? SourceLinePrefix => SourceLine?.Remove(ColumnNumber ?? 0); + public string? SourceLineHighlight => + HighlightLength is {} len ? SourceLine?.Substring(ColumnNumber ?? 0, len) + : SourceLine?.Substring(ColumnNumber ?? 0); + public string? SourceLineSuffix => + (ColumnNumber + HighlightLength) is int startIndex ? SourceLine?.Substring(startIndex) : null; + + + public CompilationDiagnosticViewModel(DotvvmCompilationDiagnostic diagnostic, string? contextLine) + : this( + diagnostic.Severity, + diagnostic.Message, + diagnostic.Location.FileName, + diagnostic.Location.LineNumber, + diagnostic.Location.ColumnNumber, + contextLine, + diagnostic.Location.LineErrorLength + ) + { + } + } } } diff --git a/src/Framework/Framework/Compilation/DotvvmCompilationDiagnostic.cs b/src/Framework/Framework/Compilation/DotvvmCompilationDiagnostic.cs new file mode 100644 index 0000000000..b729cd2d68 --- /dev/null +++ b/src/Framework/Framework/Compilation/DotvvmCompilationDiagnostic.cs @@ -0,0 +1,189 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using DotVVM.Framework.Binding.Properties; +using DotVVM.Framework.Compilation.Parser; +using DotVVM.Framework.Hosting; +using System.Linq; +using DotVVM.Framework.Compilation.Parser.Dothtml.Parser; +using System; +using DotVVM.Framework.Compilation.ControlTree.Resolved; +using DotVVM.Framework.Binding; +using Newtonsoft.Json; +using DotVVM.Framework.Binding.Expressions; + +namespace DotVVM.Framework.Compilation +{ + /// Represents a dothtml compilation error or a warning, along with its location. + public record DotvvmCompilationDiagnostic: IEquatable + { + public DotvvmCompilationDiagnostic( + string message, + DiagnosticSeverity severity, + DotvvmCompilationSourceLocation? location, + IEnumerable? notes = null, + Exception? innerException = null) + { + Message = message; + Severity = severity; + Location = location ?? DotvvmCompilationSourceLocation.Unknown; + Notes = notes?.ToImmutableArray() ?? ImmutableArray.Empty; + InnerException = innerException; + } + + public string Message { get; init; } + public Exception? InnerException { get; init; } + public DiagnosticSeverity Severity { get; init; } + public DotvvmCompilationSourceLocation Location { get; init; } + public ImmutableArray Notes { get; init; } + /// Errors with lower number are preferred when selecting the primary fault to the user. When equal, errors are sorted based on the location. 0 is default for semantic errors, 100 for parser errors and 200 for tokenizer errors. + public int Priority { get; init; } + + public bool IsError => Severity == DiagnosticSeverity.Error; + public bool IsWarning => Severity == DiagnosticSeverity.Warning; + + public override string ToString() => + $"{Severity}: {Message}\n at {Location?.ToString() ?? "unknown location"}"; + } + + public sealed record DotvvmCompilationSourceLocation + { + public string? FileName { get; init; } + [JsonIgnore] + public MarkupFile? MarkupFile { get; init; } + [JsonIgnore] + public IEnumerable? Tokens { get; init; } + public int? LineNumber { get; init; } + public int? ColumnNumber { get; init; } + public int LineErrorLength { get; init; } + [JsonIgnore] + public DothtmlNode? RelatedSyntaxNode { get; init; } + [JsonIgnore] + public ResolvedTreeNode? RelatedResolvedNode { get; init; } + public DotvvmProperty? RelatedProperty { get; init; } + public IBinding? RelatedBinding { get; init; } + + public Type? RelatedControlType => this.RelatedResolvedNode?.GetAncestors(true).OfType().FirstOrDefault()?.Metadata.Type; + + public DotvvmCompilationSourceLocation( + string? fileName, + MarkupFile? markupFile, + IEnumerable? tokens, + int? lineNumber = null, + int? columnNumber = null, + int? lineErrorLength = null) + { + if (tokens is {}) + { + tokens = tokens.ToArray(); + lineNumber ??= tokens.FirstOrDefault()?.LineNumber; + columnNumber ??= tokens.FirstOrDefault()?.ColumnNumber; + lineErrorLength ??= tokens.Where(t => t.LineNumber == lineNumber).Select(t => (int?)(t.ColumnNumber + t.Length)).LastOrDefault() - columnNumber; + } + + this.MarkupFile = markupFile; + this.FileName = fileName ?? markupFile?.FileName; + this.Tokens = tokens; + this.LineNumber = lineNumber; + this.ColumnNumber = columnNumber; + this.LineErrorLength = lineErrorLength ?? 0; + } + + public DotvvmCompilationSourceLocation( + IEnumerable tokens): this(fileName: null, null, tokens) { } + public DotvvmCompilationSourceLocation( + DothtmlNode syntaxNode, IEnumerable? tokens = null) + : this(fileName: null, null, tokens ?? syntaxNode?.Tokens) + { + RelatedSyntaxNode = syntaxNode; + } + public DotvvmCompilationSourceLocation( + ResolvedTreeNode resolvedNode, DothtmlNode? syntaxNode = null, IEnumerable? tokens = null) + : this( + syntaxNode ?? resolvedNode.GetAncestors(true).FirstOrDefault(n => n.DothtmlNode is {})?.DothtmlNode!, + tokens + ) + { + RelatedResolvedNode = resolvedNode; + if (resolvedNode.GetAncestors().OfType().FirstOrDefault() is {} property) + RelatedProperty = property.Property; + } + + public static readonly DotvvmCompilationSourceLocation Unknown = new(fileName: null, null, null); + public bool IsUnknown => FileName is null && MarkupFile is null && Tokens is null && LineNumber is null && ColumnNumber is null; + + public string[] AffectedSpans + { + get + { + if (Tokens is null || !Tokens.Any()) + return Array.Empty(); + var ts = Tokens.ToArray(); + var r = new List { ts[0].Text }; + for (int i = 1; i < ts.Length; i++) + { + if (ts[i].StartPosition == ts[i - 1].EndPosition) + r[r.Count - 1] += ts[i].Text; + else + r.Add(ts[i].Text); + } + return r.ToArray(); + } + } + + public (int start, int end)[] AffectedRanges + { + get + { + if (Tokens is null || !Tokens.Any()) + return Array.Empty<(int, int)>(); + var ts = Tokens.ToArray(); + var r = new (int start, int end)[ts.Length]; + r[0] = (ts[0].StartPosition, ts[0].EndPosition); + int ri = 0; + for (int i = 1; i < ts.Length; i++) + { + if (ts[i].StartPosition == ts[i - 1].EndPosition) + r[i].end = ts[i].EndPosition; + else + { + ri += 1; + r[ri] = (ts[i].StartPosition, ts[i].EndPosition); + } + } + return r.AsSpan(0, ri + 1).ToArray(); + } + } + + public int? EndLineNumber => Tokens?.LastOrDefault()?.LineNumber ?? LineNumber; + public int? EndColumnNumber => (Tokens?.LastOrDefault()?.ColumnNumber + Tokens?.LastOrDefault()?.Length) ?? ColumnNumber; + + public override string ToString() + { + if (IsUnknown) + return "Unknown location"; + else if (FileName is {} && LineNumber is {}) + { + // MSBuild-style file location + return $"{FileName}({LineNumber}{(ColumnNumber is {} ? "," + ColumnNumber : "")})"; + } + else + { + // only position, plus add the affected spans + var location = + LineNumber is {} && ColumnNumber is {} ? $"{LineNumber},{ColumnNumber}: " : + LineNumber is {} ? $"{LineNumber}: " : + ""; + return $"{location}{string.Join("; ", AffectedSpans)}"; + } + } + + public DotvvmLocationInfo ToRuntimeLocation() => + new DotvvmLocationInfo( + this.FileName, + this.AffectedRanges, + this.LineNumber, + this.RelatedControlType, + this.RelatedProperty + ); + } +} diff --git a/src/Framework/Framework/Compilation/DotvvmCompilationException.cs b/src/Framework/Framework/Compilation/DotvvmCompilationException.cs index c1c4fef1ac..42019fd3ec 100644 --- a/src/Framework/Framework/Compilation/DotvvmCompilationException.cs +++ b/src/Framework/Framework/Compilation/DotvvmCompilationException.cs @@ -3,55 +3,90 @@ using System.IO; using System.Linq; using System.Runtime.Serialization; +using DotVVM.Framework.Binding; +using DotVVM.Framework.Binding.Expressions; +using DotVVM.Framework.Binding.Properties; +using DotVVM.Framework.Compilation.ControlTree.Resolved; using DotVVM.Framework.Compilation.Parser; +using DotVVM.Framework.Compilation.Parser.Dothtml.Parser; +using DotVVM.Framework.Controls; +using DotVVM.Framework.Hosting; +using DotVVM.Framework.ResourceManagement; +using DotVVM.Framework.Runtime; +using Newtonsoft.Json; namespace DotVVM.Framework.Compilation { + /// Represents a failed dotvvm compilation result. The exception contains a list of all errors and warnings (). For the exception message, one error is selected as the "primary", usually it's the first encountered error. [Serializable] - public class DotvvmCompilationException : Exception + public class DotvvmCompilationException : Exception, IDotvvmException { - - public string? FileName { get; set; } + public string? FileName + { + get => CompilationError.Location.FileName; + set => SetFile(value, null); + } + [JsonIgnore] + public MarkupFile? MarkupFile => CompilationError.Location.MarkupFile; + [JsonIgnore] public string? SystemFileName => FileName?.Replace('\\', Path.DirectorySeparatorChar).Replace('/', Path.DirectorySeparatorChar); - public IEnumerable? Tokens { get; set; } + /// Affected tokens of the primary compilation error. + [JsonIgnore] + public IEnumerable? Tokens => CompilationError.Location?.Tokens; + /// Line number of the primary compilation error. + public int? LineNumber => CompilationError.Location?.LineNumber; + /// Position on the line of the primary compilation error. + [JsonIgnore] + public int? ColumnNumber => CompilationError.Location?.ColumnNumber; - public int? ColumnNumber { get; set; } + /// Text of the affected tokens of the first error. + [JsonIgnore] + public string[] AffectedSpans => CompilationError.Location?.AffectedSpans ?? Array.Empty(); - public int? LineNumber { get; set; } + /// The primary compilation error. + public DotvvmCompilationDiagnostic CompilationError { get; set; } + /// All diagnostics except the primary compilation error. + public List OtherDiagnostics { get; } = new List(); - public string[] AffectedSpans - { - get - { - if (Tokens is null || !Tokens.Any()) return new string[0]; - var ts = Tokens.ToArray(); - var r = new List { ts[0].Text }; - for (int i = 1; i < ts.Length; i++) - { - if (ts[i].StartPosition == ts[i - 1].EndPosition) - r[r.Count - 1] += ts[i].Text; - else - r.Add(ts[i].Text); - } - return r.ToArray(); - } - } + [JsonIgnore] + public IEnumerable AllDiagnostics => Enumerable.Concat(new [] { CompilationError }, OtherDiagnostics); + [JsonIgnore] + public IEnumerable AllErrors => Enumerable.Concat(new [] { CompilationError }, OtherDiagnostics.Where(d => d.Severity == DiagnosticSeverity.Error)); + + Exception IDotvvmException.TheException => this; + DotvvmProperty? IDotvvmException.RelatedProperty => this.CompilationError.Location.RelatedProperty; - public DotvvmCompilationException(string message) : base(message) { } + DotvvmBindableObject? IDotvvmException.RelatedControl => null; - public DotvvmCompilationException(string message, Exception? innerException) : base(message, innerException) { } + IBinding? IDotvvmException.RelatedBinding => this.CompilationError.Location.RelatedBinding; + + ResolvedTreeNode? IDotvvmException.RelatedResolvedControl => this.CompilationError.Location.RelatedResolvedNode; + + DothtmlNode? IDotvvmException.RelatedDothtmlNode => this.CompilationError.Location.RelatedSyntaxNode; + + IResource? IDotvvmException.RelatedResource => null; + + DotvvmLocationInfo? IDotvvmException.Location => this.CompilationError.Location.ToRuntimeLocation(); + + public DotvvmCompilationException(string message) : this(message, innerException: null) { } + + public DotvvmCompilationException(string message, Exception? innerException) : base(message, innerException) + { + CompilationError = new DotvvmCompilationDiagnostic(message, DiagnosticSeverity.Error, null, innerException: innerException); + } public DotvvmCompilationException(string message, Exception? innerException, IEnumerable? tokens) : base(message, innerException) { - if (tokens != null) - { - if (!(tokens is IList)) tokens = tokens.ToArray(); - this.Tokens = tokens; - LineNumber = tokens.FirstOrDefault()?.LineNumber; - ColumnNumber = tokens.FirstOrDefault()?.ColumnNumber; - } + var location = tokens is null ? DotvvmCompilationSourceLocation.Unknown : new DotvvmCompilationSourceLocation(tokens); + CompilationError = new DotvvmCompilationDiagnostic(message, DiagnosticSeverity.Error, location, innerException: innerException); + } + + public DotvvmCompilationException(DotvvmCompilationDiagnostic primaryError, IEnumerable allDiagnostics) : base(primaryError.Message, primaryError.InnerException) + { + this.CompilationError = primaryError; + this.OtherDiagnostics = allDiagnostics.Where(d => (object)primaryError != d).ToList(); } public DotvvmCompilationException(string message, IEnumerable? tokens) : this(message, null, tokens) { } @@ -59,6 +94,37 @@ protected DotvvmCompilationException( SerializationInfo info, StreamingContext context) : base(info, context) { + CompilationError = new DotvvmCompilationDiagnostic(this.Message, DiagnosticSeverity.Error, null, innerException: this.InnerException); + } + + /// Creates a compilation error if the provided list of diagnostics contains an error. + public static DotvvmCompilationException? TryCreateFromDiagnostics(IEnumerable diagnostics) + { + // we sort by the end position of the error range to prefer more specific errors in case there is an overlap + // for example, binding have 2 errors, one for the entire binding and a more specific error highlighting the problematic binding token + var sorted = diagnostics.OrderBy(e => (-e.Priority, e.Location.EndLineNumber ?? int.MaxValue, e.Location.EndColumnNumber ?? int.MaxValue)).ToArray(); + if (sorted.FirstOrDefault(e => e.IsError) is {} error) + { + return new DotvvmCompilationException(error, sorted); + } + return null; + } + + public void SetFile(string? fileName, MarkupFile? file) + { + if (fileName == CompilationError.Location.FileName && file == CompilationError.Location.MarkupFile) + return; + + var oldFileName = CompilationError.Location.FileName; + CompilationError = CompilationError with { Location = CompilationError.Location with { FileName = fileName, MarkupFile = file } }; + + // also change other diagnostics, if they were from the same file name + for (int i = 0; i < OtherDiagnostics.Count; i++) + { + var d = OtherDiagnostics[i]; + if (d.Location.FileName == oldFileName) + OtherDiagnostics[i] = d with { Location = d.Location with { FileName = fileName, MarkupFile = file } }; + } } } } diff --git a/src/Framework/Framework/Compilation/DotvvmLocationInfo.cs b/src/Framework/Framework/Compilation/DotvvmLocationInfo.cs index eaa5553572..fac627eac0 100644 --- a/src/Framework/Framework/Compilation/DotvvmLocationInfo.cs +++ b/src/Framework/Framework/Compilation/DotvvmLocationInfo.cs @@ -6,6 +6,7 @@ namespace DotVVM.Framework.Compilation { /// /// Contains debug information about original binding location. + /// Used at runtime, so this object avoids referencing compile-time nodes to allow their garbage collection /// public sealed record DotvvmLocationInfo( string? FileName, diff --git a/src/Framework/Framework/Compilation/DotvvmViewCompilationService.cs b/src/Framework/Framework/Compilation/DotvvmViewCompilationService.cs index a88597dbe0..4eba593baa 100644 --- a/src/Framework/Framework/Compilation/DotvvmViewCompilationService.cs +++ b/src/Framework/Framework/Compilation/DotvvmViewCompilationService.cs @@ -1,27 +1,31 @@ -using System; +using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Threading; using System.Threading.Tasks; -using DotVVM.Framework.Compilation.Parser; using DotVVM.Framework.Configuration; -using DotVVM.Framework.Controls.Infrastructure; using Microsoft.Extensions.DependencyInjection; +using DotVVM.Framework.Hosting; using DotVVM.Framework.Utils; +using DotVVM.Framework.Binding.Properties; namespace DotVVM.Framework.Compilation { public class DotvvmViewCompilationService : IDotvvmViewCompilationService { - private IControlBuilderFactory controlBuilderFactory; + private readonly IControlBuilderFactory controlBuilderFactory; + private readonly CompilationTracer tracer; + private readonly IMarkupFileLoader markupFileLoader; private readonly DotvvmConfiguration dotvvmConfiguration; - public DotvvmViewCompilationService(DotvvmConfiguration dotvvmConfiguration, IControlBuilderFactory controlBuilderFactory) + public DotvvmViewCompilationService(DotvvmConfiguration dotvvmConfiguration, IControlBuilderFactory controlBuilderFactory, CompilationTracer tracer, IMarkupFileLoader markupFileLoader) { this.dotvvmConfiguration = dotvvmConfiguration; this.controlBuilderFactory = controlBuilderFactory; + this.tracer = tracer; + this.markupFileLoader = markupFileLoader; masterPages = new Lazy>(InitMasterPagesCollection); controls = new Lazy>(InitControls); routes = new Lazy>(InitRoutes); @@ -188,22 +192,102 @@ public void RegisterCompiledView(string file, ViewCompiler.ControlBuilderDescrip routes.Value.FirstOrDefault(t => t.VirtualPath == file) ?? controls.Value.FirstOrDefault(t => t.VirtualPath == file) ?? masterPages.Value.GetOrAdd(file, path => new DotHtmlFileInfo(path)); + + var tracerData = this.tracer.CompiledViews.GetValueOrDefault(file); + fileInfo.Exception = null; + + var diagnostics = tracerData?.Diagnostics ?? Enumerable.Empty(); + if (exception is null) { fileInfo.Status = CompilationState.CompletedSuccessfully; - fileInfo.Exception = null; } else { fileInfo.Status = CompilationState.CompilationFailed; fileInfo.Exception = exception.Message; + + if (exception is DotvvmCompilationException compilationException) + { + // overwrite the tracer diagnostics to avoid presenting duplicates + diagnostics = compilationException.AllDiagnostics.Select(x => new DotHtmlFileInfo.CompilationDiagnosticViewModel(x, null)).ToArray(); + + AddSourceLines(diagnostics, compilationException.AllDiagnostics); + } } + fileInfo.Errors = diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error).ToImmutableArray(); + fileInfo.Warnings = diagnostics.Where(d => d.Severity == DiagnosticSeverity.Warning).ToImmutableArray(); + if (descriptor?.MasterPage is { FileName: {} masterPagePath }) { masterPages.Value.GetOrAdd(masterPagePath, path => new DotHtmlFileInfo(path)); } } + + private void AddSourceLines(IEnumerable viewModels, IEnumerable originalDiagnostics) + { + var markupFiles = new Dictionary(); + foreach (var d in originalDiagnostics) + { + if (d.Location.FileName is null) + continue; + + markupFiles[d.Location.FileName] = markupFiles.GetValueOrDefault(d.Location.FileName) ?? d.Location.MarkupFile; + } + var sourceCodes = new Dictionary(); + foreach (var fileName in viewModels.Where(vm => vm.FileName is {} && vm.LineNumber is {} && vm.SourceLine is null).Select(vm => vm.FileName).Distinct()) + { + var markupFile = markupFiles.GetValueOrDefault(fileName!) ?? markupFileLoader.GetMarkup(this.dotvvmConfiguration, fileName!); + var sourceCode = markupFile?.ReadContent(); + if (sourceCode is {}) + sourceCodes.Add(fileName!, sourceCode.Split('\n')); + } + foreach (var d in viewModels) + { + if (d.FileName is null || d.LineNumber is not > 0 || d.SourceLine is {}) + continue; + var source = sourceCodes.GetValueOrDefault(d.FileName); + + if (source is null || d.LineNumber!.Value > source.Length) + continue; + + d.SourceLine = source[d.LineNumber.Value - 1]; + } + } + + public class CompilationTracer : IDiagnosticsCompilationTracer + { + internal readonly ConcurrentDictionary CompiledViews = new ConcurrentDictionary(); + public IDiagnosticsCompilationTracer.Handle CompilationStarted(string file, string sourceCode) + { + return new Handle(this, file); + } + + internal sealed class Handle : IDiagnosticsCompilationTracer.Handle, IDisposable + { + private readonly CompilationTracer compilationTracer; + public string File { get; } + public DateTime CompiledAt { get; } = DateTime.UtcNow; + public List Diagnostics = new(); + + public Handle(CompilationTracer compilationTracer, string file) + { + this.compilationTracer = compilationTracer; + this.File = file; + } + + public override void CompilationDiagnostic(DotvvmCompilationDiagnostic diagnostic, string? contextLine) + { + Diagnostics.Add(new (diagnostic, contextLine)); + } + + public void Dispose() + { + compilationTracer.CompiledViews[this.File] = this; + } + } + } } } diff --git a/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs b/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs index eb3002182a..248b0b1915 100644 --- a/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs +++ b/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs @@ -1,21 +1,95 @@ using System; +using System.Collections.Generic; using System.Linq; +using DotVVM.Framework.Binding; +using DotVVM.Framework.Binding.Properties; +using DotVVM.Framework.Compilation.Binding; using DotVVM.Framework.Compilation.ControlTree.Resolved; +using DotVVM.Framework.Compilation.Parser; +using DotVVM.Framework.Compilation.Parser.Binding.Tokenizer; using DotVVM.Framework.Compilation.Parser.Dothtml.Parser; +using DotVVM.Framework.Compilation.Parser.Dothtml.Tokenizer; +using DotVVM.Framework.Hosting; using DotVVM.Framework.Utils; namespace DotVVM.Framework.Compilation { public class ErrorCheckingVisitor : ResolvedControlTreeVisitor { + public List Diagnostics { get; } = new(); + public string? FileName { get; set; } - public override void VisitControl(ResolvedControl control) + public ErrorCheckingVisitor(string? fileName) { - if (control.DothtmlNode is { HasNodeErrors: true }) + this.FileName = fileName; + } + + private void AddNodeErrors(DothtmlNode node, int priority) + { + if (!node.HasNodeErrors && !node.HasNodeWarnings) + return; + + var location = new DotvvmCompilationSourceLocation(node) { FileName = FileName }; + foreach (var error in node.NodeErrors) + { + Diagnostics.Add(new DotvvmCompilationDiagnostic(error, DiagnosticSeverity.Error, location) { Priority = priority }); + } + foreach (var warning in node.NodeWarnings) + { + Diagnostics.Add(new DotvvmCompilationDiagnostic(warning, DiagnosticSeverity.Warning, location)); + } + } + + DotvvmCompilationSourceLocation? MapBindingLocation(ResolvedBinding binding, DotvvmProperty? relatedProperty, BindingCompilationException error) + { + var tokens = error.Tokens?.ToArray(); + if (tokens is null or {Length:0}) + return null; + + var valueNode = (binding.BindingNode as DothtmlBindingNode)?.ValueNode; + var valueToken = valueNode?.ValueToken; + + if (valueToken is null) + { + // create anonymous file for the one binding + var file = new MarkupFile("anonymous binding", "anonymos binding", error.Expression ?? ""); + return new DotvvmCompilationSourceLocation(file.FileName, file, tokens) { RelatedSyntaxNode = valueNode ?? binding.DothtmlNode, RelatedResolvedNode = binding, RelatedBinding = binding.Binding, RelatedProperty = relatedProperty }; + } + else { - throw new DotvvmCompilationException(string.Join("\r\n", control.DothtmlNode.NodeErrors), control.DothtmlNode.Tokens); + tokens = tokens.Select(t => t switch { + BindingToken bt => bt.RemapPosition(valueToken), + _ => t // dothtml tokens most likely already have correct position + }).ToArray(); + return new DotvvmCompilationSourceLocation(binding, valueNode, tokens) { RelatedBinding = binding.Binding, RelatedProperty = relatedProperty }; } - base.VisitControl(control); + } + + Dictionary AnnotateBindingExceptionWithLocation(ResolvedBinding binding, DotvvmProperty? relatedProperty, IEnumerable errors) + { + var result = new Dictionary(new ReferenceEqualityComparer()); + void recurse(Exception exception, DotvvmCompilationSourceLocation? location) + { + if (result.ContainsKey(exception)) + return; + + if (exception is BindingCompilationException bce) + location = MapBindingLocation(binding, relatedProperty, bce) ?? location; + + if (location is {}) + result[exception] = location; + + if (exception is AggregateException agg) + foreach (var inner in agg.InnerExceptions) + recurse(inner, location); + else if (exception.InnerException is {}) + recurse(exception.InnerException, location); + } + + foreach (var x in errors) + recurse(x, null); + + return result; } public override void VisitPropertyBinding(ResolvedPropertyBinding propertyBinding) @@ -23,29 +97,75 @@ public override void VisitPropertyBinding(ResolvedPropertyBinding propertyBindin var errors = propertyBinding.Binding.Errors; if (errors.HasErrors) { - // TODO: aggregate all errors from the page - throw new DotvvmCompilationException( + var bindingLocation = new DotvvmCompilationSourceLocation(propertyBinding.Binding, propertyBinding.Binding.BindingNode); + var detailedLocations = AnnotateBindingExceptionWithLocation(propertyBinding.Binding, propertyBinding.Property, errors.Exceptions); + foreach (var error in + from topException in errors.Exceptions + from exception in topException.AllInnerExceptions() + where exception is not AggregateException and not BindingPropertyException { InnerException: {} } and not BindingCompilationException { InnerException: {}, Message: "Binding compilation failed" } + let location = detailedLocations.GetValueOrDefault(exception) + let message = exception.Message + orderby location?.LineNumber ?? int.MaxValue, + location?.ColumnNumber ?? int.MaxValue, + location?.LineErrorLength ?? int.MaxValue, + exception.InnerException is null ? 0 : exception is BindingCompilationException ? -1 : 2 + group (topException, exception, location, message) by message into g + select g.First()) + { + var message = $"{error.exception.GetType().Name}: {error.message}"; + Diagnostics.Add(new DotvvmCompilationDiagnostic( + message, + DiagnosticSeverity.Error, + error.location ?? bindingLocation, + innerException: error.topException + )); + } + // summary error explaining which binding properties are causing the problem + Diagnostics.Add(new DotvvmCompilationDiagnostic( errors.GetErrorMessage(propertyBinding.Binding.Binding), - errors.Exceptions.FirstOrDefault(), - propertyBinding.Binding.BindingNode?.Tokens); + DiagnosticSeverity.Error, + bindingLocation, + innerException: errors.Exceptions.FirstOrDefault() + )); } base.VisitPropertyBinding(propertyBinding); } public override void VisitView(ResolvedTreeRoot view) { - if (view.DothtmlNode.HasNodeErrors) - { - throw new DotvvmCompilationException(string.Join("\r\n", view.DothtmlNode.NodeErrors), view.DothtmlNode.Tokens); - } - foreach (var directive in ((DothtmlRootNode) view.DothtmlNode).Directives) + base.VisitView(view); + } + + public void AddTokenizerErrors(List tokens) + { + foreach (var token in tokens) { - if (directive.HasNodeErrors) + if (token.Error is { IsCritical: var critical }) { - throw new DotvvmCompilationException(string.Join("\r\n", directive.NodeErrors), directive.Tokens); + var location = new DotvvmCompilationSourceLocation(new[] { (token.Error as BeginWithLastTokenOfTypeTokenError)?.LastToken ?? token }); + Diagnostics.Add(new DotvvmCompilationDiagnostic( + token.Error.ErrorMessage, + critical ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning, + location + ) { Priority = 200 }); } } - base.VisitView(view); + } + public void AddSyntaxErrors(DothtmlNode rootNode) + { + foreach (var node in rootNode.EnumerateNodes()) + { + AddNodeErrors(node, priority: 100); + } + } + + public void ThrowOnErrors() + { + var sorted = Diagnostics.OrderBy(e => (-e.Priority, e.Location.LineNumber ?? -1, e.Location.ColumnNumber ?? -1)).ToArray(); + if (sorted.FirstOrDefault(e => e.IsError) is {} error) + { + throw new DotvvmCompilationException(error, sorted); + } } } } diff --git a/src/Framework/Framework/Compilation/Parser/Binding/Tokenizer/BindingToken.cs b/src/Framework/Framework/Compilation/Parser/Binding/Tokenizer/BindingToken.cs index 46bce1f896..7529739f95 100644 --- a/src/Framework/Framework/Compilation/Parser/Binding/Tokenizer/BindingToken.cs +++ b/src/Framework/Framework/Compilation/Parser/Binding/Tokenizer/BindingToken.cs @@ -5,5 +5,16 @@ public class BindingToken : TokenBase public BindingToken(string text, BindingTokenType type, int lineNumber, int columnNumber, int length, int startPosition) : base(text, type, lineNumber, columnNumber, length, startPosition) { } + + public BindingToken RemapPosition(TokenBase parentToken) => + RemapPosition(parentToken.LineNumber, parentToken.ColumnNumber, parentToken.StartPosition); + public BindingToken RemapPosition(int startLine, int startColumn, int startPosition) + { + return new BindingToken(Text, Type, + startLine + this.LineNumber - 1, + this.LineNumber <= 1 ? startColumn + this.ColumnNumber : this.ColumnNumber, + Length, + this.StartPosition + startPosition); + } } } diff --git a/src/Framework/Framework/Compilation/Static/CompilationReport.cs b/src/Framework/Framework/Compilation/Static/CompilationReport.cs deleted file mode 100644 index 0cc58f3051..0000000000 --- a/src/Framework/Framework/Compilation/Static/CompilationReport.cs +++ /dev/null @@ -1,71 +0,0 @@ - -using System.Collections.Generic; - -namespace DotVVM.Framework.Compilation.Static -{ - internal class CompilationReport - { - private const string UnknownError = "An unknown error occurred. This is likely a bug in the compiler."; - - public CompilationReport(string viewPath, int line, int column, string message) - { - ViewPath = viewPath; - Line = line; - Column = column; - Message = message; - } - - public CompilationReport(string viewPath, DotvvmCompilationException exception) - : this( - viewPath: viewPath, - line: exception.LineNumber ?? -1, - column: exception.ColumnNumber ?? -1, - message: !string.IsNullOrEmpty(exception.Message) - ? exception.Message - : exception.InnerException?.ToString() ?? UnknownError) - { - } - - public string Message { get; } - - public int Line { get; } - - public int Column { get; } - - public string ViewPath { get; } - - public static bool operator ==(CompilationReport? left, CompilationReport? right) - { - if (left is null) - { - return right is null; - } - - return left.Equals(right); - } - - public static bool operator !=(CompilationReport? left, CompilationReport? right) - { - return !(left == right); - } - - public override bool Equals(object? obj) - { - return obj is CompilationReport report - && Message == report.Message - && Line == report.Line - && Column == report.Column - && ViewPath == report.ViewPath; - } - - public override int GetHashCode() - { - var hashCode = -712964631; - hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(Message); - hashCode = hashCode * -1521134295 + Line.GetHashCode(); - hashCode = hashCode * -1521134295 + Column.GetHashCode(); - hashCode = hashCode * -1521134295 + EqualityComparer.Default.GetHashCode(ViewPath); - return hashCode; - } - } -} diff --git a/src/Framework/Framework/Compilation/Static/DefaultCompilationReportLogger.cs b/src/Framework/Framework/Compilation/Static/DefaultCompilationReportLogger.cs index 50422b230a..445d07cca6 100644 --- a/src/Framework/Framework/Compilation/Static/DefaultCompilationReportLogger.cs +++ b/src/Framework/Framework/Compilation/Static/DefaultCompilationReportLogger.cs @@ -7,12 +7,12 @@ namespace DotVVM.Framework.Compilation.Static { internal class DefaultCompilationReportLogger : ICompilationReportLogger { - public void Log(Stream stream, IEnumerable reports) + public void Log(Stream stream, IEnumerable diagnostics) { using var writer = new StreamWriter(stream); - foreach (var report in reports) + foreach (var d in diagnostics) { - writer.WriteLine($"{report.ViewPath}({report.Line},{report.Column}): {report.Message}"); + writer.WriteLine($"{d.Location}: {d.Severity.ToString().ToLowerInvariant()}: {d.Message}"); } } } diff --git a/src/Framework/Framework/Compilation/Static/ICompilationReportLogger.cs b/src/Framework/Framework/Compilation/Static/ICompilationReportLogger.cs index 062f0bf1ff..51302d35c8 100644 --- a/src/Framework/Framework/Compilation/Static/ICompilationReportLogger.cs +++ b/src/Framework/Framework/Compilation/Static/ICompilationReportLogger.cs @@ -6,6 +6,6 @@ namespace DotVVM.Framework.Compilation.Static { internal interface ICompilationReportLogger { - void Log(Stream stream, IEnumerable reports); + void Log(Stream stream, IEnumerable diagnostics); } } diff --git a/src/Framework/Framework/Compilation/Static/StaticViewCompiler.cs b/src/Framework/Framework/Compilation/Static/StaticViewCompiler.cs index c758d26e41..c9a4fe7394 100644 --- a/src/Framework/Framework/Compilation/Static/StaticViewCompiler.cs +++ b/src/Framework/Framework/Compilation/Static/StaticViewCompiler.cs @@ -18,31 +18,31 @@ namespace DotVVM.Framework.Compilation.Static { internal static class StaticViewCompiler { - public static ImmutableArray CompileAll( + public static ImmutableArray CompileAll( Assembly dotvvmProjectAssembly, string dotvvmProjectDir) { var configuration = ConfigurationInitializer.GetConfiguration(dotvvmProjectAssembly, dotvvmProjectDir); - var reportsBuilder = ImmutableArray.CreateBuilder(); + var diagnostics = ImmutableArray.CreateBuilder(); var markupControls = configuration.Markup.Controls.Select(c => c.Src) .Where(p => !string.IsNullOrWhiteSpace(p)) .ToImmutableArray(); foreach (var markupControl in markupControls) { - reportsBuilder.AddRange(CompileNoThrow(configuration, markupControl!)); + diagnostics.AddRange(CompileNoThrow(configuration, markupControl!)); } var views = configuration.RouteTable.Select(r => r.VirtualPath).ToImmutableArray(); foreach(var view in views) { - reportsBuilder.AddRange(CompileNoThrow(configuration, view)); + diagnostics.AddRange(CompileNoThrow(configuration, view)); } - return reportsBuilder.Distinct().ToImmutableArray(); + return diagnostics.Distinct().ToImmutableArray(); } - private static ImmutableArray CompileNoThrow( + private static ImmutableArray CompileNoThrow( DotvvmConfiguration configuration, string viewPath) { @@ -50,7 +50,7 @@ private static ImmutableArray CompileNoThrow( var file = fileLoader.GetMarkup(configuration, viewPath); if (file is null) { - return ImmutableArray.Create(); + return ImmutableArray.Create(); } var sourceCode = file.ReadContent(); @@ -62,14 +62,12 @@ private static ImmutableArray CompileNoThrow( sourceCode: sourceCode, fileName: viewPath); _ = builderFactory(); - // TODO: Reporting compiler errors solely through exceptions is dumb. I have no way of getting all of - // the parser errors at once because they're reported through exceptions one at a time. We need - // to rewrite DefaultViewCompiler and its interface if the static linter/compiler is to be useful. - return ImmutableArray.Create(); + // TODO: get warnings from compilation tracer + return ImmutableArray.Create(); } catch(DotvvmCompilationException e) { - return ImmutableArray.Create(new CompilationReport(viewPath, e)); + return e.AllDiagnostics.ToImmutableArray(); } } } diff --git a/src/Framework/Framework/Compilation/Validation/ControlUsageValidationVisitor.cs b/src/Framework/Framework/Compilation/Validation/ControlUsageValidationVisitor.cs index 20d9d476ef..7e3a2c3c2a 100644 --- a/src/Framework/Framework/Compilation/Validation/ControlUsageValidationVisitor.cs +++ b/src/Framework/Framework/Compilation/Validation/ControlUsageValidationVisitor.cs @@ -3,12 +3,14 @@ using System.Linq; using DotVVM.Framework.Binding.Properties; using DotVVM.Framework.Compilation.ControlTree.Resolved; +using FastExpressionCompiler; namespace DotVVM.Framework.Compilation.Validation { public class ControlUsageValidationVisitor: ResolvedControlTreeVisitor { - public List<(ResolvedControl control, ControlUsageError err)> Errors { get; set; } = new List<(ResolvedControl, ControlUsageError)>(); + public List Errors { get; set; } = new(); + public bool WriteErrorsToNodes { get; set; } = true; readonly IControlUsageValidator validator; public ControlUsageValidationVisitor(IControlUsageValidator validator) { @@ -20,19 +22,32 @@ public override void VisitControl(ResolvedControl control) var err = validator.Validate(control); foreach (var e in err) { - Errors.Add((control, e)); - foreach (var node in e.Nodes) + var location = new DotvvmCompilationSourceLocation(control, e.Nodes.FirstOrDefault(), e.Nodes.SelectMany(n => n.Tokens)); + var msgPrefix = $"{control.Metadata.Type.ToCode(stripNamespace: true)} validation"; + if (location.LineNumber is {}) { - switch (e.Severity) + msgPrefix += $" at line {location.LineNumber}"; + } + Errors.Add(new DotvvmCompilationDiagnostic( + msgPrefix + ": " + e.ErrorMessage, + e.Severity, + location + ) { Priority = -1 }); + if (this.WriteErrorsToNodes) + { + foreach (var node in e.Nodes) { - case DiagnosticSeverity.Error: - node.AddError(e.ErrorMessage); - break; - case DiagnosticSeverity.Warning: - node.AddWarning(e.ErrorMessage); - break; - default: - break; + switch (e.Severity) + { + case DiagnosticSeverity.Error: + node.AddError(e.ErrorMessage); + break; + case DiagnosticSeverity.Warning: + node.AddWarning(e.ErrorMessage); + break; + default: + break; + } } } } @@ -44,15 +59,9 @@ public void VisitAndAssert(ResolvedTreeRoot view) { if (this.Errors.Any()) throw new Exception("The ControlUsageValidationVisitor has already collected some errors."); VisitView(view); - if (this.Errors.FirstOrDefault(e => e.err.Severity == DiagnosticSeverity.Error) is { err: {} } controlUsageError) + if (this.Errors.FirstOrDefault(e => e.Severity == DiagnosticSeverity.Error) is { } controlUsageError) { - var lineNumber = - controlUsageError.control.GetAncestors() - .Select(c => c.DothtmlNode) - .FirstOrDefault(n => n != null) - ?.Tokens.FirstOrDefault()?.LineNumber; - var message = $"Validation error in {controlUsageError.control.Metadata.Type.Name} at line {lineNumber}: {controlUsageError.err.ErrorMessage}"; - throw new DotvvmCompilationException(message, controlUsageError.err.Nodes.SelectMany(n => n.Tokens)); + throw new DotvvmCompilationException(controlUsageError, this.Errors); } } } diff --git a/src/Framework/Framework/Compilation/ViewCompiler/DefaultViewCompiler.cs b/src/Framework/Framework/Compilation/ViewCompiler/DefaultViewCompiler.cs index 5c67f77199..9e13d6909c 100644 --- a/src/Framework/Framework/Compilation/ViewCompiler/DefaultViewCompiler.cs +++ b/src/Framework/Framework/Compilation/ViewCompiler/DefaultViewCompiler.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using DotVVM.Framework.Binding.Properties; using DotVVM.Framework.Compilation.ControlTree; using DotVVM.Framework.Compilation.ControlTree.Resolved; using DotVVM.Framework.Compilation.Parser; @@ -20,11 +21,13 @@ public class DefaultViewCompiler : IViewCompiler private readonly IBindingCompiler bindingCompiler; private readonly ViewCompilerConfiguration config; private readonly Func controlValidatorFactory; + private readonly CompositeDiagnosticsCompilationTracer tracer; private readonly ILogger? logger; - public DefaultViewCompiler(IOptions config, IControlTreeResolver controlTreeResolver, IBindingCompiler bindingCompiler, Func controlValidatorFactory, ILogger? logger = null) + public DefaultViewCompiler(IOptions config, IControlTreeResolver controlTreeResolver, IBindingCompiler bindingCompiler, Func controlValidatorFactory, CompositeDiagnosticsCompilationTracer tracer, ILogger? logger = null) { this.config = config.Value; + this.tracer = tracer; this.controlTreeResolver = controlTreeResolver; this.bindingCompiler = bindingCompiler; this.controlValidatorFactory = controlValidatorFactory; @@ -36,88 +39,127 @@ public DefaultViewCompiler(IOptions config, IControlT /// protected virtual (ControlBuilderDescriptor, Func>) CompileViewCore(string sourceCode, string fileName) { - // parse the document - var tokenizer = new DothtmlTokenizer(); - tokenizer.Tokenize(sourceCode); - var parser = new DothtmlParser(); - var node = parser.Parse(tokenizer.Tokens); - - var resolvedView = (ResolvedTreeRoot)controlTreeResolver.ResolveTree(node, fileName); - - var descriptor = resolvedView.ControlBuilderDescriptor; + var tracingHandle = tracer.CompilationStarted(fileName, sourceCode); + bool faultBlockHack(Exception e) + { + // avoids rethrowing exception and triggering the debugger by abusing + // the `filter` block to report the error + tracingHandle.Failed(e); + return false; + } + try + { + // parse the document + var tokenizer = new DothtmlTokenizer(); + tokenizer.Tokenize(sourceCode); + var parser = new DothtmlParser(); + var node = parser.Parse(tokenizer.Tokens); + tracingHandle.Parsed(tokenizer.Tokens, node); - return (descriptor, () => { + var resolvedView = (ResolvedTreeRoot)controlTreeResolver.ResolveTree(node, fileName); - var errorCheckingVisitor = new ErrorCheckingVisitor(); - resolvedView.Accept(errorCheckingVisitor); + var descriptor = resolvedView.ControlBuilderDescriptor; - foreach (var token in tokenizer.Tokens) - { - if (token.Error is { IsCritical: true }) + return (descriptor, () => { + try { - throw new DotvvmCompilationException(token.Error.ErrorMessage, new[] { (token.Error as BeginWithLastTokenOfTypeTokenError)?.LastToken ?? token }); - } - } + tracingHandle.Resolved(resolvedView, descriptor); + + // avoid visiting invalid tree, it could trigger crashes in styles + CheckErrors(fileName, sourceCode, tracingHandle, tokenizer.Tokens, node, resolvedView); + + foreach (var visitor in config.TreeVisitors) + { + var v = visitor(); + try + { + resolvedView.Accept(v); + tracingHandle.AfterVisitor(v, resolvedView); + } + finally + { + (v as IDisposable)?.Dispose(); + } + } - foreach (var n in node.EnumerateNodes()) - { - if (n.HasNodeErrors) - { - throw new DotvvmCompilationException(string.Join(", ", n.NodeErrors), n.Tokens); - } - } + var validationVisitor = this.controlValidatorFactory.Invoke(); + validationVisitor.WriteErrorsToNodes = false; + validationVisitor.DefaultVisit(resolvedView); + + // validate tree again for new errors from the visitors and warnings + var diagnostics = CheckErrors(fileName, sourceCode, tracingHandle, tokenizer.Tokens, node, resolvedView, additionalDiagnostics: validationVisitor.Errors); + LogDiagnostics(tracingHandle, diagnostics, fileName, sourceCode); - foreach (var visitor in config.TreeVisitors) - visitor().ApplyAction(resolvedView.Accept).ApplyAction(v => (v as IDisposable)?.Dispose()); + var emitter = new DefaultViewCompilerCodeEmitter(); + var compilingVisitor = new ViewCompilingVisitor(emitter, bindingCompiler); + resolvedView.Accept(compilingVisitor); - var validationVisitor = this.controlValidatorFactory.Invoke(); - validationVisitor.VisitAndAssert(resolvedView); - LogWarnings(resolvedView, sourceCode); + return compilingVisitor.BuildCompiledView; + } + catch (Exception e) when (faultBlockHack(e)) { throw; } + finally + { + (tracingHandle as IDisposable)?.Dispose(); + } + }); + } + catch (Exception e) when (faultBlockHack(e)) { throw; } + } - var emitter = new DefaultViewCompilerCodeEmitter(); - var compilingVisitor = new ViewCompilingVisitor(emitter, bindingCompiler); + private List CheckErrors(string fileName, string sourceCode, IDiagnosticsCompilationTracer.Handle tracingHandle, List tokens, DothtmlNode syntaxTree, ResolvedTreeRoot? resolvedTree, IEnumerable? additionalDiagnostics = null) + { + var errorChecker = new ErrorCheckingVisitor(fileName); + errorChecker.AddTokenizerErrors(tokens); + errorChecker.AddSyntaxErrors(syntaxTree); + resolvedTree?.Accept(errorChecker); - resolvedView.Accept(compilingVisitor); + if (additionalDiagnostics is { }) + { + errorChecker.Diagnostics.AddRange(additionalDiagnostics); + } - return compilingVisitor.BuildCompiledView; - }); + if (DotvvmCompilationException.TryCreateFromDiagnostics(errorChecker.Diagnostics) is {} error) + { + LogDiagnostics(tracingHandle, error.AllDiagnostics, fileName, sourceCode); + throw error; + } + return errorChecker.Diagnostics; } - private void LogWarnings(ResolvedTreeRoot resolvedView, string sourceCode) + private void LogDiagnostics(IDiagnosticsCompilationTracer.Handle tracingHandle, IEnumerable diagnostics, string fileName, string sourceCode) { - string[]? lines = null; - if (logger is null || resolvedView.DothtmlNode is null) return; + var warnings = diagnostics.Where(d => d.IsWarning || d.IsError).ToArray(); + if (warnings.Length == 0) return; + + var lines = sourceCode.Split('\n'); // Currently, all warnings are placed on syntax nodes (even when produced in control tree resolver) - foreach (var node in resolvedView.DothtmlNode.EnumerateNodes()) + foreach (var warning in warnings) { - if (node.HasNodeWarnings) - { - lines ??= sourceCode.Split('\n'); - var nodePosition = node.Tokens.FirstOrDefault(); - var sourceLine = nodePosition is { LineNumber: > 0 } && nodePosition.LineNumber <= lines.Length ? lines[nodePosition.LineNumber - 1] : null; - sourceLine = sourceLine?.TrimEnd(); - var highlightLength = 1; - if (sourceLine is {} && nodePosition is {}) - { - highlightLength = node.Tokens.Where(t => t.LineNumber == nodePosition?.LineNumber).Sum(t => t.Length); - highlightLength = Math.Max(1, Math.Min(highlightLength, sourceLine.Length - nodePosition.ColumnNumber + 1)); - } + var loc = warning.Location; + var sourceLine = loc.LineNumber > 0 && loc.LineNumber <= lines.Length ? lines[loc.LineNumber.Value - 1] : null; + sourceLine = sourceLine?.TrimEnd(); - foreach (var warning in node.NodeWarnings) - { - var logEvent = new CompilationWarning(warning, resolvedView.FileName, nodePosition?.LineNumber, nodePosition?.ColumnNumber, sourceLine, highlightLength); - logger.Log(LogLevel.Warning, 0, logEvent, null, (x, e) => x.ToString()); - } + var highlightLength = 1; + if (sourceLine is {} && loc is { ColumnNumber: {}, LineErrorLength: > 0 }) + { + highlightLength = loc.LineErrorLength; + highlightLength = Math.Max(1, Math.Min(highlightLength, sourceLine.Length - loc.ColumnNumber.Value + 1)); } + + var logEvent = new CompilationDiagnosticLogEvent(warning.Severity, warning.Message, fileName, loc.LineNumber, loc.ColumnNumber, sourceLine, highlightLength); + logger?.Log(warning.IsWarning ? LogLevel.Warning : LogLevel.Error, 0, logEvent, null, (x, e) => x.ToString()); + + tracingHandle.CompilationDiagnostic(warning, sourceLine); } } // custom log event implementing IEnumerable> for Serilog properties - private readonly struct CompilationWarning : IEnumerable> + private readonly struct CompilationDiagnosticLogEvent : IEnumerable> { - public CompilationWarning(string message, string? fileName, int? lineNumber, int? charPosition, string? contextLine, int highlightLength) + public CompilationDiagnosticLogEvent(DiagnosticSeverity severity, string message, string? fileName, int? lineNumber, int? charPosition, string? contextLine, int highlightLength) { + Severity = severity; Message = message; FileName = fileName; LineNumber = lineNumber; @@ -126,6 +168,7 @@ public CompilationWarning(string message, string? fileName, int? lineNumber, int HighlightLength = highlightLength; } + public DiagnosticSeverity Severity { get; } public string Message { get; } public string? FileName { get; } public int? LineNumber { get; } @@ -161,11 +204,11 @@ public override string ToString() ) ); var errorHighlight = padding + new string('^', HighlightLength); - error = $"{fileLocation}: Dotvvm Compilation Warning\n{contextLine}\n{errorHighlight} {Message}"; + error = $"{fileLocation}: Dotvvm Compilation {Severity}\n{contextLine}\n{errorHighlight} {Message}"; } else { - error = $"{fileLocation}: Dotvvm Compilation Warning: {Message}"; + error = $"{fileLocation}: Dotvvm Compilation {Severity}: {Message}"; } return error; } diff --git a/src/Framework/Framework/Configuration/DotvvmCompilationPageConfiguration.cs b/src/Framework/Framework/Configuration/DotvvmCompilationPageConfiguration.cs index 142c5838a6..a08070bc37 100644 --- a/src/Framework/Framework/Configuration/DotvvmCompilationPageConfiguration.cs +++ b/src/Framework/Framework/Configuration/DotvvmCompilationPageConfiguration.cs @@ -125,6 +125,9 @@ public void Apply(DotvvmConfiguration config) url: Url, virtualPath: "embedded://DotVVM.Framework/Diagnostics/CompilationPage.dothtml"); + config.Markup.AddMarkupControl("dotvvm-internal", "CompilationDiagnostic", "embedded://DotVVM.Framework/Diagnostics/CompilationDiagnostic.dotcontrol"); + config.Markup.AddMarkupControl("dotvvm-internal", "CompilationDiagnosticRows", "embedded://DotVVM.Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol"); + config.Security.RequireSecFetchHeaders.EnableForRoutes(RouteName); } diff --git a/src/Framework/Framework/Controls/DotvvmBindableObjectHelper.cs b/src/Framework/Framework/Controls/DotvvmBindableObjectHelper.cs index 80ebd5fada..580b3f6f7c 100644 --- a/src/Framework/Framework/Controls/DotvvmBindableObjectHelper.cs +++ b/src/Framework/Framework/Controls/DotvvmBindableObjectHelper.cs @@ -485,20 +485,26 @@ static string ValueDebugString(object? value) return toStringed; } - private static (string? prefix, string tagName) FormatControlName(DotvvmBindableObject control, DotvvmConfiguration? config = null) + internal static (string? prefix, string tagName) FormatControlName(DotvvmBindableObject control, DotvvmConfiguration? config) { var type = control.GetType(); if (type == typeof(HtmlGenericControl)) return (null, ((HtmlGenericControl)control).TagName!); + return FormatControlName(type, config); + } + internal static (string? prefix, string tagName) FormatControlName(Type type, DotvvmConfiguration? config) + { var reg = config?.Markup.Controls.FirstOrDefault(c => c.Namespace == type.Namespace && Type.GetType(c.Namespace + "." + type.Name + ", " + c.Assembly) == type) ?? config?.Markup.Controls.FirstOrDefault(c => c.Namespace == type.Namespace) ?? config?.Markup.Controls.FirstOrDefault(c => c.Assembly == type.Assembly.GetName().Name); var ns = reg?.TagPrefix ?? type.Namespace switch { + null => "_", "DotVVM.Framework.Controls" => "dot", "DotVVM.AutoUI.Controls" => "auto", "DotVVM.BusinessPack.Controls" or "DotVVM.BusinessPack.PostBackHandlers" => "bp", "DotVVM.BusinessPack.Controls.FilterOperators" => "op", "DotVVM.BusinessPack.Controls.FilterBuilderFields" => "fp", + var x when x.StartsWith("DotVVM.Contrib.") => "dc", _ => "_" }; var optionsAttribute = type.GetCustomAttribute(); @@ -538,7 +544,7 @@ public static string DebugString(this DotvvmBindableObject control, DotvvmConfig if (ancestor is {} && location.file.Equals(ancestor.TryGetValue(Internal.MarkupFileNameProperty))) { location.line = (int)ancestor.TryGetValue(Internal.MarkupLineNumberProperty)!; - var ancestorName = FormatControlName(ancestor); + var ancestorName = FormatControlName(ancestor, config); location.nearestControlInMarkup = ancestorName.prefix is null ? ancestorName.tagName : $"{ancestorName.prefix}:{ancestorName.tagName}"; } } diff --git a/src/Framework/Framework/Controls/GridViewColumn.cs b/src/Framework/Framework/Controls/GridViewColumn.cs index 7e830c66fc..5c0ab92a03 100644 --- a/src/Framework/Framework/Controls/GridViewColumn.cs +++ b/src/Framework/Framework/Controls/GridViewColumn.cs @@ -10,6 +10,7 @@ using DotVVM.Framework.Compilation.ControlTree.Resolved; using DotVVM.Framework.Compilation.Validation; using Microsoft.Extensions.DependencyInjection; +using DotVVM.Framework.Compilation.Parser.Dothtml.Parser; namespace DotVVM.Framework.Controls { @@ -268,7 +269,9 @@ public static IEnumerable ValidateUsage(ResolvedControl contr { if (control.Properties.ContainsKey(DataContextProperty)) { - yield return new ControlUsageError("Changing the DataContext property on the GridViewColumn is not supported!", control.DothtmlNode); + var node = control.Properties[DataContextProperty].DothtmlNode; + node = (node as DothtmlAttributeNode)?.ValueNode ?? node; + yield return new ControlUsageError("Changing the DataContext property on the GridViewColumn is not supported!", node); } // disallow attached properties on columns diff --git a/src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs b/src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs index e4825638b3..9efb2c044c 100644 --- a/src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs +++ b/src/Framework/Framework/DependencyInjection/DotVVMServiceCollectionExtensions.cs @@ -71,6 +71,8 @@ public static IServiceCollection RegisterDotVVMServices(IServiceCollection servi services.TryAddSingleton(); services.TryAddSingleton>(s => () => ActivatorUtilities.CreateInstance(s)); services.TryAddSingleton(); + services.AddSingleton(s => s.GetRequiredService()); + services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); @@ -130,6 +132,7 @@ public static IServiceCollection RegisterDotVVMServices(IServiceCollection servi services.TryAddSingleton(); services.TryAddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/src/Framework/Framework/Diagnostics/CompilationDiagnostic.dotcontrol b/src/Framework/Framework/Diagnostics/CompilationDiagnostic.dotcontrol new file mode 100644 index 0000000000..1c85ddd0ee --- /dev/null +++ b/src/Framework/Framework/Diagnostics/CompilationDiagnostic.dotcontrol @@ -0,0 +1,14 @@ +<%-- @property DotVVM.Framework.Compilation.DotHtmlFileInfo.CompilationDiagnosticViewModel Diagnostic --%> +@viewModel DotVVM.Framework.Compilation.DotHtmlFileInfo.CompilationDiagnosticViewModel + +
+
+
+ 
{{value: LineNumber}}: {{value: SourceLinePrefix}}{{value: SourceLineHighlight}}{{value: SourceLineSuffix}}
+
+
+ 
{{value: Severity}}: {{value: Message}}
+ <%--

+ Source File: {{value: FileName}}:{{value: LineNumber}} +

--%> +
diff --git a/src/Framework/Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol b/src/Framework/Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol new file mode 100644 index 0000000000..34cd0cdc8a --- /dev/null +++ b/src/Framework/Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol @@ -0,0 +1,26 @@ +@import DotVVM.Framework.Compilation.DotHtmlFileInfo +@property DotVVM.Framework.Compilation.DotHtmlFileInfo.CompilationDiagnosticViewModel[] Diagnostics +@property int DisplayLimit +@viewModel object +@noWrapperTag + + 0}> + + + + + + + + + _control.DisplayLimit} class=row-continues> + + plus + d.Severity == 'Error')}> + {{value: _control.Diagnostics.Skip(_control.DisplayLimit).Where(d => d.Severity == 'Error').Count()}} more errors and + + + {{value: _control.Diagnostics.Skip(_control.DisplayLimit).Where(d => d.Severity == 'Warning').Count()}} more warnings + + + diff --git a/src/Framework/Framework/Diagnostics/CompilationPage.dothtml b/src/Framework/Framework/Diagnostics/CompilationPage.dothtml index 6270e1b1a7..59fb866c61 100644 --- a/src/Framework/Framework/Diagnostics/CompilationPage.dothtml +++ b/src/Framework/Framework/Diagnostics/CompilationPage.dothtml @@ -1,4 +1,4 @@ -@viewModel DotVVM.Framework.Diagnostics.CompilationPageViewModel +@viewModel DotVVM.Framework.Diagnostics.CompilationPageViewModel @@ -35,9 +35,17 @@ class="nav" Class-active="{value: ActiveTab == 2}" /> + + +
+ +
@@ -46,7 +54,16 @@ + Class-success="{value: Status == 'CompletedSuccessfully' && Warnings.Length == 0}" /> + + + + 0} + Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} + DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> + + + Class-success="{value: Status == 'CompletedSuccessfully' && Warnings.Length == 0}" /> + + + 0} + Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} + DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> + + + Class-success="{value: Status == 'CompletedSuccessfully' && Warnings.Length == 0}" /> + + + 0} + Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} + DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> + + @@ -143,8 +176,8 @@ -
-

Errors

+
+

{{value: ActiveTab == 3 ? "Warnings" : "Errors"}}

m.Status == 'None') || Controls.AsEnumerable().Any(m => m.Status == 'None') || Routes.AsEnumerable().Any(m => m.Status == 'None')} style="color: var(--error-dark-color)"> @@ -158,8 +191,8 @@ Status Actions - r.Status == 'CompilationFailed')} WrapperTagName=tbody > - + r.Status == 'CompilationFailed' || r.Warnings.Length > 0 && ActiveTab == 3)} WrapperTagName=tbody > + Route {RouteName}"}> @@ -175,9 +208,13 @@ + 0} + Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} + DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> - r.Status == 'CompilationFailed')} WrapperTagName=tbody > - + r.Status == 'CompilationFailed' || r.Warnings.Length > 0 && ActiveTab == 3)} WrapperTagName=tbody > + Control {{value: $"{TagPrefix}:{TagName}"}} @@ -188,9 +225,13 @@ + 0} + Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} + DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> - r.Status == 'CompilationFailed')} WrapperTagName=tbody > - + r.Status == 'CompilationFailed' || r.Warnings.Length > 0 && ActiveTab == 3)} WrapperTagName=tbody > + Master page {{value: VirtualPath}} @@ -199,6 +240,10 @@ + 0} + Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} + DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> diff --git a/src/Framework/Framework/Diagnostics/CompilationPageViewModel.cs b/src/Framework/Framework/Diagnostics/CompilationPageViewModel.cs index c6007f36fc..7ff2b4121d 100644 --- a/src/Framework/Framework/Diagnostics/CompilationPageViewModel.cs +++ b/src/Framework/Framework/Diagnostics/CompilationPageViewModel.cs @@ -17,6 +17,9 @@ public class CompilationPageViewModel : DotvvmViewModelBase public int ActiveTab { get; set; } = 0; public string PathBase => Context.TranslateVirtualPath("~/"); + public bool ShowInlineDiagnostics { get; set; } = true; + public int DefaultShownDiagnosticLimit { get; set; } = 8; + public CompilationPageViewModel(IDotvvmViewCompilationService viewCompilationService) { this.viewCompilationService = viewCompilationService; diff --git a/src/Framework/Framework/DotVVM.Framework.csproj b/src/Framework/Framework/DotVVM.Framework.csproj index 4993bcc7ee..9bafd8d706 100644 --- a/src/Framework/Framework/DotVVM.Framework.csproj +++ b/src/Framework/Framework/DotVVM.Framework.csproj @@ -36,6 +36,8 @@ + + diff --git a/src/Framework/Framework/Hosting/ErrorPages/DotvvmMarkupErrorSection.cs b/src/Framework/Framework/Hosting/ErrorPages/DotvvmMarkupErrorSection.cs index 7f9e9f5a76..294c6ad9c3 100644 --- a/src/Framework/Framework/Hosting/ErrorPages/DotvvmMarkupErrorSection.cs +++ b/src/Framework/Framework/Hosting/ErrorPages/DotvvmMarkupErrorSection.cs @@ -71,24 +71,24 @@ protected virtual void WriteException(IErrorWriter w, Exception exc) private SourceModel? ExtractSourceFromDotvvmCompilationException(DotvvmCompilationException compilationException) { - if (compilationException.Tokens != null && compilationException.Tokens.Any()) + if (compilationException.LineNumber is {} || compilationException.FileName is {}) { - var errorColumn = compilationException.Tokens.FirstOrDefault()?.ColumnNumber ?? 0; - var errorLength = compilationException.Tokens.Where(t => t.LineNumber == compilationException.LineNumber).Sum(t => t.Length); - if (compilationException.FileName != null) + var errorColumn = compilationException.ColumnNumber ?? 0; + var errorLength = compilationException.CompilationError.Location.LineErrorLength; + if (compilationException.MarkupFile is {}) + return ErrorFormatter.LoadSourcePiece(compilationException.MarkupFile, compilationException.LineNumber ?? 0, + errorColumn: errorColumn, + errorLength: errorLength); + else if (compilationException.FileName != null) return ErrorFormatter.LoadSourcePiece(compilationException.FileName, compilationException.LineNumber ?? 0, errorColumn: errorColumn, errorLength: errorLength); - else + else if (compilationException.Tokens != null) { var line = string.Concat(compilationException.Tokens.Select(s => s.Text)); return CreateAnonymousLine(line, lineNumber: compilationException.Tokens.FirstOrDefault()?.LineNumber ?? 0); } } - else if (compilationException.FileName != null) - { - return ErrorFormatter.LoadSourcePiece(compilationException.FileName, compilationException.LineNumber ?? 0, errorColumn: compilationException.ColumnNumber ?? 0, errorLength: compilationException.ColumnNumber != null ? 1 : 0); - } return null; } diff --git a/src/Framework/Framework/Hosting/ErrorPages/ErrorFormatter.cs b/src/Framework/Framework/Hosting/ErrorPages/ErrorFormatter.cs index f9daa39e1c..810d184e45 100644 --- a/src/Framework/Framework/Hosting/ErrorPages/ErrorFormatter.cs +++ b/src/Framework/Framework/Hosting/ErrorPages/ErrorFormatter.cs @@ -278,16 +278,33 @@ public static SourceModel LoadSourcePiece(string? fileName, int lineNumber, { try { - var lines = File.ReadAllLines(fileName); - if (lineNumber >= 0) - { - result.CurrentLine = lines[Math.Max(0, lineNumber - 1)]; - result.PreLines = lines.Skip(lineNumber - additionalLineCount) - .TakeWhile(l => l != result.CurrentLine).ToArray(); - } - else additionalLineCount = 30; - result.PostLines = lines.Skip(lineNumber).Take(additionalLineCount).ToArray(); - return result; + return SourcePieceFromSource(fileName, File.ReadAllText(fileName), lineNumber, additionalLineCount, errorColumn, errorLength); + } + catch + { + result.LoadFailed = true; + } + } + return result; + } + + public static SourceModel LoadSourcePiece(MarkupFile? file, int lineNumber, + int additionalLineCount = 7, + int errorColumn = 0, + int errorLength = 0) + { + var result = new SourceModel { + FileName = file?.FileName, + LineNumber = lineNumber, + ErrorColumn = errorColumn, + ErrorLength = errorLength + }; + + if (file != null) + { + try + { + return SourcePieceFromSource(file.FileName, file.ReadContent(), lineNumber, additionalLineCount, errorColumn, errorLength); } catch { @@ -297,6 +314,30 @@ public static SourceModel LoadSourcePiece(string? fileName, int lineNumber, return result; } + + public static SourceModel SourcePieceFromSource(string? fileName, string sourceCode, int lineNumber, + int additionalLineCount = 7, + int errorColumn = 0, + int errorLength = 0) + { + var result = new SourceModel { + FileName = fileName, + LineNumber = lineNumber, + ErrorColumn = errorColumn, + ErrorLength = errorLength + }; + var lines = sourceCode.Split('\n'); + if (lineNumber >= 0) + { + result.CurrentLine = lines[Math.Max(0, lineNumber - 1)]; + result.PreLines = lines.Skip(lineNumber - additionalLineCount) + .TakeWhile(l => l != result.CurrentLine).ToArray(); + } + else additionalLineCount = 30; + result.PostLines = lines.Skip(lineNumber).Take(additionalLineCount).ToArray(); + return result; + } + public List> Formatters = new(); public string ErrorHtml(Exception exception, IHttpContext context) diff --git a/src/Framework/Framework/Hosting/ErrorPages/ErrorPageTemplate.cs b/src/Framework/Framework/Hosting/ErrorPages/ErrorPageTemplate.cs index 4d30b52d1a..6b52195d8e 100644 --- a/src/Framework/Framework/Hosting/ErrorPages/ErrorPageTemplate.cs +++ b/src/Framework/Framework/Hosting/ErrorPages/ErrorPageTemplate.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.IO; using System.Linq; using System.Collections; @@ -107,7 +107,7 @@ public string TransformText()

-

Server Error, HTTP {ErrorCode}: {WebUtility.HtmlEncode(ErrorDescription)}

+

Server Error, HTTP {ErrorCode}: {WebUtility.HtmlEncode(ErrorDescription)}

{WebUtility.HtmlEncode(Summary)}
@@ -164,13 +164,14 @@ public void ObjectBrowser(object? obj) ReferenceLoopHandling = ReferenceLoopHandling.Ignore, DefaultValueHandling = DefaultValueHandling.Ignore, Converters = { - new ReflectionTypeJsonConverter(), - new ReflectionAssemblyJsonConverter(), - new DotvvmTypeDescriptorJsonConverter(), - new Controls.DotvvmControlDebugJsonConverter(), - new IgnoreStuffJsonConverter(), - new BindingDebugJsonConverter() - }, + new ReflectionTypeJsonConverter(), + new ReflectionAssemblyJsonConverter(), + new DotvvmTypeDescriptorJsonConverter(), + new Controls.DotvvmControlDebugJsonConverter(), + new IgnoreStuffJsonConverter(), + new BindingDebugJsonConverter(), + new DotvvmPropertyJsonConverter() + }, // suppress any errors that occur during serialization (getters may throw exception, ...) Error = (sender, args) => { args.ErrorContext.Handled = true; diff --git a/src/Framework/Framework/ResourceManagement/ReflectionAssemblyJsonConverter.cs b/src/Framework/Framework/ResourceManagement/ReflectionAssemblyJsonConverter.cs index 18d89d6cac..77e8f75862 100644 --- a/src/Framework/Framework/ResourceManagement/ReflectionAssemblyJsonConverter.cs +++ b/src/Framework/Framework/ResourceManagement/ReflectionAssemblyJsonConverter.cs @@ -85,4 +85,22 @@ public override void WriteJson(JsonWriter writer, object? value, JsonSerializer writer.WriteValue($"{t.FullName}, {assembly}"); } } + + public class DotvvmPropertyJsonConverter : JsonConverter + { + public override bool CanConvert(Type objectType) => + typeof(IPropertyDescriptor).IsAssignableFrom(objectType) || typeof(IPropertyGroupDescriptor).IsAssignableFrom(objectType); + public override bool CanRead => false; + public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer) => + throw new NotImplementedException(); + public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer) + { + if (value is null) + { + writer.WriteNull(); + return; + } + writer.WriteValue(value.ToString()); + } + } } diff --git a/src/Framework/Framework/Resources/Styles/DotVVM.Internal.css b/src/Framework/Framework/Resources/Styles/DotVVM.Internal.css index 37e5de9539..5c6e903380 100644 --- a/src/Framework/Framework/Resources/Styles/DotVVM.Internal.css +++ b/src/Framework/Framework/Resources/Styles/DotVVM.Internal.css @@ -42,7 +42,7 @@ html { /* Variables */ :root { - --heading-color: #a82f23; + --heading-color: #333333; --nav-color: #2980b9; --activate-color: #de1212; --idle-color: #bcbcbc; @@ -50,6 +50,8 @@ html { --hint-color: #bbbbbb; --error-color: #de1212; --error-dark-color: #a82f23; + --warning-color: #ffa322; + --warning-dark-color: #940c00; --success-color: green; @@ -101,6 +103,14 @@ h3 { color: var(--hint-color) } +.error-text { + color: var(--error-dark-color) +} + +.warning-text { + color: var(--warning-dark-color) +} + /* Tables */ table { @@ -131,16 +141,19 @@ table { table th, table td { - border-right: 0.1rem var(--hint-color) solid; - border-bottom: 0.1rem var(--hint-color) solid; - border-left: 0.1rem transparent solid; - border-top: 0.1rem transparent solid; + border-right: none; + border-bottom: none; + border-left: none; + border-top: 0.1rem var(--hint-color) solid; box-sizing: border-box; } + table tr:first-child th, table tr:first-child td { + border-top: none; + } - table th:last-child, - table td:last-child { - border-right: 0.1rem transparent solid; + table tr.row-continues td { + /* border-top: 0.1rem var(--hint-color) dashed; */ + border-top: none; } table th.explosive, @@ -154,6 +167,11 @@ table { width: 1px; } + table td.center, + table th.center { + text-align: center; + } + table tr.failure { background-color: var(--error-dark-color); color: white; @@ -172,7 +190,7 @@ table { /* Source code */ .source .source-errorLine { - color: #a82f23; + color: var(--error-dark-color); } .errorUnderline { @@ -182,6 +200,17 @@ table { padding: 0.2rem; } +.source .source-warningLine { + color: var(--warning-dark-color); +} + +.warningUnderline { + background-color: color-mix(in srgb, var(--warning-color) 20%, white 80%); + border: 0.1rem solid color-mix(in srgb, var(--warning-color) 50%, white 50%); + color: var(--warning-dark-color); + padding: 0.2rem; +} + .code { font-family: var(--font-monospace); } diff --git a/src/Framework/Framework/Utils/ReferenceEqualityComparer.cs b/src/Framework/Framework/Utils/ReferenceEqualityComparer.cs new file mode 100644 index 0000000000..8d83b2f6fe --- /dev/null +++ b/src/Framework/Framework/Utils/ReferenceEqualityComparer.cs @@ -0,0 +1,12 @@ +using System.Collections.Generic; + +namespace DotVVM.Framework.Utils +{ + internal class ReferenceEqualityComparer : IEqualityComparer + where T : class + { + public bool Equals(T? x, T? y) => ReferenceEquals(x, y); + + public int GetHashCode(T obj) => obj?.GetHashCode() ?? 0; + } +} diff --git a/src/Samples/Common/DotvvmStartup.cs b/src/Samples/Common/DotvvmStartup.cs index e2cd4f4635..04dac2a63f 100644 --- a/src/Samples/Common/DotvvmStartup.cs +++ b/src/Samples/Common/DotvvmStartup.cs @@ -95,6 +95,7 @@ public void Configure(DotvvmConfiguration config, string applicationPath) ), allowGeneric: true, allowMultipleMethods: true); config.Diagnostics.CompilationPage.IsApiEnabled = true; + config.Diagnostics.CompilationPage.IsEnabled = true; config.Diagnostics.CompilationPage.ShouldCompileAllOnLoad = false; config.AssertConfigurationIsValid(); diff --git a/src/Samples/Common/Views/ControlSamples/ComboBox/ItemBinding_ItemValueBinding_Complex_Error.dothtml b/src/Samples/Common/Views/ControlSamples/ComboBox/ItemBinding_ItemValueBinding_Complex_Error.dothtml index ea891ea3d6..3aec0327e3 100644 --- a/src/Samples/Common/Views/ControlSamples/ComboBox/ItemBinding_ItemValueBinding_Complex_Error.dothtml +++ b/src/Samples/Common/Views/ControlSamples/ComboBox/ItemBinding_ItemValueBinding_Complex_Error.dothtml @@ -11,7 +11,7 @@ diff --git a/src/Samples/Common/Views/Errors/NotAllowedHardCodedPropertyValue.dothtml b/src/Samples/Common/Views/Errors/NotAllowedHardCodedPropertyValue.dothtml index f523d1ac98..b4163879a0 100644 --- a/src/Samples/Common/Views/Errors/NotAllowedHardCodedPropertyValue.dothtml +++ b/src/Samples/Common/Views/Errors/NotAllowedHardCodedPropertyValue.dothtml @@ -7,6 +7,6 @@ - + diff --git a/src/Samples/Common/Views/FeatureSamples/LambdaExpressions/LambdaExpressions.dothtml b/src/Samples/Common/Views/FeatureSamples/LambdaExpressions/LambdaExpressions.dothtml index 3662c4f15e..5ce3e7979a 100644 --- a/src/Samples/Common/Views/FeatureSamples/LambdaExpressions/LambdaExpressions.dothtml +++ b/src/Samples/Common/Views/FeatureSamples/LambdaExpressions/LambdaExpressions.dothtml @@ -36,7 +36,7 @@ --%> <%--Click="{command: SetResult(Array.Where((int item) => item % 2 == 0))}" />--%> - Click="{command SetResult(Array.Where(item => item % 2 == 0))}" /> + Click="{command: SetResult(Array.Where(item => item % 2 == 0))}" /> --%> <%--Click="{command: SetResult(Array.Where((int item) => item % 2 == 1))}" />--%> diff --git a/src/Samples/Tests/Tests/ErrorsTests.cs b/src/Samples/Tests/Tests/ErrorsTests.cs index 4057b6e87b..a3fe56d41e 100644 --- a/src/Samples/Tests/Tests/ErrorsTests.cs +++ b/src/Samples/Tests/Tests/ErrorsTests.cs @@ -40,7 +40,7 @@ public void Error_InvalidViewModel() , s => s.Contains("DotVVM.Framework.Compilation.DotvvmCompilationException", StringComparison.OrdinalIgnoreCase) && - s.Contains("Could not resolve type 'invalid'", StringComparison.OrdinalIgnoreCase) + s.Contains("Could not resolve type 'invalid_viewmodel_class'", StringComparison.OrdinalIgnoreCase) ); }); } @@ -85,11 +85,11 @@ public void Error_NotAllowedHardCodedPropertyValue() AssertUI.InnerText(browser.First("[class='exceptionMessage']") , s => - s.ToLowerInvariant().Contains("was not recognized as a valid boolean.") - , "Expected message is 'was not recognized as a valid Boolean.'"); + s.ToLowerInvariant().Contains("cannot contain hard coded value.") + , "Expected message is 'cannot contain hard coded value'"); AssertUI.InnerText(browser.First("[class='errorUnderline']") - , s => s.Contains("NotAllowedHardCodedValue")); + , s => s.Contains("true")); }); } @@ -349,7 +349,7 @@ public void Error_UnknownInnerControl() RunInAllBrowsers(browser => { browser.NavigateToUrl(SamplesRouteUrls.Errors_UnknownInnerControl); - AssertUI.InnerText(browser.First(".summary"), s => s.Contains("does not inherit from DotvvmControl and thus cannot be used in content")); + AssertUI.InnerText(browser.First(".summary"), s => s.Contains("Content control must inherit from DotvvmControl, but DotVVM.Framework.Controls.ConfirmPostBackHandler doesn't")); AssertUI.InnerText(browser.First("[class='errorUnderline']"), s => s.Contains("")); }); } @@ -454,7 +454,7 @@ public void Error_RouteLinkInvalidRouteName() browser.NavigateToUrl(SamplesRouteUrls.Errors_InvalidRouteName); AssertUI.TextEquals(browser.First("exceptionType", By.ClassName), "DotVVM.Framework.Compilation.DotvvmCompilationException"); - AssertUI.TextEquals(browser.First(".exceptionMessage"), "Validation error in RouteLink at line 18: RouteName \"NonExistingRouteName\" does not exist.", + AssertUI.TextEquals(browser.First(".exceptionMessage"), "RouteLink validation at line 18: RouteName \"NonExistingRouteName\" does not exist.", failureMessage: "Exception should contain information about the undefined route name"); }); } diff --git a/src/Tests/ControlTests/GridViewTests.cs b/src/Tests/ControlTests/GridViewTests.cs index 263fa71c4e..251657e712 100644 --- a/src/Tests/ControlTests/GridViewTests.cs +++ b/src/Tests/ControlTests/GridViewTests.cs @@ -97,7 +97,7 @@ public async Task GridViewColumn_Usage_DataContext() ")); - Assert.IsTrue(exception.Message.Contains("Changing the DataContext property on the GridViewColumn is not supported!")); + StringAssert.Contains(exception.Message, "Changing the DataContext property on the GridViewColumn is not supported!"); } [TestMethod] diff --git a/src/Tests/ControlTests/ViewModulesServerSideTests.cs b/src/Tests/ControlTests/ViewModulesServerSideTests.cs index 1b7616967e..f6d8ce5f9f 100644 --- a/src/Tests/ControlTests/ViewModulesServerSideTests.cs +++ b/src/Tests/ControlTests/ViewModulesServerSideTests.cs @@ -41,14 +41,14 @@ public async Task NamedCommandWithoutViewModule_StaticCommand() { var r = await Assert.ThrowsExceptionAsync(() => cth.RunPage(typeof(object), @" ")); - Assert.AreEqual("Validation error in NamedCommand at line 7: The NamedCommand control can be used only in pages or controls that have the @js directive.", r.Message); + Assert.AreEqual("NamedCommand validation at line 7: The NamedCommand control can be used only in pages or controls that have the @js directive.", r.Message); } [TestMethod] public async Task NamedCommandWithoutViewModule_Command() { var r = await Assert.ThrowsExceptionAsync(() => cth.RunPage(typeof(object), @" ")); - Assert.AreEqual("Validation error in NamedCommand at line 7: The NamedCommand control can be used only in pages or controls that have the @js directive.", r.Message); + Assert.AreEqual("NamedCommand validation at line 7: The NamedCommand control can be used only in pages or controls that have the @js directive.", r.Message); } [TestMethod] diff --git a/src/Tests/ControlTests/testoutputs/DotvvmErrorTests.AuthView_InvalidWrapperTagUsage.txt b/src/Tests/ControlTests/testoutputs/DotvvmErrorTests.AuthView_InvalidWrapperTagUsage.txt index 48e545c6a8..f7273e8eaa 100644 --- a/src/Tests/ControlTests/testoutputs/DotvvmErrorTests.AuthView_InvalidWrapperTagUsage.txt +++ b/src/Tests/ControlTests/testoutputs/DotvvmErrorTests.AuthView_InvalidWrapperTagUsage.txt @@ -1,2 +1,2 @@ -DotvvmCompilationException occurred: Validation error in AuthenticatedView at line 6: The WrapperTagName property cannot be set when RenderWrapperTag is false! - at void DotVVM.Framework.Compilation.Validation.ControlUsageValidationVisitor.VisitAndAssert(ResolvedTreeRoot view) +DotvvmCompilationException occurred: AuthenticatedView validation at line 6: The WrapperTagName property cannot be set when RenderWrapperTag is false! + at List DotVVM.Framework.Compilation.ViewCompiler.DefaultViewCompiler.CheckErrors(string fileName, string sourceCode, Handle tracingHandle, List tokens, DothtmlNode syntaxTree, ResolvedTreeRoot resolvedTree, IEnumerable additionalDiagnostics) diff --git a/src/Tests/ControlTests/testoutputs/DotvvmErrorTests.HtmlLiteral_InvalidWrapperTagUsage.txt b/src/Tests/ControlTests/testoutputs/DotvvmErrorTests.HtmlLiteral_InvalidWrapperTagUsage.txt index 3889337588..7612335a5a 100644 --- a/src/Tests/ControlTests/testoutputs/DotvvmErrorTests.HtmlLiteral_InvalidWrapperTagUsage.txt +++ b/src/Tests/ControlTests/testoutputs/DotvvmErrorTests.HtmlLiteral_InvalidWrapperTagUsage.txt @@ -1,2 +1,2 @@ -DotvvmCompilationException occurred: Validation error in HtmlLiteral at line 6: The WrapperTagName property cannot be set when RenderWrapperTag is false! - at void DotVVM.Framework.Compilation.Validation.ControlUsageValidationVisitor.VisitAndAssert(ResolvedTreeRoot view) +DotvvmCompilationException occurred: HtmlLiteral validation at line 6: The WrapperTagName property cannot be set when RenderWrapperTag is false! + at List DotVVM.Framework.Compilation.ViewCompiler.DefaultViewCompiler.CheckErrors(string fileName, string sourceCode, Handle tracingHandle, List tokens, DothtmlNode syntaxTree, ResolvedTreeRoot resolvedTree, IEnumerable additionalDiagnostics) diff --git a/src/Tests/Runtime/DefaultViewCompilerTests.cs b/src/Tests/Runtime/DefaultViewCompilerTests.cs index bc469dd2cf..29fd2c9285 100644 --- a/src/Tests/Runtime/DefaultViewCompilerTests.cs +++ b/src/Tests/Runtime/DefaultViewCompilerTests.cs @@ -432,8 +432,8 @@ @viewModel object var ex = Assert.ThrowsException(() => { CompileMarkup(markup); }); - Assert.IsTrue(ex.ToString().Contains("DotVVM.Framework.Binding.Properties.DataSourceLengthBinding")); - Assert.IsTrue(ex.ToString().Contains("Cannot find collection length from binding '_this'")); + Assert.IsTrue(ex.AllErrors.Any(e => e.Message.Contains("DotVVM.Framework.Binding.Properties.DataSourceLengthBinding"))); + StringAssert.Contains(ex.ToString(), "Cannot find collection length from binding '_this'"); } [TestMethod] diff --git a/src/Tests/Runtime/DotvvmControlErrorsTests.cs b/src/Tests/Runtime/DotvvmControlErrorsTests.cs index d08499c92e..980f27e2bd 100644 --- a/src/Tests/Runtime/DotvvmControlErrorsTests.cs +++ b/src/Tests/Runtime/DotvvmControlErrorsTests.cs @@ -94,7 +94,7 @@ public void Button_NonExistingCommand_ThrowsException() var dotvvmBuilder = CreateControlRenderer(control, new object()); var exc = Assert.ThrowsException(() => dotvvmBuilder()); - StringAssert.Contains(exc.Message, "Could not initialize binding"); + StringAssert.Contains(exc.Message, "Could not resolve identifier 'NonExistingCommand'"); } [TestMethod] @@ -104,7 +104,7 @@ public void CheckBox_NonExistingViewModelProperty_ThrowsException() var dotvvmBuilder = CreateControlRenderer(control, new object()); var exc = Assert.ThrowsException(() => dotvvmBuilder()); - StringAssert.Contains(exc.Message, "Could not initialize binding"); + StringAssert.Contains(exc.Message, "Could not resolve identifier 'InvalidPropertyName'"); } [TestMethod] From 394538d74436f2b7cf478dc23ad5974e335945eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Thu, 25 Jan 2024 15:15:05 +0100 Subject: [PATCH 2/3] compilation exception: fix attachement of corrent MarkupFile --- .../Compilation/DefaultControlBuilderFactory.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Framework/Framework/Compilation/DefaultControlBuilderFactory.cs b/src/Framework/Framework/Compilation/DefaultControlBuilderFactory.cs index e0f43b53dc..2621b3695f 100644 --- a/src/Framework/Framework/Compilation/DefaultControlBuilderFactory.cs +++ b/src/Framework/Framework/Compilation/DefaultControlBuilderFactory.cs @@ -106,13 +106,22 @@ public DefaultControlBuilderFactory(DotvvmConfiguration configuration, IMarkupFi var compilationService = configuration.ServiceProvider.GetService(); void editCompilationException(DotvvmCompilationException ex) { - var fileName = ex.FileName ?? file.FullPath; - if (!Path.IsPathRooted(fileName) && Path.IsPathRooted(file.FullPath)) + if (ex.FileName is null || ex.FileName == file.FullPath || ex.FileName == file.FileName) { - fileName = Path.Combine(file.FullPath.Remove(file.FullPath.Length - file.FileName.Length), fileName); + ex.SetFile(file.FullPath, file); + } + else if (ex.MarkupFile is null) + { + // try to load the markup file of this error + try + { + var exceptionFile = GetMarkupFile(ex.FileName); + ex.SetFile(exceptionFile.file.FullPath, exceptionFile.file); + } + catch { } } - ex.SetFile(fileName, file); } + try { var sw = ValueStopwatch.StartNew(); From c42dd8e451294cb5cbd4a0717693a70cf58a3a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Wed, 31 Jan 2024 22:12:31 +0100 Subject: [PATCH 3/3] compilation diagnostics refactoring: code cleanup --- .../ControlTree/ControlTreeResolverBase.cs | 2 +- .../DiagnosticsCompilationTracer.cs | 11 +++- .../DotvvmCompilationDiagnostic.cs | 51 +++++++++---------- .../Compilation/DotvvmCompilationException.cs | 3 +- .../DotvvmViewCompilationService.cs | 2 +- .../Compilation/ErrorCheckingVisitor.cs | 4 ++ .../Parser/Binding/Tokenizer/BindingToken.cs | 2 + .../ViewCompiler/DefaultViewCompiler.cs | 17 ++++--- .../DotvvmCompilationPageConfiguration.cs | 4 +- .../CompilationDiagnostic.dotcontrol | 4 -- .../CompilationDiagnosticRows.dotcontrol | 2 +- .../Diagnostics/CompilationPage.dothtml | 12 ++--- .../ErrorPages/DotvvmMarkupErrorSection.cs | 4 +- .../Hosting/ErrorPages/ErrorFormatter.cs | 6 +-- 14 files changed, 68 insertions(+), 56 deletions(-) diff --git a/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs b/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs index 6c4c325b50..20a2f21abf 100644 --- a/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs +++ b/src/Framework/Framework/Compilation/ControlTree/ControlTreeResolverBase.cs @@ -180,7 +180,7 @@ protected virtual void ResolveRootContent(DothtmlRootNode root, IAbstractControl } catch (DotvvmCompilationException ex) { - if (ex.Tokens is null) + if (ex.Tokens.IsEmpty) { var oldLoc = ex.CompilationError.Location; ex.CompilationError = ex.CompilationError with { diff --git a/src/Framework/Framework/Compilation/DiagnosticsCompilationTracer.cs b/src/Framework/Framework/Compilation/DiagnosticsCompilationTracer.cs index 01adcbe972..3b3f7c2576 100644 --- a/src/Framework/Framework/Compilation/DiagnosticsCompilationTracer.cs +++ b/src/Framework/Framework/Compilation/DiagnosticsCompilationTracer.cs @@ -8,18 +8,27 @@ namespace DotVVM.Framework.Compilation { - + /// Instrumets DotVVM view compilation, traced events are defined in . + /// The tracers are found using IServiceProvider, to register your tracer, add it to DI with service.AddSingleton<IDiagnosticsCompilationTracer, MyTracer>() public interface IDiagnosticsCompilationTracer { Handle CompilationStarted(string file, string sourceCode); + /// Traces compilation of a single file, created in the method. Note that the class can also implement . abstract class Handle { + /// Called after the DotHTML file is parsed and syntax tree is created. Called even when there are errors. public virtual void Parsed(List tokens, DothtmlRootNode syntaxTree) { } + /// Called after the entire tree has resolved types - controls have assigned type, attributes have assigned DotvvmProperty, bindings are compiled, ... public virtual void Resolved(ResolvedTreeRoot tree, ControlBuilderDescriptor descriptor) { } + /// After initial resolving, the tree is post-processed using a number of visitors (, , , ...). After each visitor processing, this method is called. public virtual void AfterVisitor(ResolvedControlTreeVisitor visitor, ResolvedTreeRoot tree) { } + /// For each compilation diagnostic (warning/error), this method is called. + /// The line of code where the error occured. public virtual void CompilationDiagnostic(DotvvmCompilationDiagnostic diagnostic, string? contextLine) { } + /// Called if the compilation fails for any reason. Normally, will be of type public virtual void Failed(Exception exception) { } } + /// Singleton tracing handle which does nothing. sealed class NopHandle: Handle { private NopHandle() { } diff --git a/src/Framework/Framework/Compilation/DotvvmCompilationDiagnostic.cs b/src/Framework/Framework/Compilation/DotvvmCompilationDiagnostic.cs index b729cd2d68..5fb1657070 100644 --- a/src/Framework/Framework/Compilation/DotvvmCompilationDiagnostic.cs +++ b/src/Framework/Framework/Compilation/DotvvmCompilationDiagnostic.cs @@ -51,7 +51,7 @@ public sealed record DotvvmCompilationSourceLocation [JsonIgnore] public MarkupFile? MarkupFile { get; init; } [JsonIgnore] - public IEnumerable? Tokens { get; init; } + public ImmutableArray Tokens { get; init; } public int? LineNumber { get; init; } public int? ColumnNumber { get; init; } public int LineErrorLength { get; init; } @@ -72,17 +72,16 @@ public DotvvmCompilationSourceLocation( int? columnNumber = null, int? lineErrorLength = null) { - if (tokens is {}) + this.Tokens = tokens?.ToImmutableArray() ?? ImmutableArray.Empty; + if (this.Tokens.Length > 0) { - tokens = tokens.ToArray(); - lineNumber ??= tokens.FirstOrDefault()?.LineNumber; - columnNumber ??= tokens.FirstOrDefault()?.ColumnNumber; + lineNumber ??= this.Tokens[0].LineNumber; + columnNumber ??= this.Tokens[0].ColumnNumber; lineErrorLength ??= tokens.Where(t => t.LineNumber == lineNumber).Select(t => (int?)(t.ColumnNumber + t.Length)).LastOrDefault() - columnNumber; } this.MarkupFile = markupFile; this.FileName = fileName ?? markupFile?.FileName; - this.Tokens = tokens; this.LineNumber = lineNumber; this.ColumnNumber = columnNumber; this.LineErrorLength = lineErrorLength ?? 0; @@ -109,53 +108,53 @@ public DotvvmCompilationSourceLocation( } public static readonly DotvvmCompilationSourceLocation Unknown = new(fileName: null, null, null); - public bool IsUnknown => FileName is null && MarkupFile is null && Tokens is null && LineNumber is null && ColumnNumber is null; + public bool IsUnknown => FileName is null && MarkupFile is null && Tokens.IsEmpty && LineNumber is null && ColumnNumber is null; + /// Text of the affected tokens. Consecutive tokens are concatenated - usually, this returns a single element array. public string[] AffectedSpans { get { - if (Tokens is null || !Tokens.Any()) + if (Tokens.IsEmpty) return Array.Empty(); - var ts = Tokens.ToArray(); - var r = new List { ts[0].Text }; - for (int i = 1; i < ts.Length; i++) + var spans = new List { Tokens[0].Text }; + for (int i = 1; i < Tokens.Length; i++) { - if (ts[i].StartPosition == ts[i - 1].EndPosition) - r[r.Count - 1] += ts[i].Text; + if (Tokens[i].StartPosition == Tokens[i - 1].EndPosition) + spans[spans.Count - 1] += Tokens[i].Text; else - r.Add(ts[i].Text); + spans.Add(Tokens[i].Text); } - return r.ToArray(); + return spans.ToArray(); } } + /// Ranges of the affected tokens (in UTF-16 codepoint positions). Consecutive rangess are merged - usually, this returns a single element array. public (int start, int end)[] AffectedRanges { get { - if (Tokens is null || !Tokens.Any()) + if (Tokens.IsEmpty) return Array.Empty<(int, int)>(); - var ts = Tokens.ToArray(); - var r = new (int start, int end)[ts.Length]; - r[0] = (ts[0].StartPosition, ts[0].EndPosition); + var ranges = new (int start, int end)[Tokens.Length]; + ranges[0] = (Tokens[0].StartPosition, Tokens[0].EndPosition); int ri = 0; - for (int i = 1; i < ts.Length; i++) + for (int i = 1; i < Tokens.Length; i++) { - if (ts[i].StartPosition == ts[i - 1].EndPosition) - r[i].end = ts[i].EndPosition; + if (Tokens[i].StartPosition == Tokens[i - 1].EndPosition) + ranges[i].end = Tokens[i].EndPosition; else { ri += 1; - r[ri] = (ts[i].StartPosition, ts[i].EndPosition); + ranges[ri] = (Tokens[i].StartPosition, Tokens[i].EndPosition); } } - return r.AsSpan(0, ri + 1).ToArray(); + return ranges.AsSpan(0, ri + 1).ToArray(); } } - public int? EndLineNumber => Tokens?.LastOrDefault()?.LineNumber ?? LineNumber; - public int? EndColumnNumber => (Tokens?.LastOrDefault()?.ColumnNumber + Tokens?.LastOrDefault()?.Length) ?? ColumnNumber; + public int? EndLineNumber => Tokens.LastOrDefault()?.LineNumber ?? LineNumber; + public int? EndColumnNumber => (Tokens.LastOrDefault()?.ColumnNumber + Tokens.LastOrDefault()?.Length) ?? ColumnNumber; public override string ToString() { diff --git a/src/Framework/Framework/Compilation/DotvvmCompilationException.cs b/src/Framework/Framework/Compilation/DotvvmCompilationException.cs index 42019fd3ec..00eb544bc3 100644 --- a/src/Framework/Framework/Compilation/DotvvmCompilationException.cs +++ b/src/Framework/Framework/Compilation/DotvvmCompilationException.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Runtime.Serialization; @@ -33,7 +34,7 @@ public string? FileName /// Affected tokens of the primary compilation error. [JsonIgnore] - public IEnumerable? Tokens => CompilationError.Location?.Tokens; + public ImmutableArray Tokens => CompilationError.Location?.Tokens ?? ImmutableArray.Empty; /// Line number of the primary compilation error. public int? LineNumber => CompilationError.Location?.LineNumber; /// Position on the line of the primary compilation error. diff --git a/src/Framework/Framework/Compilation/DotvvmViewCompilationService.cs b/src/Framework/Framework/Compilation/DotvvmViewCompilationService.cs index 4eba593baa..d227427fe4 100644 --- a/src/Framework/Framework/Compilation/DotvvmViewCompilationService.cs +++ b/src/Framework/Framework/Compilation/DotvvmViewCompilationService.cs @@ -226,6 +226,7 @@ public void RegisterCompiledView(string file, ViewCompiler.ControlBuilderDescrip } } + /// Loads the error markup file(s), adds the source line information to private void AddSourceLines(IEnumerable viewModels, IEnumerable originalDiagnostics) { var markupFiles = new Dictionary(); @@ -233,7 +234,6 @@ private void AddSourceLines(IEnumerable(); diff --git a/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs b/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs index 248b0b1915..5f6a637529 100644 --- a/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs +++ b/src/Framework/Framework/Compilation/ErrorCheckingVisitor.cs @@ -65,6 +65,10 @@ private void AddNodeErrors(DothtmlNode node, int priority) } } + /// + /// Assigns locations to the provied exceptions: + /// * if a BindingCompilationException with location, it and all its (nested) InnerException are assigned this location + /// * the locations are processed using MapBindingLocation to make them useful in the context of a dothtml file Dictionary AnnotateBindingExceptionWithLocation(ResolvedBinding binding, DotvvmProperty? relatedProperty, IEnumerable errors) { var result = new Dictionary(new ReferenceEqualityComparer()); diff --git a/src/Framework/Framework/Compilation/Parser/Binding/Tokenizer/BindingToken.cs b/src/Framework/Framework/Compilation/Parser/Binding/Tokenizer/BindingToken.cs index 7529739f95..e2505946f3 100644 --- a/src/Framework/Framework/Compilation/Parser/Binding/Tokenizer/BindingToken.cs +++ b/src/Framework/Framework/Compilation/Parser/Binding/Tokenizer/BindingToken.cs @@ -6,8 +6,10 @@ public BindingToken(string text, BindingTokenType type, int lineNumber, int colu { } + /// Returns new token with its position changed relative to the provided binding value token public BindingToken RemapPosition(TokenBase parentToken) => RemapPosition(parentToken.LineNumber, parentToken.ColumnNumber, parentToken.StartPosition); + /// Returns new token with its position changed relative to the provided binding start position public BindingToken RemapPosition(int startLine, int startColumn, int startPosition) { return new BindingToken(Text, Type, diff --git a/src/Framework/Framework/Compilation/ViewCompiler/DefaultViewCompiler.cs b/src/Framework/Framework/Compilation/ViewCompiler/DefaultViewCompiler.cs index 9e13d6909c..1866672384 100644 --- a/src/Framework/Framework/Compilation/ViewCompiler/DefaultViewCompiler.cs +++ b/src/Framework/Framework/Compilation/ViewCompiler/DefaultViewCompiler.cs @@ -127,16 +127,16 @@ private List CheckErrors(string fileName, string so return errorChecker.Diagnostics; } - private void LogDiagnostics(IDiagnosticsCompilationTracer.Handle tracingHandle, IEnumerable diagnostics, string fileName, string sourceCode) + private void LogDiagnostics(IDiagnosticsCompilationTracer.Handle tracingHandle, IEnumerable allDiagnostics, string fileName, string sourceCode) { - var warnings = diagnostics.Where(d => d.IsWarning || d.IsError).ToArray(); - if (warnings.Length == 0) return; + var diagnostics = allDiagnostics.Where(d => d.Severity >= DiagnosticSeverity.Warning).ToArray(); + if (diagnostics.Length == 0) return; var lines = sourceCode.Split('\n'); // Currently, all warnings are placed on syntax nodes (even when produced in control tree resolver) - foreach (var warning in warnings) + foreach (var diag in diagnostics) { - var loc = warning.Location; + var loc = diag.Location; var sourceLine = loc.LineNumber > 0 && loc.LineNumber <= lines.Length ? lines[loc.LineNumber.Value - 1] : null; sourceLine = sourceLine?.TrimEnd(); @@ -147,10 +147,10 @@ private void LogDiagnostics(IDiagnosticsCompilationTracer.Handle tracingHandle, highlightLength = Math.Max(1, Math.Min(highlightLength, sourceLine.Length - loc.ColumnNumber.Value + 1)); } - var logEvent = new CompilationDiagnosticLogEvent(warning.Severity, warning.Message, fileName, loc.LineNumber, loc.ColumnNumber, sourceLine, highlightLength); - logger?.Log(warning.IsWarning ? LogLevel.Warning : LogLevel.Error, 0, logEvent, null, (x, e) => x.ToString()); + var logEvent = new CompilationDiagnosticLogEvent(diag.Severity, diag.Message, fileName, loc.LineNumber, loc.ColumnNumber, sourceLine, highlightLength); + logger?.Log(diag.IsWarning ? LogLevel.Warning : LogLevel.Error, 0, logEvent, null, (x, e) => x.ToString()); - tracingHandle.CompilationDiagnostic(warning, sourceLine); + tracingHandle.CompilationDiagnostic(diag, sourceLine); } } @@ -178,6 +178,7 @@ public CompilationDiagnosticLogEvent(DiagnosticSeverity severity, string message public IEnumerator> GetEnumerator() { + // serilog "integration" yield return new("Message", Message); yield return new("FileName", FileName); yield return new("LineNumber", LineNumber); diff --git a/src/Framework/Framework/Configuration/DotvvmCompilationPageConfiguration.cs b/src/Framework/Framework/Configuration/DotvvmCompilationPageConfiguration.cs index a08070bc37..b0296d8334 100644 --- a/src/Framework/Framework/Configuration/DotvvmCompilationPageConfiguration.cs +++ b/src/Framework/Framework/Configuration/DotvvmCompilationPageConfiguration.cs @@ -125,8 +125,8 @@ public void Apply(DotvvmConfiguration config) url: Url, virtualPath: "embedded://DotVVM.Framework/Diagnostics/CompilationPage.dothtml"); - config.Markup.AddMarkupControl("dotvvm-internal", "CompilationDiagnostic", "embedded://DotVVM.Framework/Diagnostics/CompilationDiagnostic.dotcontrol"); - config.Markup.AddMarkupControl("dotvvm-internal", "CompilationDiagnosticRows", "embedded://DotVVM.Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol"); + config.Markup.AddMarkupControl("_dotvvm-internal", "CompilationDiagnostic", "embedded://DotVVM.Framework/Diagnostics/CompilationDiagnostic.dotcontrol"); + config.Markup.AddMarkupControl("_dotvvm-internal", "CompilationDiagnosticRows", "embedded://DotVVM.Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol"); config.Security.RequireSecFetchHeaders.EnableForRoutes(RouteName); } diff --git a/src/Framework/Framework/Diagnostics/CompilationDiagnostic.dotcontrol b/src/Framework/Framework/Diagnostics/CompilationDiagnostic.dotcontrol index 1c85ddd0ee..d76471315c 100644 --- a/src/Framework/Framework/Diagnostics/CompilationDiagnostic.dotcontrol +++ b/src/Framework/Framework/Diagnostics/CompilationDiagnostic.dotcontrol @@ -1,4 +1,3 @@ -<%-- @property DotVVM.Framework.Compilation.DotHtmlFileInfo.CompilationDiagnosticViewModel Diagnostic --%> @viewModel DotVVM.Framework.Compilation.DotHtmlFileInfo.CompilationDiagnosticViewModel
@@ -8,7 +7,4 @@ 
{{value: Severity}}: {{value: Message}}
- <%--

- Source File: {{value: FileName}}:{{value: LineNumber}} -

--%> diff --git a/src/Framework/Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol b/src/Framework/Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol index 34cd0cdc8a..11ebb2548f 100644 --- a/src/Framework/Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol +++ b/src/Framework/Framework/Diagnostics/CompilationDiagnosticRows.dotcontrol @@ -8,7 +8,7 @@ - + <_dotvvm-internal:CompilationDiagnostic style="margin-left: 3rem" /> diff --git a/src/Framework/Framework/Diagnostics/CompilationPage.dothtml b/src/Framework/Framework/Diagnostics/CompilationPage.dothtml index 59fb866c61..5ab36ace2d 100644 --- a/src/Framework/Framework/Diagnostics/CompilationPage.dothtml +++ b/src/Framework/Framework/Diagnostics/CompilationPage.dothtml @@ -58,7 +58,7 @@ - 0} Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> @@ -109,7 +109,7 @@ Class-success="{value: Status == 'CompletedSuccessfully' && Warnings.Length == 0}" /> - 0} Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> @@ -149,7 +149,7 @@ Class-success="{value: Status == 'CompletedSuccessfully' && Warnings.Length == 0}" /> - 0} Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> @@ -208,7 +208,7 @@ - 0} Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> @@ -225,7 +225,7 @@ - 0} Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> @@ -240,7 +240,7 @@ - 0} Diagnostics={value: Enumerable.Concat(Errors, Warnings).ToArray()} DisplayLimit={value: _root.DefaultShownDiagnosticLimit} /> diff --git a/src/Framework/Framework/Hosting/ErrorPages/DotvvmMarkupErrorSection.cs b/src/Framework/Framework/Hosting/ErrorPages/DotvvmMarkupErrorSection.cs index 294c6ad9c3..46336dbd52 100644 --- a/src/Framework/Framework/Hosting/ErrorPages/DotvvmMarkupErrorSection.cs +++ b/src/Framework/Framework/Hosting/ErrorPages/DotvvmMarkupErrorSection.cs @@ -83,10 +83,10 @@ protected virtual void WriteException(IErrorWriter w, Exception exc) return ErrorFormatter.LoadSourcePiece(compilationException.FileName, compilationException.LineNumber ?? 0, errorColumn: errorColumn, errorLength: errorLength); - else if (compilationException.Tokens != null) + else if (compilationException.Tokens.Length > 0) { var line = string.Concat(compilationException.Tokens.Select(s => s.Text)); - return CreateAnonymousLine(line, lineNumber: compilationException.Tokens.FirstOrDefault()?.LineNumber ?? 0); + return CreateAnonymousLine(line, lineNumber: compilationException.LineNumber ?? 0); } } return null; diff --git a/src/Framework/Framework/Hosting/ErrorPages/ErrorFormatter.cs b/src/Framework/Framework/Hosting/ErrorPages/ErrorFormatter.cs index 810d184e45..a1bd315b35 100644 --- a/src/Framework/Framework/Hosting/ErrorPages/ErrorFormatter.cs +++ b/src/Framework/Framework/Hosting/ErrorPages/ErrorFormatter.cs @@ -329,7 +329,7 @@ public static SourceModel SourcePieceFromSource(string? fileName, string sourceC var lines = sourceCode.Split('\n'); if (lineNumber >= 0) { - result.CurrentLine = lines[Math.Max(0, lineNumber - 1)]; + result.CurrentLine = lines[Math.Max(0, Math.Min(lines.Length, lineNumber) - 1)]; result.PreLines = lines.Skip(lineNumber - additionalLineCount) .TakeWhile(l => l != result.CurrentLine).ToArray(); } @@ -443,11 +443,11 @@ public static ErrorFormatter CreateDefault() )); f.AddInfoLoader(e => { object[]? objects = null; - if (e.Tokens != null && e.Tokens.Any()) + if (e.Tokens.Length > 0) { objects = new object[] { - $"Error in '{string.Concat(e.Tokens.Select(t => t.Text))}' at line {e.Tokens.First().LineNumber} in {e.SystemFileName}" + $"Error in '{string.Concat(e.Tokens.Select(t => t.Text))}' at line {e.LineNumber} in {e.SystemFileName}" }; } return new ExceptionAdditionalInfo(