Merge pull request microsoft#3391 from iclanton/fix-json-flag

[rush] Fix an issue where "rush list --json" prints non-json output in a repo that uses rush plugins with autoinstallers.

common/changes/@microsoft/rush/fix-json-flag_2022-05-03-21-15.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Fix an issue where \"rush list --json\" prints non-json output in a repo that uses rush plugins with autoinstallers.",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}

libraries/rush-lib/src/api/test/__snapshots__/CommandLineConfiguration.test.ts.snap

@@ -17,5 +17,3 @@ exports[`CommandLineConfiguration Forbids a misnamed phase 3`] = `"In command-li
 exports[`CommandLineConfiguration Forbids a misnamed phase 4`] = `"In command-line.json, the phase \\"_phase:A\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`;
 
 exports[`CommandLineConfiguration Forbids a misnamed phase 5`] = `"In command-line.json, the phase \\"_phase:A-\\"'s name is not a valid phase name. Phase names must begin with the required prefix \\"_phase:\\" followed by a name containing lowercase letters, numbers, or hyphens. The name must start with a letter and must not end with a hyphen."`;
-
-exports[`CommandLineConfiguration parameters does not allow a parameter to only be associated with phased commands but not have any associated phases 1`] = `"command-line.json defines a parameter \\"--flag\\" that is only associated with phased commands, but lists no associated phases."`;

libraries/rush-lib/src/cli/RushCommandLineParser.ts

@@ -72,6 +72,7 @@ export class RushCommandLineParser extends CommandLineParser {
 
   private _debugParameter!: CommandLineFlagParameter;
   private _quietParameter!: CommandLineFlagParameter;
+  private _restrictConsoleOutput: boolean = RushCommandLineParser.shouldRestrictConsoleOutput();
   private readonly _rushOptions: IRushCommandLineParserOptions;
   private readonly _terminalProvider: ConsoleTerminalProvider;
   private readonly _terminal: Terminal;
@@ -98,7 +99,7 @@ export class RushCommandLineParser extends CommandLineParser {
     try {
       const rushJsonFilename: string | undefined = RushConfiguration.tryFindRushJsonLocation({
         startingFolder: this._rushOptions.cwd,
-        showVerbose: !RushCommandLineParser.shouldRestrictConsoleOutput()
+        showVerbose: !this._restrictConsoleOutput
       });
       if (rushJsonFilename) {
         this.rushConfiguration = RushConfiguration.loadFromConfigurationFile(rushJsonFilename);
@@ -121,7 +122,8 @@ export class RushCommandLineParser extends CommandLineParser {
       rushSession: this.rushSession,
       rushConfiguration: this.rushConfiguration,
       terminal: this._terminal,
-      builtInPluginConfigurations: this._rushOptions.builtInPluginConfigurations
+      builtInPluginConfigurations: this._rushOptions.builtInPluginConfigurations,
+      restrictConsoleOutput: this._restrictConsoleOutput
     });
 
     this._populateActions();

libraries/rush-lib/src/cli/actions/InitAutoinstallerAction.ts

@@ -38,7 +38,10 @@ export class InitAutoinstallerAction extends BaseRushAction {
   protected async runAsync(): Promise<void> {
     const autoinstallerName: string = this._name.value!;
 
-    const autoinstaller: Autoinstaller = new Autoinstaller(autoinstallerName, this.rushConfiguration);
+    const autoinstaller: Autoinstaller = new Autoinstaller({
+      autoinstallerName,
+      rushConfiguration: this.rushConfiguration
+    });
 
     if (FileSystem.exists(autoinstaller.folderFullPath)) {
       // It's okay if the folder is empty

libraries/rush-lib/src/cli/actions/UpdateAutoinstallerAction.ts

@@ -13,7 +13,7 @@ export class UpdateAutoinstallerAction extends BaseRushAction {
   public constructor(parser: RushCommandLineParser) {
     super({
       actionName: 'update-autoinstaller',
-      summary: 'Updates autoinstaller package dependenices',
+      summary: 'Updates autoinstaller package dependencies',
       documentation: 'Use this command to regenerate the shrinkwrap file for an autoinstaller folder.',
       parser
     });
@@ -32,7 +32,10 @@ export class UpdateAutoinstallerAction extends BaseRushAction {
   protected async runAsync(): Promise<void> {
     const autoinstallerName: string = this._name.value!;
 
-    const autoinstaller: Autoinstaller = new Autoinstaller(autoinstallerName, this.rushConfiguration);
+    const autoinstaller: Autoinstaller = new Autoinstaller({
+      autoinstallerName,
+      rushConfiguration: this.rushConfiguration
+    });
     autoinstaller.update();
 
     console.log('\nSuccess.');

libraries/rush-lib/src/cli/scriptActions/GlobalScriptAction.ts

@@ -84,7 +84,10 @@ export class GlobalScriptAction extends BaseScriptAction<IGlobalCommandConfig> {
   }
 
   private async _prepareAutoinstallerName(): Promise<void> {
-    const autoInstaller: Autoinstaller = new Autoinstaller(this._autoinstallerName, this.rushConfiguration);
+    const autoInstaller: Autoinstaller = new Autoinstaller({
+      autoinstallerName: this._autoinstallerName,
+      rushConfiguration: this.rushConfiguration
+    });
 
     await autoInstaller.prepareAsync();
   }

libraries/rush-lib/src/cli/test/__snapshots__/CommandLineHelp.test.ts.snap

@@ -49,7 +49,7 @@ Positional arguments:
                         repo, and create or update the shrinkwrap file as 
                         needed
     update-autoinstaller
-                        Updates autoinstaller package dependenices
+                        Updates autoinstaller package dependencies
     update-cloud-credentials
                         (EXPERIMENTAL) Update the credentials used by the 
                         build cache provider.

libraries/rush-lib/src/logic/Autoinstaller.ts

@@ -15,15 +15,24 @@ import { RushGlobalFolder } from '../api/RushGlobalFolder';
 import { RushConstants } from './RushConstants';
 import { LastInstallFlag } from '../api/LastInstallFlag';
 
+interface IAutoinstallerOptions {
+  autoinstallerName: string;
+  rushConfiguration: RushConfiguration;
+  restrictConsoleOutput?: boolean;
+}
+
 export class Autoinstaller {
-  public name: string;
+  public readonly name: string;
 
-  private _rushConfiguration: RushConfiguration;
+  private readonly _rushConfiguration: RushConfiguration;
+  private readonly _restrictConsoleOutput: boolean;
 
-  public constructor(autoinstallerName: string, rushConfiguration: RushConfiguration) {
-    this._rushConfiguration = rushConfiguration;
-    Autoinstaller.validateName(autoinstallerName);
-    this.name = autoinstallerName;
+  public constructor(options: IAutoinstallerOptions) {
+    this.name = options.autoinstallerName;
+    this._rushConfiguration = options.rushConfiguration;
+    this._restrictConsoleOutput = options.restrictConsoleOutput || false;
+
+    Autoinstaller.validateName(this.name);
   }
 
   // Example: .../common/autoinstallers/my-task
@@ -68,7 +77,8 @@ export class Autoinstaller {
     await InstallHelpers.ensureLocalPackageManager(
       this._rushConfiguration,
       rushGlobalFolder,
-      RushConstants.defaultMaxInstallAttempts
+      RushConstants.defaultMaxInstallAttempts,
+      this._restrictConsoleOutput
     );
 
     // Example: common/autoinstallers/my-task/package.json
@@ -77,7 +87,7 @@ export class Autoinstaller {
       autoinstallerFullPath
     );
 
-    console.log(`Acquiring lock for "${relativePathForLogs}" folder...`);
+    this._logIfConsoleOutputIsNotRestricted(`Acquiring lock for "${relativePathForLogs}" folder...`);
 
     const lock: LockFile = await LockFile.acquire(autoinstallerFullPath, 'autoinstaller');
 
@@ -104,14 +114,14 @@ export class Autoinstaller {
       const nodeModulesFolder: string = path.join(autoinstallerFullPath, 'node_modules');
 
       if (FileSystem.exists(nodeModulesFolder)) {
-        console.log('Deleting old files from ' + nodeModulesFolder);
+        this._logIfConsoleOutputIsNotRestricted('Deleting old files from ' + nodeModulesFolder);
         FileSystem.ensureEmptyFolder(nodeModulesFolder);
       }
 
       // Copy: .../common/autoinstallers/my-task/.npmrc
       Utilities.syncNpmrc(this._rushConfiguration.commonRushConfigFolder, autoinstallerFullPath);
 
-      console.log(`Installing dependencies under ${autoinstallerFullPath}...\n`);
+      this._logIfConsoleOutputIsNotRestricted(`Installing dependencies under ${autoinstallerFullPath}...\n`);
 
       Utilities.executeCommand({
         command: this._rushConfiguration.packageManagerToolFilename,
@@ -123,9 +133,9 @@ export class Autoinstaller {
       // Create file: ../common/autoinstallers/my-task/.rush/temp/last-install.flag
       lastInstallFlag.create();
 
-      console.log('Auto install completed successfully\n');
+      this._logIfConsoleOutputIsNotRestricted('Auto install completed successfully\n');
     } else {
-      console.log('Autoinstaller folder is already up to date\n');
+      this._logIfConsoleOutputIsNotRestricted('Autoinstaller folder is already up to date\n');
     }
 
     lock.release();
@@ -138,13 +148,15 @@ export class Autoinstaller {
       throw new Error(`The specified autoinstaller path does not exist: ` + autoinstallerPackageJsonPath);
     }
 
-    console.log(`Updating autoinstaller package: ${autoinstallerPackageJsonPath}`);
+    this._logIfConsoleOutputIsNotRestricted(
+      `Updating autoinstaller package: ${autoinstallerPackageJsonPath}`
+    );
 
     let oldFileContents: string = '';
 
     if (FileSystem.exists(this.shrinkwrapFilePath)) {
       oldFileContents = FileSystem.readFile(this.shrinkwrapFilePath, { convertLineEndings: NewlineKind.Lf });
-      console.log('Deleting ' + this.shrinkwrapFilePath);
+      this._logIfConsoleOutputIsNotRestricted('Deleting ' + this.shrinkwrapFilePath);
       FileSystem.deleteFile(this.shrinkwrapFilePath);
     }
 
@@ -158,7 +170,7 @@ export class Autoinstaller {
       );
     }
 
-    console.log();
+    this._logIfConsoleOutputIsNotRestricted();
 
     Utilities.syncNpmrc(this._rushConfiguration.commonRushConfigFolder, this.folderFullPath);
 
@@ -169,18 +181,18 @@ export class Autoinstaller {
       keepEnvironment: true
     });
 
-    console.log();
+    this._logIfConsoleOutputIsNotRestricted();
 
     if (this._rushConfiguration.packageManager === 'npm') {
-      console.log(colors.bold('Running "npm shrinkwrap"...'));
+      this._logIfConsoleOutputIsNotRestricted(colors.bold('Running "npm shrinkwrap"...'));
       Utilities.executeCommand({
         command: this._rushConfiguration.packageManagerToolFilename,
         args: ['shrinkwrap'],
         workingDirectory: this.folderFullPath,
         keepEnvironment: true
       });
-      console.log('"npm shrinkwrap" completed');
-      console.log();
+      this._logIfConsoleOutputIsNotRestricted('"npm shrinkwrap" completed');
+      this._logIfConsoleOutputIsNotRestricted();
     }
 
     if (!FileSystem.exists(this.shrinkwrapFilePath)) {
@@ -193,12 +205,18 @@ export class Autoinstaller {
       convertLineEndings: NewlineKind.Lf
     });
     if (oldFileContents !== newFileContents) {
-      console.log(
+      this._logIfConsoleOutputIsNotRestricted(
         colors.green('The shrinkwrap file has been updated.') + '  Please commit the updated file:'
       );
-      console.log(`\n  ${this.shrinkwrapFilePath}`);
+      this._logIfConsoleOutputIsNotRestricted(`\n  ${this.shrinkwrapFilePath}`);
     } else {
-      console.log(colors.green('Already up to date.'));
+      this._logIfConsoleOutputIsNotRestricted(colors.green('Already up to date.'));
+    }
+  }
+
+  private _logIfConsoleOutputIsNotRestricted(message?: string): void {
+    if (!this._restrictConsoleOutput) {
+      console.log(message);
     }
   }
 }

libraries/rush-lib/src/logic/installManager/InstallHelpers.ts

@@ -74,13 +74,25 @@ export class InstallHelpers {
   public static async ensureLocalPackageManager(
     rushConfiguration: RushConfiguration,
     rushGlobalFolder: RushGlobalFolder,
-    maxInstallAttempts: number
+    maxInstallAttempts: number,
+    restrictConsoleOutput?: boolean
   ): Promise<void> {
+    let logIfConsoleOutputIsNotRestricted: (message?: string) => void;
+    if (restrictConsoleOutput) {
+      logIfConsoleOutputIsNotRestricted = () => {
+        /* noop */
+      };
+    } else {
+      logIfConsoleOutputIsNotRestricted = (message?: string) => {
+        console.log(message);
+      };
+    }
+
     // Example: "C:\Users\YourName\.rush"
     const rushUserFolder: string = rushGlobalFolder.nodeSpecificPath;
 
     if (!FileSystem.exists(rushUserFolder)) {
-      console.log('Creating ' + rushUserFolder);
+      logIfConsoleOutputIsNotRestricted('Creating ' + rushUserFolder);
       FileSystem.ensureFolder(rushUserFolder);
     }
 
@@ -95,14 +107,16 @@ export class InstallHelpers {
       node: process.versions.node
     });
 
-    console.log(`Trying to acquire lock for ${packageManagerAndVersion}`);
+    logIfConsoleOutputIsNotRestricted(`Trying to acquire lock for ${packageManagerAndVersion}`);
 
     const lock: LockFile = await LockFile.acquire(rushUserFolder, packageManagerAndVersion);
 
-    console.log(`Acquired lock for ${packageManagerAndVersion}`);
+    logIfConsoleOutputIsNotRestricted(`Acquired lock for ${packageManagerAndVersion}`);
 
     if (!packageManagerMarker.isValid() || lock.dirtyWhenAcquired) {
-      console.log(colors.bold(`Installing ${packageManager} version ${packageManagerVersion}${os.EOL}`));
+      logIfConsoleOutputIsNotRestricted(
+        colors.bold(`Installing ${packageManager} version ${packageManagerVersion}${os.EOL}`)
+      );
 
       // note that this will remove the last-install flag from the directory
       Utilities.installPackageInDirectory({
@@ -120,9 +134,13 @@ export class InstallHelpers {
         commonRushConfigFolder: rushConfiguration.commonRushConfigFolder
       });
 
-      console.log(`Successfully installed ${packageManager} version ${packageManagerVersion}`);
+      logIfConsoleOutputIsNotRestricted(
+        `Successfully installed ${packageManager} version ${packageManagerVersion}`
+      );
     } else {
-      console.log(`Found ${packageManager} version ${packageManagerVersion} in ${packageManagerToolFolder}`);
+      logIfConsoleOutputIsNotRestricted(
+        `Found ${packageManager} version ${packageManagerVersion} in ${packageManagerToolFolder}`
+      );
     }
 
     packageManagerMarker.create();
@@ -136,8 +154,8 @@ export class InstallHelpers {
       `${packageManager}-local`
     );
 
-    console.log(os.EOL + `Symlinking "${localPackageManagerToolFolder}"`);
-    console.log(`  --> "${packageManagerToolFolder}"`);
+    logIfConsoleOutputIsNotRestricted(os.EOL + `Symlinking "${localPackageManagerToolFolder}"`);
+    logIfConsoleOutputIsNotRestricted(`  --> "${packageManagerToolFolder}"`);
 
     // We cannot use FileSystem.exists() to test the existence of a symlink, because it will
     // return false for broken symlinks.  There is no way to test without catching an exception.

libraries/rush-lib/src/pluginFramework/PluginLoader/AutoinstallerPluginLoader.ts

@@ -14,6 +14,10 @@ import {
   PluginLoaderBase
 } from './PluginLoaderBase';
 
+interface IAutoinstallerPluginLoaderOptions extends IPluginLoaderOptions<IRushPluginConfiguration> {
+  restrictConsoleOutput: boolean;
+}
+
 /**
  * @beta
  */
@@ -22,12 +26,13 @@ export class AutoinstallerPluginLoader extends PluginLoaderBase<IRushPluginConfi
 
   public readonly packageFolder: string;
 
-  public constructor(options: IPluginLoaderOptions<IRushPluginConfiguration>) {
+  public constructor(options: IAutoinstallerPluginLoaderOptions) {
     super(options);
-    this._autoinstaller = new Autoinstaller(
-      options.pluginConfiguration.autoinstallerName,
-      this._rushConfiguration
-    );
+    this._autoinstaller = new Autoinstaller({
+      autoinstallerName: options.pluginConfiguration.autoinstallerName,
+      rushConfiguration: this._rushConfiguration,
+      restrictConsoleOutput: options.restrictConsoleOutput
+    });
 
     this.packageFolder = path.join(this._autoinstaller.folderFullPath, 'node_modules', this.packageName);
   }

libraries/rush-lib/src/pluginFramework/PluginManager.ts

@@ -16,6 +16,7 @@ export interface IPluginManagerOptions {
   rushConfiguration: RushConfiguration;
   rushSession: RushSession;
   builtInPluginConfigurations: IBuiltInPluginConfiguration[];
+  restrictConsoleOutput: boolean;
 }
 
 export interface ICustomCommandLineConfigurationInfo {
@@ -27,6 +28,7 @@ export class PluginManager {
   private readonly _terminal: ITerminal;
   private readonly _rushConfiguration: RushConfiguration;
   private readonly _rushSession: RushSession;
+  private readonly _restrictConsoleOutput: boolean;
   private readonly _builtInPluginLoaders: BuiltInPluginLoader[];
   private readonly _autoinstallerPluginLoaders: AutoinstallerPluginLoader[];
   private readonly _installedAutoinstallerNames: Set<string>;
@@ -38,6 +40,7 @@ export class PluginManager {
     this._terminal = options.terminal;
     this._rushConfiguration = options.rushConfiguration;
     this._rushSession = options.rushSession;
+    this._restrictConsoleOutput = options.restrictConsoleOutput;
 
     this._installedAutoinstallerNames = new Set<string>();
 
@@ -93,7 +96,8 @@ export class PluginManager {
       return new AutoinstallerPluginLoader({
         pluginConfiguration,
         rushConfiguration: this._rushConfiguration,
-        terminal: this._terminal
+        terminal: this._terminal,
+        restrictConsoleOutput: this._restrictConsoleOutput
       });
     });
   }