Skip to content

Commit 1fc5cbd

Browse files
authored
Merge pull request #223 from abraham/copilot/fix-222
[WIP] Strip complex types from property descriptions
2 parents 4f1bc5c + 6e555c4 commit 1fc5cbd

10 files changed

+82
-28
lines changed

dist/schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5120,7 +5120,7 @@
51205120
"type": "string",
51215121
"format": "uri"
51225122
},
5123-
"description": "String or Array of Strings. Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter."
5123+
"description": "Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter."
51245124
},
51255125
"scopes": {
51265126
"type": "string",

src/__tests__/generators/OpenAPIGenerator.complex.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ describe('OpenAPIGenerator Complex Parameters', () => {
2828

2929
expect(schema).toEqual({
3030
type: 'array',
31-
description:
32-
'Array of String. Include Attachment IDs to be attached as media.',
31+
description: 'Include Attachment IDs to be attached as media.',
3332
items: {
3433
type: 'string',
3534
},
@@ -103,7 +102,7 @@ describe('OpenAPIGenerator Complex Parameters', () => {
103102

104103
expect(schema).toEqual({
105104
type: 'string',
106-
description: 'String. The text content of the status.',
105+
description: 'The text content of the status.',
107106
enum: ['public', 'unlisted', 'private'],
108107
});
109108
});
@@ -119,7 +118,7 @@ describe('OpenAPIGenerator Complex Parameters', () => {
119118

120119
expect(schema).toEqual({
121120
type: 'string',
122-
description: 'String. ID of the status being replied to.',
121+
description: 'ID of the status being replied to.',
123122
});
124123
});
125124
});

src/__tests__/generators/TypeParser.email-format.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ describe('TypeParser - Email format detection', () => {
9191
expect(result).toEqual({
9292
type: 'string',
9393
format: 'email',
94-
description: 'String. The user email address',
94+
description: 'The user email address',
9595
});
9696
});
9797

@@ -107,7 +107,7 @@ describe('TypeParser - Email format detection', () => {
107107
expect(result).toEqual({
108108
type: 'string',
109109
format: 'email',
110-
description: 'String. The user e-mail address',
110+
description: 'The user e-mail address',
111111
});
112112
});
113113

@@ -122,7 +122,7 @@ describe('TypeParser - Email format detection', () => {
122122
const result = typeParser.convertParameterToSchema(param);
123123
expect(result).toEqual({
124124
type: 'string',
125-
description: 'Boolean. Whether to send a confirmation email',
125+
description: 'Whether to send a confirmation email',
126126
});
127127
expect(result.format).toBeUndefined();
128128
});
@@ -138,7 +138,7 @@ describe('TypeParser - Email format detection', () => {
138138
const result = typeParser.convertParameterToSchema(param);
139139
expect(result).toEqual({
140140
type: 'string',
141-
description: 'String. The email that will be sent for notifications',
141+
description: 'The email that will be sent for notifications',
142142
});
143143
expect(result.format).toBeUndefined();
144144
});
@@ -155,7 +155,7 @@ describe('TypeParser - Email format detection', () => {
155155
expect(result).toEqual({
156156
type: 'string',
157157
format: 'email',
158-
description: 'String. The user email for contact',
158+
description: 'The user email for contact',
159159
});
160160
});
161161

src/__tests__/generators/TypeParser.enum-fix.test.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ describe('TypeParser Enum Values Fix', () => {
2525
const schema = typeParser.convertParameterToSchema(parameter);
2626

2727
expect(schema.type).toBe('string');
28-
expect(schema.description).toBe(parameter.description);
28+
expect(schema.description).toBe(
29+
'Sets the visibility of the posted status to `public`, `unlisted`, `private`, `direct`.'
30+
);
2931
expect(schema.enum).toEqual(['public', 'unlisted', 'private', 'direct']);
3032
});
3133

@@ -47,7 +49,7 @@ describe('TypeParser Enum Values Fix', () => {
4749
const schema = typeParser.convertParameterToSchema(parameter);
4850

4951
expect(schema.type).toBe('array');
50-
expect(schema.description).toBe(parameter.description);
52+
expect(schema.description).toBe('Array of notification types');
5153
// Array properties should NOT have enum on the array itself, only on items
5254
expect(schema.enum).toBeUndefined();
5355
expect(schema.items).toBeDefined();
@@ -68,7 +70,9 @@ describe('TypeParser Enum Values Fix', () => {
6870
const schema = typeParser.convertParameterToSchema(parameter);
6971

7072
expect(schema.type).toBe('string');
71-
expect(schema.description).toBe(parameter.description);
73+
expect(schema.description).toBe(
74+
'Sets the visibility of the posted status to `public`, `unlisted`, `private`, `direct`.'
75+
);
7276
expect(schema.enum).toEqual(['public', 'unlisted', 'private', 'direct']);
7377
});
7478

@@ -86,7 +90,7 @@ describe('TypeParser Enum Values Fix', () => {
8690
const schema = typeParser.convertParameterToSchema(parameter);
8791

8892
expect(schema.type).toBe('string');
89-
expect(schema.description).toBe(parameter.description);
93+
expect(schema.description).toBe('The text content of the status.');
9094
expect(schema.enum).toBeUndefined();
9195
});
9296
});

src/__tests__/generators/TypeParser.redirect-uris-oneof.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('TypeParser - redirect_uris oneOf pattern', () => {
2323

2424
expect(schema).toEqual({
2525
description:
26-
'String or Array of Strings. Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter.',
26+
'Where the user should be redirected after authorization. To display the authorization code to the user instead of redirecting to a web page, use `urn:ietf:wg:oauth:2.0:oob` in this parameter.',
2727
oneOf: [
2828
{
2929
type: 'string',
@@ -50,7 +50,7 @@ describe('TypeParser - redirect_uris oneOf pattern', () => {
5050
const schema = typeParser.convertParameterToSchema(parameter);
5151

5252
expect(schema).toEqual({
53-
description: 'String or Array of String. Test parameter description.',
53+
description: 'Test parameter description.',
5454
oneOf: [
5555
{
5656
type: 'string',
@@ -100,7 +100,7 @@ describe('TypeParser - redirect_uris oneOf pattern', () => {
100100

101101
expect(schema).toEqual({
102102
type: 'string',
103-
description: 'String. A name for your application',
103+
description: 'A name for your application',
104104
});
105105
});
106106

@@ -115,7 +115,7 @@ describe('TypeParser - redirect_uris oneOf pattern', () => {
115115

116116
expect(schema).toEqual({
117117
type: 'string',
118-
description: 'Array of String. List of scopes.',
118+
description: 'List of scopes.',
119119
});
120120
});
121121
});

src/__tests__/integration/create-app-parameters.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ describe('Create App Method Parameters', () => {
9191

9292
expect(redirectUrisParam).toBeDefined();
9393
expect(redirectUrisParam!.description).toContain(
94-
'String or Array of Strings'
94+
'Where the user should be redirected after authorization'
9595
);
9696

9797
// Convert parameter to schema using TypeParser

src/__tests__/integration/create-app-redirect-uris-override.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('CreateApp redirect_uris override', () => {
4242

4343
// Verify the description is preserved
4444
expect(redirectUrisProperty.description).toContain(
45-
'String or Array of Strings'
45+
'Where the user should be redirected after authorization'
4646
);
4747
});
4848
});

src/__tests__/parsers/EntityParsingUtils.cleanDescription.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,39 @@ describe('EntityParsingUtils.cleanDescription - Type Stripping', () => {
186186
});
187187
});
188188

189+
test('should remove "String or Array of..." type prefixes from descriptions', () => {
190+
// Test the specific patterns mentioned in the issue
191+
const testCases = [
192+
{
193+
input:
194+
'String or Array of Strings. Where the user should be redirected after authorization.',
195+
expected: 'Where the user should be redirected after authorization.',
196+
},
197+
{
198+
input: 'String or Array of String. Test parameter description.',
199+
expected: 'Test parameter description.',
200+
},
201+
{
202+
input:
203+
'String or Array of String (URLs). Where the user should be redirected.',
204+
expected: 'Where the user should be redirected.',
205+
},
206+
{
207+
input: 'string or array of strings. Include attachment IDs.',
208+
expected: 'Include attachment IDs.',
209+
},
210+
{
211+
input: 'STRING OR ARRAY OF STRING. List of values.',
212+
expected: 'List of values.',
213+
},
214+
];
215+
216+
testCases.forEach(({ input, expected }) => {
217+
const result = EntityParsingUtils.cleanDescription(input);
218+
expect(result).toBe(expected);
219+
});
220+
});
221+
189222
test('should not strip complex type words that are not prefixes', () => {
190223
const testCases = [
191224
{

src/generators/TypeParser.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ApiParameter } from '../interfaces/ApiParameter';
22
import { HashAttribute } from '../interfaces/ApiMethod';
33
import { OpenAPIProperty, OpenAPISpec } from '../interfaces/OpenAPISchema';
4+
import { EntityParsingUtils } from '../parsers/EntityParsingUtils';
45
import { UtilityHelpers } from './UtilityHelpers';
56

67
/**
@@ -276,12 +277,19 @@ class TypeParser {
276277
*/
277278
public convertParameterToSchema(param: ApiParameter): OpenAPIProperty {
278279
// Check for "String or Array of Strings" pattern to generate oneOf schema
280+
// Also check for redirect_uris parameter specifically, which often has this pattern
279281
if (param.description) {
280282
const stringOrArrayPattern = /string\s+or\s+array\s+of\s+strings?/i.test(
281283
param.description
282284
);
283285

284-
if (stringOrArrayPattern) {
286+
// Special case for redirect_uris parameter - it's commonly "String or Array of Strings"
287+
// even if the type prefix has been stripped
288+
const isRedirectUrisParameter =
289+
param.name.toLowerCase() === 'redirect_uris' &&
290+
param.description.toLowerCase().includes('redirect');
291+
292+
if (stringOrArrayPattern || isRedirectUrisParameter) {
285293
// Detect if URIs are involved for format specification
286294
const hasUriIndicator =
287295
param.name.toLowerCase().includes('uri') ||
@@ -302,7 +310,7 @@ class TypeParser {
302310
}
303311

304312
return {
305-
description: param.description,
313+
description: EntityParsingUtils.cleanDescription(param.description),
306314
oneOf: [baseStringSchema, baseArraySchema],
307315
};
308316
}
@@ -312,7 +320,9 @@ class TypeParser {
312320
if (param.schema) {
313321
const schema: OpenAPIProperty = {
314322
type: param.schema.type,
315-
description: param.description,
323+
description: param.description
324+
? EntityParsingUtils.cleanDescription(param.description)
325+
: undefined,
316326
};
317327

318328
// Add enum values if available - for arrays, put enum on items instead of array
@@ -445,7 +455,9 @@ class TypeParser {
445455
if (hasDateTimePattern) {
446456
const parsedType = this.parseType(param.description || '');
447457
const schema: OpenAPIProperty = {
448-
description: param.description,
458+
description: param.description
459+
? EntityParsingUtils.cleanDescription(param.description)
460+
: undefined,
449461
...parsedType,
450462
};
451463

@@ -478,7 +490,9 @@ class TypeParser {
478490
const schema: OpenAPIProperty = {
479491
type: 'string',
480492
format: 'email',
481-
description: param.description,
493+
description: param.description
494+
? EntityParsingUtils.cleanDescription(param.description)
495+
: undefined,
482496
};
483497

484498
// Add enum values if available
@@ -497,7 +511,9 @@ class TypeParser {
497511
// Default fallback for other parameters
498512
const schema: OpenAPIProperty = {
499513
type: 'string',
500-
description: param.description,
514+
description: param.description
515+
? EntityParsingUtils.cleanDescription(param.description)
516+
: undefined,
501517
};
502518

503519
// Add enum values if available

src/parsers/EntityParsingUtils.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ export class EntityParsingUtils {
2626

2727
// Remove redundant type prefixes like "String.", "Boolean.", etc.
2828
// Also remove complex type prefixes like "Array of String.", "Array of Integer.", etc.
29+
// Also remove "String or Array of..." patterns
2930
// Match case-insensitive type prefix followed by period and space at start of string
3031
const typePattern =
31-
/^(String|Boolean|Integer|Number|Array(\s+of\s+(String|Boolean|Integer|Number|Object|Hash))?|Object|Hash)\.\s*/i;
32+
/^(String(\s+or\s+Array\s+of\s+(String|Strings?)(\s+\([^)]+\))?)?|Boolean|Integer|Number|Array(\s+of\s+(String|Boolean|Integer|Number|Object|Hash))?|Object|Hash)\.\s*/i;
3233
cleaned = cleaned.replace(typePattern, '');
3334

3435
return cleaned.trim();
@@ -40,9 +41,10 @@ export class EntityParsingUtils {
4041
static stripTypePrefix(description: string): string {
4142
// Remove redundant type prefixes like "String.", "Boolean.", etc.
4243
// Also remove complex type prefixes like "Array of String.", "Array of Integer.", etc.
44+
// Also remove "String or Array of..." patterns
4345
// Match case-insensitive type prefix followed by period and space at start of string
4446
const typePattern =
45-
/^(String|Boolean|Integer|Number|Array(\s+of\s+(String|Boolean|Integer|Number|Object|Hash))?|Object|Hash)\.\s*/i;
47+
/^(String(\s+or\s+Array\s+of\s+(String|Strings?)(\s+\([^)]+\))?)?|Boolean|Integer|Number|Array(\s+of\s+(String|Boolean|Integer|Number|Object|Hash))?|Object|Hash)\.\s*/i;
4648
return description.replace(typePattern, '').trim();
4749
}
4850

0 commit comments

Comments
 (0)