From 90015089638adf020268814b86bbebb68b577abe Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Fri, 25 Aug 2023 12:47:02 +0200 Subject: [PATCH] Ensure contributed problem patterns are found correctly (#12805) - Resolve pattern names correctly by removing leading '$' - Introduce utility function for dealing with variable names - Ensure 'ready' promises are already available with object creation Fixes https://github.com/eclipse-theia/theia/issues/12725 --- .../task/src/browser/task-configurations.ts | 5 ++- .../browser/task-problem-matcher-registry.ts | 18 ++++---- .../browser/task-problem-pattern-registry.ts | 9 ++-- .../task/src/browser/task-schema-updater.ts | 4 +- packages/task/src/browser/task-service.ts | 15 +++---- packages/task/src/common/index.ts | 1 + packages/task/src/common/task-util.ts | 43 +++++++++++++++++++ 7 files changed, 69 insertions(+), 26 deletions(-) create mode 100644 packages/task/src/common/task-util.ts diff --git a/packages/task/src/browser/task-configurations.ts b/packages/task/src/browser/task-configurations.ts index dac50d6a2b4b1..2ec5a9d24272e 100644 --- a/packages/task/src/browser/task-configurations.ts +++ b/packages/task/src/browser/task-configurations.ts @@ -21,7 +21,8 @@ import { TaskDefinition, TaskOutputPresentation, TaskConfigurationScope, - TaskScope + TaskScope, + asVariableName } from '../common'; import { TaskDefinitionRegistry } from './task-definition-registry'; import { ProvidedTaskConfigurations } from './provided-task-configurations'; @@ -349,7 +350,7 @@ export class TaskConfigurations implements Disposable { } else if (task.problemMatcher) { problemMatcher.push(task.problemMatcher.name!); } - customization.problemMatcher = problemMatcher.map(name => name.startsWith('$') ? name : `$${name}`); + customization.problemMatcher = problemMatcher.map(asVariableName); } if (task.group) { customization.group = task.group; diff --git a/packages/task/src/browser/task-problem-matcher-registry.ts b/packages/task/src/browser/task-problem-matcher-registry.ts index 63e05e26334e0..2c1438529759d 100644 --- a/packages/task/src/browser/task-problem-matcher-registry.ts +++ b/packages/task/src/browser/task-problem-matcher-registry.ts @@ -24,16 +24,18 @@ import { Event, Emitter } from '@theia/core/lib/common'; import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable'; import { ApplyToKind, FileLocationKind, NamedProblemMatcher, - ProblemPattern, ProblemMatcher, ProblemMatcherContribution, WatchingMatcher + ProblemPattern, ProblemMatcher, ProblemMatcherContribution, WatchingMatcher, + fromVariableName } from '../common'; import { ProblemPatternRegistry } from './task-problem-pattern-registry'; import { Severity } from '@theia/core/lib/common/severity'; +import { Deferred } from '@theia/core/lib/common/promise-util'; @injectable() export class ProblemMatcherRegistry { private readonly matchers = new Map(); - private readyPromise: Promise; + private readyPromise = new Deferred(); @inject(ProblemPatternRegistry) protected readonly problemPatternRegistry: ProblemPatternRegistry; @@ -47,13 +49,13 @@ export class ProblemMatcherRegistry { protected init(): void { this.problemPatternRegistry.onReady().then(() => { this.fillDefaults(); - this.readyPromise = new Promise((res, rej) => res(undefined)); + this.readyPromise.resolve(); this.onDidChangeProblemMatcherEmitter.fire(undefined); }); } onReady(): Promise { - return this.readyPromise; + return this.readyPromise.promise; } /** @@ -73,6 +75,7 @@ export class ProblemMatcherRegistry { this.doRegister(matcher, toDispose).then(() => this.onDidChangeProblemMatcherEmitter.fire(undefined)); return toDispose; } + protected async doRegister(matcher: ProblemMatcherContribution, toDispose: DisposableCollection): Promise { const problemMatcher = await this.getProblemMatcherFromContribution(matcher); if (toDispose.disposed) { @@ -88,10 +91,7 @@ export class ProblemMatcherRegistry { * @return the problem matcher. If the task definition is not found, `undefined` is returned. */ get(name: string): NamedProblemMatcher | undefined { - if (name.startsWith('$')) { - return this.matchers.get(name.slice(1)); - } - return this.matchers.get(name); + return this.matchers.get(fromVariableName(name)); } /** @@ -132,7 +132,7 @@ export class ProblemMatcherRegistry { if (matcher.pattern) { if (typeof matcher.pattern === 'string') { await this.problemPatternRegistry.onReady(); - const registeredPattern = this.problemPatternRegistry.get(matcher.pattern); + const registeredPattern = this.problemPatternRegistry.get(fromVariableName(matcher.pattern)); if (Array.isArray(registeredPattern)) { patterns.push(...registeredPattern); } else if (!!registeredPattern) { diff --git a/packages/task/src/browser/task-problem-pattern-registry.ts b/packages/task/src/browser/task-problem-pattern-registry.ts index 4c03c330bfaa3..3feb7e3711c52 100644 --- a/packages/task/src/browser/task-problem-pattern-registry.ts +++ b/packages/task/src/browser/task-problem-pattern-registry.ts @@ -19,23 +19,24 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable'; +import { Deferred } from '@theia/core/lib/common/promise-util'; import { injectable, postConstruct } from '@theia/core/shared/inversify'; import { NamedProblemPattern, ProblemLocationKind, ProblemPattern, ProblemPatternContribution } from '../common'; -import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable'; @injectable() export class ProblemPatternRegistry { private readonly patterns = new Map(); - private readyPromise: Promise; + private readyPromise = new Deferred(); @postConstruct() protected init(): void { this.fillDefaults(); - this.readyPromise = new Promise((res, rej) => res(undefined)); + this.readyPromise.resolve(); } onReady(): Promise { - return this.readyPromise; + return this.readyPromise.promise; } /** diff --git a/packages/task/src/browser/task-schema-updater.ts b/packages/task/src/browser/task-schema-updater.ts index ad5231af090cf..6c03dfea1f295 100644 --- a/packages/task/src/browser/task-schema-updater.ts +++ b/packages/task/src/browser/task-schema-updater.ts @@ -30,7 +30,7 @@ import { inputsSchema } from '@theia/variable-resolver/lib/browser/variable-inpu import URI from '@theia/core/lib/common/uri'; import { ProblemMatcherRegistry } from './task-problem-matcher-registry'; import { TaskDefinitionRegistry } from './task-definition-registry'; -import { TaskServer } from '../common'; +import { TaskServer, asVariableName } from '../common'; import { UserStorageUri } from '@theia/userstorage/lib/browser'; import { WorkspaceService } from '@theia/workspace/lib/browser'; import { JSONObject } from '@theia/core/shared/@phosphor/coreutils'; @@ -213,7 +213,7 @@ export class TaskSchemaUpdater implements JsonSchemaContribution { /** Gets the most up-to-date names of problem matchers from the registry and update the task schema */ private updateProblemMatcherNames(): void { - const matcherNames = this.problemMatcherRegistry.getAll().map(m => m.name.startsWith('$') ? m.name : `$${m.name}`); + const matcherNames = this.problemMatcherRegistry.getAll().map(m => asVariableName(m.name)); problemMatcherNames.length = 0; problemMatcherNames.push(...matcherNames); this.update(); diff --git a/packages/task/src/browser/task-service.ts b/packages/task/src/browser/task-service.ts index fe863c06dc2d9..16557cfed57b5 100644 --- a/packages/task/src/browser/task-service.ts +++ b/packages/task/src/browser/task-service.ts @@ -48,7 +48,8 @@ import { TaskInfo, TaskOutputPresentation, TaskOutputProcessedEvent, - TaskServer + TaskServer, + asVariableName } from '../common'; import { TaskWatcher } from '../common/task-watcher'; import { ProvidedTaskConfigurations } from './provided-task-configurations'; @@ -908,13 +909,9 @@ export class TaskService implements TaskConfigurationClient { async updateTaskConfiguration(token: number, task: TaskConfiguration, update: { [name: string]: any }): Promise { if (update.problemMatcher) { if (Array.isArray(update.problemMatcher)) { - update.problemMatcher.forEach((name, index) => { - if (!name.startsWith('$')) { - update.problemMatcher[index] = `$${update.problemMatcher[index]}`; - } - }); - } else if (!update.problemMatcher.startsWith('$')) { - update.problemMatcher = `$${update.problemMatcher}`; + update.problemMatcher.forEach((_name, index) => update.problemMatcher[index] = asVariableName(update.problemMatcher[index])); + } else { + update.problemMatcher = asVariableName(update.problemMatcher); } } this.taskConfigurations.updateTaskConfig(token, task, update); @@ -1041,7 +1038,7 @@ export class TaskService implements TaskConfigurationClient { ({ label: matcher.label, value: { problemMatchers: [matcher] }, - description: matcher.name.startsWith('$') ? matcher.name : `$${matcher.name}` + description: asVariableName(matcher.name) }) )); return items; diff --git a/packages/task/src/common/index.ts b/packages/task/src/common/index.ts index d44cfdac92e1b..40b71394e5169 100644 --- a/packages/task/src/common/index.ts +++ b/packages/task/src/common/index.ts @@ -17,3 +17,4 @@ export * from './task-protocol'; export * from './task-watcher'; export * from './problem-matcher-protocol'; +export * from './task-util'; diff --git a/packages/task/src/common/task-util.ts b/packages/task/src/common/task-util.ts new file mode 100644 index 0000000000000..b4532f818ca4f --- /dev/null +++ b/packages/task/src/common/task-util.ts @@ -0,0 +1,43 @@ +// ***************************************************************************** +// Copyright (C) 2023 EclipseSource and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0. +// +// This Source Code may also be made available under the following Secondary +// Licenses when the conditions for such availability set forth in the Eclipse +// Public License v. 2.0 are satisfied: GNU General Public License, version 2 +// with the GNU Classpath Exception which is available at +// https://www.gnu.org/software/classpath/license.html. +// +// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 +// ***************************************************************************** + +/** + * Converts the given standard name to a variable name starting with '$' if not already present. + * + * Variable names are used, for instance, to reference problem matchers, within task configurations. + * + * @param name standard name + * @returns variable name with leading '$' if not already present. + * + * @see {@link fromVariableName} for the reverse conversion. + */ +export function asVariableName(name: string): string { + return name.startsWith('$') ? name : `$${name}`; +} + +/** + * Converts a given variable name to a standard name, effectively removing a leading '$' if present. + * + * Standard names are used, for instance, in registries to store variable objects + * + * @param name variable name + * @returns variable name without leading '$' if present. + * + * @see {@link asVariableName} for the reverse conversion. + */ +export function fromVariableName(name: string): string { + return name.startsWith('$') ? name.slice(1) : name; +}