Skip to content
23 changes: 20 additions & 3 deletions src/lib/util/resolveSwagger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -92,9 +93,11 @@ function mergeParameters<T extends sm.MutableStringMap<Data>>(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" })
Expand Down Expand Up @@ -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]
Expand Down
9 changes: 5 additions & 4 deletions src/lib/validators/openApiDiff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,16 +247,17 @@ export class OpenApiDiff {
if (stderr) {
throw new Error(stderr)
}
const resolveSwagger = new ResolveSwagger(outputFilePath)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change is just moving this code earlier, and passing map to new ResolveSwagger() so it can include original locations in errors.

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,
Expand Down
37 changes: 37 additions & 0 deletions src/test/compatiblePropertiesTest.ts
Original file line number Diff line number Diff line change
@@ -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)
})
29 changes: 29 additions & 0 deletions src/test/incompatiblePropertiesTest.ts
Original file line number Diff line number Diff line change
@@ -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`
)
}
})
32 changes: 32 additions & 0 deletions src/test/specs/compatible-properties.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}
32 changes: 32 additions & 0 deletions src/test/specs/incompatible-properties.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
}
}