Skip to content

Commit 6728c86

Browse files
P0lipP0lip
authored
fix: report duplicate keys properly (#37)
* fix: report duplicate keys properly * style: make label disappear
1 parent 0aa944b commit 6728c86

File tree

2 files changed

+193
-75
lines changed

2 files changed

+193
-75
lines changed

‎src/__tests__/parseWithPointers.ts

Lines changed: 174 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -129,77 +129,188 @@ describe('json parser', () => {
129129
});
130130
});
131131

132-
test('reports duplicated properties', () => {
133-
expect(
134-
parseWithPointers('{ "foo": true, "foo": false,\n "foo": 2, "bar": true }', { ignoreDuplicateKeys: false }),
135-
).toHaveProperty('diagnostics', [
136-
{
137-
code: 20,
138-
message: 'DuplicateKey',
139-
path: ['foo'],
140-
range: {
141-
start: {
142-
line: 0,
143-
character: 15,
132+
describe('duplicate keys', () => {
133+
test('given object with no duplicate keys, does not report any errors', () => {
134+
expect(
135+
parseWithPointers(
136+
`{
137+
"type": "object",
138+
"properties": {
139+
"bike": {
140+
"type": "object",
141+
"properties": {
142+
"id": {
143+
"type": "string"
144+
},
145+
"type": {
146+
"type": "string"
147+
}
148+
},
149+
"required": ["id"]
150+
}
151+
}
152+
}`,
153+
{ ignoreDuplicateKeys: false },
154+
).diagnostics,
155+
).toEqual([]);
156+
157+
expect(
158+
parseWithPointers(
159+
`{
160+
"type": "object",
161+
"properties": [{
162+
"maxLength": {
163+
"type": "string",
164+
"maxLength": 1
165+
},
166+
"type": {
167+
"type": "string"
168+
}
169+
}, {
170+
"maxLength": {
171+
"type": "string",
172+
"maxLength": 1
173+
},
174+
"type": {
175+
"type": "string"
176+
}
177+
}]
178+
}`,
179+
{ ignoreDuplicateKeys: false },
180+
).diagnostics,
181+
).toEqual([]);
182+
});
183+
184+
test('given object containing with duplicate keys, reports them', () => {
185+
expect(
186+
parseWithPointers(
187+
`{
188+
"type": "object",
189+
"properties": {
190+
"bike": {
191+
"type": "object",
192+
"properties": {
193+
"id": {
194+
"type": "string"
195+
},
196+
"type": {
197+
"type": "string",
198+
"type": "object"
199+
},
200+
"id": ""
201+
},
202+
"required": ["id"]
203+
}
204+
}
205+
}`,
206+
{ ignoreDuplicateKeys: false },
207+
).diagnostics,
208+
).toEqual([
209+
{
210+
code: 20,
211+
message: 'DuplicateKey',
212+
path: ['properties', 'bike', 'properties', 'type', 'type'],
213+
range: {
214+
end: {
215+
character: 16,
216+
line: 11,
217+
},
218+
start: {
219+
character: 10,
220+
line: 11,
221+
},
144222
},
145-
end: {
146-
line: 0,
147-
character: 20,
223+
severity: DiagnosticSeverity.Error,
224+
},
225+
{
226+
code: 20,
227+
message: 'DuplicateKey',
228+
path: ['properties', 'bike', 'properties', 'id'],
229+
range: {
230+
end: {
231+
character: 12,
232+
line: 13,
233+
},
234+
start: {
235+
character: 8,
236+
line: 13,
237+
},
148238
},
239+
severity: DiagnosticSeverity.Error,
149240
},
150-
severity: DiagnosticSeverity.Error,
151-
},
152-
{
153-
code: 20,
154-
message: 'DuplicateKey',
155-
path: ['foo'],
156-
range: {
157-
start: {
158-
line: 1,
159-
character: 1,
241+
]);
242+
243+
expect(
244+
parseWithPointers('{ "foo": true, "foo": false,\n "foo": 2, "bar": true }', { ignoreDuplicateKeys: false }),
245+
).toHaveProperty('diagnostics', [
246+
{
247+
code: 20,
248+
message: 'DuplicateKey',
249+
path: ['foo'],
250+
range: {
251+
start: {
252+
line: 0,
253+
character: 15,
254+
},
255+
end: {
256+
line: 0,
257+
character: 20,
258+
},
160259
},
161-
end: {
162-
line: 1,
163-
character: 6,
260+
severity: DiagnosticSeverity.Error,
261+
},
262+
{
263+
code: 20,
264+
message: 'DuplicateKey',
265+
path: ['foo'],
266+
range: {
267+
start: {
268+
line: 1,
269+
character: 1,
270+
},
271+
end: {
272+
line: 1,
273+
character: 6,
274+
},
164275
},
276+
severity: DiagnosticSeverity.Error,
165277
},
166-
severity: DiagnosticSeverity.Error,
167-
},
168-
]);
169-
});
278+
]);
279+
});
170280

171-
test('generates correct path for dupes in array', () => {
172-
expect(
173-
parseWithPointers('{ "A": [{}, { "foo": true, "foo": false,\n "foo": 2 }] }', { ignoreDuplicateKeys: false }),
174-
).toHaveProperty('diagnostics', [
175-
expect.objectContaining({
176-
code: 20,
177-
message: 'DuplicateKey',
178-
path: ['A', 1, 'foo'],
179-
}),
180-
expect.objectContaining({
181-
code: 20,
182-
message: 'DuplicateKey',
183-
path: ['A', 1, 'foo'],
184-
}),
185-
]);
281+
test('generates correct paths for dupes in arrays', () => {
282+
expect(
283+
parseWithPointers('{ "A": [{}, { "foo": true, "foo": false,\n "foo": 2 }] }', { ignoreDuplicateKeys: false }),
284+
).toHaveProperty('diagnostics', [
285+
expect.objectContaining({
286+
code: 20,
287+
message: 'DuplicateKey',
288+
path: ['A', 1, 'foo'],
289+
}),
290+
expect.objectContaining({
291+
code: 20,
292+
message: 'DuplicateKey',
293+
path: ['A', 1, 'foo'],
294+
}),
295+
]);
186296

187-
expect(
188-
parseWithPointers('[{ "A": [{}, {}, { "foo": true, "foo": false,\n "foo": 2 }] }]', {
189-
ignoreDuplicateKeys: false,
190-
}),
191-
).toHaveProperty('diagnostics', [
192-
expect.objectContaining({
193-
code: 20,
194-
message: 'DuplicateKey',
195-
path: [0, 'A', 2, 'foo'],
196-
}),
197-
expect.objectContaining({
198-
code: 20,
199-
message: 'DuplicateKey',
200-
path: [0, 'A', 2, 'foo'],
201-
}),
202-
]);
297+
expect(
298+
parseWithPointers('[{ "A": [{}, {}, { "foo": true, "foo": false,\n "foo": 2 }] }]', {
299+
ignoreDuplicateKeys: false,
300+
}),
301+
).toHaveProperty('diagnostics', [
302+
expect.objectContaining({
303+
code: 20,
304+
message: 'DuplicateKey',
305+
path: [0, 'A', 2, 'foo'],
306+
}),
307+
expect.objectContaining({
308+
code: 20,
309+
message: 'DuplicateKey',
310+
path: [0, 'A', 2, 'foo'],
311+
}),
312+
]);
313+
});
203314
});
204315

205316
test('includes properties with empty string as keys', () => {

‎src/parseWithPointers.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export function parseTree<T>(
3131
let currentParent: IJsonASTNode = { type: 'array', offset: -1, length: -1, children: [], parent: void 0 }; // artificial root
3232
let currentParsedProperty: string | null = null;
3333
let currentParsedParent: any = [];
34-
const currentObjectKeys: string[] = [''];
34+
const objectKeys = new WeakMap<object, string[]>();
3535
const previousParsedParents: any[] = [];
3636

3737
function ensurePropertyComplete(endOffset: number) {
@@ -90,7 +90,7 @@ export function parseTree<T>(
9090
});
9191

9292
if (options.ignoreDuplicateKeys === false) {
93-
currentObjectKeys.length = 0;
93+
objectKeys.set(currentParent, []);
9494
}
9595

9696
onParsedComplexBegin({});
@@ -100,22 +100,29 @@ export function parseTree<T>(
100100
currentParent.children!.push({ type: 'string', value: name, offset, length, parent: currentParent });
101101

102102
if (options.ignoreDuplicateKeys === false) {
103-
if (currentObjectKeys.length === 0 || !currentObjectKeys.includes(name)) {
104-
currentObjectKeys.push(name);
105-
} else {
106-
errors.push({
107-
range: calculateRange(startLine, startCharacter, length),
108-
message: 'DuplicateKey',
109-
severity: DiagnosticSeverity.Error,
110-
path: getJsonPath(currentParent),
111-
code: 20, // 17 is the lowest safe value, but decided to bump it
112-
});
103+
const currentObjectKeys = objectKeys.get(currentParent.parent!);
104+
if (currentObjectKeys) {
105+
if (currentObjectKeys.length === 0 || !currentObjectKeys.includes(name)) {
106+
currentObjectKeys.push(name);
107+
} else {
108+
errors.push({
109+
range: calculateRange(startLine, startCharacter, length),
110+
message: 'DuplicateKey',
111+
severity: DiagnosticSeverity.Error,
112+
path: getJsonPath(currentParent),
113+
code: 20, // 17 is the lowest safe value, but decided to bump it
114+
});
115+
}
113116
}
114117
}
115118

116119
currentParsedProperty = name;
117120
},
118121
onObjectEnd: (offset: number, length, startLine, startCharacter) => {
122+
if (options.ignoreDuplicateKeys === false) {
123+
objectKeys.delete(currentParent);
124+
}
125+
119126
currentParent.length = offset + length - currentParent.offset;
120127
if (currentParent.range) {
121128
// @ts-ignore, read only ;P

0 commit comments

Comments
 (0)