Skip to content

[MutabilityWithReadOnly] Optimize to prevent excessive memory usage on large specs #813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AkhilaIlla merged 16 commits into from
Jan 30, 2026
Merged

[MutabilityWithReadOnly] Optimize to prevent excessive memory usage on large specs #813

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
98df3f9
Initial plan
Copilot Jan 28, 2026
6891fac
Fix MutabilityWithReadOnly rule to avoid out of memory issue by filte…
Copilot Jan 28, 2026
93c4e14
Remove redundant checks from mutability-with-read-only function
Copilot Jan 28, 2026
af3e536
Add Array.isArray check for x-ms-mutability defensive programming
Copilot Jan 28, 2026
7142114
Move length check to given clause for maximum efficiency
Copilot Jan 28, 2026
b71fdb6
Remove Array.isArray check that changed behavior - keep performance o…
Copilot Jan 29, 2026
aaf7003
Simplify to minimal change: only filter readOnly and x-ms-mutability …
Copilot Jan 29, 2026
ab900ea
Add comprehensive tests for all readOnly and x-ms-mutability combinat…
Copilot Jan 29, 2026
410cf09
Add tests for properties with omitted readOnly or x-ms-mutability fields
Copilot Jan 29, 2026
7d055aa
Refactor tests to reduce redundancy using helper function and consoli…
Copilot Jan 29, 2026
a61fd2f
Change helper function type from any to unknown for better type safety
Copilot Jan 29, 2026
3d9eca9
improve comment
mikeharder Jan 29, 2026
7b0dfe4
Add Rush change file for MutabilityWithReadOnly performance fix
Copilot Jan 29, 2026
7d9c1ff
Update package.json to 2.2.1 and add CHANGELOG entry
Copilot Jan 29, 2026
573ada9
improve changelog
mikeharder Jan 29, 2026
c2ad655
Apply suggestion from @mikeharder
mikeharder Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions packages/rulesets/generated/spectral/az-arm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,7 @@ const mutabilityWithReadOnly = (prop, _opts, ctx) => {
if (prop === null || typeof prop !== "object") {
return [];
}
if (prop.readOnly === undefined ||
prop["x-ms-mutability"] === undefined ||
prop["x-ms-mutability"].length === 0) {
if (prop["x-ms-mutability"].length === 0) {
return [];
}
const path = ctx.path || [];
Expand Down Expand Up @@ -1067,7 +1065,7 @@ const ruleset$1 = {
severity: "error",
resolved: true,
formats: [oas2],
given: ["$[paths,'x-ms-paths']..?(@property === 'readOnly')^"],
given: ["$[paths,'x-ms-paths']..*[?(@.readOnly !== undefined && @['x-ms-mutability'] !== undefined)]"],
then: {
function: mutabilityWithReadOnly,
},
Expand Down
6 changes: 2 additions & 4 deletions packages/rulesets/generated/spectral/az-common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ const mutabilityWithReadOnly = (prop, _opts, ctx) => {
if (prop === null || typeof prop !== "object") {
return [];
}
if (prop.readOnly === undefined ||
prop["x-ms-mutability"] === undefined ||
prop["x-ms-mutability"].length === 0) {
if (prop["x-ms-mutability"].length === 0) {
return [];
}
const path = ctx.path || [];
Expand Down Expand Up @@ -821,7 +819,7 @@ const ruleset = {
severity: "error",
resolved: true,
formats: [oas2],
given: ["$[paths,'x-ms-paths']..?(@property === 'readOnly')^"],
given: ["$[paths,'x-ms-paths']..*[?(@.readOnly !== undefined && @['x-ms-mutability'] !== undefined)]"],
then: {
function: mutabilityWithReadOnly,
},
Expand Down
6 changes: 2 additions & 4 deletions packages/rulesets/generated/spectral/az-dataplane.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ const mutabilityWithReadOnly = (prop, _opts, ctx) => {
if (prop === null || typeof prop !== "object") {
return [];
}
if (prop.readOnly === undefined ||
prop["x-ms-mutability"] === undefined ||
prop["x-ms-mutability"].length === 0) {
if (prop["x-ms-mutability"].length === 0) {
return [];
}
const path = ctx.path || [];
Expand Down Expand Up @@ -851,7 +849,7 @@ const ruleset$1 = {
severity: "error",
resolved: true,
formats: [oas2],
given: ["$[paths,'x-ms-paths']..?(@property === 'readOnly')^"],
given: ["$[paths,'x-ms-paths']..*[?(@.readOnly !== undefined && @['x-ms-mutability'] !== undefined)]"],
then: {
function: mutabilityWithReadOnly,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/rulesets/src/spectral/az-common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ const ruleset: any = {
severity: "error",
resolved: true,
formats: [oas2],
given: ["$[paths,'x-ms-paths']..?(@property === 'readOnly')^"],
given: ["$[paths,'x-ms-paths']..*[?(@.readOnly !== undefined && @['x-ms-mutability'] !== undefined)]"],
then: {
function: mutabilityWithReadOnly,
},
Expand Down
7 changes: 2 additions & 5 deletions packages/rulesets/src/spectral/functions/Extensions/mutability-with-read-only.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@ export const mutabilityWithReadOnly = (prop: any, _opts: any, ctx: any) => {
if (prop === null || typeof prop !== "object") {
return [];
}
if (
prop.readOnly === undefined ||
prop["x-ms-mutability"] === undefined ||
prop["x-ms-mutability"].length === 0
) {
// The given clause filters for readOnly !== undefined and x-ms-mutability !== undefined
Comment thread
mikeharder marked this conversation as resolved.
Outdated
if (prop["x-ms-mutability"].length === 0) {
Copy link
Copy Markdown
Member

@AkhilaIlla AkhilaIlla Jan 30, 2026

Choose a reason for hiding this comment

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

@copilot :
Is it better to have this check moved to "given" clause or keep it here in terms of performance optimization, Code correctness, Readability, Simplicity and maintainability.

Copy link
Copy Markdown
Contributor Author

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

Great question! Here's the analysis:

Moving to given clause (&& @['x-ms-mutability'].length > 0):

  • ✅ Performance: Slightly better (fewer function calls for empty arrays)
  • ✅ Correctness: Same behavior (filters earlier)
  • ⚠️ Readability: More complex JSONPath expression
  • ⚠️ Simplicity: Concentrates all filtering in one place vs separation of concerns
  • ⚠️ Maintainability: JSONPath length operator may have edge cases

Keeping in function:

  • ⚠️ Performance: Minimal cost (simple .length check on already-filtered properties)
  • ✅ Correctness: Explicit and obvious
  • ✅ Readability: Clear JavaScript check, easier to understand
  • ✅ Simplicity: Separates "must have both fields" (given) from "must be non-empty" (function)
  • ✅ Maintainability: Standard JavaScript, no JSONPath edge cases

Recommendation: Keep in function. The performance difference is negligible since we already filter out ~99% of properties in the given clause. The clarity and maintainability benefits outweigh the tiny performance gain.

return [];
}
const path = ctx.path || [];
Expand Down
37 changes: 37 additions & 0 deletions packages/rulesets/src/spectral/test/mutability-with-read-only.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,40 @@ test("MutabilityWithReadOnly should find no errors", () => {
expect(results.length).toBe(0);
});
});

test("MutabilityWithReadOnly should ignore empty x-ms-mutability arrays", () => {
Comment thread
mikeharder marked this conversation as resolved.
Outdated
const myOpenApiDocument = {
swagger: "2.0",
paths: {
"/api/Paths": {
put: {
operationId: "Path_Create",
responses: {
200: {
description: "Success",
schema: {
$ref: "#/definitions/LroStatusCodeSchema",
},
},
},
},
},
},
definitions: {
LroStatusCodeSchema: {
type: "object",
properties: {
name: {
type: "string",
readOnly: true,
"x-ms-mutability": [],
},
},
},
},
};
return linter.run(myOpenApiDocument).then((results) => {
// Empty x-ms-mutability arrays should be ignored (no errors)
expect(results.length).toBe(0);
});
});
⚡