refactor(cli): make notices display autodect part of yargs (#450)

`configuration.settings.get(['notices'])` looks like it should have the
correct value already and subsequently `shouldDisplayNotices` also
implies this. But the previous code had the auto-detection logical
_after_ this and so the config could unexpectedly be `undefined`.

This PR fixes this by moving the auto-detection logic into `yargs`
default parsing.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

Author: mrgrain

packages/@aws-cdk/user-input-gen/lib/yargs-gen.ts

@@ -14,6 +14,7 @@ export class CliHelpers extends ExternalModule {
   public readonly browserForPlatform = makeCallableExpr(this, 'browserForPlatform');
   public readonly cliVersion = makeCallableExpr(this, 'cliVersion');
   public readonly isCI = makeCallableExpr(this, 'isCI');
+  public readonly shouldDisplayNotices = makeCallableExpr(this, 'shouldDisplayNotices');
   public readonly yargsNegativeAlias = makeCallableExpr(this, 'yargsNegativeAlias');
 }
 
@@ -24,7 +25,7 @@ function makeCallableExpr(scope: IScope, name: string) {
 export async function renderYargs(config: CliConfig, helpers: CliHelpers): Promise<string> {
   const scope = new Module('aws-cdk');
 
-  scope.documentation.push( '-------------------------------------------------------------------------------------------');
+  scope.documentation.push('-------------------------------------------------------------------------------------------');
   scope.documentation.push(`GENERATED FROM ${SOURCE_OF_TRUTH}.`);
   scope.documentation.push('Do not edit by hand; all changes will be overwritten at build time from the config file.');
   scope.documentation.push('-------------------------------------------------------------------------------------------');

packages/aws-cdk/lib/cli/cli-config.ts

@@ -38,7 +38,7 @@ export async function makeConfig(): Promise<CliConfig> {
       'role-arn': { type: 'string', alias: 'r', desc: 'ARN of Role to use when invoking CloudFormation', default: undefined, requiresArg: true },
       'staging': { type: 'boolean', desc: 'Copy assets to the output directory (use --no-staging to disable the copy of assets which allows local debugging via the SAM CLI to reference the original source files)', default: true },
       'output': { type: 'string', alias: 'o', desc: 'Emits the synthesized cloud assembly into a directory (default: cdk.out)', requiresArg: true },
-      'notices': { type: 'boolean', desc: 'Show relevant notices' },
+      'notices': { type: 'boolean', desc: 'Show relevant notices', default: YARGS_HELPERS.shouldDisplayNotices() },
       'no-color': { type: 'boolean', desc: 'Removes colors and other style from console output', default: false },
       'ci': { type: 'boolean', desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr', default: YARGS_HELPERS.isCI() },
       'unstable': { type: 'array', desc: 'Opt in to unstable features. The flag indicates that the scope and API of a feature might still change. Otherwise the feature is generally production ready and fully supported. Can be specified multiple times.', default: [] },

packages/aws-cdk/lib/cli/cli.ts

@@ -1,7 +1,6 @@
 import * as cxapi from '@aws-cdk/cx-api';
 import * as chalk from 'chalk';
 import { CdkToolkit, AssetBuildTime } from './cdk-toolkit';
-import { ciSystemIsStdErrSafe } from './ci-systems';
 import type { IoMessageLevel } from './io-host';
 import { CliIoHost } from './io-host';
 import { parseCommandLineArguments } from './parse-command-line-arguments';
@@ -92,19 +91,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
   await configuration.load();
 
   const shouldDisplayNotices = configuration.settings.get(['notices']);
-  if (shouldDisplayNotices !== undefined) {
-    // Notices either go to stderr, or nowhere
-    ioHost.noticesDestination = shouldDisplayNotices ? 'stderr' : 'drop';
-  } else {
-    // If the user didn't supply either `--notices` or `--no-notices`, we do
-    // autodetection. The autodetection currently is: do write notices if we are
-    // not on CI, or are on a CI system where we know that writing to stderr is
-    // safe. We fail "closed"; that is, we decide to NOT print for unknown CI
-    // systems, even though technically we maybe could.
-    const safeToWriteToStdErr = !argv.ci || Boolean(ciSystemIsStdErrSafe());
-    ioHost.noticesDestination = safeToWriteToStdErr ? 'stderr' : 'drop';
-  }
-
+  ioHost.noticesDestination = shouldDisplayNotices ? 'stderr' : 'drop';
   const notices = Notices.create({
     ioHost,
     context: configuration.context,
@@ -160,8 +147,8 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
 
   loadPlugins(configuration.settings);
 
-  if (typeof(cmd) !== 'string') {
-    throw new ToolkitError(`First argument should be a string. Got: ${cmd} (${typeof(cmd)})`);
+  if ((typeof cmd) !== 'string') {
+    throw new ToolkitError(`First argument should be a string. Got: ${cmd} (${typeof cmd})`);
   }
 
   try {
@@ -579,7 +566,7 @@ function determineHotswapMode(hotswap?: boolean, hotswapFallback?: boolean, watc
   let hotswapMode: HotswapMode;
   if (hotswap) {
     hotswapMode = HotswapMode.HOTSWAP_ONLY;
-  /* if (hotswapFallback)*/
+    /* if (hotswapFallback)*/
   } else {
     hotswapMode = HotswapMode.FALL_BACK;
   }

packages/aws-cdk/lib/cli/parse-command-line-arguments.ts

@@ -134,7 +134,7 @@ export function parseCommandLineArguments(args: Array<string>): any {
       requiresArg: true,
     })
     .option('notices', {
-      default: undefined,
+      default: helpers.shouldDisplayNotices(),
       type: 'boolean',
       desc: 'Show relevant notices',
     })

packages/aws-cdk/lib/cli/util/yargs-helpers.ts

@@ -1,3 +1,5 @@
+import { ciSystemIsStdErrSafe } from '../ci-systems';
+import { isCI } from '../io-host';
 import * as version from '../version';
 
 export { isCI } from '../io-host';
@@ -47,3 +49,15 @@ export function browserForPlatform(): string {
   }
 }
 
+/**
+ * The default value for displaying (and refreshing) notices on all commands.
+ *
+ * If the user didn't supply either `--notices` or `--no-notices`, we do
+ * autodetection. The autodetection currently is: do write notices if we are
+ * not on CI, or are on a CI system where we know that writing to stderr is
+ * safe. We fail "closed"; that is, we decide to NOT print for unknown CI
+ * systems, even though technically we maybe could.
+ */
+export function shouldDisplayNotices(): boolean {
+  return !isCI() || Boolean(ciSystemIsStdErrSafe());
+}

packages/aws-cdk/test/cli/cli-arguments.test.ts

@@ -33,7 +33,7 @@ describe('yargs', () => {
         lookups: true,
         trace: undefined,
         unstable: [],
-        notices: undefined,
+        notices: expect.any(Boolean),
         output: undefined,
       },
       deploy: {