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 #12725

Author: martin-fleck-at

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;

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<string, NamedProblemMatcher>();
-    private readyPromise: Promise<void>;
+    private readyPromise = new Deferred<void>();
 
     @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<void>((res, rej) => res(undefined));
+            this.readyPromise.resolve();
             this.onDidChangeProblemMatcherEmitter.fire(undefined);
         });
     }
 
     onReady(): Promise<void> {
-        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<void> {
         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) {

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<string, NamedProblemPattern | NamedProblemPattern[]>();
-    private readyPromise: Promise<void>;
+    private readyPromise = new Deferred<void>();
 
     @postConstruct()
     protected init(): void {
         this.fillDefaults();
-        this.readyPromise = new Promise<void>((res, rej) => res(undefined));
+        this.readyPromise.resolve();
     }
 
     onReady(): Promise<void> {
-        return this.readyPromise;
+        return this.readyPromise.promise;
     }
 
     /**

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();

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<void> {
         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;

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';

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;
+}