Skip to content

Commit f80bcce

Browse files
avivkelleravivkeller
authored
fix(legacy-json): more robust signature parsing (#347)
1 parent a4c103c commit f80bcce

File tree

2 files changed

+276
-66
lines changed

2 files changed

+276
-66
lines changed

‎src/generators/legacy-json/utils/__tests__/parseSignature.test.mjs‎

Lines changed: 271 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -5,90 +5,299 @@ import parseSignature, {
55
parseDefaultValue,
66
findParameter,
77
parseParameters,
8+
parseNameAndOptionalStatus,
89
} from '../parseSignature.mjs';
910

10-
describe('parseDefaultValue', () => {
11-
it('extracts default value', () => {
12-
const [name, defaultVal] = parseDefaultValue('param=default');
13-
assert.equal(name, 'param');
14-
assert.equal(defaultVal, '=default');
15-
});
11+
describe('parseNameAndOptionalStatus', () => {
12+
const testCases = [
13+
{
14+
name: 'simple parameter names',
15+
input: { paramName: 'param', depth: 0 },
16+
expected: { name: 'param', depth: 0, isOptional: false },
17+
},
18+
{
19+
name: 'optional parameters with brackets',
20+
input: { paramName: '[param]', depth: 0 },
21+
expected: { name: 'param', depth: 0, isOptional: true },
22+
},
23+
{
24+
name: 'partial brackets at beginning',
25+
input: { paramName: '[param', depth: 0 },
26+
expected: { name: 'param', depth: 1, isOptional: true },
27+
},
28+
{
29+
name: 'partial brackets at end',
30+
input: { paramName: 'param]', depth: 1 },
31+
expected: { name: 'param', depth: 0, isOptional: true },
32+
},
33+
{
34+
name: 'complex nested bracket',
35+
input: { paramName: 'b[', depth: 1 },
36+
expected: { name: 'b', depth: 2, isOptional: true },
37+
},
38+
];
1639

17-
it('handles no default value', () => {
18-
const [name, defaultVal] = parseDefaultValue('param');
19-
assert.equal(name, 'param');
20-
assert.equal(defaultVal, undefined);
21-
});
40+
for (const testCase of testCases) {
41+
it(testCase.name, () => {
42+
const { paramName, depth } = testCase.input;
43+
const [name, newDepth, isOptional] = parseNameAndOptionalStatus(
44+
paramName,
45+
depth
46+
);
47+
assert.equal(name, testCase.expected.name);
48+
assert.equal(newDepth, testCase.expected.depth);
49+
assert.equal(isOptional, testCase.expected.isOptional);
50+
});
51+
}
2252
});
2353

24-
describe('findParameter', () => {
25-
it('finds parameter by index', () => {
26-
const params = [{ name: 'first' }, { name: 'second' }];
27-
const result = findParameter('first', 0, params);
28-
assert.equal(result.name, 'first');
29-
});
54+
describe('parseDefaultValue', () => {
55+
const testCases = [
56+
{
57+
name: 'extracts default value',
58+
input: 'param=default',
59+
expected: { name: 'param', defaultVal: '=default' },
60+
},
61+
{
62+
name: 'handles no default value',
63+
input: 'param',
64+
expected: { name: 'param', defaultVal: undefined },
65+
},
66+
{
67+
name: 'handles complex default values',
68+
input: 'param={x: [1,2,3]}',
69+
expected: { name: 'param', defaultVal: '={x: [1,2,3]}' },
70+
},
71+
{
72+
name: 'handles multiple equal signs',
73+
input: 'param=x=y=z',
74+
expected: { name: 'param', defaultVal: '=x=y=z' },
75+
},
76+
];
3077

31-
it('searches by name when index fails', () => {
32-
const params = [{ name: 'first' }, { name: 'second' }];
33-
const result = findParameter('second', 0, params);
34-
assert.equal(result.name, 'second');
35-
});
78+
for (const testCase of testCases) {
79+
it(testCase.name, () => {
80+
const [name, defaultVal] = parseDefaultValue(testCase.input);
81+
assert.equal(name, testCase.expected.name);
82+
assert.equal(defaultVal, testCase.expected.defaultVal);
83+
});
84+
}
85+
});
3686

37-
it('finds in nested options', () => {
38-
const params = [
87+
describe('findParameter', () => {
88+
it('handles various parameter finding scenarios', () => {
89+
const testCases = [
3990
{
40-
name: 'options',
41-
options: [{ name: 'nested' }],
91+
name: 'finds by index',
92+
input: {
93+
paramName: 'first',
94+
index: 0,
95+
params: [{ name: 'first' }, { name: 'second' }],
96+
},
97+
expected: { name: 'first' },
98+
},
99+
{
100+
name: 'searches by name',
101+
input: {
102+
paramName: 'second',
103+
index: 0,
104+
params: [{ name: 'first' }, { name: 'second' }],
105+
},
106+
expected: { name: 'second' },
107+
},
108+
{
109+
name: 'finds in nested options',
110+
input: {
111+
paramName: 'nested',
112+
index: 0,
113+
params: [
114+
{
115+
name: 'options',
116+
options: [
117+
{ name: 'nested', type: 'string', description: 'test' },
118+
],
119+
},
120+
],
121+
},
122+
expected: { name: 'nested', type: 'string', description: 'test' },
123+
},
124+
{
125+
name: 'returns default when not found',
126+
input: {
127+
paramName: 'missing',
128+
index: 0,
129+
params: [],
130+
},
131+
expected: { name: 'missing' },
42132
},
43133
];
44-
const result = findParameter('nested', 0, params);
45-
assert.equal(result.name, 'nested');
46-
});
47134

48-
it('returns default when not found', () => {
49-
const result = findParameter('missing', 0, []);
50-
assert.equal(result.name, 'missing');
135+
for (const testCase of testCases) {
136+
const { paramName, index, params } = testCase.input;
137+
const result = findParameter(paramName, index, params);
138+
139+
// Check all expected properties
140+
for (const key in testCase.expected) {
141+
assert.equal(result[key], testCase.expected[key]);
142+
}
143+
}
51144
});
52145
});
53146

54147
describe('parseParameters', () => {
55-
it('parses simple parameters', () => {
56-
const declared = ['param1', 'param2'];
57-
const markdown = [{ name: 'param1' }, { name: 'param2' }];
58-
const result = parseParameters(declared, markdown);
59-
60-
assert.equal(result.length, 2);
61-
assert.equal(result[0].name, 'param1');
62-
assert.equal(result[1].name, 'param2');
63-
});
148+
const testCases = [
149+
{
150+
name: 'parses simple parameters',
151+
input: {
152+
declared: ['param1', 'param2'],
153+
markdown: [{ name: 'param1' }, { name: 'param2' }],
154+
},
155+
expected: [{ name: 'param1' }, { name: 'param2' }],
156+
},
157+
{
158+
name: 'handles default values',
159+
input: {
160+
declared: ['param=value'],
161+
markdown: [{ name: 'param' }],
162+
},
163+
expected: [{ name: 'param', default: '=value' }],
164+
},
165+
{
166+
name: 'marks optional parameters',
167+
input: {
168+
declared: ['[optional]', 'required'],
169+
markdown: [{ name: 'optional' }, { name: 'required' }],
170+
},
171+
expected: [{ name: 'optional', optional: true }, { name: 'required' }],
172+
},
173+
{
174+
name: 'handles both brackets and default values',
175+
input: {
176+
declared: ['[param=default]'],
177+
markdown: [{ name: 'param' }],
178+
},
179+
expected: [{ name: 'param', optional: true, default: '=default' }],
180+
},
181+
];
64182

65-
it('handles default values', () => {
66-
const declared = ['param=value'];
67-
const markdown = [{ name: 'param' }];
68-
const result = parseParameters(declared, markdown);
183+
for (const testCase of testCases) {
184+
it(testCase.name, () => {
185+
const result = parseParameters(
186+
testCase.input.declared,
187+
testCase.input.markdown
188+
);
189+
assert.equal(result.length, testCase.expected.length);
69190

70-
assert.equal(result[0].default, '=value');
71-
});
191+
for (let i = 0; i < result.length; i++) {
192+
for (const key in testCase.expected[i]) {
193+
assert.deepEqual(result[i][key], testCase.expected[i][key]);
194+
}
195+
}
196+
});
197+
}
72198
});
73199

74200
describe('parseSignature', () => {
75-
it('returns empty signature for no parameters', () => {
76-
const result = parseSignature('`method()`', []);
77-
assert.deepEqual(result.params, []);
78-
});
201+
const testCases = [
202+
{
203+
name: 'returns empty signature for no parameters',
204+
input: {
205+
textRaw: '`method()`',
206+
markdown: [],
207+
},
208+
expected: { params: [] },
209+
},
210+
{
211+
name: 'extracts return value',
212+
input: {
213+
textRaw: '`method()`',
214+
markdown: [{ name: 'return', type: 'string' }],
215+
},
216+
expected: {
217+
params: [],
218+
return: { name: 'return', type: 'string' },
219+
},
220+
},
221+
{
222+
name: 'parses method with parameters',
223+
input: {
224+
textRaw: '`method(param1, param2)`',
225+
markdown: [{ name: 'param1' }, { name: 'param2' }],
226+
},
227+
expected: {
228+
params: [{ name: 'param1' }, { name: 'param2' }],
229+
},
230+
},
231+
{
232+
name: 'parses complex nested optional parameters',
233+
input: {
234+
textRaw: '`new Blob([sources[, options]])`',
235+
markdown: [{ name: 'sources' }, { name: 'options' }],
236+
},
237+
expected: {
238+
params: [
239+
{ name: 'sources', optional: true },
240+
{ name: 'options', optional: true },
241+
],
242+
},
243+
},
244+
{
245+
name: 'handles multiple levels of nested optionals',
246+
input: {
247+
textRaw: '`method(a[, b[, c]])`',
248+
markdown: [{ name: 'a' }, { name: 'b' }, { name: 'c' }],
249+
},
250+
expected: {
251+
params: [
252+
{ name: 'a' },
253+
{ name: 'b', optional: true },
254+
{ name: 'c', optional: true },
255+
],
256+
},
257+
},
258+
{
259+
name: 'handles real-world complex signatures',
260+
input: {
261+
textRaw: '`new Console(stdout[, stderr][, ignoreErrors])`',
262+
markdown: [
263+
{ name: 'stdout' },
264+
{ name: 'stderr' },
265+
{ name: 'ignoreErrors' },
266+
],
267+
},
268+
expected: {
269+
params: [
270+
{ name: 'stdout' },
271+
{ name: 'stderr', optional: true },
272+
{ name: 'ignoreErrors', optional: true },
273+
],
274+
},
275+
},
276+
];
79277

80-
it('extracts return value', () => {
81-
const markdown = [{ name: 'return', type: 'string' }];
82-
const result = parseSignature('`method()`', markdown);
278+
for (const testCase of testCases) {
279+
it(testCase.name, () => {
280+
const result = parseSignature(
281+
testCase.input.textRaw,
282+
testCase.input.markdown
283+
);
83284

84-
assert.equal(result.return.name, 'return');
85-
assert.equal(result.return.type, 'string');
86-
});
285+
if (testCase.expected.return) {
286+
assert.equal(result.return.name, testCase.expected.return.name);
287+
assert.equal(result.return.type, testCase.expected.return.type);
288+
}
87289

88-
it('parses method with parameters', () => {
89-
const markdown = [{ name: 'param1' }, { name: 'param2' }];
90-
const result = parseSignature('`method(param1, param2)`', markdown);
290+
assert.equal(result.params.length, testCase.expected.params.length);
91291

92-
assert.equal(result.params.length, 2);
93-
});
292+
for (let i = 0; i < result.params.length; i++) {
293+
for (const key in testCase.expected.params[i]) {
294+
assert.deepEqual(
295+
result.params[i][key],
296+
testCase.expected.params[i][key],
297+
`Param ${i} property ${key} mismatch`
298+
);
299+
}
300+
}
301+
});
302+
}
94303
});

‎src/generators/legacy-json/utils/parseSignature.mjs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,17 @@ export function parseNameAndOptionalStatus(parameterName, optionalDepth) {
3939
char => !OPTIONAL_LEVEL_CHANGES[char]
4040
);
4141

42+
// Extract the actual parameter name
43+
const actualName = parameterName.slice(startingIdx, endingIdx + 1);
44+
const isParameterOptional = optionalDepth > 0;
45+
4246
// Update optionalDepth based on trailing brackets
47+
// These apply to the NEXT parameter
4348
optionalDepth = [...parameterName.slice(endingIdx + 1)].reduce(
4449
updateDepth,
4550
optionalDepth
4651
);
4752

48-
// Extract the actual parameter name
49-
const actualName = parameterName.slice(startingIdx, endingIdx + 1);
50-
const isParameterOptional = optionalDepth > 0;
51-
5253
return [actualName, optionalDepth, isParameterOptional];
5354
}
5455

0 commit comments

Comments
 (0)