feat: absolute paths for string replacements for out-of-project (#1239)

* feat: absolute paths for string replacements for out-of-project

* test: refactor zip extraction

* feat: componentSetBuilder sets projectDirectory

* fix: tests use relative paths for file replacements, tighter customLabels exception

* fix: registry bug for affinityScoreDefinition

* refactor: variable rename for readability

* test: more ut for whenEnv and absolute files

src/collections/componentSetBuilder.ts

@@ -154,6 +154,7 @@ export class ComponentSetBuilder {
     componentSet = assertComponentSetIsNotUndefined(componentSet);
     componentSet.apiVersion ??= apiversion;
     componentSet.sourceApiVersion ??= sourceapiversion;
+    componentSet.projectDirectory = projectDir;
 
     logComponents(logger, componentSet);
     return componentSet;

src/convert/replacements.ts

@@ -6,7 +6,7 @@
  */
 import { readFile } from 'node:fs/promises';
 import { Transform, Readable } from 'node:stream';
-import { sep, posix } from 'node:path';
+import { sep, posix, join, isAbsolute } from 'node:path';
 import { Lifecycle, Messages, SfError, SfProject } from '@salesforce/core';
 import * as minimatch from 'minimatch';
 import { Env } from '@salesforce/kit';
@@ -91,10 +91,8 @@ export const getReplacementMarkingStream = async (
   projectDir?: string
 ): Promise<ReplacementMarkingStream | undefined> => {
   // remove any that don't agree with current env
-  const filteredReplacements = envFilter(await readReplacementsFromProject(projectDir));
-  if (filteredReplacements.length) {
-    return new ReplacementMarkingStream(filteredReplacements);
-  }
+  const filteredReplacements = (await readReplacementsFromProject(projectDir)).filter(envFilter);
+  return filteredReplacements.length ? new ReplacementMarkingStream(filteredReplacements) : undefined;
 };
 
 /**
@@ -117,8 +115,8 @@ class ReplacementMarkingStream extends Transform {
     if (!chunk.isMarkedForDelete() && this.replacementConfigs?.length) {
       try {
         chunk.replacements = await getReplacements(chunk, this.replacementConfigs);
-        if (chunk.replacements && chunk.parent && chunk.type.name === 'CustomLabel') {
-          // Set replacements on the parent of a CustomLabel as well so that recomposing
+        if (chunk.replacements && chunk.parent?.type.strategies?.transformer === 'nonDecomposed') {
+          // Set replacements on the parent of a nonDecomposed CustomLabel as well so that recomposing
           // doesn't use the non-replaced content from parent cache.
           // See RecompositionFinalizer.recompose() in convertContext.ts
           chunk.parent.replacements = chunk.replacements;
@@ -168,7 +166,7 @@ export const getReplacements = async (
           await Promise.all(
             replacementConfigs
               // filter out any that don't match the current file
-              .filter((r) => matchesFile(f, r))
+              .filter(matchesFile(f))
               .map(async (r) => ({
                 matchedFilename: f,
                 // used during replacement stream to limit warnings to explicit filenames, not globs
@@ -192,27 +190,26 @@ export const getReplacements = async (
     // filter out any that don't have any replacements
     .filter(([, replacements]) => replacements.length > 0);
 
-  if (replacementsForComponent.length) {
-    // turn into a Dictionary-style object so it's easier to lookup by filename
-    return Object.fromEntries(replacementsForComponent);
-  }
+  // turn into a Dictionary-style object so it's easier to lookup by filename
+  return replacementsForComponent.length ? Object.fromEntries(replacementsForComponent) : undefined;
 };
 
-export const matchesFile = (f: string, r: ReplacementConfig): boolean =>
-  // filenames will be absolute.  We don't have convenient access to the pkgDirs,
-  // so we need to be more open than an exact match
-  (typeof r.filename === 'string' && posixifyPaths(f).endsWith(r.filename)) ||
-  (typeof r.glob === 'string' && minimatch(f, `**/${r.glob}`));
+export const matchesFile =
+  (filename: string) =>
+  (r: ReplacementConfig): boolean =>
+    // filenames will be absolute.  We don't have convenient access to the pkgDirs,
+    // so we need to be more open than an exact match
+    (typeof r.filename === 'string' && posixifyPaths(filename).endsWith(r.filename)) ||
+    (typeof r.glob === 'string' && minimatch(filename, `**/${r.glob}`));
 
 /**
  * Regardless of any components, return the ReplacementConfig that are valid with the current env.
  * These can be checked globally and don't need to be checked per component.
  */
-const envFilter = (replacementConfigs: ReplacementConfig[] = []): ReplacementConfig[] =>
-  replacementConfigs.filter(
-    (replacement) =>
-      !replacement.replaceWhenEnv ||
-      replacement.replaceWhenEnv.every((envConditional) => process.env[envConditional.env] === envConditional.value)
+export const envFilter = (replacement: ReplacementConfig): boolean =>
+  !replacement.replaceWhenEnv ||
+  replacement.replaceWhenEnv.every(
+    (envConditional) => process.env[envConditional.env] === envConditional.value.toString()
   );
 
 /** A "getter" for envs to throw an error when an expected env is not present */
@@ -231,8 +228,8 @@ const readReplacementsFromProject = async (projectDir?: string): Promise<Replace
   try {
     const proj = await SfProject.resolve(projectDir);
     const projJson = (await proj.resolveProjectConfig()) as { replacements?: ReplacementConfig[] };
-
-    return projJson.replacements ?? [];
+    const definiteProjectDir = proj.getPath();
+    return (projJson.replacements ?? []).map(makeAbsolute(definiteProjectDir));
   } catch (e) {
     if (e instanceof SfError && e.name === 'InvalidProjectWorkspaceError') {
       return [];
@@ -249,3 +246,17 @@ export const stringToRegex = (input: string): RegExp =>
   new RegExp(input.replace(/[-\/\\^$*+?.()|[\]{}]/g, '\\$&'), 'g');
 
 export const posixifyPaths = (f: string): string => f.split(sep).join(posix.sep);
+
+/** if replaceWithFile is present, resolve it to an absolute path relative to the projectdir */
+const makeAbsolute =
+  (projectDir: string) =>
+  (replacementConfig: ReplacementConfig): ReplacementConfig =>
+    replacementConfig.replaceWithFile
+      ? {
+          ...replacementConfig,
+          // it could already be absolute?
+          replaceWithFile: isAbsolute(replacementConfig.replaceWithFile)
+            ? replacementConfig.replaceWithFile
+            : join(projectDir, replacementConfig.replaceWithFile),
+        }
+      : replacementConfig;

test/convert/replacements.test.ts

@@ -14,6 +14,7 @@ import {
   replacementIterations,
   stringToRegex,
   posixifyPaths,
+  envFilter,
 } from '../../src/convert/replacements';
 import { matchingContentFile } from '../mock';
 import * as replacementsForMock from '../../src/convert/replacements';
@@ -23,29 +24,111 @@ config.truncateThreshold = 0;
 describe('file matching', () => {
   const base = { replaceWithEnv: 'foo', stringToReplace: 'foo' };
   it('file matches string', () => {
-    expect(matchesFile('foo', { filename: 'foo', ...base })).to.be.true;
-    expect(matchesFile('bar', { filename: 'foo', ...base })).to.not.be.true;
+    expect(matchesFile('foo')({ filename: 'foo', ...base })).to.be.true;
+    expect(matchesFile('bar')({ filename: 'foo', ...base })).to.not.be.true;
   });
   it('paths with separators to cover possibility of windows paths', () => {
-    expect(matchesFile(path.join('foo', 'bar'), { filename: 'foo/bar', ...base })).to.be.true;
-    expect(matchesFile(path.join('foo', 'bar'), { filename: 'foo/baz', ...base })).to.not.be.true;
+    const fn = matchesFile(path.join('foo', 'bar'));
+    expect(fn({ filename: 'foo/bar', ...base })).to.be.true;
+    expect(fn({ filename: 'foo/baz', ...base })).to.not.be.true;
   });
   it('file matches glob (posix paths)', () => {
-    expect(matchesFile('foo/bar', { glob: 'foo/**', ...base })).to.be.true;
-    expect(matchesFile('foo/bar', { glob: 'foo/*', ...base })).to.be.true;
-    expect(matchesFile('foo/bar', { glob: 'foo', ...base })).to.be.false;
-    expect(matchesFile('foo/bar', { glob: '**/*', ...base })).to.be.true;
+    const fn = matchesFile(path.join('foo', 'bar'));
+
+    expect(fn({ glob: 'foo/**', ...base })).to.be.true;
+    expect(fn({ glob: 'foo/*', ...base })).to.be.true;
+    expect(fn({ glob: 'foo', ...base })).to.be.false;
+    expect(fn({ glob: '**/*', ...base })).to.be.true;
   });
   it('file matches glob (os-dependent paths)', () => {
-    expect(matchesFile(path.join('foo', 'bar'), { glob: 'foo/**', ...base })).to.be.true;
-    expect(matchesFile(path.join('foo', 'bar'), { glob: 'foo/*', ...base })).to.be.true;
-    expect(matchesFile(path.join('foo', 'bar'), { glob: 'foo', ...base })).to.be.false;
-    expect(matchesFile(path.join('foo', 'bar'), { glob: '**/*', ...base })).to.be.true;
+    const fn = matchesFile(path.join('foo', 'bar'));
+    expect(fn({ glob: 'foo/**', ...base })).to.be.true;
+    expect(fn({ glob: 'foo/*', ...base })).to.be.true;
+    expect(fn({ glob: 'foo', ...base })).to.be.false;
+    expect(fn({ glob: '**/*', ...base })).to.be.true;
+  });
+  it('test absolute vs. relative paths', () => {
+    const fn = matchesFile(path.join('/Usr', 'me', 'foo', 'bar'));
+    expect(fn({ glob: 'foo/**', ...base })).to.be.true;
+    expect(fn({ glob: 'foo/*', ...base })).to.be.true;
+    expect(fn({ glob: 'foo', ...base })).to.be.false;
+    expect(fn({ glob: '**/*', ...base })).to.be.true;
   });
-  it('test absolute vs. relative paths');
 });
 
-describe('env filters', () => {});
+describe('env filters', () => {
+  beforeEach(() => {
+    process.env.SHOULD_REPLACE_FOO = undefined;
+  });
+  it('true when not replaceWhenEnv', () => {
+    expect(envFilter({ stringToReplace: 'foo', filename: '*', replaceWithFile: '/some/file' })).to.equal(true);
+  });
+  it('true when env is set and value matches string', () => {
+    process.env.SHOULD_REPLACE_FOO = 'x';
+    expect(
+      envFilter({
+        stringToReplace: 'foo',
+        filename: '*',
+        replaceWithFile: '/some/file',
+        replaceWhenEnv: [{ env: 'SHOULD_REPLACE_FOO', value: 'x' }],
+      })
+    ).to.equal(true);
+  });
+  it('true when env is set and value matches boolean', () => {
+    process.env.SHOULD_REPLACE_FOO = 'true';
+    expect(
+      envFilter({
+        stringToReplace: 'foo',
+        filename: '*',
+        replaceWithFile: '/some/file',
+        replaceWhenEnv: [{ env: 'SHOULD_REPLACE_FOO', value: true }],
+      })
+    ).to.equal(true);
+  });
+  it('true when env is set and value matches number', () => {
+    process.env.SHOULD_REPLACE_FOO = '6';
+    expect(
+      envFilter({
+        stringToReplace: 'foo',
+        filename: '*',
+        replaceWithFile: '/some/file',
+        replaceWhenEnv: [{ env: 'SHOULD_REPLACE_FOO', value: 6 }],
+      })
+    ).to.equal(true);
+  });
+  it('false when env is set and does not match number', () => {
+    process.env.SHOULD_REPLACE_FOO = '6';
+    expect(
+      envFilter({
+        stringToReplace: 'foo',
+        filename: '*',
+        replaceWithFile: '/some/file',
+        replaceWhenEnv: [{ env: 'SHOULD_REPLACE_FOO', value: 7 }],
+      })
+    ).to.equal(false);
+  });
+  it('false when env is set and does not match string', () => {
+    process.env.SHOULD_REPLACE_FOO = 'x';
+    expect(
+      envFilter({
+        stringToReplace: 'foo',
+        filename: '*',
+        replaceWithFile: '/some/file',
+        replaceWhenEnv: [{ env: 'SHOULD_REPLACE_FOO', value: 'y' }],
+      })
+    ).to.equal(false);
+  });
+  it('false when env is not set', () => {
+    expect(
+      envFilter({
+        stringToReplace: 'foo',
+        filename: '*',
+        replaceWithFile: '/some/file',
+        replaceWhenEnv: [{ env: 'SHOULD_REPLACE_FOO', value: 'true' }],
+      })
+    ).to.equal(false);
+  });
+});
 
 describe('marking replacements on a component', () => {
   before(() => {

test/nuts/local/replacements/extractZip.ts

@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2023, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import * as JSZip from 'jszip';
+
+export const extractZip = async (zipBuffer: Buffer, extractPath: string) => {
+  fs.mkdirSync(extractPath);
+  const zip = await JSZip.loadAsync(zipBuffer);
+  for (const filePath of Object.keys(zip.files)) {
+    const zipObj = zip.file(filePath);
+    if (!zipObj || zipObj?.dir) {
+      fs.mkdirSync(path.join(extractPath, filePath));
+    } else {
+      // eslint-disable-next-line no-await-in-loop
+      const content = await zipObj?.async('nodebuffer');
+      if (content) {
+        fs.writeFileSync(path.join(extractPath, filePath), content);
+      }
+    }
+  }
+};

test/nuts/local/replacements/replacements.nut.ts

@@ -10,27 +10,11 @@ import * as JSZip from 'jszip';
 import { TestSession } from '@salesforce/cli-plugins-testkit';
 import { assert, expect } from 'chai';
 import { ComponentSetBuilder, MetadataConverter } from '../../../../src';
+import { extractZip } from './extractZip';
 
 describe('e2e replacements test', () => {
   let session: TestSession;
 
-  const extractZip = async (zipBuffer: Buffer, extractPath: string) => {
-    fs.mkdirSync(extractPath);
-    const zip = await JSZip.loadAsync(zipBuffer);
-    for (const filePath of Object.keys(zip.files)) {
-      const zipObj = zip.file(filePath);
-      if (!zipObj || zipObj?.dir) {
-        fs.mkdirSync(path.join(extractPath, filePath));
-      } else {
-        // eslint-disable-next-line no-await-in-loop
-        const content = await zipObj?.async('nodebuffer');
-        if (content) {
-          fs.writeFileSync(path.join(extractPath, filePath), content);
-        }
-      }
-    }
-  };
-
   before(async () => {
     session = await TestSession.create({
       project: {
@@ -42,16 +26,7 @@ describe('e2e replacements test', () => {
     // Hack: rewrite the file replacement locations relative to the project
     const projectJsonPath = path.join(session.project.dir, 'sfdx-project.json');
     const original = await fs.promises.readFile(projectJsonPath, 'utf8');
-    await fs.promises.writeFile(
-      projectJsonPath,
-      original
-        // we're putting this in a json file which doesnt like windows backslashes.  The file will require posix paths
-        .replace(
-          'replacements.txt',
-          path.join(session.project.dir, 'replacements.txt').split(path.sep).join(path.posix.sep)
-        )
-        .replace('label.txt', path.join(session.project.dir, 'label.txt').split(path.sep).join(path.posix.sep))
-    );
+    await fs.promises.writeFile(projectJsonPath, original);
   });
 
   after(async () => {

test/nuts/local/replacements/replacementsLabels.nut.ts

@@ -10,27 +10,11 @@ import * as JSZip from 'jszip';
 import { TestSession } from '@salesforce/cli-plugins-testkit';
 import { assert, expect } from 'chai';
 import { ComponentSetBuilder, MetadataConverter } from '../../../../src';
+import { extractZip } from './extractZip';
 
 describe('e2e replacements test (customLabels)', () => {
   let session: TestSession;
 
-  const extractZip = async (zipBuffer: Buffer, extractPath: string) => {
-    fs.mkdirSync(extractPath);
-    const zip = await JSZip.loadAsync(zipBuffer);
-    for (const filePath of Object.keys(zip.files)) {
-      const zipObj = zip.file(filePath);
-      if (!zipObj || zipObj?.dir) {
-        fs.mkdirSync(path.join(extractPath, filePath));
-      } else {
-        // eslint-disable-next-line no-await-in-loop
-        const content = await zipObj?.async('nodebuffer');
-        if (content) {
-          fs.writeFileSync(path.join(extractPath, filePath), content);
-        }
-      }
-    }
-  };
-
   before(async () => {
     session = await TestSession.create({
       project: {
@@ -41,16 +25,7 @@ describe('e2e replacements test (customLabels)', () => {
     // Hack: rewrite the file replacement locations relative to the project
     const projectJsonPath = path.join(session.project.dir, 'sfdx-project.json');
     const original = await fs.promises.readFile(projectJsonPath, 'utf8');
-    await fs.promises.writeFile(
-      projectJsonPath,
-      original
-        // we're putting this in a json file which doesnt like windows backslashes.  The file will require posix paths
-        .replace(
-          'replacements.txt',
-          path.join(session.project.dir, 'replacements.txt').split(path.sep).join(path.posix.sep)
-        )
-        .replace('label.txt', path.join(session.project.dir, 'label.txt').split(path.sep).join(path.posix.sep))
-    );
+    await fs.promises.writeFile(projectJsonPath, original);
   });
 
   after(async () => {