diff --git a/src/lib/util/resolveSwagger.ts b/src/lib/util/resolveSwagger.ts index bdf07b1b..8f82340d 100644 --- a/src/lib/util/resolveSwagger.ts +++ b/src/lib/util/resolveSwagger.ts @@ -3,7 +3,8 @@ import * as jsonParser from "@ts-common/json-parser" import * as jsonPointer from "json-pointer" import { toArray } from "@ts-common/iterator" -import { cloneDeep, Data, FilePosition, getFilePosition } from "@ts-common/source-map" +import { cloneDeep, Data, FilePosition, getFilePosition, getInfo, getPath, ObjectInfo } from "@ts-common/source-map" +import * as sourceMap from "source-map" import * as sm from "@ts-common/string-map" import { readFileSync, writeFileSync } from "fs" import * as path from "path" @@ -92,9 +93,11 @@ function mergeParameters>(source: T, target: export class ResolveSwagger { public innerSwagger: json.Json | undefined public file: string + public map: sourceMap.BasicSourceMapConsumer | sourceMap.IndexedSourceMapConsumer - constructor(file: string) { + constructor(file: string, map: sourceMap.BasicSourceMapConsumer | sourceMap.IndexedSourceMapConsumer) { this.file = path.resolve(file) + this.map = map } public resolve(): json.Json | undefined { const content: string = readFileSync(this.file, { encoding: "utf8" }) @@ -245,7 +248,21 @@ export class ResolveSwagger { sm.keys(allOfSchema.properties).forEach(key => { if (sm.keys(schemaList).some(k => k === key)) { if (!this.isEqual(allOfSchema.properties[key], schemaList[key])) { - throw new Error(`incompatible properties : ${key} `) + const allOfProp = allOfSchema.properties[key] + const allOfPath = getPath(getInfo(allOfProp) as ObjectInfo) + const allOfOriginalPosition = this.map.originalPositionFor(getFilePosition(allOfProp) as FilePosition) + + const schemaListProp = schemaList[key] + const schemaListPath = getPath(getInfo(schemaListProp) as ObjectInfo) + const schemaListOriginalPosition = this.map.originalPositionFor(getFilePosition(schemaListProp) as FilePosition) + + throw new Error( + `incompatible properties : ${key}\n` + + ` ${schemaListPath.join("/")}\n` + + ` at ${schemaListOriginalPosition.source}#L${schemaListOriginalPosition.line}:${schemaListOriginalPosition.column}\n` + + ` ${allOfPath.join("/")}\n` + + ` at ${allOfOriginalPosition.source}#L${allOfOriginalPosition.line}:${allOfOriginalPosition.column}` + ) } } else { schemaList[key] = allOfSchema.properties[key] diff --git a/src/lib/validators/openApiDiff.ts b/src/lib/validators/openApiDiff.ts index 23cfd934..145b0e3e 100644 --- a/src/lib/validators/openApiDiff.ts +++ b/src/lib/validators/openApiDiff.ts @@ -247,16 +247,17 @@ export class OpenApiDiff { if (stderr) { throw new Error(stderr) } - const resolveSwagger = new ResolveSwagger(outputFilePath) + + const buffer = await asyncFs.readFile(outputMapFilePath) + const map = await new sourceMap.SourceMapConsumer(buffer.toString()) + + const resolveSwagger = new ResolveSwagger(outputFilePath, map) const resolvedJson = resolveSwagger.resolve() const resolvedPath: string = resolveSwagger.getResolvedPath() if (!resolvedJson) { throw new Error("resolve failed!") } - const buffer = await asyncFs.readFile(outputMapFilePath) - const map = await new sourceMap.SourceMapConsumer(buffer.toString()) - log.debug(`outputFilePath: "${outputFilePath}"`) return { fileName: outputFilePath, diff --git a/src/test/compatiblePropertiesTest.ts b/src/test/compatiblePropertiesTest.ts new file mode 100644 index 00000000..95a1a491 --- /dev/null +++ b/src/test/compatiblePropertiesTest.ts @@ -0,0 +1,37 @@ +import * as assert from "assert" +import * as path from "path" +import * as index from "../index" +import { fileUrl } from "./fileUrl" + +// This test is part of regression test suite for https://github.com/Azure/azure-sdk-tools/issues/5981 +// Given a property with given type and name +// When another property with the same name and compatible type is referenced +// Then no issue is reported, as this is a valid scenario +test("compatible-properties", async () => { + const diff = new index.OpenApiDiff({}) + const file = "src/test/specs/compatible-properties.json" + const resultStr = await diff.compare(file, file) + const result = JSON.parse(resultStr) + const filePath = fileUrl(path.resolve(file)) + const expected = [ + { + code: "NoVersionChange", + docUrl: "https://github.com/Azure/openapi-diff/tree/master/docs/rules/1001.md", + id: "1001", + message: "The versions have not changed.", + mode: "Update", + new: { + ref: `${filePath}#`, + path: "", + location: `${filePath}:1:1` + }, + old: { + ref: `${filePath}#`, + path: "", + location: `${filePath}:1:1` + }, + type: "Info" + } + ] + assert.deepStrictEqual(result, expected) +}) diff --git a/src/test/incompatiblePropertiesTest.ts b/src/test/incompatiblePropertiesTest.ts new file mode 100644 index 00000000..0b0a3f31 --- /dev/null +++ b/src/test/incompatiblePropertiesTest.ts @@ -0,0 +1,29 @@ +import * as assert from "assert" +import * as path from "path" +import * as index from "../index" +import { fileUrl } from "./fileUrl" + +// This test is part of regression test suite for https://github.com/Azure/azure-sdk-tools/issues/5981 +// Given a property with given type and name +// When another property with the same name but an incompatible type is referenced +// Then an issue is reported, with output providing the exact source file locations of both of the occurrences of the property. +test("incompatible-properties", async () => { + const diff = new index.OpenApiDiff({}) + const file = "src/test/specs/incompatible-properties.json" + const filePath = fileUrl(path.resolve(file)) + + try { + await diff.compare(file, file) + assert.fail("expected diff.compare() to throw") + } catch (error) { + const e = error as Error + assert.equal( + e.message, + "incompatible properties : bar\n" + + " definitions/FooBarString/properties/bar\n" + + ` at ${filePath}#L13:8\n` + + " definitions/FooBarObject/properties/bar\n" + + ` at ${filePath}#L26:8` + ) + } +}) diff --git a/src/test/specs/compatible-properties.json b/src/test/specs/compatible-properties.json new file mode 100644 index 00000000..df2201d7 --- /dev/null +++ b/src/test/specs/compatible-properties.json @@ -0,0 +1,32 @@ +{ + "swagger": "2.0", + "info": { + "title": "xms-enum-name", + "version": "1.0" + }, + "paths": { + }, + "definitions": { + "FooBarString": { + "type":"object", + "properties": { + "bar": { + "type":"string" + } + }, + "allOf": [ + { + "$ref": "#/definitions/FooBarString2" + } + ] + }, + "FooBarString2": { + "type":"object", + "properties": { + "bar": { + "type":"string" + } + } + } + } +} diff --git a/src/test/specs/incompatible-properties.json b/src/test/specs/incompatible-properties.json new file mode 100644 index 00000000..35037219 --- /dev/null +++ b/src/test/specs/incompatible-properties.json @@ -0,0 +1,32 @@ +{ + "swagger": "2.0", + "info": { + "title": "xms-enum-name", + "version": "1.0" + }, + "paths": { + }, + "definitions": { + "FooBarString": { + "type":"object", + "properties": { + "bar": { + "type":"string" + } + }, + "allOf": [ + { + "$ref": "#/definitions/FooBarObject" + } + ] + }, + "FooBarObject": { + "type":"object", + "properties": { + "bar": { + "type":"object" + } + } + } + } +}