From 0c942ccc76988c4f97a0327a611f1aa90f38c13d Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 3 Apr 2018 15:14:18 -0700 Subject: [PATCH] Fix for #453 This fix adds missing line mappings for the Blazor runtime code generation. We had the correct line mappings for design time code, but they were missing in this case for runtime code - where they are used for error messages and debugging (not yet supported). Once this fix is is in the error window and output log will report the file/line/column of the original source in .cshtml. It looks like jumping to the code from the error window is currently not working correctly in VS. It works from the output window. I'm going to follow up on the VS issue in the Razor repo, since the fix won't come from the Blazor side. --- .../BlazorRuntimeNodeWriter.cs | 30 ++++++++++ .../DeclarationRazorIntegrationTest.cs | 56 +++++++++++++++++++ .../RazorIntegrationTestBase.cs | 12 +++- ...ntimeCodeGenerationRazorIntegrationTest.cs | 22 +++++++- 4 files changed, 116 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorRuntimeNodeWriter.cs b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorRuntimeNodeWriter.cs index a544c10bed..f4b72488b5 100644 --- a/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorRuntimeNodeWriter.cs +++ b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorRuntimeNodeWriter.cs @@ -50,6 +50,16 @@ namespace Microsoft.AspNetCore.Blazor.Razor public override void WriteCSharpCode(CodeRenderingContext context, CSharpCodeIntermediateNode node) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (node == null) + { + throw new ArgumentNullException(nameof(node)); + } + var isWhitespaceStatement = true; for (var i = 0; i < node.Children.Count; i++) { @@ -63,14 +73,25 @@ namespace Microsoft.AspNetCore.Blazor.Razor if (isWhitespaceStatement) { + // The runtime and design time code differ in their handling of whitespace-only + // statements. At runtime we can discard them completely. At design time we need + // to keep them for the editor. return; } + IDisposable linePragmaScope = null; + if (node.Source != null) + { + linePragmaScope = context.CodeWriter.BuildLinePragma(node.Source.Value); + context.CodeWriter.WritePadding(0, node.Source.Value, context); + } + for (var i = 0; i < node.Children.Count; i++) { if (node.Children[i] is IntermediateToken token && token.IsCSharp) { _scopeStack.IncrementCurrentScopeChildCount(context); + context.AddSourceMappingFor(token); context.CodeWriter.Write(token.Content); } else @@ -79,6 +100,15 @@ namespace Microsoft.AspNetCore.Blazor.Razor context.RenderNode(node.Children[i]); } } + + if (linePragmaScope != null) + { + linePragmaScope.Dispose(); + } + else + { + context.CodeWriter.WriteLine(); + } } public override void WriteCSharpCodeAttributeValue(CodeRenderingContext context, CSharpCodeAttributeValueIntermediateNode node) diff --git a/test/Microsoft.AspNetCore.Blazor.Build.Test/DeclarationRazorIntegrationTest.cs b/test/Microsoft.AspNetCore.Blazor.Build.Test/DeclarationRazorIntegrationTest.cs index 555b41f670..e2d29f2e5b 100644 --- a/test/Microsoft.AspNetCore.Blazor.Build.Test/DeclarationRazorIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Build.Test/DeclarationRazorIntegrationTest.cs @@ -100,6 +100,62 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test Assert.Empty(frames); } + [Fact] // Regression test for https://github.com/aspnet/Blazor/issues/453 + public void DeclarationConfiguration_FunctionsBlockHasLineMappings_MappingsApplyToError() + { + // Arrange & Act 1 + var generated = CompileToCSharp(@" +@functions { + public StringBuilder Builder { get; set; } +} +"); + + // Assert 1 + AssertSourceEquals(@" +// +#pragma warning disable 1591 +#pragma warning disable 0414 +#pragma warning disable 0649 +#pragma warning disable 0169 + +namespace Test +{ + #line hidden + using System; + using System.Collections.Generic; + using System.Linq; + using System.Threading.Tasks; + public class TestComponent : Microsoft.AspNetCore.Blazor.Components.BlazorComponent + { + #pragma warning disable 1998 + protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) + { + } + #pragma warning restore 1998 +#line 1 ""x:\dir\subdir\Test\TestComponent.cshtml"" + + public StringBuilder Builder { get; set; } + +#line default +#line hidden + } +} +#pragma warning restore 1591 +", generated); + + // Act 2 + var assembly = CompileToAssembly(generated, throwOnFailure: false); + + // Assert 2 + var diagnostic = Assert.Single(assembly.Diagnostics); + + // This error should map to line 2 of the generated file, the test + // says 1 because Roslyn's line/column data structures are 0-based. + var position = diagnostic.Location.GetMappedLineSpan(); + Assert.EndsWith(".cshtml", position.Path); + Assert.Equal(1, position.StartLinePosition.Line); + } + public class BaseClass : BlazorComponent { } diff --git a/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorIntegrationTestBase.cs b/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorIntegrationTestBase.cs index 97b5e3e513..975d7d97c5 100644 --- a/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorIntegrationTestBase.cs +++ b/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorIntegrationTestBase.cs @@ -208,7 +208,7 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test return CompileToAssembly(cSharpResult); } - protected CompileToAssemblyResult CompileToAssembly(CompileToCSharpResult cSharpResult) + protected CompileToAssemblyResult CompileToAssembly(CompileToCSharpResult cSharpResult, bool throwOnFailure = true) { if (cSharpResult.Diagnostics.Any()) { @@ -227,10 +227,18 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test .GetDiagnostics() .Where(d => d.Severity != DiagnosticSeverity.Hidden); - if (diagnostics.Any()) + if (diagnostics.Any() && throwOnFailure) { throw new CompilationFailedException(compilation); } + else if (diagnostics.Any()) + { + return new CompileToAssemblyResult + { + Compilation = compilation, + Diagnostics = diagnostics, + }; + } using (var peStream = new MemoryStream()) { diff --git a/test/Microsoft.AspNetCore.Blazor.Build.Test/RuntimeCodeGenerationRazorIntegrationTest.cs b/test/Microsoft.AspNetCore.Blazor.Build.Test/RuntimeCodeGenerationRazorIntegrationTest.cs index db989dac4c..b6d346e3cc 100644 --- a/test/Microsoft.AspNetCore.Blazor.Build.Test/RuntimeCodeGenerationRazorIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Build.Test/RuntimeCodeGenerationRazorIntegrationTest.cs @@ -49,6 +49,7 @@ namespace Test protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) { base.BuildRenderTree(builder); + builder.OpenComponent(0); builder.CloseComponent(); } @@ -110,6 +111,7 @@ namespace Test protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) { base.BuildRenderTree(builder); + builder.OpenComponent(0); builder.AddAttribute(1, ""IntProperty"", 123); builder.AddAttribute(2, ""BoolProperty"", true); @@ -164,6 +166,7 @@ namespace Test protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) { base.BuildRenderTree(builder); + builder.OpenComponent(0); builder.AddAttribute(1, ""StringProperty"", 42.ToString()); builder.CloseComponent(); @@ -214,6 +217,7 @@ namespace Test protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) { base.BuildRenderTree(builder); + builder.OpenComponent(0); builder.AddAttribute(1, ""some-attribute"", ""foo""); builder.AddAttribute(2, ""another-attribute"", 43.ToString()); @@ -276,20 +280,26 @@ namespace Test protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) { base.BuildRenderTree(builder); + builder.OpenComponent(0); builder.AddAttribute(1, ""OnClick"", new Microsoft.AspNetCore.Blazor.UIEventHandler(e => { Increment(); })); builder.CloseComponent(); builder.AddContent(2, ""\n\n""); } #pragma warning restore 1998 - +#line 4 ""x:\dir\subdir\Test\TestComponent.cshtml"" + private int counter; private void Increment() { counter++; } + +#line default +#line hidden } } #pragma warning restore 1591 + ", generated); } @@ -344,20 +354,26 @@ namespace Test protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) { base.BuildRenderTree(builder); + builder.OpenComponent(0); builder.AddAttribute(1, ""OnClick"", new Microsoft.AspNetCore.Blazor.UIEventHandler(Increment)); builder.CloseComponent(); builder.AddContent(2, ""\n\n""); } #pragma warning restore 1998 - +#line 5 ""x:\dir\subdir\Test\TestComponent.cshtml"" + private int counter; private void Increment(UIEventArgs e) { counter++; } + +#line default +#line hidden } } #pragma warning restore 1591 + ", generated); } @@ -404,6 +420,7 @@ namespace Test protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) { base.BuildRenderTree(builder); + builder.OpenComponent(0); builder.AddAttribute(1, ""MyAttr"", ""abc""); builder.AddAttribute(2, ""ChildContent"", (Microsoft.AspNetCore.Blazor.RenderFragment)((builder2) => { @@ -466,6 +483,7 @@ namespace Test protected override void BuildRenderTree(Microsoft.AspNetCore.Blazor.RenderTree.RenderTreeBuilder builder) { base.BuildRenderTree(builder); + builder.OpenComponent(0); builder.CloseComponent(); }