Skip to content

Ensure @typeparam directive descriptor is always set #33456

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 11, 2021
Merged

Ensure @typeparam directive descriptor is always set #33456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7bb7a4
Ensure @typeparam directive descriptor is always set
captainsafia Jun 11, 2021
bd08923
Set fallback RazorLang value for test scenarios
captainsafia Jun 11, 2021
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
32 changes: 32 additions & 0 deletions ...rosoft.AspNetCore.Razor.Language/src/Components/ComponentConstrainedTypeParamDirective.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;

namespace Microsoft.AspNetCore.Razor.Language.Components
{
internal class ComponentConstrainedTypeParamDirective
{
public static DirectiveDescriptor Directive = DirectiveDescriptor.CreateDirective(
"typeparam",
DirectiveKind.SingleLine,
builder =>
{
builder.AddMemberToken(ComponentResources.TypeParamDirective_Token_Name, ComponentResources.TypeParamDirective_Token_Description);
builder.AddOptionalGenericTypeConstraintToken(ComponentResources.TypeParamDirective_Constraint_Name, ComponentResources.TypeParamDirective_Constraint_Description);
builder.Usage = DirectiveUsage.FileScopedMultipleOccurring;
builder.Description = ComponentResources.TypeParamDirective_Description;
});

public static RazorProjectEngineBuilder Register(RazorProjectEngineBuilder builder)
{
if (builder == null)
{
throw new ArgumentNullException(nameof(builder));
}

builder.AddDirective(Directive, FileKinds.Component, FileKinds.ComponentImport);
return builder;
}
}
}
7 changes: 6 additions & 1 deletion ...zor/Microsoft.AspNetCore.Razor.Language/src/Components/ComponentDocumentClassifierPass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,12 @@ protected override void OnDocumentStructureCreated(
{
@class.BaseType = ComponentsApi.ComponentBase.FullTypeName;

var typeParamReferences = documentNode.FindDirectiveReferences(ComponentTypeParamDirective.Directive);
// Constrained type parameters are only supported in Razor language versions v6.0
var razorLanguageVersion = codeDocument.GetParserOptions()?.Version ?? RazorLanguageVersion.Latest;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't we want to assume lowest version instead of highest if there isn't one specified?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, the only scenario where it is undefined is in some test cases for this pass that don't fully configure the ComponentDocumentClassifierPass with parser options. For that scenario, I figured it made sense to update to the latest.

Assuming this happens to a user, I think it also makes sense to default to the latest since the constrained type param directive can still support unconstrained scenarios.

var directiveType = razorLanguageVersion.CompareTo(RazorLanguageVersion.Version_6_0) >= 0
? ComponentConstrainedTypeParamDirective.Directive
: ComponentTypeParamDirective.Directive;
var typeParamReferences = documentNode.FindDirectiveReferences(directiveType);
for (var i = 0; i < typeParamReferences.Count; i++)
{
var typeParamNode = (DirectiveIntermediateNode)typeParamReferences[i].Node;
Expand Down
42 changes: 10 additions & 32 deletions src/Razor/Microsoft.AspNetCore.Razor.Language/src/Components/ComponentTypeParamDirective.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,45 +7,23 @@ namespace Microsoft.AspNetCore.Razor.Language.Components
{
internal class ComponentTypeParamDirective
{
public static DirectiveDescriptor Directive;
public static DirectiveDescriptor Directive = DirectiveDescriptor.CreateDirective(
"typeparam",
DirectiveKind.SingleLine,
builder =>
{
builder.AddMemberToken(ComponentResources.TypeParamDirective_Token_Name, ComponentResources.TypeParamDirective_Token_Description);
builder.Usage = DirectiveUsage.FileScopedMultipleOccurring;
builder.Description = ComponentResources.TypeParamDirective_Description;
});

public static RazorProjectEngineBuilder Register(RazorProjectEngineBuilder builder, bool supportConstraints)
public static RazorProjectEngineBuilder Register(RazorProjectEngineBuilder builder)
{
if (builder == null)
{
throw new ArgumentNullException(nameof(builder));
}

if (Directive == null)
{
// Do nothing and assume the first registration wins. In real life this directive is only ever registered once.
if (supportConstraints)
{
Directive = DirectiveDescriptor.CreateDirective(
"typeparam",
DirectiveKind.SingleLine,
builder =>
{
builder.AddMemberToken(ComponentResources.TypeParamDirective_Token_Name, ComponentResources.TypeParamDirective_Token_Description);
builder.AddOptionalGenericTypeConstraintToken(ComponentResources.TypeParamDirective_Constraint_Name, ComponentResources.TypeParamDirective_Constraint_Description);
builder.Usage = DirectiveUsage.FileScopedMultipleOccurring;
builder.Description = ComponentResources.TypeParamDirective_Description;
});
}
else
{
Directive = DirectiveDescriptor.CreateDirective(
"typeparam",
DirectiveKind.SingleLine,
builder =>
{
builder.AddMemberToken(ComponentResources.TypeParamDirective_Token_Name, ComponentResources.TypeParamDirective_Token_Description);
builder.Usage = DirectiveUsage.FileScopedMultipleOccurring;
builder.Description = ComponentResources.TypeParamDirective_Description;
});
}
}

builder.AddDirective(Directive, FileKinds.Component, FileKinds.ComponentImport);
return builder;
}
Expand Down
16 changes: 10 additions & 6 deletions src/Razor/Microsoft.AspNetCore.Razor.Language/src/RazorProjectEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,16 @@ private static void AddComponentFeatures(RazorProjectEngineBuilder builder, Razo
ComponentLayoutDirective.Register(builder);
ComponentPageDirective.Register(builder);

// Unconditionally enable the feature for the time being until we flow the SDK with the flow version
// into the repository.
ComponentTypeParamDirective.Register(
builder,
supportConstraints: true);
//supportConstraints: razorLanguageVersion.CompareTo(RazorLanguageVersion.Version_6_0) >= 0);


if (razorLanguageVersion.CompareTo(RazorLanguageVersion.Version_6_0) >= 0)
{
ComponentConstrainedTypeParamDirective.Register(builder);
}
else
{
ComponentTypeParamDirective.Register(builder);
}

if (razorLanguageVersion.CompareTo(RazorLanguageVersion.Version_5_0) >= 0)
{
Expand Down