diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs index a9fd60572b6..124895c8782 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryCodes.cs @@ -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"; } diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs index 6e14b99ddf5..2a775985a2f 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Logging/LogEntryHelper.cs @@ -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, diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ExternalUnusedRule.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ExternalUnusedRule.cs new file mode 100644 index 00000000000..74b9833a0cc --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/PreMergeValidation/Rules/ExternalUnusedRule.cs @@ -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; + +/// +/// This rule ensures that every field marked as @external in a source schema is actually +/// used by that source schema in a @provides directive. +/// +/// +/// Specification +/// +internal sealed class ExternalUnusedRule : IEventHandler +{ + public void Handle(OutputFieldEvent @event, CompositionContext context) + { + var (field, type, schema) = @event; + + if (ValidationHelper.IsExternal(field)) + { + var referencingFields = + schema.Types + .OfType() + .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)); + } + } + } +} diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.Designer.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.Designer.cs index a36dca256ca..12a930d9e85 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.Designer.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.Designer.cs @@ -131,6 +131,15 @@ internal static string LogEntryHelper_ExternalMissingOnBase { } } + /// + /// Looks up a localized string similar to External field '{0}' in schema '{1}' is not referenced by an @provides directive in the schema.. + /// + internal static string LogEntryHelper_ExternalUnused { + get { + return ResourceManager.GetString("LogEntryHelper_ExternalUnused", resourceCulture); + } + } + /// /// Looks up a localized string similar to Field '{0}' has a different type shape in schema '{1}' than it does in schema '{2}'.. /// diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx index df1b4ed6217..f7b806e4753 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/Properties/CompositionResources.resx @@ -42,6 +42,9 @@ External field '{0}' in schema '{1}' is not defined (non-external) in any other schema. + + External field '{0}' in schema '{1}' is not referenced by an @provides directive in the schema. + Field '{0}' has a different type shape in schema '{1}' than it does in schema '{2}'. diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs index ef997e408b7..c87eb7d4731 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/SourceSchemaMerger.cs @@ -49,6 +49,7 @@ private CompositionResult MergeSchemaDefinitions(CompositionCo new DisallowedInaccessibleElementsRule(), new ExternalArgumentDefaultMismatchRule(), new ExternalMissingOnBaseRule(), + new ExternalUnusedRule(), new OutputFieldTypesMergeableRule() ]; } diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/ValidationHelper.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/ValidationHelper.cs index b84d9c171ba..20dc84fa68d 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/ValidationHelper.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/ValidationHelper.cs @@ -1,3 +1,4 @@ +using HotChocolate.Language; using HotChocolate.Skimmed; namespace HotChocolate.Fusion; @@ -14,6 +15,28 @@ public static bool IsExternal(IDirectivesProvider type) return type.Directives.ContainsName(WellKnownDirectiveNames.External); } + /// + /// Returns true if the specified has an @provides + /// directive that references the specified . + /// + 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().Any(f => f.Name.Value == fieldName); + } + public static bool SameTypeShape(ITypeDefinition typeA, ITypeDefinition typeB) { while (true) diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownArgumentNames.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownArgumentNames.cs new file mode 100644 index 00000000000..82f03e1d481 --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownArgumentNames.cs @@ -0,0 +1,6 @@ +namespace HotChocolate.Fusion; + +internal static class WellKnownArgumentNames +{ + public const string Fields = "fields"; +} diff --git a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs index d27781211ab..628d7f00a25 100644 --- a/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs +++ b/src/HotChocolate/Fusion-vnext/src/Fusion.Composition/WellKnownDirectiveNames.cs @@ -4,4 +4,5 @@ internal static class WellKnownDirectiveNames { public const string External = "external"; public const string Inaccessible = "inaccessible"; + public const string Provides = "provides"; } diff --git a/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ExternalUnusedRuleTests.cs b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ExternalUnusedRuleTests.cs new file mode 100644 index 00000000000..43b7429547f --- /dev/null +++ b/src/HotChocolate/Fusion-vnext/test/Fusion.Composition.Tests/PreMergeValidation/Rules/ExternalUnusedRuleTests.cs @@ -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 ValidExamplesData() + { + return new TheoryData + { + // 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 InvalidExamplesData() + { + return new TheoryData + { + // 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." + ] + } + }; + } +}