Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3710 #3714

Merged
merged 3 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ public ConnectJupyterKernel(string connectedKernelName) : base(connectedKernelNa

public string InitScript { get; set; }

[JsonPropertyName("url")]
public string TargetUrl { get; set; }


[JsonPropertyName("bearer")]
public bool UseBearerAuth { get; set; }

public string Token { get; set; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// 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 Microsoft.DotNet.Interactive.Connection;
Expand Down Expand Up @@ -31,7 +31,10 @@ public ConnectJupyterKernelDirective() : base("jupyter", "Connects a Jupyter ker
};

public KernelDirectiveParameter InitScriptParameter { get; } =
new("--init-script", "Script to run on kernel initialization");
new("--init-script", "Script to run on kernel initialization")
{
TypeHint = "file"
};

public ConnectJupyterKernelDirective AddConnectionOptions(IJupyterKernelConnectionOptions connectionOptions)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,38 @@ public void When_the_value_for_a_required_parameter_is_missing_then_an_error_is_
.Be("Missing value for required parameter '--required'");
}

[Fact]
public void When_a_required_parameter_on_a_subcommand_is_missing_then_an_error_is_produced()
{
var tree = Parse("#!connect jupyter --kernel-spec .net-csharp");

tree.RootNode
.GetDiagnostics()
.Should()
.ContainSingle()
.Which
.GetMessage()
.Should()
.Be("Missing required parameter '--kernel-name'");
}

[Theory]
[InlineData("#!connect jupyter --kernel-spec .net-csharp --kernel-name")]
[InlineData("#!connect jupyter --kernel-name --kernel-spec .net-csharp")]
public void When_the_value_for_a_required_parameter_on_a_subcommand_is_missing_then_an_error_is_produced(string code)
{
var tree = Parse(code);

tree.RootNode
.GetDiagnostics()
.Should()
.ContainSingle()
.Which
.GetMessage()
.Should()
.Be("Missing value for required parameter '--kernel-name'");
}

[Theory]
[InlineData("""
"just a JSON string"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Linq;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.CodeAnalysis;
using Microsoft.DotNet.Interactive.Directives;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Tags;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Parsing;

namespace Microsoft.DotNet.Interactive.Directives;

Expand Down
74 changes: 42 additions & 32 deletions src/Microsoft.DotNet.Interactive/Parsing/DirectiveNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,39 +47,9 @@ internal DirectiveNode(

if (TryGetDirective(out var directive))
{
foreach (var namedParameter in directive.Parameters)
foreach (var diagnostic in GetDiagnosticsForMissingParameters(directive, this))
{
if (namedParameter.Required)
{
var matchingNodes = ChildNodes.OfType<DirectiveParameterNode>()
.Where(p => p.NameNode is null
? namedParameter.AllowImplicitName
: p.NameNode?.Text == namedParameter.Name)
.ToArray();

if (!matchingNodes.Any())
{
yield return CreateDiagnostic(
new(PolyglotSyntaxParser.ErrorCodes.MissingRequiredParameter,
"Missing required parameter '{0}'",
DiagnosticSeverity.Error,
namedParameter.Name));
}
else
{
foreach (var parameterNode in matchingNodes)
{
if (parameterNode.ValueNode is null)
{
yield return CreateDiagnostic(
new(PolyglotSyntaxParser.ErrorCodes.MissingRequiredParameter,
"Missing value for required parameter '{0}'",
DiagnosticSeverity.Error,
namedParameter.Name));
}
}
}
}
yield return diagnostic;
}
}

Expand All @@ -103,6 +73,46 @@ internal DirectiveNode(
}
}

internal static IEnumerable<CodeAnalysis.Diagnostic> GetDiagnosticsForMissingParameters(
KernelDirective directive,
SyntaxNode node)
{
foreach (var namedParameter in directive.Parameters)
{
if (namedParameter.Required)
{
var matchingNodes = node.ChildNodes.OfType<DirectiveParameterNode>()
.Where(p => p.NameNode is null
? namedParameter.AllowImplicitName
: p.NameNode?.Text == namedParameter.Name)
.ToArray();

if (!matchingNodes.Any())
{
yield return node.CreateDiagnostic(
new(PolyglotSyntaxParser.ErrorCodes.MissingRequiredParameter,
"Missing required parameter '{0}'",
DiagnosticSeverity.Error,
namedParameter.Name));
}
else
{
foreach (var parameterNode in matchingNodes)
{
if (parameterNode.ValueNode is null)
{
yield return node.CreateDiagnostic(
new(PolyglotSyntaxParser.ErrorCodes.MissingRequiredParameter,
"Missing value for required parameter '{0}'",
DiagnosticSeverity.Error,
namedParameter.Name));
}
}
}
}
}
}

public bool TryGetActionDirective(out KernelActionDirective directive)
{
if (GetKernelInfo() is { } kernelInfo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#nullable enable
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Text;
using Microsoft.DotNet.Interactive.Directives;

Expand Down Expand Up @@ -36,6 +38,22 @@ public void Add(DirectiveParameterValueNode node)
HasParameters = true;
}

public override IEnumerable<CodeAnalysis.Diagnostic> GetDiagnostics()
{
foreach (var diagnostic in base.GetDiagnostics())
{
yield return diagnostic;
}

if (TryGetSubcommand(out var directive))
{
foreach (var diagnostic in DirectiveNode.GetDiagnosticsForMissingParameters(directive, this))
{
yield return diagnostic;
}
}
}

public bool TryGetSubcommand([NotNullWhen(true)] out KernelActionDirective? subcommandDirective)
{
if (Parent is DirectiveNode parentDirectiveNode)
Expand Down