refactor: no mutate params, no NamedError

src/config/configStore.ts

@@ -144,18 +144,19 @@ export abstract class BaseConfigStore<
    * @param value The value.
    */
   public set<K extends Key<P>>(key: K, value: P[K]): void {
+    let resolvedValue = value;
     if (this.hasEncryption()) {
-      if (isJsonMap(value)) {
-        value = this.recursiveEncrypt(value, key as string) as P[K];
+      if (isJsonMap(resolvedValue)) {
+        resolvedValue = this.recursiveEncrypt(resolvedValue, key as string) as P[K];
       } else if (this.isCryptoKey(key as string)) {
-        value = this.encrypt(value) as P[K];
+        resolvedValue = this.encrypt(resolvedValue) as P[K];
       }
     }
     // set(key, undefined) means unset
-    if (value === undefined) {
+    if (resolvedValue === undefined) {
       this.unset(key);
     } else {
-      this.contents.set(key, value);
+      this.contents.set(key, resolvedValue);
     }
   }
 
@@ -276,10 +277,8 @@ export abstract class BaseConfigStore<
    * @param contents The contents.
    */
   protected setContents(contents: P = {} as P): void {
-    if (this.hasEncryption()) {
-      contents = this.recursiveEncrypt(contents);
-    }
-    entriesOf(contents).map(([key, value]) => {
+    const maybeEncryptedContents = this.hasEncryption() ? this.recursiveEncrypt(contents) : contents;
+    entriesOf(maybeEncryptedContents).map(([key, value]) => {
       this.contents.set(key, value);
     });
   }
@@ -420,6 +419,8 @@ export abstract class BaseConfigStore<
         this.recursiveCrypto(method, [...keyPaths, key, newKey], value);
       }
     } else if (this.isCryptoKey(key)) {
+      // I think this side effect is intentional
+      // eslint-disable-next-line no-param-reassign
       data[key] = method(value);
     }
   }

src/global.ts

@@ -96,15 +96,15 @@ export class Global {
    * @param dirPath The directory path to be created within {@link Global.SFDX_DIR}.
    */
   public static async createDir(dirPath?: string): Promise<void> {
-    dirPath = dirPath ? path.join(Global.SFDX_DIR, dirPath) : Global.SFDX_DIR;
+    const resolvedPath = dirPath ? path.join(Global.SFDX_DIR, dirPath) : Global.SFDX_DIR;
     try {
       if (process.platform === 'win32') {
-        await fs.promises.mkdir(dirPath, { recursive: true });
+        await fs.promises.mkdir(resolvedPath, { recursive: true });
       } else {
-        await fs.promises.mkdir(dirPath, { recursive: true, mode: 0o700 });
+        await fs.promises.mkdir(resolvedPath, { recursive: true, mode: 0o700 });
       }
     } catch (error) {
-      throw new SfError(`Failed to create directory or set permissions for: ${dirPath}`);
+      throw new SfError(`Failed to create directory or set permissions for: ${resolvedPath}`);
     }
   }
 }

src/logger/logger.ts

@@ -256,11 +256,11 @@ export class Logger {
    * @see {@Link LoggerLevel}
    */
   public static getLevelByName(levelName: string): LoggerLevelValue {
-    levelName = levelName.toUpperCase();
-    if (!isKeyOf(LoggerLevel, levelName)) {
-      throw new SfError(`Invalid log level "${levelName}".`, 'UnrecognizedLoggerLevelNameError');
+    const upperLevel = levelName.toUpperCase();
+    if (!isKeyOf(LoggerLevel, upperLevel)) {
+      throw new SfError(`Invalid log level "${upperLevel}".`, 'UnrecognizedLoggerLevelNameError');
     }
-    return LoggerLevel[levelName];
+    return LoggerLevel[upperLevel];
   }
 
   /** get the bare (pino) logger instead of using the class hierarchy */
@@ -309,11 +309,8 @@ export class Logger {
    * ```
    */
   public setLevel(level?: LoggerLevelValue): Logger {
-    if (level == null) {
-      const logLevelFromEnvVar = new Env().getString('SF_LOG_LEVEL');
-      level = logLevelFromEnvVar ? Logger.getLevelByName(logLevelFromEnvVar) : Logger.DEFAULT_LEVEL;
-    }
-    this.pinoLogger.level = this.pinoLogger.levels.labels[level] ?? this.pinoLogger.levels.labels[Logger.DEFAULT_LEVEL];
+    this.pinoLogger.level =
+      this.pinoLogger.levels.labels[level ?? getDefaultLevel()] ?? this.pinoLogger.levels.labels[Logger.DEFAULT_LEVEL];
     return this;
   }
 
@@ -342,10 +339,7 @@ export class Logger {
    */
   public readLogContentsAsText(): string {
     if (this.memoryLogger) {
-      return this.memoryLogger.loggedData.reduce((accum, line) => {
-        accum += JSON.stringify(line) + os.EOL;
-        return accum;
-      }, '');
+      return this.memoryLogger?.loggedData.map((line) => JSON.stringify(line)).join(os.EOL);
     } else {
       this.pinoLogger.warn(
         'readLogContentsAsText is not supported for file streams, only used when useMemoryLogging is true'
@@ -511,3 +505,8 @@ const numberToLevel = (level: number): string =>
   pino.levels.labels[level] ??
   Object.entries(pino.levels.labels).find(([value]) => Number(value) > level)?.[1] ??
   'warn';
+
+const getDefaultLevel = (): LoggerLevel => {
+  const logLevelFromEnvVar = new Env().getString('SF_LOG_LEVEL');
+  return logLevelFromEnvVar ? Logger.getLevelByName(logLevelFromEnvVar) : Logger.DEFAULT_LEVEL;
+};

src/messages.ts

@@ -11,7 +11,7 @@ import * as path from 'node:path';
 import * as util from 'node:util';
 import { fileURLToPath } from 'node:url';
 import { AnyJson, asString, ensureJsonMap, ensureString, isJsonMap, isObject } from '@salesforce/ts-types';
-import { ensureArray, NamedError, upperFirst } from '@salesforce/kit';
+import { ensureArray, upperFirst } from '@salesforce/kit';
 import { SfError } from './sfError';
 
 export type Tokens = Array<string | boolean | number | null | undefined>;
@@ -362,17 +362,7 @@ export class Messages<T extends string> {
       moduleMessagesDirPath = projectRoot;
     }
 
-    if (!packageName) {
-      const errMessage = `Invalid or missing package.json file at '${moduleMessagesDirPath}'. If not using a package.json, pass in a packageName.`;
-      try {
-        packageName = asString(ensureJsonMap(Messages.readFile(path.join(moduleMessagesDirPath, 'package.json'))).name);
-        if (!packageName) {
-          throw new NamedError('MissingPackageName', errMessage);
-        }
-      } catch (err) {
-        throw new NamedError('MissingPackageName', errMessage, err as Error);
-      }
-    }
+    const resolvedPackageName = packageName ?? resolvePackageName(moduleMessagesDirPath);
 
     moduleMessagesDirPath += `${path.sep}messages`;
 
@@ -385,7 +375,7 @@ export class Messages<T extends string> {
           // When we support other locales, load them from /messages/<local>/<bundleName>.json
           // Change generateFileLoaderFunction to handle loading locales.
         } else if (stat.isFile()) {
-          this.importMessageFile(packageName, filePath);
+          this.importMessageFile(resolvedPackageName, filePath);
         }
       }
     }
@@ -423,7 +413,7 @@ export class Messages<T extends string> {
     }
 
     // Don't use messages inside messages
-    throw new NamedError('MissingBundleError', `Missing bundle ${key} for locale ${Messages.getLocale()}.`);
+    throw new SfError(`Missing bundle ${key} for locale ${Messages.getLocale()}.`, 'MissingBundleError');
   }
 
   /**
@@ -591,9 +581,9 @@ export class Messages<T extends string> {
     const msg = map.get(key);
     if (!msg) {
       // Don't use messages inside messages
-      throw new NamedError(
-        'MissingMessageError',
-        `Missing message ${this.bundleName}:${key} for locale ${Messages.getLocale()}.`
+      throw new SfError(
+        `Missing message ${this.bundleName}:${key} for locale ${Messages.getLocale()}.`,
+        'MissingMessageError'
       );
     }
     const messages = ensureArray(msg);
@@ -603,3 +593,18 @@ export class Messages<T extends string> {
     });
   }
 }
+
+const resolvePackageName = (moduleMessagesDirPath: string): string => {
+  const errMessage = `Invalid or missing package.json file at '${moduleMessagesDirPath}'. If not using a package.json, pass in a packageName.`;
+  try {
+    const resolvedPackageName = asString(
+      ensureJsonMap(Messages.readFile(path.join(moduleMessagesDirPath, 'package.json'))).name
+    );
+    if (!resolvedPackageName) {
+      throw new SfError(errMessage, 'MissingPackageName');
+    }
+    return resolvedPackageName;
+  } catch (err) {
+    throw new SfError('errMessage', 'MissingPackageName', undefined, err as Error);
+  }
+};

src/org/authInfo.ts

@@ -334,10 +334,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
    */
   public static getAuthorizationUrl(options: JwtOAuth2Config & { scope?: string }, oauth2?: OAuth2): string {
     // Unless explicitly turned off, use a code verifier for enhanced security
-    if (options.useVerifier !== false) {
-      options.useVerifier = true;
-    }
-    const oauth2Verifier = oauth2 ?? new OAuth2(options);
+    const oauth2Verifier = oauth2 ?? new OAuth2({ useVerifier: true, ...options });
 
     // The state parameter allows the redirectUri callback listener to ignore request
     // that don't contain the state value.
@@ -613,45 +610,40 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
    * Get the auth fields (decrypted) needed to make a connection.
    */
   public getConnectionOptions(): ConnectionOptions {
-    let opts: ConnectionOptions;
-
     const decryptedCopy = this.getFields(true);
     const { accessToken, instanceUrl, loginUrl } = decryptedCopy;
 
     if (this.isAccessTokenFlow()) {
       this.logger.info('Returning fields for a connection using access token.');
 
       // Just auth with the accessToken
-      opts = { accessToken, instanceUrl, loginUrl };
-    } else if (this.isJwt()) {
+      return { accessToken, instanceUrl, loginUrl };
+    }
+    if (this.isJwt()) {
       this.logger.info('Returning fields for a connection using JWT config.');
-      opts = {
-        accessToken,
-        instanceUrl,
-        refreshFn: this.refreshFn.bind(this),
-      };
-    } else {
-      // @TODO: figure out loginUrl and redirectUri (probably get from config class)
-      //
-      // redirectUri: org.config.getOauthCallbackUrl()
-      // loginUrl: this.fields.instanceUrl || this.config.getAppConfig().sfdcLoginUrl
-      this.logger.info('Returning fields for a connection using OAuth config.');
-
-      // Decrypt a user provided client secret or use the default.
-      opts = {
-        oauth2: {
-          loginUrl: instanceUrl ?? SfdcUrl.PRODUCTION,
-          clientId: this.getClientId(),
-          redirectUri: this.getRedirectUri(),
-        },
+      return {
         accessToken,
         instanceUrl,
         refreshFn: this.refreshFn.bind(this),
       };
     }
+    // @TODO: figure out loginUrl and redirectUri (probably get from config class)
+    //
+    // redirectUri: org.config.getOauthCallbackUrl()
+    // loginUrl: this.fields.instanceUrl || this.config.getAppConfig().sfdcLoginUrl
+    this.logger.info('Returning fields for a connection using OAuth config.');
 
-    // decrypt the fields
-    return opts;
+    // Decrypt a user provided client secret or use the default.
+    return {
+      oauth2: {
+        loginUrl: instanceUrl ?? SfdcUrl.PRODUCTION,
+        clientId: this.getClientId(),
+        redirectUri: this.getRedirectUri(),
+      },
+      accessToken,
+      instanceUrl,
+      refreshFn: this.refreshFn.bind(this),
+    };
   }
 
   public getClientId(): string {
@@ -849,26 +841,26 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
     let authConfig: AuthFields;
 
     if (options) {
-      options = structuredClone(options);
+      const clonedOptions = structuredClone(options);
 
-      if (this.isTokenOptions(options)) {
-        authConfig = options;
+      if (this.isTokenOptions(clonedOptions)) {
+        authConfig = clonedOptions;
         const userInfo = await this.retrieveUserInfo(
-          ensureString(options.instanceUrl),
-          ensureString(options.accessToken)
+          ensureString(clonedOptions.instanceUrl),
+          ensureString(clonedOptions.accessToken)
         );
         this.update({ username: userInfo?.username, orgId: userInfo?.organizationId });
       } else {
         if (this.options.parentUsername) {
           const parentFields = await this.loadDecryptedAuthFromConfig(this.options.parentUsername);
 
-          options.clientId = parentFields.clientId;
+          clonedOptions.clientId = parentFields.clientId;
 
           if (process.env.SFDX_CLIENT_SECRET) {
-            options.clientSecret = process.env.SFDX_CLIENT_SECRET;
+            clonedOptions.clientSecret = process.env.SFDX_CLIENT_SECRET;
           } else {
             // Grab whatever flow is defined
-            Object.assign(options, {
+            Object.assign(clonedOptions, {
               clientSecret: parentFields.clientSecret,
               privateKey: parentFields.privateKey ? pathResolve(parentFields.privateKey) : parentFields.privateKey,
             });
@@ -877,20 +869,20 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
 
         // jwt flow
         // Support both sfdx and jsforce private key values
-        if (!options.privateKey && options.privateKeyFile) {
-          options.privateKey = pathResolve(options.privateKeyFile);
+        if (!clonedOptions.privateKey && clonedOptions.privateKeyFile) {
+          clonedOptions.privateKey = pathResolve(clonedOptions.privateKeyFile);
         }
 
-        if (options.privateKey) {
-          authConfig = await this.authJwt(options);
-        } else if (!options.authCode && options.refreshToken) {
+        if (clonedOptions.privateKey) {
+          authConfig = await this.authJwt(clonedOptions);
+        } else if (!clonedOptions.authCode && clonedOptions.refreshToken) {
           // refresh token flow (from sfdxUrl or OAuth refreshFn)
-          authConfig = await this.buildRefreshTokenConfig(options);
+          authConfig = await this.buildRefreshTokenConfig(clonedOptions);
         } else if (this.options.oauth2 instanceof OAuth2) {
           // authcode exchange / web auth flow
-          authConfig = await this.exchangeToken(options, this.options.oauth2);
+          authConfig = await this.exchangeToken(clonedOptions, this.options.oauth2);
         } else {
-          authConfig = await this.exchangeToken(options);
+          authConfig = await this.exchangeToken(clonedOptions);
         }
       }
 
@@ -1054,21 +1046,20 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
 
   // Build OAuth config for a refresh token auth flow
   private async buildRefreshTokenConfig(options: JwtOAuth2Config): Promise<AuthFields> {
-    // Ideally, this would be removed at some point in the distant future when all auth files
-    // now have the clientId stored in it.
-    if (!options.clientId) {
-      options.clientId = DEFAULT_CONNECTED_APP_INFO.clientId;
-      options.clientSecret = DEFAULT_CONNECTED_APP_INFO.clientSecret;
-    }
-
-    if (!options.redirectUri) {
-      options.redirectUri = this.getRedirectUri();
-    }
+    const fullOptions: JwtOAuth2Config = {
+      ...options,
+      redirectUri: options.redirectUri ?? this.getRedirectUri(),
+      // Ideally, this would be removed at some point in the distant future when all auth files
+      // now have the clientId stored in it.
+      ...(options.clientId
+        ? {}
+        : { clientId: DEFAULT_CONNECTED_APP_INFO.clientId, clientSecret: DEFAULT_CONNECTED_APP_INFO.clientSecret }),
+    };
 
-    const oauth2 = new OAuth2(options);
+    const oauth2 = new OAuth2(fullOptions);
     let authFieldsBuilder: TokenResponse;
     try {
-      authFieldsBuilder = await oauth2.refreshToken(ensure(options.refreshToken));
+      authFieldsBuilder = await oauth2.refreshToken(ensure(fullOptions.refreshToken));
     } catch (err) {
       throw messages.createError('refreshTokenAuthError', [(err as Error).message]);
     }
@@ -1087,10 +1078,10 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
       username,
       accessToken: authFieldsBuilder.access_token,
       instanceUrl: authFieldsBuilder.instance_url,
-      loginUrl: options.loginUrl ?? authFieldsBuilder.instance_url,
-      refreshToken: options.refreshToken,
-      clientId: options.clientId,
-      clientSecret: options.clientSecret,
+      loginUrl: fullOptions.loginUrl ?? authFieldsBuilder.instance_url,
+      refreshToken: fullOptions.refreshToken,
+      clientId: fullOptions.clientId,
+      clientSecret: fullOptions.clientSecret,
     };
   }
 
@@ -1102,9 +1093,11 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
    */
   private async exchangeToken(options: JwtOAuth2Config, oauth2: OAuth2 = new OAuth2(options)): Promise<AuthFields> {
     if (!oauth2.redirectUri) {
+      // eslint-disable-next-line no-param-reassign
       oauth2.redirectUri = this.getRedirectUri();
     }
     if (!oauth2.clientId) {
+      // eslint-disable-next-line no-param-reassign
       oauth2.clientId = this.getClientId();
     }