Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

Updating CSharpCodeVisitor to generate implicit expressions on a single line #171

Closed
wants to merge 2 commits into from
Closed

Updating CSharpCodeVisitor to generate implicit expressions on a single line #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
663a5f2
Updating CSharpCodeVisitor to generate implicit expressions on a single
pranavkm Sep 25, 2014
b7b2fd0
Updating per PR comments
pranavkm Oct 1, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpCodeWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,27 @@ public CSharpCodeWriter WriteReturn(string value, bool endLine)
return WriteLine();
}

/// <summary>
/// Writes a <c>#line</c> pragma directive for the line number at the specified <paramref name="location"/>.
/// </summary>
/// <param name="location">The location to generate the line pragma for.</param>
/// <param name="file">The file to generate the line pragma for.</param>
/// <returns>The current instance of <see cref="CSharpCodeWriter"/>.</returns>
public CSharpCodeWriter WriteLineNumberDirective(SourceLocation location, string file)
{
return WriteLineNumberDirective(location.LineIndex + 1, file);
}

public CSharpCodeWriter WriteLineNumberDirective(int lineNumber, string file)
{
return Write("#line ").Write(lineNumber.ToString()).Write(" \"").Write(file).WriteLine("\"");
if (!string.IsNullOrEmpty(LastWrite) &&
!LastWrite.EndsWith(Environment.NewLine, StringComparison.Ordinal))
{
WriteLine();
}

var lineNumberAsString = lineNumber.ToString(CultureInfo.InvariantCulture);
return Write("#line ").Write(lineNumberAsString).Write(" \"").Write(file).WriteLine("\"");
}

public CSharpCodeWriter WriteStartMethodInvocation(string methodName)
Expand Down
83 changes: 46 additions & 37 deletions src/Microsoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/CSharpLineMappingWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq;
using Microsoft.AspNet.Razor.Text;

namespace Microsoft.AspNet.Razor.Generator.Compiler.CSharp
Expand All @@ -15,16 +14,21 @@ public class CSharpLineMappingWriter : IDisposable
private int _startIndent;
private int _generatedContentLength;
private bool _writePragmas;
private bool _addLineMapping;

public CSharpLineMappingWriter(CSharpCodeWriter writer, SourceLocation documentLocation, int contentLength)
private CSharpLineMappingWriter([NotNull] CSharpCodeWriter writer,
bool addLineMappings)
{
_writer = writer;
_documentMapping = new MappingLocation(documentLocation, contentLength);

_addLineMapping = addLineMappings;
_startIndent = _writer.CurrentIndent;
_generatedContentLength = 0;
_writer.ResetIndent();

}

public CSharpLineMappingWriter(CSharpCodeWriter writer, SourceLocation documentLocation, int contentLength)
: this(writer, addLineMappings: true)
{
_documentMapping = new MappingLocation(documentLocation, contentLength);
_generatedLocation = _writer.GetCurrentSourceLocation();
}

