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

[Fusion] Added pre-merge validation rule "ExternalUnusedRule" #7864

Merged
merged 2 commits into from
Dec 24, 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 @@ -5,5 +5,6 @@ public static class LogEntryCodes
public const string DisallowedInaccessible = "DISALLOWED_INACCESSIBLE";
public const string ExternalArgumentDefaultMismatch = "EXTERNAL_ARGUMENT_DEFAULT_MISMATCH";
public const string ExternalMissingOnBase = "EXTERNAL_MISSING_ON_BASE";
public const string ExternalUnused = "EXTERNAL_UNUSED";
public const string OutputFieldTypesNotMergeable = "OUTPUT_FIELD_TYPES_NOT_MERGEABLE";
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,22 @@ public static LogEntry ExternalMissingOnBase(
schema);
}

public static LogEntry ExternalUnused(
OutputFieldDefinition externalField,
INamedTypeDefinition type,
SchemaDefinition schema)
{
var coordinate = new SchemaCoordinate(type.Name, externalField.Name);

return new LogEntry(
string.Format(LogEntryHelper_ExternalUnused, coordinate, schema.Name),
LogEntryCodes.ExternalUnused,
LogSeverity.Error,
coordinate,
externalField,
schema);
}

public static LogEntry OutputFieldTypesNotMergeable(
OutputFieldDefinition field,
string typeName,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using System.Collections.Immutable;
using HotChocolate.Fusion.Events;
using HotChocolate.Skimmed;
using static HotChocolate.Fusion.Logging.LogEntryHelper;

namespace HotChocolate.Fusion.PreMergeValidation.Rules;

/// <summary>
/// This rule ensures that every field marked as <c>@external</c> in a source schema is actually
/// used by that source schema in a <c>@provides</c> directive.
/// </summary>
/// <seealso href="https://graphql.github.io/composite-schemas-spec/draft/#sec-External-Unused">
/// Specification
/// </seealso>
internal sealed class ExternalUnusedRule : IEventHandler<OutputFieldEvent>
{
public void Handle(OutputFieldEvent @event, CompositionContext context)
{
var (field, type, schema) = @event;

if (ValidationHelper.IsExternal(field))
{
var referencingFields =
schema.Types
.OfType<ComplexTypeDefinition>()
.SelectMany(t => t.Fields)
.Where(f => f.Type == type)
.ToImmutableArray();

var isReferenced =
referencingFields.Any(f => ValidationHelper.ProvidesFieldName(f, field.Name));

if (!isReferenced)
{
context.Log.Write(ExternalUnused(field, type, schema));
}
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
<data name="LogEntryHelper_ExternalMissingOnBase" xml:space="preserve">
<value>External field '{0}' in schema '{1}' is not defined (non-external) in any other schema.</value>
</data>
<data name="LogEntryHelper_ExternalUnused" xml:space="preserve">
<value>External field '{0}' in schema '{1}' is not referenced by an @provides directive in the schema.</value>
</data>
<data name="LogEntryHelper_OutputFieldTypesNotMergeable" xml:space="preserve">
<value>Field '{0}' has a different type shape in schema '{1}' than it does in schema '{2}'.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ private CompositionResult<SchemaDefinition> MergeSchemaDefinitions(CompositionCo
new DisallowedInaccessibleElementsRule(),
new ExternalArgumentDefaultMismatchRule(),
new ExternalMissingOnBaseRule(),
new ExternalUnusedRule(),
new OutputFieldTypesMergeableRule()
];
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using HotChocolate.Language;
using HotChocolate.Skimmed;

namespace HotChocolate.Fusion;
Expand All @@ -14,6 +15,28 @@ public static bool IsExternal(IDirectivesProvider type)
return type.Directives.ContainsName(WellKnownDirectiveNames.External);
}

/// <summary>
/// Returns <c>true</c> if the specified <paramref name="field"/> has an <c>@provides</c>
/// directive that references the specified <paramref name="fieldName"/>.
/// </summary>
public static bool ProvidesFieldName(OutputFieldDefinition field, string fieldName)
{
var providesDirective = field.Directives.FirstOrDefault(WellKnownDirectiveNames.Provides);

var fieldsArgumentValueNode =
providesDirective?.Arguments.GetValueOrDefault(WellKnownArgumentNames.Fields);

if (fieldsArgumentValueNode is not StringValueNode fieldsArgumentStringNode)
{
return false;
}

var selectionSet =
Utf8GraphQLParser.Syntax.ParseSelectionSet($"{{{fieldsArgumentStringNode.Value}}}");

return selectionSet.Selections.OfType<FieldNode>().Any(f => f.Name.Value == fieldName);
}

public static bool SameTypeShape(ITypeDefinition typeA, ITypeDefinition typeB)
{
while (true)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace HotChocolate.Fusion;

internal static class WellKnownArgumentNames
{
public const string Fields = "fields";
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ internal static class WellKnownDirectiveNames
{
public const string External = "external";
public const string Inaccessible = "inaccessible";
public const string Provides = "provides";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
using HotChocolate.Fusion.Logging;
using HotChocolate.Fusion.PreMergeValidation;
using HotChocolate.Fusion.PreMergeValidation.Rules;

namespace HotChocolate.Composition.PreMergeValidation.Rules;

public sealed class ExternalUnusedRuleTests : CompositionTestBase
{
private readonly PreMergeValidator _preMergeValidator = new([new ExternalUnusedRule()]);

[Theory]
[MemberData(nameof(ValidExamplesData))]
public void Examples_Valid(string[] sdl)
{
// arrange
var context = CreateCompositionContext(sdl);

// act
var result = _preMergeValidator.Validate(context);

// assert
Assert.True(result.IsSuccess);
Assert.True(context.Log.IsEmpty);
}

[Theory]
[MemberData(nameof(InvalidExamplesData))]
public void Examples_Invalid(string[] sdl, string[] errorMessages)
{
// arrange
var context = CreateCompositionContext(sdl);

// act
var result = _preMergeValidator.Validate(context);

// assert
Assert.True(result.IsFailure);
Assert.Equal(errorMessages, context.Log.Select(e => e.Message).ToArray());
Assert.True(context.Log.All(e => e.Code == "EXTERNAL_UNUSED"));
Assert.True(context.Log.All(e => e.Severity == LogSeverity.Error));
}

public static TheoryData<string[]> ValidExamplesData()
{
return new TheoryData<string[]>
{
// In this example, the `name` field is marked with @external and is used by the
// @provides directive, satisfying the rule.
{
[
"""
# Source schema A
type Product {
id: ID
name: String @external
}

type Query {
productByName(name: String): Product @provides(fields: "name")
}
"""
]
},
// Provides two fields.
{
[
"""
# Source schema A
type Product {
id: ID
name: String @external
}

type Query {
productByName(name: String): Product @provides(fields: "id name")
}
"""
]
}
};
}

public static TheoryData<string[], string[]> InvalidExamplesData()
{
return new TheoryData<string[], string[]>
{
// In this example, the `name` field is marked with @external but is not used by the
// @provides directive, violating the rule.
{
[
"""
# Source schema A
type Product {
title: String @external
author: Author
}
"""
],
[
"External field 'Product.title' in schema 'A' is not referenced by an " +
"@provides directive in the schema."
]
},
// Provides different field.
{
[
"""
# Source schema A
type Product {
title: String @external
author: Author
}

type Query {
productByName(name: String): Product @provides(fields: "author")
}
"""
],
[
"External field 'Product.title' in schema 'A' is not referenced by an " +
"@provides directive in the schema."
]
}
};
}
}
Loading