feat: Add the schema coordinate to all CompositionHints in composition-js (#2658)

Author: swcollard

.changeset/fuzzy-vans-hide.md

@@ -0,0 +1,6 @@
+---
+"@apollo/composition": minor
+---
+
+Includes an optional Schema Coordinate field in the Composition Hints returned by composition
+  

composition-js/src/__tests__/hints.test.ts

@@ -143,6 +143,7 @@ export class ComposeDirectiveManager {
           this.pushHint(new CompositionHint(
             HINTS.DIRECTIVE_COMPOSITION_INFO,
             `Non-composed core feature "${coreIdentity}" has major version mismatch across subgraphs`,
+            undefined,
             this.coreFeatureASTs(coreIdentity),
           ));
           raisedHint = true;
@@ -174,6 +175,7 @@ export class ComposeDirectiveManager {
       this.pushHint(new CompositionHint(
         HINTS.DIRECTIVE_COMPOSITION_INFO,
         `Directive "@${directive.name}" should not be explicitly manually composed since it is a federation directive composed by default`,
+        directive,
         composeInstance.sourceAST ? {
           ...composeInstance.sourceAST,
           subgraph: sg.name,
@@ -390,6 +392,7 @@ export class ComposeDirectiveManager {
         this.pushHint(new CompositionHint(
           HINTS.DIRECTIVE_COMPOSITION_WARN,
           `Composed directive "@${name}" is named differently in a subgraph that doesn't export it. Consistent naming will be required to export it.`,
+          undefined,
           subgraphsWithDifferentNaming
             .map((subgraph : Subgraph): SubgraphASTNode | undefined => {
               const ast = subgraph.schema.coreFeatures?.getByIdentity(items[0].feature.url.identity)?.directive.sourceAST;

composition-js/src/composeDirectiveManager.ts

@@ -1,4 +1,4 @@
-import { SubgraphASTNode } from "@apollo/federation-internals";
+import { NamedSchemaElement, SubgraphASTNode } from "@apollo/federation-internals";
 import { printLocation } from "graphql";
 
 export enum HintLevel {
@@ -231,15 +231,18 @@ export const HINTS = {
 
 export class CompositionHint {
   public readonly nodes?: readonly SubgraphASTNode[];
+  public readonly coordinate?: string;
 
   constructor(
     readonly definition: HintCodeDefinition,
     readonly message: string,
+    readonly element: NamedSchemaElement<any, any, any> | undefined,
     nodes?: readonly SubgraphASTNode[] | SubgraphASTNode
   ) {
     this.nodes = nodes
       ? (Array.isArray(nodes) ? (nodes.length === 0 ? undefined : nodes) : [nodes])
       : undefined;
+    this.coordinate = element?.coordinate;
   }
 
   toString(): string {

composition-js/src/hints.ts

@@ -1166,6 +1166,7 @@ class Merger {
         this.hints.push(new CompositionHint(
           HINTS.FROM_SUBGRAPH_DOES_NOT_EXIST,
           `Source subgraph "${sourceSubgraphName}" for field "${dest.coordinate}" on subgraph "${subgraphName}" does not exist.${extraMsg}`,
+          dest,
           overridingSubgraphASTNode,
         ));
       } else if (sourceSubgraphName === subgraphName) {
@@ -1182,6 +1183,7 @@ class Merger {
         this.hints.push(new CompositionHint(
           HINTS.OVERRIDE_DIRECTIVE_CAN_BE_REMOVED,
           `Field "${dest.coordinate}" on subgraph "${subgraphName}" no longer exists in the from subgraph. The @override directive can be removed.`,
+          dest,
           overridingSubgraphASTNode,
         ));
       } else {
@@ -1224,20 +1226,23 @@ class Merger {
             this.hints.push(new CompositionHint(
               HINTS.OVERRIDE_DIRECTIVE_CAN_BE_REMOVED,
               `Field "${dest.coordinate}" on subgraph "${subgraphName}" is not resolved anymore by the from subgraph (it is marked "@external" in "${sourceSubgraphName}"). The @override directive can be removed.`,
+              dest,
               overridingSubgraphASTNode,
             ));
           } else if (this.metadata(fromIdx).isFieldUsed(fromField)) {
             result.setUsedOverridden(fromIdx);
             this.hints.push(new CompositionHint(
               HINTS.OVERRIDDEN_FIELD_CAN_BE_REMOVED,
               `Field "${dest.coordinate}" on subgraph "${sourceSubgraphName}" is overridden. It is still used in some federation directive(s) (@key, @requires, and/or @provides) and/or to satisfy interface constraint(s), but consider marking it @external explicitly or removing it along with its references.`,
+              dest,
               overriddenSubgraphASTNode,
             ));
           } else {
             result.setUnusedOverridden(fromIdx);
             this.hints.push(new CompositionHint(
               HINTS.OVERRIDDEN_FIELD_CAN_BE_REMOVED,
               `Field "${dest.coordinate}" on subgraph "${sourceSubgraphName}" is overridden. Consider removing it.`,
+              dest,
               overriddenSubgraphASTNode,
             ));
           }
@@ -1969,6 +1974,7 @@ class Merger {
       this.hints.push(new CompositionHint(
         HINTS.UNUSED_ENUM_TYPE,
         `Enum type "${dest}" is defined but unused. It will be included in the supergraph with all the values appearing in any subgraph ("as if" it was only used as an output type).`,
+        dest
       ));
     }
 
@@ -2046,6 +2052,7 @@ class Merger {
           message: `Value "${value}" of enum type "${dest}" will not be part of the supergraph as it is not defined in all the subgraphs defining "${dest}": `,
           supergraphElement: dest,
           subgraphElements: sources,
+          targetedElement: value,
           elementToString: (type) => type.value(value.name) ? 'yes' : 'no',
           supergraphElementPrinter: (_, subgraphs) => `"${value}" is defined in ${subgraphs}`,
           otherElementsPrinter: (_, subgraphs) => ` but not in ${subgraphs}`,
@@ -2056,7 +2063,7 @@ class Merger {
         value.remove();
       }
     } else if (position === 'Output') {
-      this.hintOnInconsistentOutputEnumValue(sources, dest, value.name);
+      this.hintOnInconsistentOutputEnumValue(sources, dest, value);
     }
   }
 
@@ -2083,8 +2090,9 @@ class Merger {
   private hintOnInconsistentOutputEnumValue(
     sources: (EnumType | undefined)[],
     dest: EnumType,
-    valueName: string
+    value: EnumValue,
   ) {
+    const valueName: string = value.name
     for (const source of sources) {
       // As soon as we find a subgraph that has the type but not the member, we hint.
       if (source && !source.value(valueName)) {
@@ -2093,6 +2101,7 @@ class Merger {
           message: `Value "${valueName}" of enum type "${dest}" has been added to the supergraph but is only defined in a subset of the subgraphs defining "${dest}": `,
           supergraphElement: dest,
           subgraphElements: sources,
+          targetedElement: value,
           elementToString: type => type.value(valueName) ? 'yes' : 'no',
           supergraphElementPrinter: (_, subgraphs) => `"${valueName}" is defined in ${subgraphs}`,
           otherElementsPrinter: (_, subgraphs) => ` but not in ${subgraphs}`,
@@ -2491,6 +2500,7 @@ class Merger {
           this.mismatchReporter.pushHint(new CompositionHint(
             HINTS.MERGED_NON_REPEATABLE_DIRECTIVE_ARGUMENTS,
             `Directive @${name} is applied to "${(dest as any)['coordinate'] ?? dest}" in multiple subgraphs with different arguments. Merging strategies used by arguments: ${info.argumentsMerger}`,
+            undefined,
           ));
         } else {
           const idx = indexOfMax(counts);

composition-js/src/merging/merge.ts

@@ -1,4 +1,4 @@
-import { addSubgraphToASTNode, assert, ErrorCodeDefinition, joinStrings, MultiMap, printSubgraphNames, SubgraphASTNode } from '@apollo/federation-internals';
+import { addSubgraphToASTNode, assert, ErrorCodeDefinition, joinStrings, MultiMap, NamedSchemaElement, printSubgraphNames, SubgraphASTNode } from '@apollo/federation-internals';
 import { ASTNode, GraphQLError } from 'graphql';
 import { CompositionHint, HintCodeDefinition } from '../hints';
 
@@ -101,6 +101,7 @@ export class MismatchReporter {
     message,
     supergraphElement,
     subgraphElements,
+    targetedElement,
     elementToString,
     supergraphElementPrinter,
     otherElementsPrinter,
@@ -112,6 +113,7 @@ export class MismatchReporter {
     message: string,
     supergraphElement: TMismatched,
     subgraphElements: (TMismatched | undefined)[],
+    targetedElement?: NamedSchemaElement<any, any, any>
     elementToString: (elt: TMismatched, isSupergraph: boolean) => string | undefined,
     supergraphElementPrinter: (elt: string, subgraphs: string | undefined) => string,
     otherElementsPrinter: (elt: string, subgraphs: string) => string,
@@ -129,6 +131,7 @@ export class MismatchReporter {
         this.pushHint(new CompositionHint(
           code,
           message + distribution[0] + joinStrings(distribution.slice(1), ' and ') + (noEndOfMessageDot ? '' : '.'),
+          targetedElement ?? ((supergraphElement instanceof NamedSchemaElement) ? supergraphElement as NamedSchemaElement<any, any, any> : undefined),
           astNodes
         ));
       },

composition-js/src/merging/reporter.ts

@@ -143,6 +143,7 @@ function shareableFieldMismatchedRuntimeTypesHint(
   return new CompositionHint(
     HINTS.INCONSISTENT_RUNTIME_TYPES_FOR_SHAREABLE_RETURN,
     message,
+    field,
     subgraphNodes(state, (s) => (s.type(field.parent.name) as CompositeType | undefined)?.field(field.name)?.sourceAST),
   );
 }