Expand All @@ -33,17 +37,27 @@ public CSharpLineMappingWriter(CSharpCodeWriter writer, SourceLocation documentL
{
_writePragmas = true;

// TODO: Should this just be '\n'?
if (!_writer.LastWrite.EndsWith("\n"))
{
_writer.WriteLine();
}

_writer.WriteLineNumberDirective(documentLocation.LineIndex + 1, sourceFilename);

_writer.WriteLineNumberDirective(documentLocation, sourceFilename);
_generatedLocation = _writer.GetCurrentSourceLocation();
}

/// <summary>
/// Initializes a new instance of <see cref="CSharpLineMappingWriter"/> used for generation of runtime
/// line mappings. The constructed instance of <see cref="CSharpLineMappingWriter"/> does not track
/// mappings between the Razor content and the generated content.
/// </summary>
/// <param name="writer">The <see cref="CSharpCodeWriter"/> to write output to.</param>
/// <param name="documentLocation">The <see cref="SourceLocation"/> of the Razor content being mapping.</param>
/// <param name="sourceFileName">The input file path.</param>
public CSharpLineMappingWriter([NotNull] CSharpCodeWriter writer,
[NotNull] SourceLocation documentLocation,
[NotNull] string sourceFileName)
: this(writer, addLineMappings: false)
{
_writePragmas = true;
_writer.WriteLineNumberDirective(documentLocation, sourceFileName);
}

public void MarkLineMappingStart()
{
_generatedLocation = _writer.GetCurrentSourceLocation();
Expand All @@ -54,9 +68,9 @@ public void MarkLineMappingEnd()
_generatedContentLength = _writer.GenerateCode().Length - _generatedLocation.AbsoluteIndex;
}

protected virtual void Dispose(bool disposing)
public void Dispose()
{
if(disposing)
if (_addLineMapping)
{
// Verify that the generated length has not already been calculated
if (_generatedContentLength == 0)
Expand All @@ -73,34 +87,29 @@ protected virtual void Dispose(bool disposing)
_writer.LineMappingManager.AddMapping(
documentLocation: _documentMapping,
generatedLocation: new MappingLocation(_generatedLocation, _generatedContentLength));
}

if (_writePragmas)
{
// 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 endsWithNewline = _writer.GenerateCode().EndsWith("\n");

// Always write at least 1 empty line to potentially separate code from pragmas.
_writer.WriteLine();
if (_writePragmas)
{
// 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 endsWithNewline = _writer.GenerateCode().EndsWith("\n");

// Check if the previous empty line wasn't enough to separate code from pragmas.
if (!endsWithNewline)
{
_writer.WriteLine();
}
// Always write at least 1 empty line to potentially separate code from pragmas.
_writer.WriteLine();

_writer.WriteLineDefaultDirective()
.WriteLineHiddenDirective();
// Check if the previous empty line wasn't enough to separate code from pragmas.
if (!endsWithNewline)
{
_writer.WriteLine();
}

// Reset indent back to when it was started
_writer.SetIndent(_startIndent);
_writer.WriteLineDefaultDirective()
.WriteLineHiddenDirective();
}
}

public void Dispose()
{
Dispose(disposing: true);
// Reset indent back to when it was started
_writer.SetIndent(_startIndent);
}
}
}
97 changes: 67 additions & 30 deletions ...icrosoft.AspNet.Razor/Generator/Compiler/CodeBuilder/CSharp/Visitors/CSharpCodeVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ protected override void Visit(ExpressionBlockChunk chunk)

protected override void Visit(ExpressionChunk chunk)
{
CreateExpressionCodeMapping(chunk.Code, chunk);
Writer.Write(chunk.Code);
}

protected override void Visit(StatementChunk chunk)
Expand Down Expand Up @@ -343,66 +343,103 @@ public void RenderDesignTimeExpressionBlockChunk(ExpressionBlockChunk chunk)

public void RenderRuntimeExpressionBlockChunk(ExpressionBlockChunk chunk)
{
var generateInstrumentation = ShouldGenerateInstrumentationForExpressions();
Span contentSpan = null;
// For expression chunks, such as @value, @(value) etc, pick the first Code or Markup span
// from the expression (in this case "value") and use that to calculate the length. This works
// accurately for most parts. The scenarios that don't work are
// (a) Expressions with inline comments (e.g. @(a @* comment *@ b)) - these have multiple code spans
// (b) Expressions with inline templates (e.g. @Foo(@<p>Hello world</p>)).
// Tracked via https://github.com/aspnet/Razor/issues/153

var block = (Block)chunk.Association;
var contentSpan = block.Children
.OfType<Span>()
.FirstOrDefault(s => s.Kind == SpanKind.Code || s.Kind == SpanKind.Markup);

if (generateInstrumentation)
if (Context.ExpressionRenderingMode == ExpressionRenderingMode.InjectCode)
{
Accept(chunk.Children);
}
else if (Context.ExpressionRenderingMode == ExpressionRenderingMode.WriteToOutput)
{
// For expression chunks, such as @value, @(value) etc, pick the first Code or Markup span
// from the expression (in this case "value") and use that to calculate the length. This works
// accurately for most parts. The scenarios that don't work are
// (a) Expressions with inline comments (e.g. @(a @* comment *@ b)) - these have multiple code spans
// (b) Expressions with inline templates (e.g. @Foo(@<p>Hello world</p>)).
// Tracked via https://github.com/aspnet/Razor/issues/153

var block = (Block)chunk.Association;
contentSpan = block.Children
.OfType<Span>()
.FirstOrDefault(s => s.Kind == SpanKind.Code || s.Kind == SpanKind.Markup);

if (contentSpan != null)
{
Writer.WriteStartInstrumentationContext(Context, contentSpan, isLiteral: false);
RenderRuntimeExpressionBlockChunkWithContentSpan(chunk, contentSpan);
}
else
{
if (!string.IsNullOrEmpty(Context.TargetWriterName))
{
Writer.WriteStartMethodInvocation(Context.Host.GeneratedClassContext.WriteToMethodName)
.Write(Context.TargetWriterName)
.WriteParameterSeparator();
}
else
{
Writer.WriteStartMethodInvocation(Context.Host.GeneratedClassContext.WriteMethodName);
}

Accept(chunk.Children);

Writer.WriteEndMethodInvocation()
.WriteLine();
}
}
}

