From addfb9422160cf2c1ef613c52607bb7fff58b8c6 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Fri, 1 Sep 2023 09:15:37 -0700 Subject: [PATCH 1/6] add combinatorial tests for extra trivia and progressively typed code --- .../ParserTests.Combinatorial.cs | 89 ++++++++++++++++--- .../ParserTests.Version.cs | 38 ++++++++ .../Utility/HttpBodyNodeSyntaxSpec.cs | 40 +++++++-- .../Parser/HttpRequestParser.cs | 66 ++++++++------ .../Parser/HttpSyntaxNode.cs | 7 +- 5 files changed, 187 insertions(+), 53 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs index 1c54c64c41..5db22c9c4e 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs @@ -1,10 +1,10 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.Collections.Generic; using System.Linq; using FluentAssertions; -using Microsoft.DotNet.Interactive.Formatting; using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility; using Xunit; using Xunit.Abstractions; @@ -41,6 +41,25 @@ public void Valid_syntax_produces_expected_parse_tree_and_no_diagnostics(ISyntax syntaxSpec.Validate(parseResult.SyntaxTree.RootNode.ChildNodes.Single()); } + [Theory] + [MemberData(nameof(GenerateValidRequestsWithExtraTrivia))] + public void Valid_syntax_with_extra_trivia_produces_expected_parse_tree_and_no_diagnostics(ISyntaxSpec syntaxSpec, int index) + { + var code = syntaxSpec.ToString(); + + var parseResult = HttpRequestParser.Parse(code); + + _output.WriteLine($""" + === Generation #{index} === + + {code} + """); + + parseResult.GetDiagnostics().Should().BeEmpty(); + + syntaxSpec.Validate(parseResult.SyntaxTree.RootNode.ChildNodes.Single()); + } + [Theory] [MemberData(nameof(GenerateInvalidRequests))] public void Invalid_syntax_produces_diagnostics(ISyntaxSpec syntaxSpec, int index) @@ -59,10 +78,30 @@ public void Invalid_syntax_produces_diagnostics(ISyntaxSpec syntaxSpec, int inde syntaxSpec.Validate(parseResult.SyntaxTree.RootNode.ChildNodes.Single()); } - + + [Theory] + [MemberData(nameof(GenerateValidRequestsWithExtraTrivia))] + public void Code_that_a_user_has_not_finished_typing_round_trips_correctly_and_does_not_throw(ISyntaxSpec syntaxSpec, int index) + { + var code = syntaxSpec.ToString(); + + for (var truncateAfter = 0; truncateAfter < code.Length; truncateAfter++) + { + var truncatedCode = code[..truncateAfter]; + + _output.WriteLine($""" + === Generation #{index} truncated after {truncateAfter} characters === + + {truncatedCode} + """); + + Parse(truncatedCode); + } + } + public static IEnumerable GenerateValidRequests() { - var i = 0; + var generationNumber = 0; foreach (var method in ValidMethods()) foreach (var url in ValidUrls()) @@ -70,18 +109,40 @@ public static IEnumerable GenerateValidRequests() foreach (var headerSection in ValidHeaderSections()) foreach (var bodySection in ValidBodySections()) { - ++i; + ++generationNumber; yield return new object[] { new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection), - i + generationNumber + }; + } + } + + public static IEnumerable GenerateValidRequestsWithExtraTrivia() + { + var generationNumber = 0; + + foreach (var method in ValidMethods()) + foreach (var url in ValidUrls()) + foreach (var version in ValidVersions()) + foreach (var headerSection in ValidHeaderSections()) + foreach (var bodySection in ValidBodySections()) + { + ++generationNumber; + yield return new object[] + { + new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection) + { + ExtraTriviaRandomizer = new Random(1) + }, + generationNumber }; } } public static IEnumerable GenerateInvalidRequests() { - var i = 0; + var generationNumber = 0; foreach (var method in InvalidMethods()) foreach (var url in ValidUrls()) @@ -89,11 +150,11 @@ public static IEnumerable GenerateInvalidRequests() foreach (var headerSection in ValidHeaderSections()) foreach (var bodySection in ValidBodySections()) { - ++i; + ++generationNumber; yield return new object[] { new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection), - i + generationNumber }; } @@ -103,11 +164,11 @@ public static IEnumerable GenerateInvalidRequests() foreach (var headerSection in ValidHeaderSections()) foreach (var bodySection in ValidBodySections()) { - ++i; + ++generationNumber; yield return new object[] { new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection), - i + generationNumber }; } @@ -117,11 +178,11 @@ public static IEnumerable GenerateInvalidRequests() foreach (var headerSection in ValidHeaderSections()) foreach (var bodySection in ValidBodySections()) { - ++i; + ++generationNumber; yield return new object[] { new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection), - i + generationNumber }; } @@ -131,11 +192,11 @@ public static IEnumerable GenerateInvalidRequests() foreach (var headerSection in InvalidHeaderSections()) foreach (var bodySection in ValidBodySections()) { - ++i; + ++generationNumber; yield return new object[] { new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection), - i + generationNumber }; } } diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Version.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Version.cs index 3335c46f10..0b224b6a13 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Version.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Version.cs @@ -42,5 +42,43 @@ public void http_version_containing_whitespace_produces_a_diagnostic() versionNode.GetDiagnostics().Should().ContainSingle() .Which.Message.Should().Be("Invalid HTTP version"); } + + [Fact] + public void extra_whitespace_around_HTTP_version_does_not_produce_diagnostics() + { + var code = """ + + https://example.com HTTP/1.1 + Accept: */* + Accept-Encoding: gzip, deflate, br + Accept-Language: en-US,en;q=0.9 + Content-Length: 7060 + Cookie: expor=;HSD=Ak_1ZasdqwASDASD;SSID=SASASSDFsdfsdf213123;APISID=WRQWRQWRQWRcc123123; + Origin: https://www.bing.com + Referer: https://www.bing.com/ + + + + Gambardella, Matthew + XML Developer's Guide + Computer + 44.95 + 2000-10-01 + An in-depth look at creating applications + with XML. + + + + """; + + var result = Parse(code); + + var httpVersionNode = result.SyntaxTree.RootNode.DescendantNodesAndTokens().Should().ContainSingle() + .Which; + + httpVersionNode.Text.Should().Be("HTTP/1.1"); + + httpVersionNode.GetDiagnostics().Should().BeEmpty(); + } } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/Utility/HttpBodyNodeSyntaxSpec.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/Utility/HttpBodyNodeSyntaxSpec.cs index 34dca2c474..e053e6156c 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/Utility/HttpBodyNodeSyntaxSpec.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/Utility/HttpBodyNodeSyntaxSpec.cs @@ -87,6 +87,8 @@ public HttpRequestNodeSyntaxSpec( public HttpHeadersNodeSyntaxSpec HeadersSection { get; } public HttpBodyNodeSyntaxSpec BodySection { get; } + + public Random ExtraTriviaRandomizer { get; set; } public override void Validate(HttpRequestNode requestNode) { @@ -163,6 +165,7 @@ public override string ToString() var sb = new StringBuilder(); sb.Append(MaybeNewLines()); + sb.Append(MaybeLineComment()); sb.Append(MaybeWhitespace()); sb.Append(Method); @@ -180,6 +183,7 @@ public override string ToString() sb.Append(MaybeWhitespace()); sb.AppendLine(); + sb.Append(MaybeLineComment()); sb.Append(MaybeNewLines()); if (HeadersSection is not null) @@ -197,18 +201,36 @@ public override string ToString() sb.Append(MaybeNewLines()); } - return sb.ToString(); - } + sb.Append(MaybeLineComment()); - private string MaybeWhitespace() - { - return ""; + return sb.ToString(); } - private string MaybeNewLines() - { - return ""; - } + private string MaybeWhitespace() => + ExtraTriviaRandomizer?.NextDouble() switch + { + < .2 => " ", + > .2 and < .4 => " ", + > .4 and < .6 => "\t", + > .6 and < .8 => "\t ", + _ => "" + }; + + private string MaybeNewLines() => + ExtraTriviaRandomizer?.NextDouble() switch + { + < .2 => "\n", + > .2 and < .4 => "\r\n", + _ => "" + }; + + private string MaybeLineComment() => + ExtraTriviaRandomizer?.NextDouble() switch + { + // FIX: (MaybeLineComment) + < 0 => "# random line comment", + _ => "" + }; } internal class HttpMethodNodeSyntaxSpec : SyntaxSpecBase diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs index 8f6c9976aa..0c91b02932 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs @@ -187,21 +187,17 @@ private HttpVariableDeclarationNode ParseVariableDeclaration() private HttpSyntaxToken CurrentToken => _tokens![_currentTokenIndex]; - private HttpSyntaxToken? NextToken + private HttpSyntaxToken? CurrentTokenPlus(int offset) { - get + var nextTokenIndex = _currentTokenIndex + offset; + + if (nextTokenIndex >= _tokens!.Count) { - var nextTokenIndex = _currentTokenIndex + 1; - return nextTokenIndex >= _tokens!.Count ? null : _tokens![nextTokenIndex]; + return null; } - } - - private HttpSyntaxToken? NextNextToken - { - get + else { - var nextNextTokenIndex = _currentTokenIndex + 2; - return nextNextTokenIndex >= _tokens!.Count ? null : _tokens![nextNextTokenIndex]; + return _tokens![nextTokenIndex]; } } @@ -229,8 +225,8 @@ private T ParseLeadingTrivia(T node) where T : HttpSyntaxNode ConsumeCurrentTokenInto(node); } else if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && - !(NextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && - NextNextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" })) + !(CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && + CurrentTokenPlus(2) is { Kind: HttpTokenKind.Punctuation } and { Text: "#" })) { if (ParseComment() is { } commentNode) { @@ -238,7 +234,7 @@ private T ParseLeadingTrivia(T node) where T : HttpSyntaxNode } } else if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && - NextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) + CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) { if (ParseComment() is { } commentNode) { @@ -379,7 +375,7 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto if (MoreTokens()) { if (CurrentToken.Kind is HttpTokenKind.Word && - NextToken?.Kind is HttpTokenKind.Whitespace) + CurrentTokenPlus(1)?.Kind is HttpTokenKind.Whitespace) { node = new HttpMethodNode(_sourceText, _syntaxTree); @@ -472,7 +468,7 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto private bool IsAtStartOfEmbeddedExpression() { return CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "{" } && - NextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "{" }; + CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "{" }; } private HttpEmbeddedExpressionNode ParseEmbeddedExpression() @@ -481,7 +477,11 @@ private HttpEmbeddedExpressionNode ParseEmbeddedExpression() node.Add(ParseExpressionStart()); node.Add(ParseExpression()); - node.Add(ParseExpressionEnd()); + var expressionEnd = ParseExpressionEnd(); + if (expressionEnd is not null) + { + node.Add(expressionEnd); + } return node; } @@ -502,7 +502,8 @@ private HttpExpressionNode ParseExpression() ParseLeadingTrivia(node); while (MoreTokens() && - !(CurrentToken is { Text: "}" } && NextToken is { Text: "}" })) + !(CurrentToken is { Text: "}" } && + CurrentTokenPlus(1) is { Text: "}" })) { ConsumeCurrentTokenInto(node); } @@ -510,14 +511,23 @@ private HttpExpressionNode ParseExpression() return ParseTrailingTrivia(node); } - private HttpExpressionEndNode ParseExpressionEnd() + private HttpExpressionEndNode? ParseExpressionEnd() { - var node = new HttpExpressionEndNode(_sourceText, _syntaxTree); + if (MoreTokens() && + CurrentToken.Text is "}" && + CurrentTokenPlus(1)?.Text == "}") + { + var node = new HttpExpressionEndNode(_sourceText, _syntaxTree); - ConsumeCurrentTokenInto(node); // parse the first } - ConsumeCurrentTokenInto(node); // parse the second } + ConsumeCurrentTokenInto(node); // parse the first } + ConsumeCurrentTokenInto(node); // parse the second } - return node; + return node; + } + else + { + return null; + } } private HttpVersionNode? ParseVersion() @@ -619,7 +629,7 @@ private HttpHeaderValueNode ParseHeaderValue() while (MoreTokens() && CurrentToken.Kind is not HttpTokenKind.NewLine) { if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "{" } && - NextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "{" }) + CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "{" }) { node.Add(ParseEmbeddedExpression()); } @@ -729,7 +739,7 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && } if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && - NextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) + CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) { var node = new HttpCommentStartNode(_sourceText, _syntaxTree); @@ -750,7 +760,7 @@ private bool IsComment() return true; } else if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && - NextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) + CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) { return true; } @@ -765,8 +775,8 @@ private bool IsComment() private bool IsRequestSeparator() { return CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && - (NextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && - NextNextToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" }); + (CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && + CurrentTokenPlus(2) is { Kind: HttpTokenKind.Punctuation } and { Text: "#" }); } private static LinePosition GetLinePositionFromCursorOffset(SourceText code, int cursorOffset) diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpSyntaxNode.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpSyntaxNode.cs index 302e3920a2..2b8877808e 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpSyntaxNode.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpSyntaxNode.cs @@ -60,13 +60,16 @@ private protected HttpSyntaxNode( protected bool TextContainsWhitespace() { - // ignore whitespace if it's the first or last token + // We ignore whitespace if it's the first or last token, OR ignore the first or last token if it's not whitespace. For this reason, the first and last tokens aren't interesting. for (var i = 1; i < _childNodesAndTokens.Count - 1; i++) { var nodeOrToken = _childNodesAndTokens[i]; if (nodeOrToken is HttpSyntaxToken { Kind: HttpTokenKind.Whitespace }) { - return true; + if (nodeOrToken.Span.OverlapsWith(_span)) + { + return true; + } } } From f624feba697cc9a031f19e6ab06b6723353b5ae7 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Fri, 8 Sep 2023 13:28:30 -0700 Subject: [PATCH 2/6] improve comment parsing; more combinatorial testing --- .../InteractiveDocument.cs | 7 +- .../InteractiveDocumentElement.cs | 2 +- .../PlainTextSummaryFormatter.cs | 1 - .../ParserTests.Body.cs | 41 ++- .../ParserTests.Combinatorial.cs | 3 + .../ParserTests.Headers.cs | 22 +- .../ParserTests.Url.cs | 6 +- .../ParserTests.cs | 9 +- .../Utility/HttpBodyNodeSyntaxSpec.cs | 7 +- .../Parser/HttpBodySeparatorNode.cs | 15 -- .../Parser/HttpRequestNode.cs | 12 - .../Parser/HttpRequestParser.cs | 249 +++++++++--------- 12 files changed, 184 insertions(+), 190 deletions(-) delete mode 100644 src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpBodySeparatorNode.cs diff --git a/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs b/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs index 4498dc1de3..f50b7e54b8 100644 --- a/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs +++ b/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs @@ -65,7 +65,7 @@ public async IAsyncEnumerable GetImportsAsync(bool recursiv { EnsureImportFieldParserIsInitialized(); - if (!TryGetKernelInfosFromMetadata(out var kernelInfos)) + if (!TryGetKernelInfosFromMetadata(Metadata, out var kernelInfos)) { kernelInfos = new(); } @@ -198,6 +198,7 @@ public static async Task LoadAsync( public string? GetDefaultKernelName() { + // FIX: (GetDefaultKernelName) remove from public API if (TryGetKernelInfosFromMetadata(Metadata, out var kernelInfo)) { return kernelInfo.DefaultKernelName; @@ -259,10 +260,6 @@ internal static void MergeKernelInfos(KernelInfoCollection destination, KernelIn destination.AddRange(source.Where(ki => added.Add(ki.Name))); } - public bool TryGetKernelInfosFromMetadata( - [NotNullWhen(true)] out KernelInfoCollection? kernelInfos) => - TryGetKernelInfosFromMetadata(Metadata, out kernelInfos); - internal static bool TryGetKernelInfosFromMetadata( IDictionary? metadata, [NotNullWhen(true)] out KernelInfoCollection? kernelInfos) diff --git a/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocumentElement.cs b/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocumentElement.cs index 24a8a6fd2d..fb7f47fba7 100644 --- a/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocumentElement.cs +++ b/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocumentElement.cs @@ -6,7 +6,7 @@ namespace Microsoft.DotNet.Interactive.Documents; -public class InteractiveDocumentElement +public sealed class InteractiveDocumentElement { [JsonConstructor] public InteractiveDocumentElement() diff --git a/src/Microsoft.DotNet.Interactive.Formatting/PlainTextSummaryFormatter.cs b/src/Microsoft.DotNet.Interactive.Formatting/PlainTextSummaryFormatter.cs index f874c2132e..7fbc8a32b6 100644 --- a/src/Microsoft.DotNet.Interactive.Formatting/PlainTextSummaryFormatter.cs +++ b/src/Microsoft.DotNet.Interactive.Formatting/PlainTextSummaryFormatter.cs @@ -8,7 +8,6 @@ namespace Microsoft.DotNet.Interactive.Formatting; public static class PlainTextSummaryFormatter { - // FIX: (PlainTextSummaryFormatter) rename this public const string MimeType = "text/plain+summary"; public static ITypeFormatter GetPreferredFormatterFor(Type type) diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Body.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Body.cs index 901a0523d4..cf03bf63bf 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Body.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Body.cs @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -using System.Linq; +using System; using FluentAssertions; using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility; using Xunit; @@ -12,27 +12,6 @@ public partial class ParserTests { public class Body { - [Fact] - public void body_separator_is_present() - { - var result = Parse( - """ - POST https://example.com/comments HTTP/1.1 - Content-Type: application/xml - Authorization: token xxx - - - sample - - - """); - - var requestNode = result.SyntaxTree.RootNode - .ChildNodes.Should().ContainSingle().Which; - - requestNode.BodySeparatorNode.ChildTokens.First().Kind.Should().Be(HttpTokenKind.NewLine); - } - [Fact] public void body_is_parsed_correctly_when_headers_are_not_present() { @@ -87,5 +66,23 @@ public void multiple_new_lines_before_body_are_parsed_correctly() """); } + + [Fact] + public void Whitespace_after_headers_is_not_parsed_as_body() + { + var code = """ + + hptps://example.com + Accept: */* + + + + + """; + + var result = Parse(code); + + result.SyntaxTree.RootNode.DescendantNodesAndTokens().Should().NotContain(n => n is HttpBodyNode); + } } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs index 5db22c9c4e..3b74cf5a9e 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using FluentAssertions; +using Microsoft.DotNet.Interactive.Formatting; using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility; using Xunit; using Xunit.Abstractions; @@ -13,6 +14,8 @@ namespace Microsoft.DotNet.Interactive.HttpRequest.Tests; public partial class ParserTests { + private readonly ITestOutputHelper _output; + public class Combinatorial { private readonly ITestOutputHelper _output; diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Headers.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Headers.cs index b2a62f3d82..8f8b91e73f 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Headers.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Headers.cs @@ -1,6 +1,7 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.Linq; using FluentAssertions; using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility; @@ -74,6 +75,25 @@ public void header_separator_is_parsed() .Which.Text.Should().Be(":"); } + [Fact] + public void Comments_can_precede_headers() + { + var code = """ + POST https://example.com + # this is a comment + Accept: */* + Accept-Encoding: gzip, deflate, br + """; + + var result = Parse(code); + + var requestNode = result.SyntaxTree.RootNode.DescendantNodesAndTokens() + .Should().ContainSingle().Which; + + requestNode.DescendantNodesAndTokens().Should().ContainSingle().Which.Text.Should().Be("# this is a comment"); + requestNode.ChildNodes.Should().ContainSingle(); + } + [Fact] public void headers_are_parsed_correctly() { diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Url.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Url.cs index 32765fc395..cbe914ef34 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Url.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Url.cs @@ -32,9 +32,9 @@ public void newline_is_legal_at_the_after_url() """); - result.SyntaxTree.RootNode - .ChildNodes.Should().ContainSingle().Which - .ChildTokens.Last().Kind.Should().Be(HttpTokenKind.NewLine); + result.SyntaxTree.RootNode.ChildNodes.Should().ContainSingle(); + + result.GetDiagnostics().Should().BeEmpty(); } [Theory] diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs index ad92f444f5..e6231093c3 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs @@ -2,8 +2,10 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Linq; using FluentAssertions; using FluentAssertions.Execution; +using Xunit.Abstractions; namespace Microsoft.DotNet.Interactive.HttpRequest.Tests; @@ -11,8 +13,9 @@ public partial class ParserTests : IDisposable { private readonly AssertionScope _assertionScope; - public ParserTests() + public ParserTests(ITestOutputHelper output) { + _output = output; _assertionScope = new AssertionScope(); } @@ -24,9 +27,9 @@ public void Dispose() private static HttpRequestParseResult Parse(string code) { var result = HttpRequestParser.Parse(code); - - result.SyntaxTree.RootNode.FullText.Should().Be(code); + result.SyntaxTree.RootNode.FullText.Should().Be(code); + return result; } } diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/Utility/HttpBodyNodeSyntaxSpec.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/Utility/HttpBodyNodeSyntaxSpec.cs index e053e6156c..bd18736182 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/Utility/HttpBodyNodeSyntaxSpec.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/Utility/HttpBodyNodeSyntaxSpec.cs @@ -184,7 +184,6 @@ public override string ToString() sb.Append(MaybeWhitespace()); sb.AppendLine(); sb.Append(MaybeLineComment()); - sb.Append(MaybeNewLines()); if (HeadersSection is not null) { @@ -201,8 +200,6 @@ public override string ToString() sb.Append(MaybeNewLines()); } - sb.Append(MaybeLineComment()); - return sb.ToString(); } @@ -227,8 +224,8 @@ private string MaybeNewLines() => private string MaybeLineComment() => ExtraTriviaRandomizer?.NextDouble() switch { - // FIX: (MaybeLineComment) - < 0 => "# random line comment", + < .2 => "# random line comment followed by a LF\n", + > .2 and < .4 => "# random line comment followed by a CRLF\r\n", _ => "" }; } diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpBodySeparatorNode.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpBodySeparatorNode.cs deleted file mode 100644 index 7fc81106a5..0000000000 --- a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpBodySeparatorNode.cs +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -#nullable enable - -using Microsoft.CodeAnalysis.Text; - -namespace Microsoft.DotNet.Interactive.HttpRequest; - -internal class HttpBodySeparatorNode : HttpSyntaxNode -{ - internal HttpBodySeparatorNode(SourceText sourceText, HttpSyntaxTree? syntaxTree) : base(sourceText, syntaxTree) - { - } -} diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestNode.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestNode.cs index 9e005e579c..5a0604ad44 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestNode.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestNode.cs @@ -27,8 +27,6 @@ internal HttpRequestNode(SourceText sourceText, HttpSyntaxTree? syntaxTree) : ba public HttpHeadersNode? HeadersNode { get; private set; } - public HttpBodySeparatorNode? BodySeparatorNode { get; private set; } - public HttpBodyNode? BodyNode { get; private set; } public void Add(HttpMethodNode node) @@ -71,16 +69,6 @@ public void Add(HttpHeadersNode node) AddInternal(node); } - public void Add(HttpBodySeparatorNode node) - { - if (BodySeparatorNode is not null) - { - throw new InvalidOperationException($"{nameof(BodySeparatorNode)} was already added."); - } - BodySeparatorNode = node; - AddInternal(node); - } - public void Add(HttpBodyNode node) { if (BodyNode is not null) diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs index 0c91b02932..1698a56d93 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs @@ -3,7 +3,6 @@ #nullable enable -using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Text; using System.Collections.Generic; using System.Diagnostics; @@ -74,17 +73,36 @@ public HttpSyntaxTree Parse() if (ParseRequestSeparator() is { } separatorNode) { + // FIX: (Parse) uncovered _syntaxTree.RootNode.Add(separatorNode); } - } return _syntaxTree; } - private IEnumerable? ParseVariableDeclarations() + private static void AppendCommentsIfAny(List commentsToAppend, HttpSyntaxNode prependToNode) { + foreach (var comment in commentsToAppend) + { + prependToNode.Add(comment, addBefore: false); + } + + commentsToAppend.Clear(); + } + + private static void PrependCommentsIfAny(List commentsToPrepend, HttpSyntaxNode prependToNode) + { + foreach (var comment in commentsToPrepend) + { + prependToNode.Add(comment, addBefore: true); + } + + commentsToPrepend.Clear(); + } + private IEnumerable? ParseVariableDeclarations() + { while (MoreTokens()) { if (GetNextSignificantToken() is { Kind: HttpTokenKind.Punctuation } and { Text: "@" }) @@ -104,7 +122,6 @@ public HttpSyntaxTree Parse() break; } } - } private HttpVariableValueNode? ParseVariableValue() @@ -120,7 +137,7 @@ public HttpSyntaxTree Parse() { node = new HttpVariableValueNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); } else { @@ -139,7 +156,7 @@ public HttpSyntaxTree Parse() } return node is not null - ? ParseTrailingTrivia(node, stopAfterNewLine: true) + ? ParseTrailingWhitespace(node, stopAfterNewLine: true) : null; } @@ -147,14 +164,14 @@ private HttpVariableAssignmentNode ParserVariableAssignment() { var node = new HttpVariableAssignmentNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); - if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Word } and { Text: "=" }) + if (MoreTokens() && CurrentToken is { Text: "=" }) { ConsumeCurrentTokenInto(node); } - return ParseTrailingTrivia(node); + return ParseTrailingWhitespace(node); } private HttpVariableDeclarationNode ParseVariableDeclaration() @@ -163,11 +180,11 @@ private HttpVariableDeclarationNode ParseVariableDeclaration() if (MoreTokens()) { - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); while (MoreTokens()) { - if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "@" }) + if (CurrentToken is { Text: "@" }) { ConsumeCurrentTokenInto(node); } @@ -182,7 +199,7 @@ private HttpVariableDeclarationNode ParseVariableDeclaration() } - return ParseTrailingTrivia(node); + return ParseTrailingWhitespace(node); } private HttpSyntaxToken CurrentToken => _tokens![_currentTokenIndex]; @@ -204,15 +221,17 @@ private HttpVariableDeclarationNode ParseVariableDeclaration() [DebuggerStepThrough] private bool MoreTokens() => _tokens!.Count > _currentTokenIndex; + [DebuggerStepThrough] private void AdvanceToNextToken() => _currentTokenIndex++; + [DebuggerStepThrough] private void ConsumeCurrentTokenInto(HttpSyntaxNode node) { node.Add(CurrentToken); AdvanceToNextToken(); } - private T ParseLeadingTrivia(T node) where T : HttpSyntaxNode + private T ParseLeadingWhitespaceAndComments(T node) where T : HttpSyntaxNode { while (MoreTokens()) { @@ -224,22 +243,9 @@ private T ParseLeadingTrivia(T node) where T : HttpSyntaxNode { ConsumeCurrentTokenInto(node); } - else if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && - !(CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && - CurrentTokenPlus(2) is { Kind: HttpTokenKind.Punctuation } and { Text: "#" })) + else if (ParseComment() is { } commentNode) { - if (ParseComment() is { } commentNode) - { - node.Add(commentNode, addBefore: true); - } - } - else if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && - CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) - { - if (ParseComment() is { } commentNode) - { - node.Add(commentNode, addBefore: true); - } + node.Add(commentNode, addBefore: true); } else { @@ -250,13 +256,13 @@ private T ParseLeadingTrivia(T node) where T : HttpSyntaxNode return node; } - private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool stopBeforeNewline = false) where T : HttpSyntaxNode + private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool stopBeforeNewLine = false) where T : HttpSyntaxNode { while (MoreTokens()) { if (CurrentToken.Kind is HttpTokenKind.NewLine) { - if (stopBeforeNewline) + if (stopBeforeNewLine) { break; } @@ -281,16 +287,17 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto private HttpRequestNode? ParseRequest() { - if (IsComment()) { + // FIX: (ParseRequest) this looks suspect... test with a comment at the start of the request node? return null; } + var requestNode = new HttpRequestNode( _sourceText, _syntaxTree); - ParseLeadingTrivia(requestNode); + ParseLeadingWhitespaceAndComments(requestNode); var methodNode = ParseMethod(); if (methodNode is not null) @@ -318,30 +325,37 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto { requestNode.Add(versionNode); } - else + else { - ParseTrailingTrivia(requestNode, stopAfterNewLine: true); + ParseTrailingWhitespace(requestNode, stopAfterNewLine: true); + } + + var commentsToPrepend = new List(); + if (ParseComment() is { } comment) + { + commentsToPrepend.Add(comment); } var headersNode = ParseHeaders(); if (headersNode is not null) { + PrependCommentsIfAny(commentsToPrepend, headersNode); requestNode.Add(headersNode); } - var bodySeparatorNode = ParseBodySeparator(); - if (bodySeparatorNode is not null) - { - requestNode.Add(bodySeparatorNode); - } + ParseTrailingWhitespace(requestNode); var bodyNode = ParseBody(); if (bodyNode is not null) { requestNode.Add(bodyNode); } + else + { + PrependCommentsIfAny(commentsToPrepend, requestNode); + } - ParseTrailingTrivia(requestNode); + ParseTrailingWhitespace(requestNode); return requestNode; } @@ -351,18 +365,13 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto if (MoreTokens() && IsRequestSeparator()) { var node = new HttpRequestSeparatorNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); ConsumeCurrentTokenInto(node); ConsumeCurrentTokenInto(node); ConsumeCurrentTokenInto(node); - - while (MoreTokens() && CurrentToken.Kind is not (HttpTokenKind.NewLine or HttpTokenKind.Whitespace)) - { - ConsumeCurrentTokenInto(node); - } - - ParseTrailingTrivia(node); + + ParseTrailingWhitespace(node); } return null; @@ -379,7 +388,7 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto { node = new HttpMethodNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); if (CurrentToken.Text.ToLower() is not ("get" or "post" or "patch" or "put" or "delete" or "head" or "options" or "trace")) { @@ -392,7 +401,7 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto ConsumeCurrentTokenInto(node); - ParseTrailingTrivia(node, stopBeforeNewline: true); + ParseTrailingWhitespace(node, stopBeforeNewLine: true); } } @@ -412,7 +421,7 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto { node = new HttpUrlNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); } else { @@ -431,7 +440,7 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto } return node is not null - ? ParseTrailingTrivia(node, stopBeforeNewline: true) + ? ParseTrailingWhitespace(node, stopBeforeNewLine: true) : null; } @@ -465,11 +474,9 @@ private T ParseTrailingTrivia(T node, bool stopAfterNewLine = false, bool sto return null; } - private bool IsAtStartOfEmbeddedExpression() - { - return CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "{" } && - CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "{" }; - } + private bool IsAtStartOfEmbeddedExpression() => + CurrentToken is { Text: "{" } && + CurrentTokenPlus(1) is { Text: "{" }; private HttpEmbeddedExpressionNode ParseEmbeddedExpression() { @@ -493,13 +500,13 @@ private HttpExpressionStartNode ParseExpressionStart() ConsumeCurrentTokenInto(node); // parse the first { ConsumeCurrentTokenInto(node); // parse the second { - return ParseTrailingTrivia(node); + return ParseTrailingWhitespace(node); } private HttpExpressionNode ParseExpression() { var node = new HttpExpressionNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); while (MoreTokens() && !(CurrentToken is { Text: "}" } && @@ -508,7 +515,7 @@ private HttpExpressionNode ParseExpression() ConsumeCurrentTokenInto(node); } - return ParseTrailingTrivia(node); + return ParseTrailingWhitespace(node); } private HttpExpressionEndNode? ParseExpressionEnd() @@ -537,7 +544,7 @@ CurrentToken.Text is "}" && { var node = new HttpVersionNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); ConsumeCurrentTokenInto(node); while (MoreTokens() && CurrentToken.Kind is not HttpTokenKind.NewLine && @@ -546,7 +553,7 @@ CurrentToken.Text is "}" && ConsumeCurrentTokenInto(node); } - return ParseTrailingTrivia(node, stopAfterNewLine: true); + return ParseTrailingWhitespace(node, stopAfterNewLine: true); } return null; @@ -557,7 +564,7 @@ CurrentToken.Text is "}" && HttpHeadersNode? headersNode = null; while (MoreTokens() && - (CurrentToken is { Kind: HttpTokenKind.Word } || CurrentToken is { Text: ":" })) + CurrentToken is { Kind: HttpTokenKind.Word } or { Text: ":" }) { headersNode ??= new HttpHeadersNode(_sourceText, _syntaxTree); @@ -585,7 +592,7 @@ private HttpHeaderNameNode ParseHeaderName() if (MoreTokens() && CurrentToken is not { Kind: HttpTokenKind.NewLine } and not { Kind: HttpTokenKind.Punctuation, Text: ":" }) { - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); if (MoreTokens()) { @@ -603,28 +610,28 @@ private HttpHeaderNameNode ParseHeaderName() } } - return ParseTrailingTrivia(node); + return ParseTrailingWhitespace(node); } private HttpHeaderSeparatorNode ParserHeaderSeparator() { var node = new HttpHeaderSeparatorNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: ":" }) { ConsumeCurrentTokenInto(node); } - return ParseTrailingTrivia(node); + return ParseTrailingWhitespace(node); } private HttpHeaderValueNode ParseHeaderValue() { var node = new HttpHeaderValueNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); while (MoreTokens() && CurrentToken.Kind is not HttpTokenKind.NewLine) { @@ -639,34 +646,23 @@ private HttpHeaderValueNode ParseHeaderValue() } } - return ParseTrailingTrivia(node, stopAfterNewLine: true); + return ParseTrailingWhitespace(node, stopAfterNewLine: true); } - private HttpBodySeparatorNode? ParseBodySeparator() + private HttpBodyNode? ParseBody() { - if (!MoreTokens() || IsRequestSeparator()) + if (!MoreTokens()) { return null; } - var node = new HttpBodySeparatorNode(_sourceText, _syntaxTree); - - ParseLeadingTrivia(node); - - return ParseTrailingTrivia(node); - } - - private HttpBodyNode? ParseBody() - { - if (!MoreTokens() || IsRequestSeparator()) + if (IsRequestSeparator()) { return null; } var node = new HttpBodyNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); - if (MoreTokens() && CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && !IsRequestSeparator()) @@ -686,29 +682,34 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && } } - return ParseTrailingTrivia(node); + ParseTrailingWhitespace(node); + + return node; } private HttpCommentNode? ParseComment() { - var commentNode = new HttpCommentNode(_sourceText, _syntaxTree); + if (MoreTokens() && IsComment()) + { + var commentNode = new HttpCommentNode(_sourceText, _syntaxTree); - var commentStartNode = ParseCommentStart(); + var commentStartNode = ParseCommentStart(); - if (commentStartNode is null) - { - return null; - } + if (commentStartNode is not null) + { + commentNode.Add(commentStartNode); + } - commentNode.Add(commentStartNode); + var commentBodyNode = ParseCommentBody(); + if (commentBodyNode is not null) + { + commentNode.Add(commentBodyNode); + } - var commentBodyNode = ParseCommentBody(); - if (commentBodyNode is not null) - { - commentNode.Add(commentBodyNode); + return commentNode; } - return commentNode; + return null; } private HttpCommentBodyNode? ParseCommentBody() @@ -719,13 +720,15 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && } var node = new HttpCommentBodyNode(_sourceText, _syntaxTree); - ParseLeadingTrivia(node); + ParseLeadingWhitespaceAndComments(node); while (MoreTokens() && CurrentToken.Kind is not HttpTokenKind.NewLine) { ConsumeCurrentTokenInto(node); } + ParseTrailingWhitespace(node); + return node; } @@ -735,7 +738,7 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && { var node = new HttpCommentStartNode(_sourceText, _syntaxTree); ConsumeCurrentTokenInto(node); - return ParseTrailingTrivia(node); + return ParseTrailingWhitespace(node); } if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && @@ -745,7 +748,7 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && ConsumeCurrentTokenInto(node); ConsumeCurrentTokenInto(node); - return ParseTrailingTrivia(node); + return ParseTrailingWhitespace(node); } return null; @@ -753,14 +756,14 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && private bool IsComment() { - if (MoreTokens()) + if (MoreTokens() && !IsRequestSeparator()) { - if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" }) + if (CurrentToken is { Text: "#" }) { return true; } - else if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && - CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) + else if (CurrentToken is { Text: "/" } && + CurrentTokenPlus(1) is { Text: "/" }) { return true; } @@ -769,15 +772,14 @@ private bool IsComment() return false; } } + return false; } - private bool IsRequestSeparator() - { - return CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && - (CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "#" } && - CurrentTokenPlus(2) is { Kind: HttpTokenKind.Punctuation } and { Text: "#" }); - } + private bool IsRequestSeparator() => + CurrentToken is { Text: "#" } && + CurrentTokenPlus(1) is { Text: "#" } && + CurrentTokenPlus(2) is { Text: "#" }; private static LinePosition GetLinePositionFromCursorOffset(SourceText code, int cursorOffset) { @@ -837,17 +839,21 @@ public IReadOnlyList Lex() { ' ' or '\t' => HttpTokenKind.Whitespace, '\n' or '\r' or '\v' => HttpTokenKind.NewLine, - _ => char.IsPunctuation(currentCharacter) - ? HttpTokenKind.Punctuation - : HttpTokenKind.Word, + _ => char.IsLetterOrDigit(currentCharacter) + ? HttpTokenKind.Word + : HttpTokenKind.Punctuation, }; if (previousTokenKind is { } previousTokenKindValue) { - if (!IsCurrentTokenANewLinePrecededByACarriageReturn(previousTokenKindValue, previousCharacter, - currentTokenKind, currentCharacter) && (previousTokenKind != currentTokenKind || currentTokenKind - is HttpTokenKind.NewLine || - currentTokenKind is HttpTokenKind.Punctuation)) + if (!IsCurrentTokenANewLinePrecededByACarriageReturn( + previousTokenKindValue, + previousCharacter, + currentTokenKind, + currentCharacter) && + (previousTokenKind != currentTokenKind || currentTokenKind + is HttpTokenKind.NewLine || + currentTokenKind is HttpTokenKind.Punctuation)) { FlushToken(previousTokenKindValue); } @@ -894,11 +900,11 @@ private bool IsCurrentTokenANewLinePrecededByACarriageReturn( HttpTokenKind previousTokenKindValue, char previousCharacter, HttpTokenKind currentTokenKind, - char currentCharacter) - { - return (currentTokenKind is HttpTokenKind.NewLine && previousTokenKindValue is HttpTokenKind.NewLine - && previousCharacter is '\r' && currentCharacter is '\n'); - } + char currentCharacter) => + currentTokenKind is HttpTokenKind.NewLine && + previousTokenKindValue is HttpTokenKind.NewLine + && + previousCharacter is '\r' && currentCharacter is '\n'; } private class TextWindow @@ -936,5 +942,4 @@ public void Advance() public override string ToString() => $"[{Start}..{End}]"; } - } From 9e3f60ead21b33a2e5079138c270771c114b69b2 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Mon, 11 Sep 2023 09:29:25 -0700 Subject: [PATCH 3/6] PR feedback --- .../ParserTests.RequestSeparator.cs | 31 ++++ .../ParserTests.cs | 169 +----------------- .../Parser/HttpRequestParser.cs | 15 +- 3 files changed, 35 insertions(+), 180 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.RequestSeparator.cs diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.RequestSeparator.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.RequestSeparator.cs new file mode 100644 index 0000000000..330f05d322 --- /dev/null +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.RequestSeparator.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using FluentAssertions; +using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility; +using Xunit; + +namespace Microsoft.DotNet.Interactive.HttpRequest.Tests; + +public partial class ParserTests +{ + public class RequestSeparator + { + + [Fact] + public void request_separator_at_start_of_request_is_valid() + { + var code = """ + ### + GET https://example.com + """; + + var result = Parse(code); + + result.SyntaxTree.RootNode.ChildNodes + .Should().ContainSingle() + .Which.UrlNode.Text.Should().Be("https://example.com"); + } + + } +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs index e6231093c3..9c39bbf05d 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -using System.Linq; using FluentAssertions; using FluentAssertions.Execution; using Xunit.Abstractions; @@ -29,171 +28,7 @@ private static HttpRequestParseResult Parse(string code) var result = HttpRequestParser.Parse(code); result.SyntaxTree.RootNode.FullText.Should().Be(code); - - return result; - } -} - - -// TODO: Test string with variable declarations but no requests - -/* -[TestClass] -public class TokenTest -{ - - [Fact] - public async Task RequestWithEmbeddedDynamicVariableWithSpacesAsync() - { - string text = $"\r{Environment.NewLine}GET https://example.com?id={{{{$randomInt 1 2}}}}&id2=value&id3={{{{$randomInt 2 3}}}} http/1.1"; - - IRestDocumentSnapshot doc = await TestHelpers.CreateDocumentSnapshotAsync(text); - - Request request = doc.Requests[0]; - - Assert.IsNotNull(request.Version); - Assert.AreEqual("http/1.1", request.Version.Text); - - Assert.AreEqual("https://example.com?id={{$randomInt 1 2}}&id2=value&id3={{$randomInt 2 3}}", request.Url.Text); - } - - [Fact] - public async Task MultipleRequestsAsync() - { - string text = """ - get http://example.com - - ### - - post http://bing.com - """; - - IRestDocumentSnapshot doc = await TestHelpers.CreateDocumentSnapshotAsync(text); - - Assert.AreEqual(2, doc.Requests.Count); - } - - [Fact] - public async Task RequestWithHeaderAndBodyAndCommentAsync() - { - string text = """ - DELETE https://example.com - User-Agent: ost - #ost:hat - - { - "enabled": true - } - - ### - """; - - IRestDocumentSnapshot doc = await TestHelpers.CreateDocumentSnapshotAsync(text); - Request first = doc.Requests[0]; - - Assert.IsNotNull(first); - Assert.AreEqual(1, doc.Requests.Count); - Assert.AreEqual(ItemType.HeaderName, first.Children[2].Type); - Assert.AreEqual(ItemType.HeaderValue, first.Children[3].Type); - Assert.AreEqual(ItemType.Comment, first.Children[4].Type); - Assert.AreEqual(ItemType.EmptyLine, first.Children[5].Type); - Assert.AreEqual(ItemType.Body, first.Children[6].Type); - Assert.AreEqual("{\r\n \"enabled\": true\r\n}\r\n", first.Body); - } - - [Fact] - public async Task BodyAfterCommentAsync() - { - string text = """ - TraCe https://{{host}}/authors/{{name}} - Content-Type: at{{contentType}}svin - #ost - mads: ost - - { - "content": "foo bar", - "created_at": "{{createdAt}}", - - "modified_by": "$test$" - } - - - """; - - IRestDocumentSnapshot doc = await TestHelpers.CreateDocumentSnapshotAsync(text); - Request request = doc.Requests[0]; - - Assert.IsNotNull(request.Body); - Assert.IsTrue(request.Body.Contains("$test$")); - Assert.IsTrue(request.Body.Trim().EndsWith("}")); - } - - [Fact] - public async Task VariableTokenizationAsync() - { - string text = $"@name = value"; - - IRestDocumentSnapshot doc = await TestHelpers.CreateDocumentSnapshotAsync(text); - ParseItem name = doc.Items[0]; - ParseItem nextItem = doc.Items[1]; - - Assert.AreEqual(0, name.Start); - Assert.AreEqual(5, name.Length); - Assert.AreEqual(8, nextItem.Start); - Assert.AreEqual(5, nextItem.Length); - } - - [Fact] - public async Task CommentInBetweenHeadersAsync() - { - string text = """ - POST https://example.com - Content-Type:application/json - #comment - Accept: gzip - """; - - IRestDocumentSnapshot doc = await TestHelpers.CreateDocumentSnapshotAsync(text); - - Assert.AreEqual(8, doc.Items.Count); - } - - [Theory] - [InlineData(" foo: bar")] - [InlineData("foo : bar")] - [InlineData("foo")] - public async Task InvalidHeaders_ShouldBeTreatedAsBody_WithError(string contentType) - { - string text = $""" - POST https://example.com - {contentType} - """; - - IRestDocumentSnapshot doc = await TestHelpers.CreateDocumentSnapshotAsync(text); - - Assert.AreEqual(4, doc.Items.Count); - Assert.AreEqual(ItemType.Body, doc.Items[3].Type); - Assert.AreEqual(contentType, doc.Items[3].Text); - Assert.AreEqual(1, doc.Items[3].Errors.Count); - Assert.AreEqual(string.Format(Strings.NotAValidHttpHeader, contentType), doc.Items[3].Errors[0].Message); - } - - [Fact] - public async Task HeaderMissingValue_ShouldBeTreatedAsBody_WithError() - { - string text = $""" - POST https://example.com - foo: - """; - - IRestDocumentSnapshot doc = await TestHelpers.CreateDocumentSnapshotAsync(text); - - Assert.AreEqual(4, doc.Items.Count); - Assert.AreEqual(ItemType.Body, doc.Items[3].Type); - Assert.AreEqual("foo:", doc.Items[3].Text); - Assert.AreEqual(1, doc.Items[3].Errors.Count); - Assert.AreEqual(string.Format(Strings.HttpHeaderMissingValue, "foo:"), doc.Items[3].Errors[0].Message); + return result; } - -}*/ \ No newline at end of file +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs index 1698a56d93..525f2f0394 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs @@ -73,7 +73,6 @@ public HttpSyntaxTree Parse() if (ParseRequestSeparator() is { } separatorNode) { - // FIX: (Parse) uncovered _syntaxTree.RootNode.Add(separatorNode); } } @@ -81,21 +80,11 @@ public HttpSyntaxTree Parse() return _syntaxTree; } - private static void AppendCommentsIfAny(List commentsToAppend, HttpSyntaxNode prependToNode) - { - foreach (var comment in commentsToAppend) - { - prependToNode.Add(comment, addBefore: false); - } - - commentsToAppend.Clear(); - } - - private static void PrependCommentsIfAny(List commentsToPrepend, HttpSyntaxNode prependToNode) + private static void PrependCommentsIfAny(List commentsToPrepend, HttpSyntaxNode toNode) { foreach (var comment in commentsToPrepend) { - prependToNode.Add(comment, addBefore: true); + toNode.Add(comment, addBefore: true); } commentsToPrepend.Clear(); From 50f1cb605cbdc07a86fbe863c07f697b34965896 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Mon, 11 Sep 2023 15:54:12 -0700 Subject: [PATCH 4/6] fix for leading request separator; CurrentToken => null instead of throw --- .../Parser/HttpRequestParser.cs | 103 +++++++++--------- 1 file changed, 54 insertions(+), 49 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs index 525f2f0394..cd64aabb6e 100644 --- a/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs +++ b/src/Microsoft.DotNet.Interactive.HttpRequestParser/Parser/HttpRequestParser.cs @@ -92,7 +92,8 @@ private static void PrependCommentsIfAny(List commentsToPrepend private IEnumerable? ParseVariableDeclarations() { - while (MoreTokens()) + while (MoreTokens() && + !IsRequestSeparator()) { if (GetNextSignificantToken() is { Kind: HttpTokenKind.Punctuation } and { Text: "@" }) { @@ -105,8 +106,8 @@ private static void PrependCommentsIfAny(List commentsToPrepend variableNode.Add(valueNode); yield return variableNode; } - - } else + } + else { break; } @@ -117,8 +118,8 @@ private static void PrependCommentsIfAny(List commentsToPrepend { HttpVariableValueNode? node = null; - while (MoreTokens() && - CurrentToken.Kind is not HttpTokenKind.NewLine) + while (MoreTokens() && + CurrentToken is not { Kind: HttpTokenKind.NewLine }) { if (node is null) { @@ -191,7 +192,10 @@ private HttpVariableDeclarationNode ParseVariableDeclaration() return ParseTrailingWhitespace(node); } - private HttpSyntaxToken CurrentToken => _tokens![_currentTokenIndex]; + private HttpSyntaxToken? CurrentToken => + MoreTokens() + ? _tokens![_currentTokenIndex] + : null; private HttpSyntaxToken? CurrentTokenPlus(int offset) { @@ -216,19 +220,22 @@ private HttpVariableDeclarationNode ParseVariableDeclaration() [DebuggerStepThrough] private void ConsumeCurrentTokenInto(HttpSyntaxNode node) { - node.Add(CurrentToken); - AdvanceToNextToken(); + if (CurrentToken is { } token) + { + node.Add(token); + AdvanceToNextToken(); + } } private T ParseLeadingWhitespaceAndComments(T node) where T : HttpSyntaxNode { while (MoreTokens()) { - if (CurrentToken.Kind is HttpTokenKind.Whitespace) + if (CurrentToken?.Kind is HttpTokenKind.Whitespace) { ConsumeCurrentTokenInto(node); } - else if (CurrentToken.Kind is HttpTokenKind.NewLine) + else if (CurrentToken?.Kind is HttpTokenKind.NewLine) { ConsumeCurrentTokenInto(node); } @@ -249,7 +256,7 @@ private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool { while (MoreTokens()) { - if (CurrentToken.Kind is HttpTokenKind.NewLine) + if (CurrentToken?.Kind is HttpTokenKind.NewLine) { if (stopBeforeNewLine) { @@ -263,7 +270,7 @@ private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool } } - if (CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine)) + if (CurrentToken is not { Kind: HttpTokenKind.Whitespace } and not { Kind: HttpTokenKind.NewLine }) { break; } @@ -278,7 +285,11 @@ private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool { if (IsComment()) { - // FIX: (ParseRequest) this looks suspect... test with a comment at the start of the request node? + return null; + } + + if (IsRequestSeparator()) + { return null; } @@ -351,7 +362,7 @@ private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool private HttpRequestSeparatorNode? ParseRequestSeparator() { - if (MoreTokens() && IsRequestSeparator()) + if (IsRequestSeparator()) { var node = new HttpRequestSeparatorNode(_sourceText, _syntaxTree); ParseLeadingWhitespaceAndComments(node); @@ -360,7 +371,7 @@ private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool ConsumeCurrentTokenInto(node); ConsumeCurrentTokenInto(node); - ParseTrailingWhitespace(node); + return ParseTrailingWhitespace(node); } return null; @@ -370,28 +381,25 @@ private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool { HttpMethodNode? node = null; - if (MoreTokens()) + while (CurrentToken?.Kind is HttpTokenKind.Word && + CurrentTokenPlus(1)?.Kind is HttpTokenKind.Whitespace) { - if (CurrentToken.Kind is HttpTokenKind.Word && - CurrentTokenPlus(1)?.Kind is HttpTokenKind.Whitespace) - { - node = new HttpMethodNode(_sourceText, _syntaxTree); + node = new HttpMethodNode(_sourceText, _syntaxTree); - ParseLeadingWhitespaceAndComments(node); + ParseLeadingWhitespaceAndComments(node); - if (CurrentToken.Text.ToLower() is not ("get" or "post" or "patch" or "put" or "delete" or "head" or "options" or "trace")) - { - var message = $"Unrecognized HTTP verb {CurrentToken.Text}"; + if (CurrentToken.Text.ToLower() is not ("get" or "post" or "patch" or "put" or "delete" or "head" or "options" or "trace")) + { + var message = $"Unrecognized HTTP verb {CurrentToken.Text}"; - var diagnostic = CurrentToken.CreateDiagnostic(message); + var diagnostic = CurrentToken.CreateDiagnostic(message); - node.AddDiagnostic(diagnostic); - } + node.AddDiagnostic(diagnostic); + } - ConsumeCurrentTokenInto(node); + ConsumeCurrentTokenInto(node); - ParseTrailingWhitespace(node, stopBeforeNewLine: true); - } + ParseTrailingWhitespace(node, stopBeforeNewLine: true); } return node; @@ -401,8 +409,7 @@ private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool { HttpUrlNode? node = null; - while (MoreTokens() && - CurrentToken.Kind is HttpTokenKind.Word or HttpTokenKind.Punctuation) + while (CurrentToken?.Kind is HttpTokenKind.Word or HttpTokenKind.Punctuation) { if (node is null) { @@ -440,7 +447,7 @@ private T ParseTrailingWhitespace(T node, bool stopAfterNewLine = false, bool while (MoreTokens()) { - if (token.IsSignificant) + if (token?.IsSignificant == true) { return token; } @@ -509,9 +516,8 @@ private HttpExpressionNode ParseExpression() private HttpExpressionEndNode? ParseExpressionEnd() { - if (MoreTokens() && - CurrentToken.Text is "}" && - CurrentTokenPlus(1)?.Text == "}") + if (CurrentToken?.Text is "}" && + CurrentTokenPlus(1)?.Text is "}") { var node = new HttpExpressionEndNode(_sourceText, _syntaxTree); @@ -528,8 +534,7 @@ CurrentToken.Text is "}" && private HttpVersionNode? ParseVersion() { - if (MoreTokens() && - CurrentToken.Kind is HttpTokenKind.Word) + if (CurrentToken?.Kind is HttpTokenKind.Word) { var node = new HttpVersionNode(_sourceText, _syntaxTree); @@ -552,8 +557,7 @@ CurrentToken.Text is "}" && { HttpHeadersNode? headersNode = null; - while (MoreTokens() && - CurrentToken is { Kind: HttpTokenKind.Word } or { Text: ":" }) + while (CurrentToken is { Kind: HttpTokenKind.Word } or { Text: ":" }) { headersNode ??= new HttpHeadersNode(_sourceText, _syntaxTree); @@ -578,7 +582,7 @@ private HttpHeaderNameNode ParseHeaderName() { var node = new HttpHeaderNameNode(_sourceText, _syntaxTree); - if (MoreTokens() && + if (MoreTokens() && CurrentToken is not { Kind: HttpTokenKind.NewLine } and not { Kind: HttpTokenKind.Punctuation, Text: ":" }) { ParseLeadingWhitespaceAndComments(node); @@ -608,7 +612,7 @@ private HttpHeaderSeparatorNode ParserHeaderSeparator() ParseLeadingWhitespaceAndComments(node); - if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: ":" }) + if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: ":" }) { ConsumeCurrentTokenInto(node); } @@ -622,7 +626,7 @@ private HttpHeaderValueNode ParseHeaderValue() ParseLeadingWhitespaceAndComments(node); - while (MoreTokens() && CurrentToken.Kind is not HttpTokenKind.NewLine) + while (MoreTokens() && CurrentToken is not { Kind: HttpTokenKind.NewLine }) { if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "{" } && CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "{" }) @@ -652,8 +656,8 @@ private HttpHeaderValueNode ParseHeaderValue() var node = new HttpBodyNode(_sourceText, _syntaxTree); - if (MoreTokens() && - CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && + if (MoreTokens() && + CurrentToken is not { Kind: HttpTokenKind.Whitespace } and not { Kind: HttpTokenKind.NewLine } && !IsRequestSeparator()) { ConsumeCurrentTokenInto(node); @@ -678,7 +682,7 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && private HttpCommentNode? ParseComment() { - if (MoreTokens() && IsComment()) + if (IsComment()) { var commentNode = new HttpCommentNode(_sourceText, _syntaxTree); @@ -711,7 +715,8 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && var node = new HttpCommentBodyNode(_sourceText, _syntaxTree); ParseLeadingWhitespaceAndComments(node); - while (MoreTokens() && CurrentToken.Kind is not HttpTokenKind.NewLine) + while (MoreTokens() && + CurrentToken is not { Kind: HttpTokenKind.NewLine }) { ConsumeCurrentTokenInto(node); } @@ -723,14 +728,14 @@ CurrentToken.Kind is not (HttpTokenKind.Whitespace or HttpTokenKind.NewLine) && private HttpCommentStartNode? ParseCommentStart() { - if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" }) + if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" }) { var node = new HttpCommentStartNode(_sourceText, _syntaxTree); ConsumeCurrentTokenInto(node); return ParseTrailingWhitespace(node); } - if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && + if (CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && CurrentTokenPlus(1) is { Kind: HttpTokenKind.Punctuation } and { Text: "/" }) { var node = new HttpCommentStartNode(_sourceText, _syntaxTree); From c7fa38461197977184ed124e69b20a11f2e74dcd Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 12 Sep 2023 11:19:42 -0700 Subject: [PATCH 5/6] remove redundant GetDefaultKernelName logic --- .../InteractiveDocument.cs | 29 +++---------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs b/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs index f50b7e54b8..62d3e4e1c1 100644 --- a/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs +++ b/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs @@ -198,10 +198,9 @@ public static async Task LoadAsync( public string? GetDefaultKernelName() { - // FIX: (GetDefaultKernelName) remove from public API - if (TryGetKernelInfosFromMetadata(Metadata, out var kernelInfo)) + if (TryGetKernelInfosFromMetadata(Metadata, out var kernelInfos)) { - return kernelInfo.DefaultKernelName; + return kernelInfos.DefaultKernelName; } return null; @@ -213,28 +212,8 @@ public static async Task LoadAsync( { return kernelInfoCollection.DefaultKernelName; } - - if (Metadata.TryGetValue("kernelspec", out var kernelspecObj)) - { - if (kernelspecObj is IDictionary kernelspecDict) - { - if (kernelspecDict.TryGetValue("language", out var languageObj) && - languageObj is string defaultLanguage) - { - return defaultLanguage; - } - } - } - - if (kernelInfos.DefaultKernelName is { } defaultFromKernelInfos) - { - if (kernelInfos.TryGetByAlias(defaultFromKernelInfos, out var info)) - { - return info.Name; - } - } - - return null; + + return kernelInfos.DefaultKernelName; } internal static void MergeKernelInfos(InteractiveDocument document, KernelInfoCollection kernelInfos) From 13a5b8653405d7ee8b7fc3a83b7c0c18327154a1 Mon Sep 17 00:00:00 2001 From: Jon Sequeira Date: Tue, 12 Sep 2023 11:20:03 -0700 Subject: [PATCH 6/6] update API baseline --- ...piCompatibilityTests.Document_api_is_not_changed.approved.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.DotNet.Interactive.ApiCompatibility.Tests/ApiCompatibilityTests.Document_api_is_not_changed.approved.txt b/src/Microsoft.DotNet.Interactive.ApiCompatibility.Tests/ApiCompatibilityTests.Document_api_is_not_changed.approved.txt index b2b7a3bdf0..d7e8c50b94 100644 --- a/src/Microsoft.DotNet.Interactive.ApiCompatibility.Tests/ApiCompatibilityTests.Document_api_is_not_changed.approved.txt +++ b/src/Microsoft.DotNet.Interactive.ApiCompatibility.Tests/ApiCompatibilityTests.Document_api_is_not_changed.approved.txt @@ -31,7 +31,6 @@ Microsoft.DotNet.Interactive.Documents public System.Collections.Generic.IAsyncEnumerable GetImportsAsync(System.Boolean recursive = False) public System.Collections.Generic.IEnumerable GetInputFields() public System.Collections.Generic.IEnumerable GetMagicCommandLines() - public System.Boolean TryGetKernelInfosFromMetadata(ref KernelInfoCollection& kernelInfos) public class InteractiveDocumentElement .ctor() .ctor(System.String contents = null, System.String kernelName = null, System.Collections.Generic.IEnumerable outputs = null)