From 14d84c01e49400168694e3e56a0032992e8954d4 Mon Sep 17 00:00:00 2001
From: Nicholas Lim <18374483+niclim@users.noreply.github.com>
Date: Mon, 4 Mar 2024 08:11:24 -0500
Subject: [PATCH] Have better allof warning (#2766)

---
 .../__snapshots__/denormalize.test.ts.snap    | 14 +++++++++
 .../__tests__/denormalize.test.ts             |  5 ++--
 .../src/denormalizers/denormalizeProperty.ts  | 30 ++++++++++++++++---
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/projects/openapi-io/src/denormalizers/__tests__/__snapshots__/denormalize.test.ts.snap b/projects/openapi-io/src/denormalizers/__tests__/__snapshots__/denormalize.test.ts.snap
index ba56bba2cd..ec5ade7c5e 100644
--- a/projects/openapi-io/src/denormalizers/__tests__/__snapshots__/denormalize.test.ts.snap
+++ b/projects/openapi-io/src/denormalizers/__tests__/__snapshots__/denormalize.test.ts.snap
@@ -110,6 +110,12 @@ paths:
 }
 `;
 
+exports[`denormalize allOf merging does not merge allOf when all items are not all objects: warnings 1`] = `
+[
+  "invalid allOf variant at src/denormalizers/__tests__/specs/allOf/no-merge.yaml:26:578 (/paths/~1example/get/requestBody/content/application~1json/schema/properties/d/allOf/1)",
+]
+`;
+
 exports[`denormalize allOf merging merges allOf when all items are objects 1`] = `
 {
   "jsonLike": {
@@ -242,6 +248,8 @@ paths:
 }
 `;
 
+exports[`denormalize allOf merging merges allOf when all items are objects: warnings 1`] = `[]`;
+
 exports[`denormalize allOf merging merges allOf with only one item 1`] = `
 {
   "jsonLike": {
@@ -305,6 +313,8 @@ paths:
 }
 `;
 
+exports[`denormalize allOf merging merges allOf with only one item: warnings 1`] = `[]`;
+
 exports[`denormalize allOf merging merges allOfs in type array object / items 1`] = `
 {
   "jsonLike": {
@@ -536,6 +546,8 @@ paths:
 }
 `;
 
+exports[`denormalize allOf merging merges allOfs in type array object / items: warnings 1`] = `[]`;
+
 exports[`denormalize allOf merging merges nested allOf 1`] = `
 {
   "jsonLike": {
@@ -691,6 +703,8 @@ paths:
 }
 `;
 
+exports[`denormalize allOf merging merges nested allOf: warnings 1`] = `[]`;
+
 exports[`denormalize denormalizes shared path parameters 1`] = `
 {
   "jsonLike": {
diff --git a/projects/openapi-io/src/denormalizers/__tests__/denormalize.test.ts b/projects/openapi-io/src/denormalizers/__tests__/denormalize.test.ts
index a8e922c2e6..a779367ee5 100644
--- a/projects/openapi-io/src/denormalizers/__tests__/denormalize.test.ts
+++ b/projects/openapi-io/src/denormalizers/__tests__/denormalize.test.ts
@@ -60,10 +60,11 @@ describe('denormalize', () => {
       const spec = await parseOpenAPIWithSourcemap(
         path.resolve(openapiFilePath)
       );
-
-      const denormalized = denormalize(spec);
+      const warnings: string[] = [];
+      const denormalized = denormalize(spec, warnings);
 
       expect(prepSnapshot(denormalized)).toMatchSnapshot();
+      expect(warnings).toMatchSnapshot('warnings');
     });
   });
 });
diff --git a/projects/openapi-io/src/denormalizers/denormalizeProperty.ts b/projects/openapi-io/src/denormalizers/denormalizeProperty.ts
index ac61685cb7..80248c5677 100644
--- a/projects/openapi-io/src/denormalizers/denormalizeProperty.ts
+++ b/projects/openapi-io/src/denormalizers/denormalizeProperty.ts
@@ -1,5 +1,10 @@
 import { jsonPointerHelpers } from '@useoptic/json-pointer-helpers';
-import { FlatOpenAPIV3, OAS3 } from '@useoptic/openapi-utilities';
+import {
+  FlatOpenAPIV3,
+  OAS3,
+  getSourcemapLink,
+  sourcemapReader,
+} from '@useoptic/openapi-utilities';
 import { JsonSchemaSourcemap } from '../parser/sourcemap';
 import { logPointer } from './pointer';
 
@@ -119,10 +124,27 @@ export function denormalizeProperty(
           sourcemap,
           pointers
         );
+        let maybeReader:
+          | ReturnType<typeof sourcemapReader>['findFileAndLines']
+          | undefined = undefined;
+        if (sourcemap) {
+          maybeReader = sourcemapReader(sourcemap).findFileAndLines;
+        }
         warnings.push(
-          ...invalidChildren.map(
-            ([, i]) => `invalid allOf variant at ${pointers.old}/allOf/${i}`
-          ),
+          ...invalidChildren.map(([, i]) => {
+            const jsonPath = jsonPointerHelpers.append(
+              pointers.old,
+              'allOf',
+              String(i)
+            );
+            const maybeSourcemap = maybeReader
+              ? maybeReader(jsonPath)
+              : maybeReader;
+            const location = maybeSourcemap
+              ? `${getSourcemapLink(maybeSourcemap)} (${jsonPath})`
+              : jsonPath;
+            return `invalid allOf variant at ${location}`;
+          }),
           ...w
         );
         effectiveObject = obj;