Preset file expansion on open/save and expansion validation

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 = [];
+    }
+}