Skip to content

Commit 59ac947

Browse files
authored
Fix #3710 (#3714)
* diagnostics for missing subcommand parameters * fix #3710 * rename local
1 parent 7deef8b commit 59ac947

File tree

7 files changed

+100
-37
lines changed

7 files changed

+100
-37
lines changed

src/Microsoft.DotNet.Interactive.Jupyter/ConnectJupyterKernel.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@ public ConnectJupyterKernel(string connectedKernelName) : base(connectedKernelNa
1919

2020
public string InitScript { get; set; }
2121

22+
[JsonPropertyName("url")]
2223
public string TargetUrl { get; set; }
23-
24+
25+
[JsonPropertyName("bearer")]
2426
public bool UseBearerAuth { get; set; }
2527

2628
public string Token { get; set; }

src/Microsoft.DotNet.Interactive.Jupyter/ConnectJupyterKernelDirective.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation and contributors. All rights reserved.
1+
// Copyright (c) .NET Foundation and contributors. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
using Microsoft.DotNet.Interactive.Connection;
@@ -31,7 +31,10 @@ public ConnectJupyterKernelDirective() : base("jupyter", "Connects a Jupyter ker
3131
};
3232

3333
public KernelDirectiveParameter InitScriptParameter { get; } =
34-
new("--init-script", "Script to run on kernel initialization");
34+
new("--init-script", "Script to run on kernel initialization")
35+
{
36+
TypeHint = "file"
37+
};
3538

3639
public ConnectJupyterKernelDirective AddConnectionOptions(IJupyterKernelConnectionOptions connectionOptions)
3740
{

src/Microsoft.DotNet.Interactive.Parsing.Tests/PolyglotSyntaxParserTests.DirectiveParameters.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,38 @@ public void When_the_value_for_a_required_parameter_is_missing_then_an_error_is_
299299
.Be("Missing value for required parameter '--required'");
300300
}
301301

302+
[Fact]
303+
public void When_a_required_parameter_on_a_subcommand_is_missing_then_an_error_is_produced()
304+
{
305+
var tree = Parse("#!connect jupyter --kernel-spec .net-csharp");
306+
307+
tree.RootNode
308+
.GetDiagnostics()
309+
.Should()
310+
.ContainSingle()
311+
.Which
312+
.GetMessage()
313+
.Should()
314+
.Be("Missing required parameter '--kernel-name'");
315+
}
316+
317+
[Theory]
318+
[InlineData("#!connect jupyter --kernel-spec .net-csharp --kernel-name")]
319+
[InlineData("#!connect jupyter --kernel-name --kernel-spec .net-csharp")]
320+
public void When_the_value_for_a_required_parameter_on_a_subcommand_is_missing_then_an_error_is_produced(string code)
321+
{
322+
var tree = Parse(code);
323+
324+
tree.RootNode
325+
.GetDiagnostics()
326+
.Should()
327+
.ContainSingle()
328+
.Which
329+
.GetMessage()
330+
.Should()
331+
.Be("Missing value for required parameter '--kernel-name'");
332+
}
333+
302334
[Theory]
303335
[InlineData("""
304336
"just a JSON string"

src/Microsoft.DotNet.Interactive.Parsing.Tests/PolyglotSyntaxParserTests.Subcommands.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System;
55
using System.Linq;
6-
using System.Threading.Tasks;
76
using FluentAssertions;
87
using Microsoft.CodeAnalysis;
98
using Microsoft.DotNet.Interactive.Directives;

src/Microsoft.DotNet.Interactive/Directives/KernelActionDirective.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
using System.Threading.Tasks;
1212
using Microsoft.CodeAnalysis.Tags;
1313
using Microsoft.DotNet.Interactive.Events;
14-
using Microsoft.DotNet.Interactive.Parsing;
1514

1615
namespace Microsoft.DotNet.Interactive.Directives;
1716

src/Microsoft.DotNet.Interactive/Parsing/DirectiveNode.cs

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -47,39 +47,9 @@ internal DirectiveNode(
4747

4848
if (TryGetDirective(out var directive))
4949
{
50-
foreach (var namedParameter in directive.Parameters)
50+
foreach (var diagnostic in GetDiagnosticsForMissingParameters(directive, this))
5151
{
52-
if (namedParameter.Required)
53-
{
54-
var matchingNodes = ChildNodes.OfType<DirectiveParameterNode>()
55-
.Where(p => p.NameNode is null
56-
? namedParameter.AllowImplicitName
57-
: p.NameNode?.Text == namedParameter.Name)
58-
.ToArray();
59-
60-
if (!matchingNodes.Any())
61-
{
62-
yield return CreateDiagnostic(
63-
new(PolyglotSyntaxParser.ErrorCodes.MissingRequiredParameter,
64-
"Missing required parameter '{0}'",
65-
DiagnosticSeverity.Error,
66-
namedParameter.Name));
67-
}
68-
else
69-
{
70-
foreach (var parameterNode in matchingNodes)
71-
{
72-
if (parameterNode.ValueNode is null)
73-
{
74-
yield return CreateDiagnostic(
75-
new(PolyglotSyntaxParser.ErrorCodes.MissingRequiredParameter,
76-
"Missing value for required parameter '{0}'",
77-
DiagnosticSeverity.Error,
78-
namedParameter.Name));
79-
}
80-
}
81-
}
82-
}
52+
yield return diagnostic;
8353
}
8454
}
8555

@@ -103,6 +73,46 @@ internal DirectiveNode(
10373
}
10474
}
10575

76+
internal static IEnumerable<CodeAnalysis.Diagnostic> GetDiagnosticsForMissingParameters(
77+
KernelDirective directive,
78+
SyntaxNode node)
79+
{
80+
foreach (var namedParameter in directive.Parameters)
81+
{
82+
if (namedParameter.Required)
83+
{
84+
var matchingNodes = node.ChildNodes.OfType<DirectiveParameterNode>()
85+
.Where(p => p.NameNode is null
86+
? namedParameter.AllowImplicitName
87+
: p.NameNode?.Text == namedParameter.Name)
88+
.ToArray();
89+
90+
if (!matchingNodes.Any())
91+
{
92+
yield return node.CreateDiagnostic(
93+
new(PolyglotSyntaxParser.ErrorCodes.MissingRequiredParameter,
94+
"Missing required parameter '{0}'",
95+
DiagnosticSeverity.Error,
96+
namedParameter.Name));
97+
}
98+
else
99+
{
100+
foreach (var parameterNode in matchingNodes)
101+
{
102+
if (parameterNode.ValueNode is null)
103+
{
104+
yield return node.CreateDiagnostic(
105+
new(PolyglotSyntaxParser.ErrorCodes.MissingRequiredParameter,
106+
"Missing value for required parameter '{0}'",
107+
DiagnosticSeverity.Error,
108+
namedParameter.Name));
109+
}
110+
}
111+
}
112+
}
113+
}
114+
}
115+
106116
public bool TryGetActionDirective(out KernelActionDirective directive)
107117
{
108118
if (GetKernelInfo() is { } kernelInfo)

src/Microsoft.DotNet.Interactive/Parsing/DirectiveSubcommandNode.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

44
#nullable enable
5+
using System.Collections.Generic;
56
using System.Diagnostics.CodeAnalysis;
7+
using Microsoft.CodeAnalysis;
68
using Microsoft.CodeAnalysis.Text;
79
using Microsoft.DotNet.Interactive.Directives;
810

@@ -36,6 +38,22 @@ public void Add(DirectiveParameterValueNode node)
3638
HasParameters = true;
3739
}
3840

41+
public override IEnumerable<CodeAnalysis.Diagnostic> GetDiagnostics()
42+
{
43+
foreach (var diagnostic in base.GetDiagnostics())
44+
{
45+
yield return diagnostic;
46+
}
47+
48+
if (TryGetSubcommand(out var directive))
49+
{
50+
foreach (var diagnostic in DirectiveNode.GetDiagnosticsForMissingParameters(directive, this))
51+
{
52+
yield return diagnostic;
53+
}
54+
}
55+
}
56+
3957
public bool TryGetSubcommand([NotNullWhen(true)] out KernelActionDirective? subcommandDirective)
4058
{
4159
if (Parent is DirectiveNode parentDirectiveNode)

0 commit comments

Comments
 (0)