From 6ea8d7721b77c8aee8f9035837a438712c9d66ad Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Wed, 5 Mar 2014 11:24:06 -0800 Subject: [PATCH] Re-applied code review changes for formatting fix. The previous fix was accidentally overridden. Also changed how we render chunk block's children. New way avoids casts and removes logic from base. --- .../CSharp/CSharpLineMappingWriter.cs | 6 +- .../CSharp/CSharpPaddingBuilder.cs | 12 +++- .../CSharp/Visitors/CSharpCodeVisitor.cs | 65 +++++++++---------- .../Compiler/CodeBuilder/CodeVisitor.cs | 1 - .../CodeGenerator/CS/Output/DesignTime.cs | 10 +-- .../CS/Output/EmptyExplicitExpression.cs | 2 +- .../CS/Output/EmptyImplicitExpression.cs | 2 +- .../EmptyImplicitExpressionInCode.Tabs.cs | 2 +- .../Output/EmptyImplicitExpressionInCode.cs | 2 +- .../CS/Output/ExplicitExpressionAtEOF.cs | 2 +- .../Output/FunctionsBlock.DesignTime.Tabs.cs | 2 +- .../CS/Output/FunctionsBlock.DesignTime.cs | 2 +- .../CS/Output/ImplicitExpressionAtEOF.cs | 2 +- .../CS/Output/Imports.DesignTime.cs | 4 +- .../CS/Output/Inherits.Designtime.cs | 2 +- .../CS/Output/RazorComments.DesignTime.cs | 4 +- .../Output/UnfinishedExpressionInCode.Tabs.cs | 2 +- .../CS/Output/UnfinishedExpressionInCode.cs | 2 +- .../TestFiles/DesignTime/Simple.txt | 4 +- 19 files changed, 65 insertions(+), 63 deletions(-) diff --git a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpLineMappingWriter.cs b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpLineMappingWriter.cs index 5a9b30fede..c2fb3cd4c5 100644 --- a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpLineMappingWriter.cs +++ b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpLineMappingWriter.cs @@ -31,7 +31,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp _writePragmas = true; // TODO: Should this just be '\n'? - if (_writer.LastWrite.Last() != '\n') + if (!_writer.LastWrite.EndsWith("\n")) { _writer.WriteLine(); } @@ -73,13 +73,13 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp { // Need to add an additional line at the end IF there wasn't one already written. // This is needed to work with the C# editor's handling of #line ... - bool writeExtraLine = _writer.ToString().Last() != '\n'; + bool endsWithNewline = _writer.ToString().EndsWith("\n"); // Always write at least 1 empty line to potentially separate code from pragmas. _writer.WriteLine(); // Check if the previous empty line wasn't enough to separate code from pragmas. - if (writeExtraLine) + if (!endsWithNewline) { _writer.WriteLine(); } diff --git a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpPaddingBuilder.cs b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpPaddingBuilder.cs index cc7ea0bd19..663564d139 100644 --- a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpPaddingBuilder.cs +++ b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpPaddingBuilder.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp throw new ArgumentNullException("target"); } - int padding = CalculatePadding(target, 0); + int padding = CalculatePadding(target, generatedStart: 0); // We treat statement padding specially so for brace positioning, so that in the following example: // @if (foo > 0) @@ -44,7 +44,12 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp return generatedCode; } - public string BuildExpressionPadding(Span target, int generatedStart = 0) + public string BuildExpressionPadding(Span target) + { + return BuildExpressionPadding(target, generatedStart: 0); + } + + public string BuildExpressionPadding(Span target, int generatedStart) { int padding = CalculatePadding(target, generatedStart); @@ -69,7 +74,8 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp // In design time: __o = somecode(); // In Run time: Write(somecode()); // - // In both cases the padding would have been 1 space to remote the space the @ symbol takes, which will be smaller than the 6 chars the hidden generated code takes. + // In both cases the padding would have been 1 space to remote the space the @ symbol takes, which will be smaller than the 6 + // chars the hidden generated code takes. if (padding < 0) { padding = 0; diff --git a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs index 54ede121ed..f9f8367828 100644 --- a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs +++ b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs @@ -40,7 +40,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp using (Writer.BuildLambda(endLine: false, parameterNames: TemplateBlockCodeGenerator.TemplateWriterName)) { - Visit((ChunkBlock)chunk); + Accept(chunk.Children); } Context.TargetWriterName = currentTargetWriterName; @@ -182,7 +182,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp using (Writer.BuildLambda(endLine: false, parameterNames: ValueWriterName)) { - Visit((ChunkBlock)chunk); + Accept(chunk.Children); } Writer.WriteEndMethodInvocation(false) @@ -217,7 +217,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp ExpressionRenderingMode currentRenderingMode = Context.ExpressionRenderingMode; Context.ExpressionRenderingMode = ExpressionRenderingMode.InjectCode; - Visit((ChunkBlock)chunk); + Accept(chunk.Children); Context.ExpressionRenderingMode = currentRenderingMode; @@ -262,7 +262,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp .WriteParameterSeparator() .WriteLocationTaggedString(chunk.Suffix); - Visit((ChunkBlock)chunk); + Accept(chunk.Children); Writer.WriteEndMethodInvocation(); } @@ -275,7 +275,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp using (Writer.BuildLambda(false)) { - Visit((ChunkBlock)chunk); + Accept(chunk.Children); } Writer.WriteEndMethodInvocation(); @@ -285,38 +285,35 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp { // TODO: Handle instrumentation - int currentIndent = Writer.CurrentIndent; - string designTimeAssignment = "__o = "; - - // The first child should never be null, it should always be a transition span, that's what - // defines an expression block chunk. var firstChild = (ExpressionChunk)chunk.Children.FirstOrDefault(); - Writer.ResetIndent() - .WriteLineNumberDirective(1, "This is here only for document formatting.") - // We build the padding with an offset of the design time assignment statement. - .Write(_paddingBuilder.BuildExpressionPadding((Span)firstChild.Association, designTimeAssignment.Length)) - .Write(designTimeAssignment); - - // We map the first line of code but do not write the line pragmas associated with it. - CreateRawCodeMapping(firstChild.Code, firstChild.Association.Start); - - // This is a temporary block that is indentical to the current one with the exception of its children. - var subBlock = new ChunkBlock + if (firstChild != null) { - Start = chunk.Start, - Association = chunk.Association, - Children = chunk.Children.Skip(1).ToList() - }; + int currentIndent = Writer.CurrentIndent; + string designTimeAssignment = "__o = "; + Writer.ResetIndent(); - // Render all children (except for the first one) - Visit(subBlock); + // This is only here to enable accurate formatting by the C# editor. + Writer.WriteLineNumberDirective(1, "------------------------------------------"); - Writer.WriteLine(";") - .WriteLine() - .WriteLineDefaultDirective() - .WriteLineHiddenDirective() - .SetIndent(currentIndent); + // We build the padding with an offset of the design time assignment statement. + Writer.Write(_paddingBuilder.BuildExpressionPadding((Span)firstChild.Association, designTimeAssignment.Length)) + .Write(designTimeAssignment); + + // We map the first line of code but do not write the line pragmas associated with it. + CreateRawCodeMapping(firstChild.Code, firstChild.Association.Start); + + // Render all but the first child. + // The reason why we render the other children differently is because when formatting the C# code + // the formatter expects the start line to have the assignment statement on it. + Accept(chunk.Children.Skip(1).ToList()); + + Writer.WriteLine(";") + .WriteLine() + .WriteLineDefaultDirective() + .WriteLineHiddenDirective() + .SetIndent(currentIndent); + } } public void RenderRuntimeExpressionBlockChunk(ExpressionBlockChunk chunk) @@ -325,7 +322,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp if (Context.ExpressionRenderingMode == ExpressionRenderingMode.InjectCode) { - Visit((ChunkBlock)chunk); + Accept(chunk.Children); } else if (Context.ExpressionRenderingMode == ExpressionRenderingMode.WriteToOutput) { @@ -340,7 +337,7 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp Writer.WriteStartMethodInvocation(Context.Host.GeneratedClassContext.WriteMethodName); } - Visit((ChunkBlock)chunk); + Accept(chunk.Children); Writer.WriteEndMethodInvocation() .WriteLine(); diff --git a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CodeVisitor.cs b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CodeVisitor.cs index 7a9ba511de..a142556ddc 100644 --- a/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CodeVisitor.cs +++ b/src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CodeVisitor.cs @@ -23,7 +23,6 @@ namespace Microsoft.AspNet.Razor.Generator.Compiler } protected override void Visit(ChunkBlock chunk) { - Accept(chunk.Children); } protected override void Visit(DynamicCodeAttributeChunk chunk) { diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/DesignTime.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/DesignTime.cs index 5504a275aa..2510fdf6e3 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/DesignTime.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/DesignTime.cs @@ -54,7 +54,7 @@ Foo() { #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = i; #line default @@ -66,14 +66,14 @@ Foo() { #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = Foo(Bar.Baz); #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = Foo(item => new Template((__razor_template_writer) => { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = baz; #line default @@ -90,7 +90,7 @@ __o = Foo(item => new Template((__razor_template_writer) => { #line default #line hidden DefineSection("Footer", () => { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = bar; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyExplicitExpression.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyExplicitExpression.cs index 8a0886ee89..ac92c7e9dc 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyExplicitExpression.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyExplicitExpression.cs @@ -17,7 +17,7 @@ namespace TestOutput public override void Execute() { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = ; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpression.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpression.cs index 073eb4ea45..c86605fdd7 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpression.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpression.cs @@ -17,7 +17,7 @@ namespace TestOutput public override void Execute() { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = ; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpressionInCode.Tabs.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpressionInCode.Tabs.cs index 1622c89b65..a3804d066b 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpressionInCode.Tabs.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpressionInCode.Tabs.cs @@ -24,7 +24,7 @@ namespace TestOutput #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = ; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpressionInCode.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpressionInCode.cs index 83b3308935..fb47cc9f69 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpressionInCode.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/EmptyImplicitExpressionInCode.cs @@ -24,7 +24,7 @@ namespace TestOutput #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = ; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/ExplicitExpressionAtEOF.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/ExplicitExpressionAtEOF.cs index b809681249..62e76ef3aa 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/ExplicitExpressionAtEOF.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/ExplicitExpressionAtEOF.cs @@ -17,7 +17,7 @@ namespace TestOutput public override void Execute() { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = ; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/FunctionsBlock.DesignTime.Tabs.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/FunctionsBlock.DesignTime.Tabs.cs index 2900ee527b..636c86c8e9 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/FunctionsBlock.DesignTime.Tabs.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/FunctionsBlock.DesignTime.Tabs.cs @@ -32,7 +32,7 @@ namespace TestOutput public override void Execute() { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = RandomInt(); #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/FunctionsBlock.DesignTime.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/FunctionsBlock.DesignTime.cs index b55d9bcf3c..889840fc0b 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/FunctionsBlock.DesignTime.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/FunctionsBlock.DesignTime.cs @@ -32,7 +32,7 @@ namespace TestOutput public override void Execute() { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = RandomInt(); #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/ImplicitExpressionAtEOF.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/ImplicitExpressionAtEOF.cs index 425a7e3e76..c9fa1434b3 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/ImplicitExpressionAtEOF.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/ImplicitExpressionAtEOF.cs @@ -17,7 +17,7 @@ namespace TestOutput public override void Execute() { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = ; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/Imports.DesignTime.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/Imports.DesignTime.cs index 222a1b95a6..8707ff9009 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/Imports.DesignTime.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/Imports.DesignTime.cs @@ -34,12 +34,12 @@ using System public override void Execute() { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = typeof(Path).FullName; #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = typeof(Foo).FullName; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/Inherits.Designtime.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/Inherits.Designtime.cs index a582bda285..2ccdbaf6fa 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/Inherits.Designtime.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/Inherits.Designtime.cs @@ -22,7 +22,7 @@ namespace TestOutput public override void Execute() { -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = foo(); #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/RazorComments.DesignTime.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/RazorComments.DesignTime.cs index a33874b795..910c73260e 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/RazorComments.DesignTime.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/RazorComments.DesignTime.cs @@ -46,12 +46,12 @@ namespace TestOutput #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = bar; #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = a #line 15 "RazorComments.cshtml" b diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/UnfinishedExpressionInCode.Tabs.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/UnfinishedExpressionInCode.Tabs.cs index 6e8bb67d1e..6b1f3e0ca6 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/UnfinishedExpressionInCode.Tabs.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/UnfinishedExpressionInCode.Tabs.cs @@ -23,7 +23,7 @@ namespace TestOutput #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = DateTime.; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/UnfinishedExpressionInCode.cs b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/UnfinishedExpressionInCode.cs index f711457239..ada83a30be 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/UnfinishedExpressionInCode.cs +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/CodeGenerator/CS/Output/UnfinishedExpressionInCode.cs @@ -23,7 +23,7 @@ namespace TestOutput #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = DateTime.; #line default diff --git a/test/Microsoft.AspNet.Razor.Test/TestFiles/DesignTime/Simple.txt b/test/Microsoft.AspNet.Razor.Test/TestFiles/DesignTime/Simple.txt index 5f8871c4e6..66e682213f 100644 --- a/test/Microsoft.AspNet.Razor.Test/TestFiles/DesignTime/Simple.txt +++ b/test/Microsoft.AspNet.Razor.Test/TestFiles/DesignTime/Simple.txt @@ -23,7 +23,7 @@ #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = hello; #line default @@ -34,7 +34,7 @@ #line default #line hidden -#line 1 "This is here only for document formatting." +#line 1 "------------------------------------------" __o = c; #line default