Skip to content

Commit edcd5a7

Browse files
committed
refactor: address tatomyr review feedback (round 4)
- Remove as any casts from normalizeVisitors — types infer correctly - Remove redundant propertyCount (always equals totalSchemaProperties) - Fix constraintCount mixing across media types — keep all three correlated metrics from the same schema walk - Consolidate CurrentOperationContext with OperationMetrics field names, simplify buildOperationMetrics to destructuring spread - Export Oas3MediaType from openapi-core, use in hasExample signature - Remove any type on MediaType.enter — visitor infers type by default - Move collectDocumentMetrics to __tests__/collect-metrics-helper.ts since it is test-only code Made-with: Cursor
1 parent 47a9667 commit edcd5a7

14 files changed

Lines changed: 145 additions & 177 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import {
2+
normalizeTypes,
3+
getTypes,
4+
resolveDocument,
5+
BaseResolver,
6+
Source,
7+
type Document,
8+
type SpecVersion,
9+
type WalkContext,
10+
} from '@redocly/openapi-core';
11+
12+
import { collectMetrics, type CollectMetricsResult } from '../collect-metrics.js';
13+
14+
/**
15+
* Convenience wrapper that resolves a parsed OpenAPI document and collects metrics.
16+
* Useful in tests where you don't already have resolved types.
17+
*/
18+
export async function collectDocumentMetrics(
19+
parsed: Record<string, unknown>,
20+
options?: { specVersion?: SpecVersion; debugOperationId?: string }
21+
): Promise<CollectMetricsResult> {
22+
const specVersion: SpecVersion = options?.specVersion ?? 'oas3_0';
23+
const types = normalizeTypes(getTypes(specVersion), {});
24+
const source = new Source('score.yaml', JSON.stringify(parsed));
25+
const document: Document = { source, parsed };
26+
const externalRefResolver = new BaseResolver();
27+
const resolvedRefMap = await resolveDocument({
28+
rootDocument: document,
29+
rootType: types.Root,
30+
externalRefResolver,
31+
});
32+
const ctx: WalkContext = { problems: [], specVersion, visitorsData: {} };
33+
34+
return collectMetrics({
35+
document,
36+
types,
37+
resolvedRefMap,
38+
ctx,
39+
debugOperationId: options?.debugOperationId,
40+
});
41+
}

packages/cli/src/commands/score/__tests__/dependency-graph.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ function makeOp(path: string, method: string, refs: string[]): OperationMetrics
1515
polymorphismCount: 0,
1616
anyOfCount: 0,
1717
hasDiscriminator: false,
18-
propertyCount: 0,
1918
operationDescriptionPresent: false,
2019
schemaPropertiesWithDescription: 0,
2120
totalSchemaProperties: 0,

packages/cli/src/commands/score/__tests__/document-metrics.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { parseYaml } from '@redocly/openapi-core';
22
import { outdent } from 'outdent';
33

4-
import { collectDocumentMetrics } from '../collect-metrics.js';
4+
import { collectDocumentMetrics } from './collect-metrics-helper.js';
55

66
const yaml = (source: string) => parseYaml(source) as Record<string, unknown>;
77

@@ -131,7 +131,7 @@ describe('collectDocumentMetrics', () => {
131131
const op = metrics.operations.get('createItem')!;
132132
expect(op.requestBodyPresent).toBe(true);
133133
expect(op.requestExamplePresent).toBe(true);
134-
expect(op.propertyCount).toBe(2);
134+
expect(op.totalSchemaProperties).toBe(2);
135135
expect(op.constraintCount).toBe(2);
136136
expect(op.schemaPropertiesWithDescription).toBe(1);
137137
});

packages/cli/src/commands/score/__tests__/example-coverage.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { collectDocumentMetrics } from '../collect-metrics.js';
21
import { computeOperationSubscores, computeAgentReadiness } from '../scoring.js';
2+
import { collectDocumentMetrics } from './collect-metrics-helper.js';
33

