Variable declaration addition #3152
Conversation
| @@ -39,24 +41,151 @@ public HttpSyntaxTree Parse() | |||
| { | |||
| _tokens = new HttpLexer(_sourceText, _syntaxTree).Lex(); | |||
|
|
|||
| var unmatchedComments = new List<HttpCommentNode>(); | |||
There was a problem hiding this comment.
I'd probably just call this comments. They're matched, they're just not parented yet. Or maybe something like commentsToPrepend?
|
|
||
| } | ||
|
|
||
| private HttpVariableValueNode? ParseVariableExpression() |
There was a problem hiding this comment.
| private HttpVariableValueNode? ParseVariableExpression() | |
| private HttpVariableValueNode? ParseVariableValue() |
| { | ||
| break; | ||
| } | ||
| }*/ |
There was a problem hiding this comment.
Feel free to delete leftover comments.
| return null; | ||
| } | ||
|
|
||
| private HttpSyntaxToken? GetNextNodeType() |
There was a problem hiding this comment.
This name looks inaccurate. How is this different from GetNextSignificantToken?
| if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "#" }) | ||
| { | ||
| return true; | ||
| } else if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && |
There was a problem hiding this comment.
You can refactor this to dedupe the MoreTokens call.
| { | ||
| return true; | ||
| } | ||
| else if (MoreTokens() && CurrentToken is { Kind: HttpTokenKind.Punctuation } and { Text: "/" } && |
There was a problem hiding this comment.
You can refactor this to dedupe the MoreTokens call.
| { | ||
| public HttpVariableDeclarationNode? DeclarationNode { get; private set; } | ||
| public HttpVariableAssignmentNode? AssignmentNode { get; private set; } | ||
| public HttpVariableValueNode? ExpressionNode { get; private set; } |
There was a problem hiding this comment.
Calling this property ValueNode is probably a better match to the property's type name at this point.
| yield return diagnostic; | ||
| } | ||
|
|
||
| yield return base.CreateDiagnostic("Variable name expected."); |
There was a problem hiding this comment.
This looks like a bug (and presumably a missing test?) unless there's a condition around it, e.g. if (Span.Length ==0).
There was a problem hiding this comment.
I don't fully get what you mean here
There was a problem hiding this comment.
This will always return a diagnostic even if the node is perfectly valid.
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpVariableDeclarationNode.cs
Show resolved
Hide resolved
Head branch was pushed to by a user without write access
No description provided.