Skip to content

Commit db73799

Browse files
committed
Logic for propagating suppressions.
1 parent 65b436b commit db73799

File tree

8 files changed

+187
-27
lines changed

8 files changed

+187
-27
lines changed

src/definitions.ts

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ import {
55
parseReference,
66
ReferenceMetadata,
77
toSorted,
8+
getRegistryName,
89
} from "./util.js";
9-
import * as fs from "fs";
1010
import path from "path";
11-
import { SuppressionRegistry } from "./suppression.js";
1211

1312
/** The registry to look up the name within. */
1413
export enum RegistryKind {
@@ -33,7 +32,7 @@ class CollectionRegistry {
3332
if (subdata !== undefined) {
3433
for (const [name, _] of toSorted(Object.entries(subdata))) {
3534
const resolvedPath = getResolvedPath(filepath).replace(/\\/g, "/");
36-
const pathKey = `${resolvedPath}#/${this.getRegistryName()}/${name}`;
35+
const pathKey = `${resolvedPath}#/${getRegistryName(this.kind)}/${name}`;
3736
// we don't care about unreferenced common-types
3837
if (!resolvedPath.includes("common-types")) {
3938
this.unreferenced.add(pathKey);
@@ -43,19 +42,6 @@ class CollectionRegistry {
4342
}
4443
}
4544

46-
getRegistryName(): string {
47-
switch (this.kind) {
48-
case RegistryKind.Definition:
49-
return "definitions";
50-
case RegistryKind.Parameter:
51-
return "parameters";
52-
case RegistryKind.Response:
53-
return "responses";
54-
case RegistryKind.SecurityDefinition:
55-
return "securityDefinitions";
56-
}
57-
}
58-
5945
/** Add or update an item. */
6046
add(itemPath: string, name: string, value: any) {
6147
const resolvedPath = getResolvedPath(itemPath);
@@ -77,7 +63,7 @@ class CollectionRegistry {
7763
countReference(path: string, name: string, kind: RegistryKind) {
7864
// convert backslashes to forward slashes
7965
path = path.replace(/\\/g, "/");
80-
const pathKey = `${path}#/${this.getRegistryName()}/${name}`;
66+
const pathKey = `${path}#/${getRegistryName(this.kind)}/${name}`;
8167
this.unreferenced.delete(pathKey);
8268
}
8369

src/diff-client.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@ export interface DiffClientConfig {
2424

2525
export class DiffClient {
2626
public args: any;
27+
public suppressions?: SuppressionRegistry;
28+
2729
private rules: RuleSignature[];
2830
private lhsParser?: SwaggerParser;
2931
private rhsParser?: SwaggerParser;
30-
private suppressions?: SuppressionRegistry;
3132
/** Tracks if shortenKeys has been called to avoid re-running the algorithm needlessly. */
3233
private keysShortened: boolean = false;
3334

@@ -251,7 +252,10 @@ export class DiffClient {
251252

252253
// if a violation is suppressed, keep metadata the same but change it from a
253254
// flagged or assumed violation to `RuleResult.Suppressed`.
254-
const isSuppressed = this.suppressions.has(getUrlEncodedPath(data.path));
255+
const urlEncodedPath = getUrlEncodedPath(data.path);
256+
const isSuppressed = this.suppressions
257+
? this.suppressions.has(urlEncodedPath)
258+
: false;
255259
const finalRuleResult =
256260
(Array.isArray(finalResult) ? finalResult[0] : finalResult) ??
257261
RuleResult.AssumedViolation;

src/parser.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { OpenAPIV2 } from "openapi-types";
33
import { DefinitionRegistry, RegistryKind } from "./definitions.js";
44
import {
55
forceArray,
6+
getUrlEncodedPath,
67
isReference,
78
loadPaths,
89
parseReference,
@@ -27,7 +28,9 @@ export class SwaggerParser {
2728
private host?: string;
2829
private result: any = {};
2930
private defRegistry?: DefinitionRegistry;
31+
private client?: DiffClient;
3032
private swaggerMap?: Map<string, any>;
33+
private currentPath?: string[];
3134

3235
/**
3336
* Creates a new SwaggerParser instance asynchronously.
@@ -44,6 +47,7 @@ export class SwaggerParser {
4447
const pathMap = await loadPaths(forceArray(paths), rootPath, client.args);
4548
parser.defRegistry = new DefinitionRegistry(pathMap, client);
4649
parser.swaggerMap = pathMap;
50+
parser.client = client;
4751
return parser;
4852
}
4953

@@ -55,6 +59,7 @@ export class SwaggerParser {
5559
if (!this.defRegistry) {
5660
throw new Error("Definition registry is not initialized.");
5761
}
62+
this.currentPath = [];
5863
const allPathsUnsorted: any = {};
5964
for (const [_, data] of this.swaggerMap.entries()) {
6065
// Retrieve any top-level defaults that need to be normalized later on.
@@ -74,12 +79,15 @@ export class SwaggerParser {
7479

7580
// combine the paths and x-ms-paths objects and merge into overall paths object
7681
const allPaths = { ...paths, ...xMsPaths };
82+
this.currentPath?.push("paths");
7783
const newPaths = this.#parsePaths(allPaths);
7884
for (const [path, data] of Object.entries(newPaths)) {
7985
allPathsUnsorted[path] = data;
8086
}
87+
this.currentPath?.pop();
8188

8289
for (const [key, val] of toSorted(Object.entries(data))) {
90+
this.currentPath?.push(key);
8391
switch (key) {
8492
case "swagger":
8593
case "info":
@@ -116,6 +124,7 @@ export class SwaggerParser {
116124
default:
117125
throw new Error(`Unhandled root key: ${key}`);
118126
}
127+
this.currentPath?.pop();
119128
}
120129
}
121130
// sort all the paths and add into the result
@@ -194,6 +203,7 @@ export class SwaggerParser {
194203
#parseResponse(value: any): any {
195204
let result: any = {};
196205
for (const [key, val] of toSorted(Object.entries(value))) {
206+
this.currentPath?.push(key);
197207
if (key === "headers") {
198208
result[key] = {};
199209
for (const [headerKey, headerVal] of toSorted(
@@ -205,6 +215,7 @@ export class SwaggerParser {
205215
} else {
206216
result[key] = this.#parseNode(val);
207217
}
218+
this.currentPath?.pop();
208219
}
209220
return result;
210221
}
@@ -238,6 +249,7 @@ export class SwaggerParser {
238249
#parseResponses(value: any): any {
239250
let result: any = {};
240251
for (const [code, data] of toSorted(Object.entries(value))) {
252+
this.currentPath?.push(code);
241253
if (code === "default") {
242254
// Don't expand the default response. We will handle this in a special way.
243255
const errorName = this.#parseErrorName(data);
@@ -252,6 +264,7 @@ export class SwaggerParser {
252264
} else {
253265
result[code] = this.#parseResponse(data);
254266
}
267+
this.currentPath?.pop();
255268
}
256269
return result;
257270
}
@@ -262,6 +275,7 @@ export class SwaggerParser {
262275
value["consumes"] = value["consumes"] ?? this.defaultConsumes;
263276
value["produces"] = value["produces"] ?? this.defaultProduces;
264277
for (const [key, val] of toSorted(Object.entries(value))) {
278+
this.currentPath?.push(key);
265279
if (key === "parameters") {
266280
// mix in any parameters from parameterized host
267281
const hostParams = this.parameterizedHost?.parameters ?? [];
@@ -301,6 +315,7 @@ export class SwaggerParser {
301315
} else {
302316
result[key] = this.#parseNode(value[key]);
303317
}
318+
this.currentPath?.pop();
304319
}
305320
return result;
306321
}
@@ -309,7 +324,9 @@ export class SwaggerParser {
309324
#parseVerbs(value: any): any {
310325
let result: any = {};
311326
for (const [verb, data] of toSorted(Object.entries(value))) {
327+
this.currentPath?.push(verb);
312328
result[verb] = this.#parseOperation(data);
329+
this.currentPath?.pop();
313330
}
314331
return result;
315332
}
@@ -320,7 +337,10 @@ export class SwaggerParser {
320337
for (const [operationPath, pathData] of Object.entries(value)) {
321338
// normalize the path to coerce the naming convention
322339
const normalizedPath = this.#normalizePath(operationPath);
340+
const urlEncodedPath = getUrlEncodedPath([normalizedPath])!;
341+
this.currentPath?.push(urlEncodedPath);
323342
result[normalizedPath] = this.#parseVerbs(pathData);
343+
this.currentPath?.pop();
324344
}
325345
return result;
326346
}
@@ -330,8 +350,10 @@ export class SwaggerParser {
330350
if (value.length > 0 && typeof value[0] === "object") {
331351
const values: any[] = [];
332352
for (let i = 0; i < value.length; i++) {
353+
this.currentPath?.push(i.toString());
333354
const item = value[i];
334355
values.push(this.#parseNode(item));
356+
this.currentPath?.pop();
335357
}
336358
return values;
337359
} else {
@@ -377,12 +399,18 @@ export class SwaggerParser {
377399
refResult.registry
378400
);
379401
}
402+
this.client?.suppressions?.propagateSuppression(
403+
refResult,
404+
this.currentPath
405+
);
380406
return this.#parseObject(resolved);
381407
}
382408
const result: any = {};
383409
// visit each key in the object in sorted order
384410
for (const [key, val] of toSorted(Object.entries(value))) {
411+
this.currentPath?.push(key);
385412
result[key] = this.#parseNode(val);
413+
this.currentPath?.pop();
386414
}
387415
return result;
388416
}

src/suppression.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
11
import * as fs from "fs";
22
import { parse } from "yaml";
3+
import { getRegistryName, ReferenceMetadata } from "./util.js";
34

45
export interface SuppressionMetadata {
56
path: string;
67
reason: string;
78
}
89

910
export class SuppressionRegistry {
10-
data = new Set<string>();
11+
private data = new Set<string>();
1112

1213
constructor(filepath: string) {
1314
const contents = fs.readFileSync(filepath, "utf8");
14-
const data: SuppressionMetadata[] = parse(contents);
15+
const data: SuppressionMetadata[] = parse(contents) ?? [];
1516
for (const item of data) {
1617
let path = item.path.trim().toLowerCase();
17-
this.data.add(path);
18+
this.add(path);
1819
}
1920
}
2021

@@ -24,9 +25,8 @@ export class SuppressionRegistry {
2425
* @param key the Swagger path which serves as a key
2526
* @param path the transformed path to add to the suppression list
2627
*/
27-
add(key: string, path: string) {
28+
add(path: string) {
2829
path = path.trim().toLowerCase();
29-
key = key.toLowerCase();
3030
this.data.add(path);
3131
}
3232

@@ -37,6 +37,20 @@ export class SuppressionRegistry {
3737
has(path: string | undefined): boolean {
3838
if (!path) return false;
3939
path = path.trim().toLowerCase();
40-
return this.data.has(path);
40+
const value = this.data.has(path);
41+
return value;
42+
}
43+
44+
propagateSuppression(ref: ReferenceMetadata, basePath: string[] | undefined) {
45+
if (!basePath) throw new Error("basePath is undefined");
46+
const base = basePath.join("/");
47+
const target = `${getRegistryName(ref.registry)}/${ref.name}`.toLowerCase();
48+
for (const item of this.data) {
49+
if (item.startsWith(target)) {
50+
// create a new string suppression propagating the suppression onto the base path
51+
const newItem = item.replace(target, base);
52+
this.add(newItem);
53+
}
54+
}
4155
}
4256
}

src/util.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,3 +428,19 @@ export function getUrlEncodedPath(
428428
if (segments === undefined) return undefined;
429429
return segments.map((x: string) => encodeURIComponent(x)).join("/");
430430
}
431+
432+
/**
433+
* Returns the string key for the `RegistryKind` enum.
434+
*/
435+
export function getRegistryName(kind: RegistryKind): string {
436+
switch (kind) {
437+
case RegistryKind.Definition:
438+
return "definitions";
439+
case RegistryKind.Parameter:
440+
return "parameters";
441+
case RegistryKind.Response:
442+
return "responses";
443+
case RegistryKind.SecurityDefinition:
444+
return "securityDefinitions";
445+
}
446+
}

suppression.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,2 @@
11
- path: definitions/Foo/properties/age/format
22
reason: Because a really good reason.
3-
- path: paths/%2F/get/responses/200/schema/properties/age/format
4-
reason: This reason is amazing.

test/files/suppressions1a.json

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
{
2+
"swagger": "2.0",
3+
"info": {
4+
"title": "(title)",
5+
"version": "0000-00-00",
6+
"x-typespec-generated": [
7+
{
8+
"emitter": "@azure-tools/typespec-autorest"
9+
}
10+
]
11+
},
12+
"schemes": ["https"],
13+
"produces": ["application/json"],
14+
"consumes": ["application/json"],
15+
"tags": [],
16+
"paths": {
17+
"/": {
18+
"put": {
19+
"operationId": "CreateFoo",
20+
"parameters": [
21+
{
22+
"name": "body",
23+
"in": "body",
24+
"required": true,
25+
"schema": {
26+
"$ref": "#/definitions/Foo"
27+
}
28+
}
29+
],
30+
"responses": {
31+
"200": {
32+
"description": "The request has succeeded.",
33+
"schema": {
34+
"$ref": "#/definitions/Foo"
35+
}
36+
}
37+
}
38+
}
39+
}
40+
},
41+
"definitions": {
42+
"Foo": {
43+
"type": "object",
44+
"properties": {
45+
"name": {
46+
"type": "string"
47+
},
48+
"age": {
49+
"type": "integer",
50+
"format": "int16"
51+
}
52+
},
53+
"required": ["name", "age"]
54+
}
55+
},
56+
"parameters": {}
57+
}

0 commit comments

Comments
 (0)