44
async function collect(doc: Record<string, unknown>) {
55
return (await collectDocumentMetrics(doc)).metrics;

packages/cli/src/commands/score/__tests__/hotspots.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ function makeMetrics(overrides: Partial<OperationMetrics> = {}): OperationMetric
1616
polymorphismCount: 0,
1717
anyOfCount: 0,
1818
hasDiscriminator: false,
19-
propertyCount: 0,
2019
operationDescriptionPresent: true,
2120
schemaPropertiesWithDescription: 0,
2221
totalSchemaProperties: 0,

packages/cli/src/commands/score/__tests__/index.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ function makeTestMetrics(overrides: Partial<OperationMetrics> = {}): OperationMe
5151
polymorphismCount: 0,
5252
anyOfCount: 0,
5353
hasDiscriminator: false,
54-
propertyCount: 1,
5554
operationDescriptionPresent: true,
5655
schemaPropertiesWithDescription: 0,
5756
totalSchemaProperties: 1,

packages/cli/src/commands/score/__tests__/scoring.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ function makeBaseMetrics(overrides: Partial<OperationMetrics> = {}): OperationMe
2323
polymorphismCount: 0,
2424
anyOfCount: 0,
2525
hasDiscriminator: false,
26-
propertyCount: 0,
2726
operationDescriptionPresent: true,
2827
schemaPropertiesWithDescription: 0,
2928
totalSchemaProperties: 0,
@@ -132,7 +131,6 @@ describe('scoring', () => {
132131
parameterCount: 1,
133132
requiredParameterCount: 1,
134133
paramsWithDescription: 0,
135-
propertyCount: 1,
136134
totalSchemaProperties: 1,
137135
maxResponseSchemaDepth: 1,
138136
}),
@@ -146,7 +144,6 @@ describe('scoring', () => {
146144
requestBodyPresent: true,
147145
requestExamplePresent: true,
148146
constraintCount: 1,
149-
propertyCount: 2,
150147
totalSchemaProperties: 2,
151148
maxRequestSchemaDepth: 1,
152149
maxResponseSchemaDepth: 1,

packages/cli/src/commands/score/collect-metrics.ts

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,13 @@
11
import {
2-
normalizeTypes,
3-
getTypes,
2+
type normalizeTypes,
43
normalizeVisitors,
54
walkDocument,
6-
resolveDocument,
7-
BaseResolver,
8-
Source,
5+
type resolveDocument,
96
isPlainObject,
107
isRef,
118
isMappingRef,
129
unescapePointerFragment,
1310
type Document,
14-
type SpecVersion,
1511
type WalkContext,
1612
} from '@redocly/openapi-core';
1713

@@ -57,7 +53,7 @@ export function collectMetrics({
5753
const schemaWalkState = createSchemaWalkState();
5854
const schemaVisitor = createSchemaMetricVisitor(schemaWalkState);
5955
const normalizedSchemaVisitors = normalizeVisitors(
60-
[{ severity: 'warn', ruleId: 'score-schema', visitor: schemaVisitor as any }],
56+
[{ severity: 'warn', ruleId: 'score-schema', visitor: schemaVisitor }],
6157
types
6258
);
6359

@@ -87,7 +83,6 @@ export function collectMetrics({
8783
polymorphismCount: 0,
8884
anyOfCount: 0,
8985
hasDiscriminator: false,
90-
propertyCount: 0,
9186
totalSchemaProperties: 0,
9287
schemaPropertiesWithDescription: 0,
9388
constraintCount: 0,
@@ -102,7 +97,6 @@ export function collectMetrics({
10297
polymorphismCount: a.polymorphismCount + b.polymorphismCount,
10398
anyOfCount: a.anyOfCount + b.anyOfCount,
10499
hasDiscriminator: a.hasDiscriminator || b.hasDiscriminator,
105-
propertyCount: a.propertyCount + b.propertyCount,
106100
totalSchemaProperties: a.totalSchemaProperties + b.totalSchemaProperties,
107101
schemaPropertiesWithDescription:
108102
a.schemaPropertiesWithDescription + b.schemaPropertiesWithDescription,
@@ -172,7 +166,7 @@ export function collectMetrics({
172166
let maxBranch = walkSchema(polyBranches[0], debug);
173167
for (let i = 1; i < polyBranches.length; i++) {
174168
const branchStats = walkSchema(polyBranches[i], debug);
175-
if (branchStats.propertyCount > maxBranch.propertyCount) {
169+
if (branchStats.totalSchemaProperties > maxBranch.totalSchemaProperties) {
176170
maxBranch = branchStats;
177171
}
178172
}
@@ -191,7 +185,7 @@ export function collectMetrics({
191185
let maxBranch = walkSchema(discriminatorRefs[0], debug);
192186
for (let i = 1; i < discriminatorRefs.length; i++) {
193187
const branchStats = walkSchema(discriminatorRefs[i], debug);
194-
if (branchStats.propertyCount > maxBranch.propertyCount) {
188+
if (branchStats.totalSchemaProperties > maxBranch.totalSchemaProperties) {
195189
maxBranch = branchStats;
196190
}
197191
}
@@ -235,7 +229,7 @@ export function collectMetrics({
235229
const scoreVisitor = createScoreVisitor(accumulator);
236230

237231
const normalizedVisitors = normalizeVisitors(
238-
[{ severity: 'warn', ruleId: 'score', visitor: scoreVisitor as any }],
232+
[{ severity: 'warn', ruleId: 'score', visitor: scoreVisitor }],
239233
types
240234
);
241235

@@ -252,32 +246,3 @@ export function collectMetrics({
252246
debugLogs: accumulator.debugLogs,
253247
};
254248
}
255-
256-
/**
257-
* Convenience wrapper that resolves a parsed OpenAPI document and collects metrics.
258-
* Useful in tests and standalone usage where you don't already have resolved types.
259-
*/
260-
export async function collectDocumentMetrics(
261-
parsed: Record<string, unknown>,
262-
options?: { specVersion?: SpecVersion; debugOperationId?: string }
263-
): Promise<CollectMetricsResult> {
264-
const specVersion: SpecVersion = options?.specVersion ?? 'oas3_0';
265-
const types = normalizeTypes(getTypes(specVersion), {});
266-
const source = new Source('score.yaml', JSON.stringify(parsed));
267-
const document: Document = { source, parsed };
268-
const externalRefResolver = new BaseResolver();
269-
const resolvedRefMap = await resolveDocument({
270-
rootDocument: document,
271-
rootType: types.Root,
272-
externalRefResolver,
273-
});
274-
const ctx: WalkContext = { problems: [], specVersion, visitorsData: {} };
275-
276-
return collectMetrics({
277-
document,
278-
types,
279-
resolvedRefMap,
280-
ctx,
281-
debugOperationId: options?.debugOperationId,
282-
});
283-
}

packages/cli/src/commands/score/collectors/document-metrics.ts

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
isNotEmptyObject,
33
isPlainObject,
4+
type Oas3MediaType,
45
type Oas3Visitor,
56
type UserContext,
67
type Oas3PathItem,
@@ -49,7 +50,6 @@ export interface SchemaWalkState {
4950
polymorphismCount: number;
5051
anyOfCount: number;
5152
hasDiscriminator: boolean;
52-
propertyCount: number;
5353
totalSchemaProperties: number;
5454
schemaPropertiesWithDescription: number;
5555
constraintCount: number;
@@ -68,7 +68,6 @@ export function createSchemaWalkState(): SchemaWalkState {
6868
polymorphismCount: 0,
6969
anyOfCount: 0,
7070
hasDiscriminator: false,
71-
propertyCount: 0,
7271
totalSchemaProperties: 0,
7372
schemaPropertiesWithDescription: 0,
7473
constraintCount: 0,
@@ -122,7 +121,6 @@ export function createSchemaMetricVisitor(state: SchemaWalkState): Oas3Visitor {
122121
if (isPlainObject(schema.properties)) {
123122
const props = Object.entries(schema.properties) as [string, any][];
124123
state.totalSchemaProperties += props.length;
125-
state.propertyCount += props.length;
126124

127125
for (const [name, prop] of props) {
128126
localPropertyNames.push(name);
@@ -166,15 +164,14 @@ interface CurrentOperationContext {
166164

167165
maxRequestSchemaDepth: number;
168166
maxResponseSchemaDepth: number;
169-
propertyCount: number;
170167
totalSchemaProperties: number;
171168
schemaPropertiesWithDescription: number;
172169
constraintCount: number;
173170
polymorphismCount: number;
174171
anyOfCount: number;
175172
hasDiscriminator: boolean;
176173

177-
writableTopLevelFieldCount: number;
174+
topLevelWritableFieldCount: number;
178175

179176
requestBodyPresent: boolean;
180177
requestExamplePresent: boolean;
@@ -233,15 +230,14 @@ function createOperationContext(
233230

234231
maxRequestSchemaDepth: 0,
235232
maxResponseSchemaDepth: 0,
236-
propertyCount: 0,
237233
totalSchemaProperties: 0,
238234
schemaPropertiesWithDescription: 0,
239235
constraintCount: 0,
240236
polymorphismCount: 0,
241237
anyOfCount: 0,
242238
hasDiscriminator: false,
243239

244-
writableTopLevelFieldCount: 0,
240+
topLevelWritableFieldCount: 0,
245241

246242
requestBodyPresent: false,
247243
requestExamplePresent: false,
@@ -259,32 +255,14 @@ function createOperationContext(
259255
}
260256

261257
function buildOperationMetrics(ctx: CurrentOperationContext): OperationMetrics {
262-
return {
263-
path: ctx.path,
264-
method: ctx.method,
265-
operationId: ctx.operationId,
266-
parameterCount: ctx.parameterCount,
267-
requiredParameterCount: ctx.requiredParameterCount,
268-
paramsWithDescription: ctx.paramsWithDescription,
269-
requestBodyPresent: ctx.requestBodyPresent,
270-
topLevelWritableFieldCount: ctx.writableTopLevelFieldCount,
271-
maxRequestSchemaDepth: ctx.maxRequestSchemaDepth,
272-
maxResponseSchemaDepth: ctx.maxResponseSchemaDepth,
273-
polymorphismCount: ctx.polymorphismCount,
274-
anyOfCount: ctx.anyOfCount,
275-
hasDiscriminator: ctx.hasDiscriminator,
276-
propertyCount: ctx.propertyCount,
277-
operationDescriptionPresent: ctx.operationDescriptionPresent,
278-
schemaPropertiesWithDescription: ctx.schemaPropertiesWithDescription,
279-
totalSchemaProperties: ctx.totalSchemaProperties,
280-
constraintCount: ctx.constraintCount,
281-
requestExamplePresent: ctx.requestExamplePresent,
282-
responseExamplePresent: ctx.responseExamplePresent,
283-
structuredErrorResponseCount: ctx.structuredErrorResponseCount,
284-
totalErrorResponses: ctx.totalErrorResponses,
285-
ambiguousIdentifierCount: ctx.ambiguousIdentifierCount,
286-
refsUsed: ctx.refsUsed,
287-
};
258+
const {
259+
inRequestBody: _1,
260+
inResponse: _2,
261+
currentResponseCode: _3,
262+
errorStructuredCounted: _4,
263+
...metrics
264+
} = ctx;
265+
return metrics;
288266
}
289267

290268
export function createScoreVisitor(accumulator: ScoreAccumulator): Oas3Visitor {
@@ -324,7 +302,7 @@ export function createScoreVisitor(accumulator: ScoreAccumulator): Oas3Visitor {
324302
},
325303
},
326304
MediaType: {
327-
enter(mediaType: any) {
305+
enter(mediaType) {
328306
const current = accumulator.current;
329307
if (!current) return;
330308

@@ -350,12 +328,11 @@ export function createScoreVisitor(accumulator: ScoreAccumulator): Oas3Visitor {
350328

351329
const stats = accumulator.walkSchema(mediaType.schema, isDebugTarget);
352330

353-
current.propertyCount = Math.max(current.propertyCount, stats.propertyCount);
354331
if (stats.totalSchemaProperties > current.totalSchemaProperties) {
355332
current.totalSchemaProperties = stats.totalSchemaProperties;
356333
current.schemaPropertiesWithDescription = stats.schemaPropertiesWithDescription;
334+
current.constraintCount = stats.constraintCount;
357335
}
358-
current.constraintCount = Math.max(current.constraintCount, stats.constraintCount);
359336
current.polymorphismCount = Math.max(current.polymorphismCount, stats.polymorphismCount);
360337
current.anyOfCount = Math.max(current.anyOfCount, stats.anyOfCount);
361338
if (stats.hasDiscriminator) current.hasDiscriminator = true;
@@ -369,8 +346,8 @@ export function createScoreVisitor(accumulator: ScoreAccumulator): Oas3Visitor {
369346

370347
if (current.inRequestBody) {
371348
current.maxRequestSchemaDepth = Math.max(current.maxRequestSchemaDepth, stats.maxDepth);
372-
current.writableTopLevelFieldCount = Math.max(
373-
current.writableTopLevelFieldCount,
349+
current.topLevelWritableFieldCount = Math.max(
350+
current.topLevelWritableFieldCount,
374351
stats.writableTopLevelFields
375352
);
376353
}
@@ -388,7 +365,7 @@ export function createScoreVisitor(accumulator: ScoreAccumulator): Oas3Visitor {
388365
accumulator.debugLogs.push({
389366
context,
390367
entries: stats.debugEntries,
391-
totalProperties: stats.propertyCount,
368+
totalProperties: stats.totalSchemaProperties,
392369
totalPolymorphism: stats.polymorphismCount,
393370
totalConstraints: stats.constraintCount,
394371
maxDepth: stats.maxDepth,
@@ -460,7 +437,7 @@ function mergeParameters(
460437
return merged;
461438
}
462439

463-
function hasExample(mediaType: { example?: unknown; examples?: Record<string, unknown> }): boolean {
440+
function hasExample(mediaType: Oas3MediaType): boolean {
464441
return mediaType.example !== undefined || isNotEmptyObject(mediaType.examples);
465442
}
466443

0 commit comments

Comments
 (0)