From 7182b28f066ca625ee99bf7a388078ff02db92e0 Mon Sep 17 00:00:00 2001 From: Ajay Bhargav Baaskaran Date: Wed, 19 Feb 2020 11:03:25 -0800 Subject: [PATCH] Respect tabs vs spaces when generating code --- .../CodeGeneration/CodeWriter.cs | 50 ++++++++++++++++--- .../CodeGeneration/CodeWriterExtensions.cs | 30 ++--------- .../CodeGeneration/DefaultDocumentWriter.cs | 2 +- .../DefaultProjectSnapshotManagerAccessor.cs | 13 ++++- .../DefaultRazorConfigurationService.cs | 32 ++++++++++-- .../RazorLSPOptions.cs | 10 +++- ...moteProjectSnapshotProjectEngineFactory.cs | 42 +++++++++++++++- .../DefaultRazorConfigurationServiceTest.cs | 12 +++-- .../Formatting/RazorFormattingEndpointTest.cs | 2 +- .../RazorLSPOptionsMonitorTest.cs | 4 +- .../TestComponent.codegen.cs | 2 +- .../TestComponent.mappings.txt | 2 +- 12 files changed, 148 insertions(+), 53 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/CodeWriter.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/CodeWriter.cs index 5b44fbc7fc3..88807bc4d14 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/CodeWriter.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/CodeWriter.cs @@ -18,9 +18,16 @@ public sealed class CodeWriter private int _currentLineIndex; private int _currentLineCharacterIndex; - public CodeWriter() + public CodeWriter() : this(Environment.NewLine, 4, false) { - NewLine = Environment.NewLine; + } + + public CodeWriter(string newLine, int tabSize, bool indentWithTabs) + { + NewLine = newLine; + TabSize = tabSize; + IndentWithTabs = indentWithTabs; + _builder = new StringBuilder(); } @@ -47,6 +54,10 @@ public string NewLine } } + public int TabSize { get; set; } + + public bool IndentWithTabs { get; set; } + public SourceLocation Location => new SourceLocation(_absoluteIndex, _currentLineIndex, _currentLineCharacterIndex); public char this[int index] @@ -62,15 +73,40 @@ public char this[int index] } } - // Internal for testing. - internal CodeWriter Indent(int size) + public CodeWriter Indent(int size) { if (Length == 0 || this[Length - 1] == '\n') { - _builder.Append(' ', size); + var actualSize = 0; + if (IndentWithTabs) + { + // Avoid writing directly to the StringBuilder here, that will throw off the manual indexing + // done by the base class. + var tabs = size / TabSize; + actualSize += tabs; + for (var i = 0; i < tabs; i++) + { + _builder.Append("\t"); + } + + var spaces = size % TabSize; + actualSize += spaces; + for (var i = 0; i < spaces; i++) + { + _builder.Append(" "); + } + } + else + { + actualSize = size; + for (var i = 0; i < size; i++) + { + _builder.Append(" "); + } + } - _currentLineCharacterIndex += size; - _absoluteIndex += size; + _currentLineCharacterIndex += actualSize; + _absoluteIndex += actualSize; } return this; diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/CodeWriterExtensions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/CodeWriterExtensions.cs index eee9f37927c..c92b53f2853 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/CodeWriterExtensions.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/CodeWriterExtensions.cs @@ -48,29 +48,7 @@ public static CodeWriter WritePadding(this CodeWriter writer, int offset, Source var basePadding = CalculatePadding(); var resolvedPadding = Math.Max(basePadding - offset, 0); - if (context.Options.IndentWithTabs) - { - // Avoid writing directly to the StringBuilder here, that will throw off the manual indexing - // done by the base class. - var tabs = resolvedPadding / context.Options.IndentSize; - for (var i = 0; i < tabs; i++) - { - writer.Write("\t"); - } - - var spaces = resolvedPadding % context.Options.IndentSize; - for (var i = 0; i < spaces; i++) - { - writer.Write(" "); - } - } - else - { - for (var i = 0; i < resolvedPadding; i++) - { - writer.Write(" "); - } - } + writer.Indent(resolvedPadding); return writer; @@ -86,7 +64,7 @@ int CalculatePadding() } else if (@char == '\t') { - spaceCount += context.Options.IndentSize; + spaceCount += writer.TabSize; } else { @@ -569,11 +547,11 @@ public struct CSharpCodeWritingScope : IDisposable private int _tabSize; private int _startIndent; - public CSharpCodeWritingScope(CodeWriter writer, int tabSize = 4, bool autoSpace = true) + public CSharpCodeWritingScope(CodeWriter writer, bool autoSpace = true) { _writer = writer; _autoSpace = autoSpace; - _tabSize = tabSize; + _tabSize = writer.TabSize; _startIndent = -1; // Set in WriteStartScope WriteStartScope(); diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/DefaultDocumentWriter.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/DefaultDocumentWriter.cs index c5fb4de90e2..774d25830a0 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/DefaultDocumentWriter.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.Language/CodeGeneration/DefaultDocumentWriter.cs @@ -32,7 +32,7 @@ public override RazorCSharpDocument WriteDocument(RazorCodeDocument codeDocument } var context = new DefaultCodeRenderingContext( - new CodeWriter(), + new CodeWriter(Environment.NewLine, _options.IndentSize, _options.IndentWithTabs), _codeTarget.CreateNodeWriter(), codeDocument, documentNode, diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultProjectSnapshotManagerAccessor.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultProjectSnapshotManagerAccessor.cs index e4f9a761131..9027337334b 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultProjectSnapshotManagerAccessor.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultProjectSnapshotManagerAccessor.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.Razor; using Microsoft.CodeAnalysis.Razor.ProjectSystem; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Razor.LanguageServer { @@ -17,12 +18,14 @@ internal class DefaultProjectSnapshotManagerAccessor : ProjectSnapshotManagerAcc private readonly ForegroundDispatcher _foregroundDispatcher; private readonly IEnumerable _changeTriggers; private readonly FilePathNormalizer _filePathNormalizer; + private readonly IOptionsMonitor _optionsMonitor; private ProjectSnapshotManagerBase _instance; public DefaultProjectSnapshotManagerAccessor( ForegroundDispatcher foregroundDispatcher, IEnumerable changeTriggers, - FilePathNormalizer filePathNormalizer) + FilePathNormalizer filePathNormalizer, + IOptionsMonitor optionsMonitor) { if (foregroundDispatcher == null) { @@ -39,9 +42,15 @@ public DefaultProjectSnapshotManagerAccessor( throw new ArgumentNullException(nameof(filePathNormalizer)); } + if (optionsMonitor is null) + { + throw new ArgumentNullException(nameof(optionsMonitor)); + } + _foregroundDispatcher = foregroundDispatcher; _changeTriggers = changeTriggers; _filePathNormalizer = filePathNormalizer; + _optionsMonitor = optionsMonitor; } public override ProjectSnapshotManagerBase Instance @@ -53,7 +62,7 @@ public override ProjectSnapshotManagerBase Instance var services = AdhocServices.Create( workspaceServices: new[] { - new RemoteProjectSnapshotProjectEngineFactory(_filePathNormalizer) + new RemoteProjectSnapshotProjectEngineFactory(_filePathNormalizer, _optionsMonitor) }, razorLanguageServices: Enumerable.Empty()); var workspace = new AdhocWorkspace(services); diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs index 8230089aea3..59e5f1187c2 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs @@ -41,6 +41,10 @@ public async override Task GetLatestOptionsAsync() { Items = new[] { + new ConfigurationItem() + { + Section = "editor" + }, new ConfigurationItem() { Section = "razor" @@ -49,16 +53,22 @@ public async override Task GetLatestOptionsAsync() }; var result = await _server.Client.SendRequest("workspace/configuration", request); - if (result == null || result.Length < 1 || result[0] == null) + if (result == null || result.Length < 2 || result[0] == null) { _logger.LogWarning("Client failed to provide the expected configuration."); return null; } - var jsonString = result[0].ToString(); var builder = new ConfigurationBuilder(); - using var stream = new MemoryStream(Encoding.UTF8.GetBytes(jsonString)); - builder.AddJsonStream(stream); + + var editorJsonString = result[0].ToString(); + using var editorStream = new MemoryStream(Encoding.UTF8.GetBytes(editorJsonString)); + builder.AddJsonStream(editorStream); + + var razorJsonString = result[1].ToString(); + using var razorStream = new MemoryStream(Encoding.UTF8.GetBytes(razorJsonString)); + builder.AddJsonStream(razorStream); + var config = builder.Build(); var instance = BuildOptions(config); @@ -83,7 +93,19 @@ private RazorLSPOptions BuildOptions(IConfiguration config) enableFormatting = parsedEnableFormatting; } - return new RazorLSPOptions(trace, enableFormatting); + var tabSize = instance.TabSize; + if (int.TryParse(config["tabSize"], out var parsedTabSize)) + { + tabSize = parsedTabSize; + } + + var insertSpaces = instance.InsertSpaces; + if (bool.TryParse(config["insertSpaces"], out var parsedInsertSpaces)) + { + insertSpaces = parsedInsertSpaces; + } + + return new RazorLSPOptions(trace, enableFormatting, tabSize, insertSpaces); } } } diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs index 6201ce99dbb..80bfc950681 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs @@ -10,13 +10,15 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer { internal class RazorLSPOptions : IEquatable { - public RazorLSPOptions(Trace trace, bool enableFormatting) + public RazorLSPOptions(Trace trace, bool enableFormatting, int tabSize, bool insertSpaces) { Trace = trace; EnableFormatting = enableFormatting; + TabSize = tabSize; + InsertSpaces = insertSpaces; } - public static RazorLSPOptions Default => new RazorLSPOptions(trace: default, enableFormatting: true); + public static RazorLSPOptions Default => new RazorLSPOptions(trace: default, enableFormatting: true, tabSize: 4, insertSpaces: true); public Trace Trace { get; } @@ -24,6 +26,10 @@ public RazorLSPOptions(Trace trace, bool enableFormatting) public bool EnableFormatting { get; } + public int TabSize { get; } + + public bool InsertSpaces { get; } + public static LogLevel GetLogLevelForTrace(Trace trace) { return trace switch diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RemoteProjectSnapshotProjectEngineFactory.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RemoteProjectSnapshotProjectEngineFactory.cs index e8acfc5b503..359ff19b8c2 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RemoteProjectSnapshotProjectEngineFactory.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RemoteProjectSnapshotProjectEngineFactory.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Razor.LanguageServer.Common; using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem; using Microsoft.CodeAnalysis.Razor; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Razor.LanguageServer { @@ -15,8 +16,9 @@ internal class RemoteProjectSnapshotProjectEngineFactory : DefaultProjectSnapsho public static readonly IFallbackProjectEngineFactory FallbackProjectEngineFactory = new FallbackProjectEngineFactory(); private readonly FilePathNormalizer _filePathNormalizer; + private readonly IOptionsMonitor _optionsMonitor; - public RemoteProjectSnapshotProjectEngineFactory(FilePathNormalizer filePathNormalizer) : + public RemoteProjectSnapshotProjectEngineFactory(FilePathNormalizer filePathNormalizer, IOptionsMonitor optionsMonitor) : base(FallbackProjectEngineFactory, ProjectEngineFactories.Factories) { if (filePathNormalizer == null) @@ -24,7 +26,13 @@ public RemoteProjectSnapshotProjectEngineFactory(FilePathNormalizer filePathNorm throw new ArgumentNullException(nameof(filePathNormalizer)); } + if (optionsMonitor is null) + { + throw new ArgumentNullException(nameof(optionsMonitor)); + } + _filePathNormalizer = filePathNormalizer; + _optionsMonitor = optionsMonitor; } public override RazorProjectEngine Create( @@ -39,7 +47,37 @@ public override RazorProjectEngine Create( } var remoteFileSystem = new RemoteRazorProjectFileSystem(defaultFileSystem.Root, _filePathNormalizer); - return base.Create(configuration, remoteFileSystem, configure); + return base.Create(configuration, remoteFileSystem, Configure); + + void Configure(RazorProjectEngineBuilder builder) + { + configure(builder); + builder.Features.Add(new RemoteCodeGenerationOptionsFeature(_optionsMonitor)); + } + } + + private class RemoteCodeGenerationOptionsFeature : RazorEngineFeatureBase, IConfigureRazorCodeGenerationOptionsFeature + { + private readonly IOptionsMonitor _optionsMonitor; + + public RemoteCodeGenerationOptionsFeature(IOptionsMonitor optionsMonitor) + { + if (optionsMonitor is null) + { + throw new ArgumentNullException(nameof(optionsMonitor)); + } + + _optionsMonitor = optionsMonitor; + } + + public int Order { get; set; } + + public void Configure(RazorCodeGenerationOptionsBuilder options) + { + // We don't need to explicitly subscribe to options changing because this method will be run on every parse. + options.IndentSize = _optionsMonitor.CurrentValue.TabSize; + options.IndentWithTabs = !_optionsMonitor.CurrentValue.InsertSpaces; + } } } } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs index ff6602f29b9..89b9c783f1a 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs @@ -17,8 +17,14 @@ public class DefaultRazorConfigurationServiceTest : LanguageServerTestBase public async Task GetLatestOptionsAsync_ReturnsExpectedOptions() { // Arrange - var expectedOptions = new RazorLSPOptions(Trace.Messages, enableFormatting: false); - var jsonString = @" + var expectedOptions = new RazorLSPOptions(Trace.Messages, enableFormatting: false, tabSize: 8, insertSpaces: true); + var editorJsonString = @" +{ + ""tabSize"": 8, + ""insertSpaces"": ""true"" +} +".Trim(); + var razorJsonString = @" { ""trace"": ""Messages"", ""format"": { @@ -26,7 +32,7 @@ public async Task GetLatestOptionsAsync_ReturnsExpectedOptions() } } ".Trim(); - var result = new object[] { jsonString }; + var result = new object[] { editorJsonString, razorJsonString }; var languageServer = GetLanguageServer(result); var configurationService = new DefaultRazorConfigurationService(languageServer, LoggerFactory); diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/RazorFormattingEndpointTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/RazorFormattingEndpointTest.cs index aa9a4f202fb..3df6cc83102 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/RazorFormattingEndpointTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Formatting/RazorFormattingEndpointTest.cs @@ -111,7 +111,7 @@ public async Task Handle_FormattingDisabled_ReturnsNull() private static IOptionsMonitor GetOptionsMonitor(bool enableFormatting) { var monitor = new Mock>(); - monitor.SetupGet(m => m.CurrentValue).Returns(new RazorLSPOptions(default, enableFormatting)); + monitor.SetupGet(m => m.CurrentValue).Returns(new RazorLSPOptions(default, enableFormatting, tabSize: 4, insertSpaces: true)); return monitor.Object; } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/RazorLSPOptionsMonitorTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/RazorLSPOptionsMonitorTest.cs index 4935e002844..60fc0db84ca 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/RazorLSPOptionsMonitorTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/RazorLSPOptionsMonitorTest.cs @@ -23,7 +23,7 @@ public RazorLSPOptionsMonitorTest() public async Task UpdateAsync_Invokes_OnChangeRegistration() { // Arrange - var expectedOptions = new RazorLSPOptions(Trace.Messages, enableFormatting: false); + var expectedOptions = new RazorLSPOptions(Trace.Messages, enableFormatting: false, tabSize: 4, insertSpaces: true); var configService = Mock.Of(f => f.GetLatestOptionsAsync() == Task.FromResult(expectedOptions)); var optionsMonitor = new RazorLSPOptionsMonitor(configService, Cache); var called = false; @@ -42,7 +42,7 @@ public async Task UpdateAsync_Invokes_OnChangeRegistration() public async Task UpdateAsync_DoesNotInvoke_OnChangeRegistration_AfterDispose() { // Arrange - var expectedOptions = new RazorLSPOptions(Trace.Messages, enableFormatting: false); + var expectedOptions = new RazorLSPOptions(Trace.Messages, enableFormatting: false, tabSize: 8, insertSpaces: false); var configService = Mock.Of(f => f.GetLatestOptionsAsync() == Task.FromResult(expectedOptions)); var optionsMonitor = new RazorLSPOptionsMonitor(configService, Cache); var called = false; diff --git a/src/Razor/test/RazorLanguage.Test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/SingleLineControlFlowStatements_InCodeBlock/TestComponent.codegen.cs b/src/Razor/test/RazorLanguage.Test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/SingleLineControlFlowStatements_InCodeBlock/TestComponent.codegen.cs index 4b5f6a8517b..c72365f1eb5 100644 --- a/src/Razor/test/RazorLanguage.Test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/SingleLineControlFlowStatements_InCodeBlock/TestComponent.codegen.cs +++ b/src/Razor/test/RazorLanguage.Test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/SingleLineControlFlowStatements_InCodeBlock/TestComponent.codegen.cs @@ -45,7 +45,7 @@ protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Renderin #line default #line hidden #nullable disable - + } #pragma warning restore 1998 diff --git a/src/Razor/test/RazorLanguage.Test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/SingleLineControlFlowStatements_InCodeBlock/TestComponent.mappings.txt b/src/Razor/test/RazorLanguage.Test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/SingleLineControlFlowStatements_InCodeBlock/TestComponent.mappings.txt index 34c80001e6a..fe1a4a0917e 100644 --- a/src/Razor/test/RazorLanguage.Test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/SingleLineControlFlowStatements_InCodeBlock/TestComponent.mappings.txt +++ b/src/Razor/test/RazorLanguage.Test/TestFiles/IntegrationTests/ComponentDesignTimeCodeGenerationTest/SingleLineControlFlowStatements_InCodeBlock/TestComponent.mappings.txt @@ -24,7 +24,7 @@ Generated Location: (1301:42,16 [6] ) Source Location: (216:6,26 [2] x:\dir\subdir\Test\TestComponent.cshtml) | | -Generated Location: (1398:47,38 [2] ) +Generated Location: (1386:47,26 [2] ) | |