From 2d652d41a7cde3d8f13d84760ffe28f4fc6bc4fc Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Tue, 30 Sep 2025 21:34:30 +0200 Subject: [PATCH] Tweaks to the problem collection API. --- ...oblems-result-to-set_2025-09-30-19-33.json | 10 ++ common/reviews/api/terminal.api.md | 10 +- libraries/terminal/src/IProblemCollector.ts | 7 +- libraries/terminal/src/ProblemCollector.ts | 25 ++-- .../src/test/ProblemCollector.test.ts | 111 ++++++++++-------- 5 files changed, 87 insertions(+), 76 deletions(-) create mode 100644 common/changes/@rushstack/terminal/change-problems-result-to-set_2025-09-30-19-33.json diff --git a/common/changes/@rushstack/terminal/change-problems-result-to-set_2025-09-30-19-33.json b/common/changes/@rushstack/terminal/change-problems-result-to-set_2025-09-30-19-33.json new file mode 100644 index 00000000000..72b4a6eb492 --- /dev/null +++ b/common/changes/@rushstack/terminal/change-problems-result-to-set_2025-09-30-19-33.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/terminal", + "comment": "", + "type": "none" + } + ], + "packageName": "@rushstack/terminal" +} \ No newline at end of file diff --git a/common/reviews/api/terminal.api.md b/common/reviews/api/terminal.api.md index 6494e673eba..32147a273f5 100644 --- a/common/reviews/api/terminal.api.md +++ b/common/reviews/api/terminal.api.md @@ -145,12 +145,12 @@ export interface IPrefixProxyTerminalProviderOptionsBase { terminalProvider: ITerminalProvider; } -// @public +// @beta export interface IProblemCollector { - getProblems(): ReadonlyArray; + get problems(): ReadonlySet; } -// @public +// @beta export interface IProblemCollectorOptions extends ITerminalWritableOptions { matcherJson?: IProblemMatcherJson[]; matchers?: IProblemMatcher[]; @@ -301,12 +301,12 @@ export class PrintUtilities { static wrapWordsToLines(text: string, maxLineLength?: number, indentOrLinePrefix?: number | string): string[]; } -// @public +// @beta export class ProblemCollector extends TerminalWritable implements IProblemCollector { constructor(options: IProblemCollectorOptions); - getProblems(): ReadonlyArray; protected onClose(): void; protected onWriteChunk(chunk: ITerminalChunk): void; + get problems(): ReadonlySet; } // @public diff --git a/libraries/terminal/src/IProblemCollector.ts b/libraries/terminal/src/IProblemCollector.ts index a8d7b5bbf6b..5f0c0813aad 100644 --- a/libraries/terminal/src/IProblemCollector.ts +++ b/libraries/terminal/src/IProblemCollector.ts @@ -6,12 +6,11 @@ import type { IProblem } from '@rushstack/problem-matcher'; /** * Collects problems (errors/warnings/info) encountered during an operation. * - * @public + * @beta */ export interface IProblemCollector { /** - * Returns the collected problems. - * @throws Error if the collector is not yet closed. + * Returns the collected problems so far. */ - getProblems(): ReadonlyArray; + get problems(): ReadonlySet; } diff --git a/libraries/terminal/src/ProblemCollector.ts b/libraries/terminal/src/ProblemCollector.ts index 0efd78ec30b..79efb2ed3a3 100644 --- a/libraries/terminal/src/ProblemCollector.ts +++ b/libraries/terminal/src/ProblemCollector.ts @@ -10,7 +10,7 @@ import type { IProblemCollector } from './IProblemCollector'; /** * Constructor options for {@link ProblemCollector}. - * @public + * @beta */ export interface IProblemCollectorOptions extends ITerminalWritableOptions { /** @@ -33,15 +33,11 @@ export interface IProblemCollectorOptions extends ITerminalWritableOptions { * by a `"\n"` character (for example when preceded by {@link StderrLineTransform} / `StdioLineTransform`). * If a chunk does not end with a newline an error is thrown to surface incorrect pipeline wiring early. * - * Call `close()` before retrieving results via getProblems. Similar to other collectors, attempting - * to read results before closure throws. - * @see getProblems - * - * @public + * @beta */ export class ProblemCollector extends TerminalWritable implements IProblemCollector { private readonly _matchers: IProblemMatcher[]; - private readonly _problems: IProblem[] = []; + private readonly _problems: Set = new Set(); public constructor(options: IProblemCollectorOptions) { super(options); @@ -66,10 +62,7 @@ export class ProblemCollector extends TerminalWritable implements IProblemCollec /** * {@inheritdoc IProblemCollector} */ - public getProblems(): ReadonlyArray { - if (this.isOpen) { - throw new Error('Problems cannot be retrieved until after close() is called.'); - } + public get problems(): ReadonlySet { return this._problems; } @@ -89,10 +82,11 @@ export class ProblemCollector extends TerminalWritable implements IProblemCollec for (const matcher of this._matchers) { const problem: IProblem | false = matcher.exec(text); if (problem) { - this._problems.push({ + const finalized: IProblem = { ...problem, matcherName: matcher.name - }); + }; + this._problems.add(finalized); } } } @@ -106,10 +100,11 @@ export class ProblemCollector extends TerminalWritable implements IProblemCollec const flushed: IProblem[] = matcher.flush(); if (flushed && flushed.length > 0) { for (const problem of flushed) { - this._problems.push({ + const finalized: IProblem = { ...problem, matcherName: matcher.name - }); + }; + this._problems.add(finalized); } } } diff --git a/libraries/terminal/src/test/ProblemCollector.test.ts b/libraries/terminal/src/test/ProblemCollector.test.ts index 6b083e58dfb..eacf96031d6 100644 --- a/libraries/terminal/src/test/ProblemCollector.test.ts +++ b/libraries/terminal/src/test/ProblemCollector.test.ts @@ -43,14 +43,15 @@ describe('ProblemCollector', () => { }); collector.close(); - const problems = collector.getProblems(); - expect(problems.length).toBe(2); - expect(problems[0].message).toBe('something bad happened in stdout'); - expect(problems[0].severity).toBe('error'); - expect(problems[0].matcherName).toBe('errorLine'); - expect(problems[1].message).toBe('something bad happened in stderr'); - expect(problems[1].severity).toBe('error'); - expect(problems[1].matcherName).toBe('errorLine'); + const { problems } = collector; + expect(problems.size).toBe(2); + const [firstProblem, secondProblem] = Array.from(problems); + expect(firstProblem.message).toBe('something bad happened in stdout'); + expect(firstProblem.severity).toBe('error'); + expect(firstProblem.matcherName).toBe('errorLine'); + expect(secondProblem.message).toBe('something bad happened in stderr'); + expect(secondProblem.severity).toBe('error'); + expect(secondProblem.matcherName).toBe('errorLine'); }); }); @@ -72,12 +73,13 @@ describe('VSCodeProblemMatcherAdapter - additional location formats', () => { const collector = new ProblemCollector({ matchers }); collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'src/a.c(10,5): something happened\n' }); collector.close(); - const problems = collector.getProblems(); - expect(problems.length).toBe(1); - expect(problems[0].file).toBe('src/a.c'); - expect(problems[0].line).toBe(10); - expect(problems[0].column).toBe(5); - expect(problems[0].message).toContain('something happened'); + const { problems } = collector; + expect(problems.size).toBe(1); + const [firstProblem] = Array.from(problems); + expect(firstProblem.file).toBe('src/a.c'); + expect(firstProblem.line).toBe(10); + expect(firstProblem.column).toBe(5); + expect(firstProblem.message).toContain('something happened'); }); it('parses explicit endLine and endColumn groups', () => { @@ -100,14 +102,15 @@ describe('VSCodeProblemMatcherAdapter - additional location formats', () => { const collector = new ProblemCollector({ matchers }); collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'lib/x.c(10,5,12,20): multi-line issue\n' }); collector.close(); - const problems = collector.getProblems(); - expect(problems.length).toBe(1); - expect(problems[0].file).toBe('lib/x.c'); - expect(problems[0].line).toBe(10); - expect(problems[0].column).toBe(5); - expect(problems[0].endLine).toBe(12); - expect(problems[0].endColumn).toBe(20); - expect(problems[0].message).toContain('multi-line issue'); + const { problems } = collector; + expect(problems.size).toBe(1); + const [firstProblem] = Array.from(problems); + expect(firstProblem.file).toBe('lib/x.c'); + expect(firstProblem.line).toBe(10); + expect(firstProblem.column).toBe(5); + expect(firstProblem.endLine).toBe(12); + expect(firstProblem.endColumn).toBe(20); + expect(firstProblem.message).toContain('multi-line issue'); }); }); @@ -134,13 +137,14 @@ describe('VSCodeProblemMatcherAdapter', () => { text: "src/file.ts(10,5): error TS1005: ' ; ' expected\n" }); collector.close(); - const probs = collector.getProblems(); - expect(probs.length).toBe(1); - expect(probs[0].file).toBe('src/file.ts'); - expect(probs[0].line).toBe(10); - expect(probs[0].column).toBe(5); - expect(probs[0].code).toBe('TS1005'); - expect(probs[0].severity).toBe('error'); + const { problems } = collector; + expect(problems.size).toBe(1); + const [firstProblem] = Array.from(problems); + expect(firstProblem.file).toBe('src/file.ts'); + expect(firstProblem.line).toBe(10); + expect(firstProblem.column).toBe(5); + expect(firstProblem.code).toBe('TS1005'); + expect(firstProblem.severity).toBe('error'); }); it('converts and matches a multi-line pattern', () => { @@ -174,13 +178,14 @@ describe('VSCodeProblemMatcherAdapter', () => { collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'Line 42, Col 7\n' }); collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'error: something bad happened\n' }); collector.close(); - const problems = collector.getProblems(); - expect(problems.length).toBe(1); - expect(problems[0].file).toBe('src/foo.c'); - expect(problems[0].line).toBe(42); - expect(problems[0].column).toBe(7); - expect(problems[0].severity).toBe('error'); - expect(problems[0].message).toContain('something bad'); + const { problems } = collector; + expect(problems.size).toBe(1); + const [firstProblem] = Array.from(problems); + expect(firstProblem.file).toBe('src/foo.c'); + expect(firstProblem.line).toBe(42); + expect(firstProblem.column).toBe(7); + expect(firstProblem.severity).toBe('error'); + expect(firstProblem.message).toContain('something bad'); }); it('handles a multi-line pattern whose last pattern loops producing multiple problems', () => { @@ -227,17 +232,17 @@ describe('VSCodeProblemMatcherAdapter', () => { } collector.close(); - const problems = collector.getProblems(); - expect(problems.length).toBe(6); - // First problem's message will include the count line plus the first error message (due to how captures accumulate before loop resets) - expect(problems[0].message).toContain('Unused @ts-expect-error directive.'); - // Ensure every problem has expected properties and severity propagated from matcher default. - for (let i = 0; i < problems.length; i++) { - const p = problems[i]; + const { problems } = collector; + expect(problems.size).toBe(6); + + const problemLineNumbers: number[] = [9, 11, 19, 24, 26, 34]; + const problemsArray = Array.from(problems); + for (let i = 0; i < 6; i++) { + const p = problemsArray[i]; expect(p.file).toContain( 'vscode-extensions/debug-certificate-manager-vscode-extension/src/certificates.ts' ); - expect(p.line).toBeGreaterThan(0); + expect(p.line).toBe(problemLineNumbers[i]); expect(p.column).toBe(3); // All sample lines have column 3 expect(p.code).toBe('TS2578'); expect(p.severity).toBe('error'); @@ -279,13 +284,15 @@ describe('VSCodeProblemMatcherAdapter', () => { collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: 'Start Problems\n' }); for (const l of lines) collector.writeChunk({ kind: TerminalChunkKind.Stdout, text: l + '\n' }); collector.close(); - const problems = collector.getProblems(); - expect(problems.length).toBe(4); - expect(problems.map((p) => p.severity)).toEqual(['error', 'warning', 'error', 'info']); - expect(problems.map((p) => p.code)).toEqual(['CODE100', 'CODE200', 'CODE300', 'CODE400']); - expect(problems[0].file).toBe('lib/a.ts'); - expect(problems[1].file).toBe('lib/b.ts'); - expect(problems[2].file).toBe('lib/c.ts'); - expect(problems[3].file).toBe('lib/d.ts'); + const { problems } = collector; + expect(problems.size).toBe(4); + + const problemsArray = Array.from(problems); + expect(problemsArray.map((p) => p.severity)).toEqual(['error', 'warning', 'error', 'info']); + expect(problemsArray.map((p) => p.code)).toEqual(['CODE100', 'CODE200', 'CODE300', 'CODE400']); + expect(problemsArray[0].file).toBe('lib/a.ts'); + expect(problemsArray[1].file).toBe('lib/b.ts'); + expect(problemsArray[2].file).toBe('lib/c.ts'); + expect(problemsArray[3].file).toBe('lib/d.ts'); }); });