Skip to content

Commit 9cdace4

Browse files
authored
[terminal] Add support for onProblem notification callback to IOperationExecutionResult.problemCollector (#5382)
1 parent 6f52a12 commit 9cdace4

6 files changed

Lines changed: 144 additions & 62 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/problem-matcher",
5+
"comment": "Fix multi-line looping problem matcher message parsing",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@rushstack/problem-matcher"
10+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/terminal",
5+
"comment": "Add ProblemCollector.onProblem notification callback",
6+
"type": "minor"
7+
}
8+
],
9+
"packageName": "@rushstack/terminal"
10+
}

common/reviews/api/terminal.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ export interface IProblemCollector {
154154
export interface IProblemCollectorOptions extends ITerminalWritableOptions {
155155
matcherJson?: IProblemMatcherJson[];
156156
matchers?: IProblemMatcher[];
157+
onProblem?: (problem: IProblem) => void;
157158
}
158159

159160
// @public

libraries/problem-matcher/src/ProblemMatcher.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,10 @@ function finalizeProblem(
258258
captures: ICapturesMutable,
259259
defaultSeverity: ProblemSeverity | undefined
260260
): IProblem {
261+
// For multi-line patterns, use only the last non-empty message part
262+
const message: string =
263+
captures.messageParts.length > 0 ? captures.messageParts[captures.messageParts.length - 1] : '';
264+
261265
return {
262266
matcherName,
263267
file: captures.file,
@@ -267,7 +271,7 @@ function finalizeProblem(
267271
endColumn: captures.endColumn,
268272
severity: captures.severity || defaultSeverity,
269273
code: captures.code,
270-
message: captures.messageParts.join('\n')
274+
message: message
271275
};
272276
}
273277

libraries/terminal/src/ProblemCollector.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ export interface IProblemCollectorOptions extends ITerminalWritableOptions {
2222
* {@link @rushstack/problem-matcher#IProblemMatcher | IProblemMatcher} definitions.
2323
*/
2424
matcherJson?: IProblemMatcherJson[];
25+
/**
26+
* Optional callback invoked immediately whenever a problem is produced.
27+
*/
28+
onProblem?: (problem: IProblem) => void;
2529
}
2630

2731
/**
@@ -38,6 +42,7 @@ export interface IProblemCollectorOptions extends ITerminalWritableOptions {
3842
export class ProblemCollector extends TerminalWritable implements IProblemCollector {
3943
private readonly _matchers: IProblemMatcher[];
4044
private readonly _problems: Set<IProblem> = new Set();
45+
private readonly _onProblem: ((problem: IProblem) => void) | undefined;
4146

4247
public constructor(options: IProblemCollectorOptions) {
4348
super(options);
@@ -57,6 +62,7 @@ export class ProblemCollector extends TerminalWritable implements IProblemCollec
5762
if (this._matchers.length === 0) {
5863
throw new Error('ProblemCollector requires at least one problem matcher.');
5964
}
65+
this._onProblem = options.onProblem;
6066
}
6167

6268
/**
@@ -87,6 +93,7 @@ export class ProblemCollector extends TerminalWritable implements IProblemCollec
8793
matcherName: matcher.name
8894
};
8995
this._problems.add(finalized);
96+
this._onProblem?.(finalized);
9097
}
9198
}
9299
}
@@ -105,6 +112,7 @@ export class ProblemCollector extends TerminalWritable implements IProblemCollec
105112
matcherName: matcher.name
106113
};
107114
this._problems.add(finalized);
115+
this._onProblem?.(finalized);
108116
}
109117
}
110118
}

libraries/terminal/src/test/ProblemCollector.test.ts

Lines changed: 110 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@ class ErrorLineMatcher implements IProblemMatcher {
2828

2929
describe('ProblemCollector', () => {
3030
it('collects a simple error line', () => {
31+
const onProblemSpy = jest.fn<void, [IProblem]>();
3132
const collector: ProblemCollector = new ProblemCollector({
32-
matchers: [new ErrorLineMatcher()]
33+
matchers: [new ErrorLineMatcher()],
34+
onProblem: onProblemSpy
3335
});
3436

3537
collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'hello world\n' });
@@ -45,13 +47,17 @@ describe('ProblemCollector', () => {
4547

4648
const { problems } = collector;
4749
expect(problems.size).toBe(2);
48-
const [firstProblem, secondProblem] = Array.from(problems);
49-
expect(firstProblem.message).toBe('something bad happened in stdout');
50-
expect(firstProblem.severity).toBe('error');
51-
expect(firstProblem.matcherName).toBe('errorLine');
52-
expect(secondProblem.message).toBe('something bad happened in stderr');
53-
expect(secondProblem.severity).toBe('error');
54-
expect(secondProblem.matcherName).toBe('errorLine');
50+
expect(onProblemSpy).toHaveBeenCalledTimes(2);
51+
expect(onProblemSpy).toHaveBeenNthCalledWith(1, {
52+
matcherName: 'errorLine',
53+
message: 'something bad happened in stdout',
54+
severity: 'error'
55+
});
56+
expect(onProblemSpy).toHaveBeenNthCalledWith(2, {
57+
matcherName: 'errorLine',
58+
message: 'something bad happened in stderr',
59+
severity: 'error'
60+
});
5561
});
5662
});
5763

@@ -70,16 +76,24 @@ describe('VSCodeProblemMatcherAdapter - additional location formats', () => {
7076
} satisfies IProblemMatcherJson;
7177

7278
const matchers = parseProblemMatchersJson([matcherPattern]);
73-
const collector = new ProblemCollector({ matchers });
79+
const onProblemSpy = jest.fn<void, [IProblem]>();
80+
const collector = new ProblemCollector({ matchers, onProblem: onProblemSpy });
7481
collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'src/a.c(10,5): something happened\n' });
7582
collector.close();
7683
const { problems } = collector;
7784
expect(problems.size).toBe(1);
78-
const [firstProblem] = Array.from(problems);
79-
expect(firstProblem.file).toBe('src/a.c');
80-
expect(firstProblem.line).toBe(10);
81-
expect(firstProblem.column).toBe(5);
82-
expect(firstProblem.message).toContain('something happened');
85+
expect(onProblemSpy).toHaveBeenCalledTimes(1);
86+
expect(onProblemSpy).toHaveBeenNthCalledWith(1, {
87+
matcherName: 'loc-group',
88+
file: 'src/a.c',
89+
line: 10,
90+
column: 5,
91+
message: 'something happened',
92+
code: undefined,
93+
endColumn: undefined,
94+
endLine: undefined,
95+
severity: undefined
96+
} satisfies IProblem);
8397
});
8498

8599
it('parses explicit endLine and endColumn groups', () => {
@@ -99,18 +113,24 @@ describe('VSCodeProblemMatcherAdapter - additional location formats', () => {
99113
} satisfies IProblemMatcherJson;
100114

101115
const matchers = parseProblemMatchersJson([matcherPattern]);
102-
const collector = new ProblemCollector({ matchers });
116+
const onProblemSpy = jest.fn<void, [IProblem]>();
117+
const collector = new ProblemCollector({ matchers, onProblem: onProblemSpy });
103118
collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'lib/x.c(10,5,12,20): multi-line issue\n' });
104119
collector.close();
105120
const { problems } = collector;
106121
expect(problems.size).toBe(1);
107-
const [firstProblem] = Array.from(problems);
108-
expect(firstProblem.file).toBe('lib/x.c');
109-
expect(firstProblem.line).toBe(10);
110-
expect(firstProblem.column).toBe(5);
111-
expect(firstProblem.endLine).toBe(12);
112-
expect(firstProblem.endColumn).toBe(20);
113-
expect(firstProblem.message).toContain('multi-line issue');
122+
expect(onProblemSpy).toHaveBeenCalledTimes(1);
123+
expect(onProblemSpy).toHaveBeenNthCalledWith(1, {
124+
matcherName: 'end-range',
125+
file: 'lib/x.c',
126+
line: 10,
127+
column: 5,
128+
endLine: 12,
129+
endColumn: 20,
130+
message: 'multi-line issue',
131+
code: undefined,
132+
severity: undefined
133+
} satisfies IProblem);
114134
});
115135
});
116136

@@ -131,20 +151,27 @@ describe('VSCodeProblemMatcherAdapter', () => {
131151
} satisfies IProblemMatcherJson;
132152

133153
const matchers = parseProblemMatchersJson([matcherPattern]);
134-
const collector = new ProblemCollector({ matchers });
154+
const onProblemSpy = jest.fn<void, [IProblem]>();
155+
const collector = new ProblemCollector({ matchers, onProblem: onProblemSpy });
135156
collector.writeChunk({
136157
kind: TerminalChunkKind.Stderr,
137158
text: "src/file.ts(10,5): error TS1005: ' ; ' expected\n"
138159
});
139160
collector.close();
140161
const { problems } = collector;
141162
expect(problems.size).toBe(1);
142-
const [firstProblem] = Array.from(problems);
143-
expect(firstProblem.file).toBe('src/file.ts');
144-
expect(firstProblem.line).toBe(10);
145-
expect(firstProblem.column).toBe(5);
146-
expect(firstProblem.code).toBe('TS1005');
147-
expect(firstProblem.severity).toBe('error');
163+
expect(onProblemSpy).toHaveBeenCalledTimes(1);
164+
expect(onProblemSpy).toHaveBeenNthCalledWith(1, {
165+
matcherName: 'tsc-like',
166+
file: 'src/file.ts',
167+
line: 10,
168+
column: 5,
169+
code: 'TS1005',
170+
severity: 'error',
171+
message: "' ; ' expected",
172+
endColumn: undefined,
173+
endLine: undefined
174+
} satisfies IProblem);
148175
});
149176

150177
it('converts and matches a multi-line pattern', () => {
@@ -173,19 +200,26 @@ describe('VSCodeProblemMatcherAdapter', () => {
173200
]
174201
} satisfies IProblemMatcherJson;
175202
const matchers = parseProblemMatchersJson([matcherPattern]);
176-
const collector = new ProblemCollector({ matchers });
203+
const onProblemSpy = jest.fn<void, [IProblem]>();
204+
const collector = new ProblemCollector({ matchers, onProblem: onProblemSpy });
177205
collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'In file src/foo.c\n' });
178206
collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'Line 42, Col 7\n' });
179207
collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'error: something bad happened\n' });
180208
collector.close();
181209
const { problems } = collector;
182210
expect(problems.size).toBe(1);
183-
const [firstProblem] = Array.from(problems);
184-
expect(firstProblem.file).toBe('src/foo.c');
185-
expect(firstProblem.line).toBe(42);
186-
expect(firstProblem.column).toBe(7);
187-
expect(firstProblem.severity).toBe('error');
188-
expect(firstProblem.message).toContain('something bad');
211+
expect(onProblemSpy).toHaveBeenCalledTimes(1);
212+
expect(onProblemSpy).toHaveBeenNthCalledWith(1, {
213+
matcherName: 'multi',
214+
file: 'src/foo.c',
215+
line: 42,
216+
column: 7,
217+
severity: 'error',
218+
message: 'something bad happened',
219+
code: undefined,
220+
endColumn: undefined,
221+
endLine: undefined
222+
} satisfies IProblem);
189223
});
190224

191225
it('handles a multi-line pattern whose last pattern loops producing multiple problems', () => {
@@ -217,36 +251,39 @@ describe('VSCodeProblemMatcherAdapter', () => {
217251

218252
const errorLines: string[] = [
219253
'Encountered 6 errors',
220-
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:9:3 - (TS2578) Unused @ts-expect-error directive.',
221-
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:11:3 - (TS2578) Unused @ts-expect-error directive.',
222-
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:19:3 - (TS2578) Unused @ts-expect-error directive.',
223-
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:24:3 - (TS2578) Unused @ts-expect-error directive.',
224-
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:26:3 - (TS2578) Unused @ts-expect-error directive.',
225-
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:34:3 - (TS2578) Unused @ts-expect-error directive.'
254+
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:9:3 - (TS2578) Unused @ts-expect-error directive 1.',
255+
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:11:3 - (TS2578) Unused @ts-expect-error directive 2.',
256+
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:19:3 - (TS2578) Unused @ts-expect-error directive 3.',
257+
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:24:3 - (TS2578) Unused @ts-expect-error directive 4.',
258+
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:26:3 - (TS2578) Unused @ts-expect-error directive 5.',
259+
' [build:typescript] vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts:34:3 - (TS2578) Unused @ts-expect-error directive 6.'
226260
];
227261

228262
const matchers = parseProblemMatchersJson([matcherPattern]);
229-
const collector = new ProblemCollector({ matchers });
263+
const onProblemSpy = jest.fn<void, [IProblem]>();
264+
const collector = new ProblemCollector({ matchers, onProblem: onProblemSpy });
230265
for (const line of errorLines) {
231266
collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: line + '\n' });
232267
}
233268
collector.close();
234269

235270
const { problems } = collector;
236271
expect(problems.size).toBe(6);
272+
expect(onProblemSpy).toHaveBeenCalledTimes(6);
237273

238274
const problemLineNumbers: number[] = [9, 11, 19, 24, 26, 34];
239-
const problemsArray = Array.from(problems);
240275
for (let i = 0; i < 6; i++) {
241-
const p = problemsArray[i];
242-
expect(p.file).toContain(
243-
'vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts'
244-
);
245-
expect(p.line).toBe(problemLineNumbers[i]);
246-
expect(p.column).toBe(3); // All sample lines have column 3
247-
expect(p.code).toBe('TS2578');
248-
expect(p.severity).toBe('error');
249-
expect(p.message).toContain('Unused @ts-expect-error directive.');
276+
expect(onProblemSpy).toHaveBeenNthCalledWith(i + 1, {
277+
matcherName: 'ts-loop-errors',
278+
file: 'vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts',
279+
line: problemLineNumbers[i],
280+
column: 3,
281+
code: 'TS2578',
282+
severity: 'error',
283+
message: `Unused @ts-expect-error directive ${i + 1}.`,
284+
endColumn: undefined,
285+
endLine: undefined
286+
} satisfies IProblem);
250287
}
251288
});
252289

@@ -280,19 +317,31 @@ describe('VSCodeProblemMatcherAdapter', () => {
280317
];
281318

282319
const matchers = parseProblemMatchersJson([matcherPattern]);
283-
const collector = new ProblemCollector({ matchers });
320+
const onProblemSpy = jest.fn<void, [IProblem]>();
321+
const collector = new ProblemCollector({ matchers, onProblem: onProblemSpy });
284322
collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'Start Problems\n' });
285323
for (const l of lines) collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: l + '\n' });
286324
collector.close();
287325
const { problems } = collector;
288326
expect(problems.size).toBe(4);
327+
expect(onProblemSpy).toHaveBeenCalledTimes(4);
289328

290-
const problemsArray = Array.from(problems);
291-
expect(problemsArray.map((p) => p.severity)).toEqual(['error', 'warning', 'error', 'info']);
292-
expect(problemsArray.map((p) => p.code)).toEqual(['CODE100', 'CODE200', 'CODE300', 'CODE400']);
293-
expect(problemsArray[0].file).toBe('lib/a.ts');
294-
expect(problemsArray[1].file).toBe('lib/b.ts');
295-
expect(problemsArray[2].file).toBe('lib/c.ts');
296-
expect(problemsArray[3].file).toBe('lib/d.ts');
329+
const problemCodes: string[] = ['CODE100', 'CODE200', 'CODE300', 'CODE400'];
330+
const problemColumns: number[] = [5, 1, 9, 2];
331+
const problemSeverities: ('error' | 'warning' | 'info')[] = ['error', 'warning', 'error', 'info'];
332+
const problemMessages: string[] = ['First thing', 'Second thing', 'Third thing', 'Fourth thing'];
333+
for (let i = 0; i < 4; i++) {
334+
expect(onProblemSpy).toHaveBeenNthCalledWith(i + 1, {
335+
matcherName: 'loop-with-severity',
336+
file: `lib/${String.fromCharCode('a'.charCodeAt(0) + i)}.ts`,
337+
line: (i + 1) * 10,
338+
column: problemColumns[i],
339+
code: problemCodes[i],
340+
severity: problemSeverities[i],
341+
message: problemMessages[i],
342+
endColumn: undefined,
343+
endLine: undefined
344+
} satisfies IProblem);
345+
}
297346
});
298347
});

0 commit comments

Comments
 (0)