Skip to content

Commit 9e34dfe

Browse files
authored
[LG-5930] fix(CodeEditor): prevent tooltip range errors (#3486)
* feat(tooltip-extension): enhance tooltip bounds checking and diagnostics handling * Lint fix
1 parent bb18968 commit 9e34dfe

File tree

2 files changed

+227
-32
lines changed

2 files changed

+227
-32
lines changed

packages/code-editor/src/CodeEditor/hooks/extensions/useTooltipExtension.spec.ts

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,66 @@ import {
77

88
import { useTooltipExtension } from './useTooltipExtension';
99

10+
/** Mock CodeMirror document with specified line count */
11+
const createMockDoc = (lineCount: number) => {
12+
const lines = Array.from({ length: lineCount }, (_, i) => `line ${i + 1}`);
13+
const content = lines.join('\n');
14+
15+
const lineInfos = lines.map((lineContent, index) => {
16+
const previousLength = lines
17+
.slice(0, index)
18+
.reduce((sum, l) => sum + l.length + 1, 0);
19+
20+
return {
21+
from: previousLength,
22+
to: previousLength + lineContent.length,
23+
text: lineContent,
24+
};
25+
});
26+
27+
return {
28+
lines: lineCount,
29+
length: content.length,
30+
line: (n: number) => {
31+
if (n < 1 || n > lineCount) {
32+
throw new RangeError(
33+
`Invalid line number ${n} in ${lineCount}-line document`,
34+
);
35+
}
36+
37+
return lineInfos[n - 1];
38+
},
39+
};
40+
};
41+
42+
/** Mock @codemirror/lint module that captures the linter callback */
43+
const createTestingLintModule = () => {
44+
let capturedLinterFn: ((view: any) => any) | null = null;
45+
46+
return {
47+
module: {
48+
linter: (fn: (view: any) => any) => {
49+
capturedLinterFn = fn;
50+
51+
return { LINTER_EXT: 'function' };
52+
},
53+
} as unknown as typeof import('@codemirror/lint'),
54+
invokeLinter: (lineCount: number) => {
55+
if (!capturedLinterFn) {
56+
throw new Error('Linter function was not captured');
57+
}
58+
59+
const mockView = {
60+
state: {
61+
doc: createMockDoc(lineCount),
62+
},
63+
};
64+
65+
return capturedLinterFn(mockView);
66+
},
67+
};
68+
};
69+
1070
describe('useTooltipExtension', () => {
1171
const fakeStateModule = createMockStateModule();
1272
const fakeLintModule = createMockLintModule();
@@ -46,4 +106,125 @@ describe('useTooltipExtension', () => {
46106
);
47107
expect(result.current).toHaveProperty('LINTER_EXT', 'function');
48108
});
109+
110+
describe('bounds checking', () => {
111+
test('filters out tooltips referencing lines that do not exist in the document', () => {
112+
const testingLintModule = createTestingLintModule();
113+
114+
renderHook(() =>
115+
useTooltipExtension({
116+
editorViewInstance: null,
117+
props: {
118+
tooltips: [
119+
{
120+
line: 1,
121+
column: 1,
122+
length: 4,
123+
messages: ['valid tooltip'],
124+
severity: 'info',
125+
links: [],
126+
},
127+
{
128+
line: 5, // Out of bounds - document only has 2 lines
129+
column: 1,
130+
length: 4,
131+
messages: ['invalid tooltip'],
132+
severity: 'error',
133+
links: [],
134+
},
135+
{
136+
line: 0, // Invalid - lines are 1-based
137+
column: 1,
138+
length: 4,
139+
messages: ['invalid tooltip'],
140+
severity: 'info',
141+
links: [],
142+
},
143+
],
144+
},
145+
modules: {
146+
'@codemirror/state': fakeStateModule,
147+
'@codemirror/lint': testingLintModule.module,
148+
},
149+
}),
150+
);
151+
152+
const diagnostics = testingLintModule.invokeLinter(2);
153+
154+
expect(diagnostics).toHaveLength(1);
155+
expect(diagnostics[0].from).toBe(0);
156+
});
157+
158+
test('clamps column and length to stay within line bounds', () => {
159+
const testingLintModule = createTestingLintModule();
160+
161+
renderHook(() =>
162+
useTooltipExtension({
163+
editorViewInstance: null,
164+
props: {
165+
tooltips: [
166+
{
167+
line: 1,
168+
column: 100, // Exceeds "line 1" length (6 characters)
169+
length: 4,
170+
messages: ['tooltip with large column'],
171+
severity: 'info',
172+
links: [],
173+
},
174+
{
175+
line: 2,
176+
column: 1,
177+
length: 100, // Exceeds line length
178+
messages: ['tooltip with large length'],
179+
severity: 'info',
180+
links: [],
181+
},
182+
],
183+
},
184+
modules: {
185+
'@codemirror/state': fakeStateModule,
186+
'@codemirror/lint': testingLintModule.module,
187+
},
188+
}),
189+
);
190+
191+
const diagnostics = testingLintModule.invokeLinter(3);
192+
193+
expect(diagnostics).toHaveLength(2);
194+
expect(diagnostics[0].from).toBe(6);
195+
expect(diagnostics[1].from).toBe(7);
196+
expect(diagnostics[1].to).toBe(13);
197+
});
198+
199+
test('does not throw when tooltips reference lines that no longer exist after document changes', () => {
200+
const testingLintModule = createTestingLintModule();
201+
202+
renderHook(() =>
203+
useTooltipExtension({
204+
editorViewInstance: null,
205+
props: {
206+
tooltips: [
207+
{
208+
line: 5,
209+
column: 1,
210+
length: 4,
211+
messages: ['tooltip on line that no longer exists'],
212+
severity: 'error',
213+
links: [],
214+
},
215+
],
216+
},
217+
modules: {
218+
'@codemirror/state': fakeStateModule,
219+
'@codemirror/lint': testingLintModule.module,
220+
},
221+
}),
222+
);
223+
224+
expect(() => testingLintModule.invokeLinter(1)).not.toThrow();
225+
226+
const diagnostics = testingLintModule.invokeLinter(1);
227+
expect(diagnostics).toHaveLength(0);
228+
});
229+
});
49230
});

packages/code-editor/src/CodeEditor/hooks/extensions/useTooltipExtension.ts

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -49,41 +49,55 @@ export function useTooltipExtension({
4949

5050
return module.linter(
5151
linterView => {
52-
const diagnostics: Array<Diagnostic> = tooltips.map(
53-
({
54-
line,
55-
column = 1,
56-
severity = 'info',
57-
length,
58-
messages,
59-
links,
60-
}: CodeEditorTooltipType) => {
61-
const lineInfo = linterView.state.doc.line(line);
62-
const from = lineInfo.from + column - 1;
63-
const to = from + length;
52+
const doc = linterView.state.doc;
53+
const totalLines = doc.lines;
6454

65-
const renderMessage = () => {
66-
const dom = document.createElement('div');
67-
dom.innerHTML = renderToString(
68-
React.createElement(CodeEditorTooltip, {
69-
messages,
70-
links,
71-
darkMode: props.darkMode,
72-
baseFontSize: props.baseFontSize,
73-
}),
55+
const diagnostics: Array<Diagnostic> = tooltips
56+
.filter(({ line }: CodeEditorTooltipType) => {
57+
return line >= 1 && line <= totalLines;
58+
})
59+
.map(
60+
({
61+
line,
62+
column = 1,
63+
severity = 'info',
64+
length,
65+
messages,
66+
links,
67+
}: CodeEditorTooltipType) => {
68+
const lineInfo = doc.line(line);
69+
const lineLength = lineInfo.to - lineInfo.from;
70+
const safeColumn = Math.max(
71+
1,
72+
Math.min(column, lineLength + 1),
7473
);
75-
return dom;
76-
};
74+
const from = lineInfo.from + safeColumn - 1;
75+
const maxLength = Math.max(0, lineInfo.to - from);
76+
const safeLength = Math.min(length, maxLength);
77+
const to = from + safeLength;
7778

78-
return {
79-
from,
80-
to,
81-
severity,
82-
message: ' ', // Provide a non-empty string to satisfy Diagnostic type
83-
renderMessage,
84-
};
85-
},
86-
);
79+
const renderMessage = () => {
80+
const dom = document.createElement('div');
81+
dom.innerHTML = renderToString(
82+
React.createElement(CodeEditorTooltip, {
83+
messages,
84+
links,
85+
darkMode: props.darkMode,
86+
baseFontSize: props.baseFontSize,
87+
}),
88+
);
89+
return dom;
90+
};
91+
92+
return {
93+
from,
94+
to,
95+
severity,
96+
message: ' ', // Provide a non-empty string to satisfy Diagnostic type
97+
renderMessage,
98+
};
99+
},
100+
);
87101
return diagnostics;
88102
},
89103
{

0 commit comments

Comments
 (0)