if (Context.ExpressionRenderingMode == ExpressionRenderingMode.InjectCode)
private void RenderRuntimeExpressionBlockChunkWithContentSpan(ExpressionBlockChunk chunk, Span contentSpan)
{
var generateInstrumentation = ShouldGenerateInstrumentationForExpressions();

if (generateInstrumentation)
{
Accept(chunk.Children);
Writer.WriteStartInstrumentationContext(Context, contentSpan, isLiteral: false);
}
else if (Context.ExpressionRenderingMode == ExpressionRenderingMode.WriteToOutput)

using (var mappingWriter = new CSharpLineMappingWriter(Writer, chunk.Start, Context.SourceFile))
{
if (!String.IsNullOrEmpty(Context.TargetWriterName))
if (!string.IsNullOrEmpty(Context.TargetWriterName))
{
Writer.WriteStartMethodInvocation(Context.Host.GeneratedClassContext.WriteToMethodName)
var generatedStart = Context.Host.GeneratedClassContext.WriteToMethodName.Length +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we could potentially calculate the generatedStart preset value once on the creation of the visitor. Up to you if you think its worth it.

Context.TargetWriterName.Length +
3; // 1 for the opening '(' and 2 for ', '

var padding = _paddingBuilder.BuildExpressionPadding(contentSpan, generatedStart);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: extra line.


Writer.Write(padding)
.WriteStartMethodInvocation(Context.Host.GeneratedClassContext.WriteToMethodName)
.Write(Context.TargetWriterName)
.WriteParameterSeparator();
}
else
{
Writer.WriteStartMethodInvocation(Context.Host.GeneratedClassContext.WriteMethodName);
var generatedStart = Context.Host.GeneratedClassContext.WriteMethodName.Length +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we could potentially calculate the generatedStart preset value once on the creation of the visitor. Up to you if you think its worth it.

1; // for the opening '('
var padding = _paddingBuilder.BuildExpressionPadding(contentSpan, generatedStart);

Writer.Write(padding)
.WriteStartMethodInvocation(Context.Host.GeneratedClassContext.WriteMethodName);
}

Accept(chunk.Children);

Writer.WriteEndMethodInvocation()
.WriteLine();
Writer.WriteEndMethodInvocation();
}

if (contentSpan != null)
if (generateInstrumentation)
{
Writer.WriteEndInstrumentationContext(Context);
}
}

public void CreateExpressionCodeMapping(string code, Chunk chunk)
public void CreateStatementCodeMapping(string code, Chunk chunk)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why re-order these methods? Or am I missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad merge. CreateExpressionCodeMapping can be removed since it's not referenced

{
CreateCodeMapping(_paddingBuilder.BuildExpressionPadding((Span)chunk.Association), code, chunk);
CreateCodeMapping(_paddingBuilder.BuildStatementPadding((Span)chunk.Association), code, chunk);
}

public void CreateStatementCodeMapping(string code, Chunk chunk)
public void CreateExpressionCodeMapping(string code, Chunk chunk)
{
CreateCodeMapping(_paddingBuilder.BuildStatementPadding((Span)chunk.Association), code, chunk);
CreateCodeMapping(_paddingBuilder.BuildExpressionPadding((Span)chunk.Association), code, chunk);
}

public void CreateCodeMapping(string padding, string code, Chunk chunk)
Expand Down
5 changes: 1 addition & 4 deletions test/Microsoft.AspNet.Razor.Test/Generator/CSharpRazorCodeGeneratorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,6 @@ public void CSharpCodeGeneratorCorrectlyGeneratesMappingsForRazorCommentsAtDesig
BuildLineMapping(238, 11, 899, 45, 2, 24),
BuildLineMapping(310, 12, 1036, 51, 45, 3),
BuildLineMapping(323, 14, 2, 1112, 56, 6, 1),
BuildLineMapping(328, 14, 1155, 58, 7, 1)

});
}

Expand Down Expand Up @@ -296,8 +294,7 @@ public void CSharpCodeGeneratorCorrectlyGeneratesDesignTimePragmasMarkupAndExpre
BuildLineMapping(113, 7, 2, 1262, 71, 6, 12),
BuildLineMapping(129, 8, 1, 1343, 76, 6, 4),
BuildLineMapping(142, 8, 1443, 78, 14, 3),
BuildLineMapping(153, 8, 1540, 85, 25, 1),
BuildLineMapping(204, 13, 5, 1725, 95, 6, 3)
BuildLineMapping(204, 13, 5, 1638, 90, 6, 3)
});
}

Expand Down
Loading