Preset file expansion on open/save and expansion validation (#3905)

This change changes the way we do preset file expansion. Expansion now occurs upfront on file open or save, and if there are no errors, the expanded presets will be cached so repeated expansion is not necessary every time getAll{type of preset}Presets() is called.

The only time expansion will happen again is on set preset, which will then also apply the VS developer environment as needed. This is avoided on the initial expansion for performance.

If there are errors in expansion, the errors are shown in the problems panel per file. The presets file will also be set to undefined, which will invalidate that file and not allow any presets in that file to be recognized/selected as valid presets. This is in line with the CMake command line experience.

Some notes:

- The ${generator} macro should not work for packagePresets, but should now expand correctly for all other presets and evaluate conditions correctly
- Tertiary preset cache was added (really its just the original presets cache but used in a new way) to account for included json files that wouldnt be in the original presets files

CHANGELOG.md

@@ -11,6 +11,7 @@ Improvements:
 - Add `Unspecified` option for selecting a kit variant to allow CMake itself to select the build type. [#3821](https://github.com/microsoft/vscode-cmake-tools/issues/3821)
 - Skip loading variants when using CMakePresets. [#3300](https://github.com/microsoft/vscode-cmake-tools/issues/3300)
 - Add setting to only show the cmake log on target failure. [#3785](https://github.com/microsoft/vscode-cmake-tools/pull/3785) [@stepeos](https://github.com/stepeos)
+- Preset expansion occurs on `CMakePresets.json` or `CMakeUserPresets.json` save, and if there are no errors the expanded presets are cached. The VS Developer Environment will only be applied to a preset if it is selected. Expansion errors will show in the problems panel and preset files with errors will be invalid, and any presets they contain cannot be used. [#3905](https://github.com/microsoft/vscode-cmake-tools/pull/3905)
 
 Bug Fixes:
 

src/cmakeProject.ts

@@ -266,11 +266,23 @@ export class CMakeProject {
             return undefined;
         }
         log.debug(localize('resolving.config.preset', 'Resolving the selected configure preset'));
+
+        // We want to use the original unexpanded preset file to apply the dev env in expandConfigurePreset
+        // we have to first check if the preset is valid in expandedPresets since we won't be expanding the whole file here, only the path up for this preset
+        if (!preset.getPresetByName(preset.configurePresets(this.folderPath), configurePreset) && !preset.getPresetByName(preset.userConfigurePresets(this.folderPath), configurePreset)) {
+            return undefined;
+        }
+
+        // TODO: move applyDevEnv here to decouple from expandConfigurePreset
+
+        // modify the preset parent environment, in certain cases, to apply the Vs Dev Env on top of process.env.
         const expandedConfigurePreset = await preset.expandConfigurePreset(this.folderPath,
             configurePreset,
             lightNormalizePath(this.folderPath || '.'),
             this.sourceDir,
+            true,
             true);
+
         if (!expandedConfigurePreset) {
             log.error(localize('failed.resolve.config.preset', 'Failed to resolve configure preset: {0}', configurePreset));
             return undefined;
@@ -298,6 +310,7 @@ export class CMakeProject {
 
         if (configurePreset) {
             const expandedConfigurePreset: preset.ConfigurePreset | undefined = await this.expandConfigPresetbyName(configurePreset);
+
             if (!expandedConfigurePreset) {
                 await this.resetPresets(drv);
                 return;

src/diagnostics/collections.ts

@@ -38,6 +38,7 @@ class LazyCollection implements vscode.Disposable {
 class Collections {
     private readonly _cmake = new LazyCollection('cmake-configure-diags');
     private readonly _build = new LazyCollection('cmake-build-diags');
+    private readonly _presets = new LazyCollection('cmake-presets-diags');
 
     /**
      * The `DiagnosticCollection` for the CMake configure diagnostics.
@@ -53,9 +54,17 @@ class Collections {
         return this._build.getOrCreate();
     }
 
+    /**
+     * The `DiagnosticCollection` for presets diagnostics
+     */
+    get presets(): vscode.DiagnosticCollection {
+        return this._presets.getOrCreate();
+    }
+
     reset() {
         this._cmake.dispose();
         this._build.dispose();
+        this._presets.dispose();
     }
 }
 

src/expand.ts

@@ -99,18 +99,22 @@ export interface ExpansionOptions {
     doNotSupportCommands?: boolean;
 }
 
+export interface ExpansionErrorHandler {
+    errorList: [string, string][];
+    tempErrorList: [string, string][];
+}
+
 /**
  * Replace ${variable} references in the given string with their corresponding
  * values.
  * @param input The input string
  * @param opts Options for the expansion process
  * @returns A string with the variable references replaced
  */
-export async function expandString<T>(input: string | T, opts: ExpansionOptions): Promise<string | T> {
+export async function expandString<T>(input: string | T, opts: ExpansionOptions, errorHandler?: ExpansionErrorHandler): Promise<string | T> {
     if (typeof input !== 'string') {
         return input;
     }
-
     const inputString = input as string;
     try {
 
@@ -122,22 +126,24 @@ export async function expandString<T>(input: string | T, opts: ExpansionOptions)
         let i = 0;
         do {
             // TODO: consider a full circular reference check?
-            const expansion = await expandStringHelper(result, opts);
+            const expansion = await expandStringHelper(result, opts, errorHandler);
             result = expansion.result;
             didReplacement = expansion.didReplacement;
             circularReference = expansion.circularReference;
             i++;
         } while (i < maxRecursion && opts.recursive && didReplacement && !circularReference);
 
         if (circularReference) {
-            log.error(localize('circular.variable.reference', 'Circular variable reference found: {0}', circularReference));
+            log.error(localize('circular.variable.reference.full', 'Circular variable reference found: {0}', circularReference));
         } else if (i === maxRecursion) {
             log.error(localize('reached.max.recursion', 'Reached max string expansion recursion. Possible circular reference.'));
+            errorHandler?.tempErrorList.push([localize('max.recursion', 'Max string expansion recursion'), ""]);
         }
 
         return replaceAll(result, '${dollar}', '$');
     } catch (e) {
-        log.warning(localize('exception.expanding.string', 'Exception while expanding string {0}: {1}', inputString, errorToString(e)));
+        log.warning(localize('exception.expanding.string.full', 'Exception while expanding string {0}: {1}', inputString, errorToString(e)));
+        errorHandler?.tempErrorList.push([localize('exception.expanding.string', 'Exception expanding string'), inputString]);
     }
 
     return input;
@@ -148,7 +154,7 @@ export async function expandString<T>(input: string | T, opts: ExpansionOptions)
 // as few times as possible, expanding as needed (lazy)
 const varValueRegexp = ".+?";
 
-async function expandStringHelper(input: string, opts: ExpansionOptions) {
+async function expandStringHelper(input: string, opts: ExpansionOptions, errorHandler?: ExpansionErrorHandler) {
     const envPreNormalize = opts.envOverride ? opts.envOverride : process.env;
     const env = EnvironmentUtils.create(envPreNormalize);
     const replacements = opts.vars;
@@ -169,7 +175,8 @@ async function expandStringHelper(input: string, opts: ExpansionOptions) {
             // Replace dollar sign at the very end of the expanding process
             const replacement = replacements[key];
             if (!replacement) {
-                log.warning(localize('invalid.variable.reference', 'Invalid variable reference {0} in string: {1}', full, input));
+                log.warning(localize('invalid.variable.reference.full', 'Invalid variable reference {0} in string: {1}', full, input));
+                errorHandler?.tempErrorList.push([localize('invalid.variable.reference', 'Invalid variable reference'), input]);
             } else {
                 subs.set(full, replacement);
             }
@@ -206,6 +213,7 @@ async function expandStringHelper(input: string, opts: ExpansionOptions) {
         const varNameReplacement = mat2 ? mat2[1] : undefined;
         if (varNameReplacement && varNameReplacement === varName) {
             circularReference = `\"${varName}\" : \"${input}\"`;
+            errorHandler?.tempErrorList.push([localize('circular.variable.reference', 'Circular variable reference'), full]);
             break;
         }
         subs.set(full, replacement);
@@ -254,7 +262,8 @@ async function expandStringHelper(input: string, opts: ExpansionOptions) {
             const result = await vscode.commands.executeCommand(command, opts.vars.workspaceFolder);
             subs.set(full, `${result}`);
         } catch (e) {
-            log.warning(localize('exception.executing.command', 'Exception while executing command {0} for string: {1} {2}', command, input, errorToString(e)));
+            log.warning(localize('exception.executing.command.full', 'Exception while executing command {0} for string: {1} {2}', command, input, errorToString(e)));
+            errorHandler?.tempErrorList.push([localize('exception.executing.command', 'Exception executing command'), input]);
         }
     }
 
@@ -298,3 +307,12 @@ export function getParentEnvSubstitutions(input: string, subs: Map<string, strin
 
     return subs;
 }
+
+export function errorHandlerHelper(presetName: string, errorHandler?: ExpansionErrorHandler) {
+    if (errorHandler) {
+        for (const error of errorHandler.tempErrorList || []) {
+            errorHandler.errorList.push([error[0], `'${error[1]}' in preset '${presetName}'`]);
+        }
+        errorHandler.tempErrorList = [];
+    }
+}