feat: offer suggestions for unresolved metadata types (#948)

* fix: offer suggestions for unresolved metadata types * chore: bump core * Sm/pr feedback 948 (#953) * fix: ut and better answers for case errors * test: another case guess test * fix: pr feedback * chore: bump core * chore: graceful-fs types * fix: better manifest detection * chore: better manifest detection * fix: better meta suffix guesses * fix: filename lookup * fix: guess nuts --------- Co-authored-by: Shane McLaughlin <[email protected]>
forcedotcom · May 8, 2023 · c4633b2 · c4633b2
1 parent 1e5309a
commit c4633b2
Show file tree

Hide file tree

Showing 23 changed files with 624 additions and 12 deletions.
diff --git a/messages/sdr.md b/messages/sdr.md
@@ -157,3 +157,17 @@ No uniqueIdElement found in registry for %s (reading %s at %s).
 # uniqueIdElementNotInChild
 
 The uniqueIdElement %s was not found the child (reading %s at %s).
+
+# suggest_type_header
+
+A metadata type lookup for "%s" found the following close matches:
+
+# suggest_type_did_you_mean
+
+-- Did you mean ".%s%s" instead for the "%s" metadata type?
+
+# suggest_type_more_suggestions
+
+Additional suggestions:
+Confirm the file name, extension, and directory names are correct. Validate against the registry at:
+https://github.com/forcedotcom/source-deploy-retrieve/blob/main/src/registry/metadataRegistry.json
diff --git a/package.json b/package.json
@@ -29,6 +29,7 @@
     "@salesforce/kit": "^1.9.2",
     "@salesforce/ts-types": "^1.7.2",
     "archiver": "^5.3.1",
+    "fast-levenshtein": "^3.0.0",
     "fast-xml-parser": "^4.1.4",
     "got": "^11.8.6",
     "graceful-fs": "^4.2.11",
@@ -47,6 +48,7 @@
     "@salesforce/ts-sinon": "^1.4.6",
     "@types/archiver": "^5.3.1",
     "@types/deep-equal-in-any-order": "^1.0.1",
+    "@types/fast-levenshtein": "^0.0.2",
     "@types/graceful-fs": "^4.1.6",
     "@types/mime": "2.0.3",
     "@types/minimatch": "^5.1.2",
@@ -101,6 +103,7 @@
     "test": "wireit",
     "test:nuts": "mocha \"test/nuts/local/**/*.nut.ts\" --timeout 500000",
     "test:nuts:scale": "mocha \"test/nuts/scale/eda.nut.ts\" --timeout 500000; mocha \"test/nuts/scale/lotsOfClasses.nut.ts\" --timeout 500000; mocha \"test/nuts/scale/lotsOfClassesOneDir.nut.ts\" --timeout 500000",
+    "test:nuts:suggest": "mocha \"test/nuts/suggestType/suggestType.nut.ts\" --timeout 10000",
     "test:only": "wireit",
     "test:registry": "mocha ./test/registry/registryCompleteness.test.ts --timeout 50000",
     "update-registry": "npx ts-node scripts/update-registry/update2.ts",
@@ -188,4 +191,4 @@
       "output": []
     }
   }
-}
+}
diff --git a/src/client/metadataTransfer.ts b/src/client/metadataTransfer.ts
@@ -162,13 +162,13 @@ export abstract class MetadataTransfer<
   }
 
   protected async maybeSaveTempDirectory(target: SfdxFileFormat, cs?: ComponentSet): Promise<void> {
-    const mdapiTempDir = process.env.SFDX_MDAPI_TEMP_DIR;
+    const mdapiTempDir = process.env.SF_MDAPI_TEMP_DIR;
     if (mdapiTempDir) {
       await Lifecycle.getInstance().emitWarning(
-        'The SFDX_MDAPI_TEMP_DIR environment variable is set, which may degrade performance'
+        'The SF_MDAPI_TEMP_DIR environment variable is set, which may degrade performance'
       );
       this.logger.debug(
-        `Converting metadata to: ${mdapiTempDir} because the SFDX_MDAPI_TEMP_DIR environment variable is set`
+        `Converting metadata to: ${mdapiTempDir} because the SF_MDAPI_TEMP_DIR environment variable is set`
       );
       try {
         const source = cs ?? this.components ?? new ComponentSet();

diff --git a/src/registry/registryAccess.ts b/src/registry/registryAccess.ts
@@ -5,6 +5,7 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 import { Messages, SfError } from '@salesforce/core';
+import * as Levenshtein from 'fast-levenshtein';
 import { registry as defaultRegistry } from './registry';
 import { MetadataRegistry, MetadataType } from './types';
 
@@ -60,12 +61,44 @@ export class RegistryAccess {
    * @returns The corresponding metadata type object
    */
   public getTypeBySuffix(suffix: string): MetadataType | undefined {
-    if (this.registry.suffixes?.[suffix]) {
+    if (this.registry.suffixes[suffix]) {
       const typeId = this.registry.suffixes[suffix];
       return this.getTypeByName(typeId);
     }
   }
 
+  /**
+   * Find similar metadata type matches by its file suffix
+   *
+   * @param suffix - File suffix of the metadata type
+   * @returns An array of similar suffix and metadata type matches
+   */
+  public guessTypeBySuffix(
+    suffix: string
+  ): Array<{ suffixGuess: string; metadataTypeGuess: MetadataType }> | undefined {
+    const registryKeys = Object.keys(this.registry.suffixes);
+
+    const scores = registryKeys.map((registryKey) => ({
+      registryKey,
+      score: Levenshtein.get(suffix, registryKey, { useCollator: true }),
+    }));
+    const sortedScores = scores.sort((a, b) => a.score - b.score);
+    const lowestScore = sortedScores[0].score;
+    // Levenshtein uses positive integers for scores, find all scores that match the lowest score
+    const guesses = sortedScores.filter((score) => score.score === lowestScore);
+
+    if (guesses.length > 0) {
+      return guesses.map((guess) => {
+        const typeId = this.registry.suffixes[guess.registryKey];
+        const metadataType = this.getTypeByName(typeId);
+        return {
+          suffixGuess: guess.registryKey,
+          metadataTypeGuess: metadataType,
+        };
+      });
+    }
+  }
+
   /**
    * Searches for the first metadata type in the registry that returns `true`
    * for the given predicate function.

diff --git a/src/registry/types.ts b/src/registry/types.ts
@@ -11,7 +11,7 @@
  */
 export interface MetadataRegistry {
   types: TypeIndex;
-  suffixes?: SuffixIndex;
+  suffixes: SuffixIndex;
   strictDirectoryNames: {
     [directoryName: string]: string;
   };

diff --git a/src/resolve/metadataResolver.ts b/src/resolve/metadataResolver.ts
@@ -5,7 +5,7 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 import { basename, dirname, join, sep } from 'path';
-import { Lifecycle, Messages, SfError } from '@salesforce/core';
+import { Lifecycle, Messages, SfError, Logger } from '@salesforce/core';
 import { extName, parentName, parseMetadataXml } from '../utils';
 import { MetadataType, RegistryAccess } from '../registry';
 import { ComponentSet } from '../collections';
@@ -25,6 +25,7 @@ const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sd
  */
 export class MetadataResolver {
   public forceIgnoredPaths: Set<string>;
+  protected logger: Logger;
   private forceIgnore?: ForceIgnore;
   private sourceAdapterFactory: SourceAdapterFactory;
   private folderContentTypeDirNames?: string[];
@@ -38,6 +39,7 @@ export class MetadataResolver {
     private tree: TreeContainer = new NodeFSTreeContainer(),
     private useFsForceIgnore = true
   ) {
+    this.logger = Logger.childFromRoot(this.constructor.name);
     this.sourceAdapterFactory = new SourceAdapterFactory(this.registry, tree);
     this.forceIgnoredPaths = new Set<string>();
   }
@@ -139,13 +141,39 @@ export class MetadataResolver {
         !adapter.allowMetadataWithContent();
       return shouldResolve ? adapter.getComponent(fsPath, isResolvingSource) : undefined;
     }
+
+    // Perform some additional checks to see if this is a package manifest
+    if (fsPath.endsWith('.xml') && !fsPath.endsWith(META_XML_SUFFIX)) {
+      // If it is named the default package.xml, assume it is a package manifest
+      if (fsPath.endsWith('package.xml')) return undefined;
+      try {
+        // If the file contains the string "<Package xmlns", it is a package manifest
+        if (this.tree.readFileSync(fsPath).toString().includes('<Package xmlns')) return undefined;
+      } catch (err) {
+        const error = err as Error;
+        if (error.message === 'Method not implemented') {
+          // Currently readFileSync is not implemented for zipTreeContainer
+          // Ignoring since this would have been ignored in the past
+          this.logger.warn(
+            `Type could not be inferred for ${fsPath}. It is likely this is a package manifest. Skipping...`
+          );
+          return undefined;
+        }
+      }
+    }
+
     void Lifecycle.getInstance().emitTelemetry({
       eventName: 'metadata_resolver_type_inference_error',
       library: 'SDR',
       function: 'resolveComponent',
       path: fsPath,
     });
-    throw new SfError(messages.getMessage('error_could_not_infer_type', [fsPath]), 'TypeInferenceError');
+
+    // The metadata type could not be inferred
+    // Attempt to guess the type and throw an error with actions
+    const actions = this.getSuggestionsForUnresolvedTypes(fsPath);
+
+    throw new SfError(messages.getMessage('error_could_not_infer_type', [fsPath]), 'TypeInferenceError', actions);
   }
 
   private resolveTypeFromStrictFolder(fsPath: string): MetadataType | undefined {
@@ -202,6 +230,15 @@ export class MetadataResolver {
     // attempt 3 - try treating the file extension name as a suffix
     if (!resolvedType) {
       resolvedType = this.registry.getTypeBySuffix(extName(fsPath));
+
+      // Metadata types with `strictDirectoryName` should have been caught in "attempt 1".
+      // If the metadata returned from this lookup has a `strictDirectoryName`, something is wrong.
+      // It is likely that the metadata file is misspelled or has the wrong suffix.
+      // A common occurrence is that a misspelled metadata file will fall back to
+      // `EmailServicesFunction` because that is the default for the `.xml` suffix
+      if (resolvedType?.strictDirectoryName === true) {
+        resolvedType = undefined;
+      }
     }
 
     // attempt 4 - try treating the content as metadata
@@ -215,6 +252,47 @@ export class MetadataResolver {
     return resolvedType;
   }
 
+  /**
+   * Attempt to find similar types for types that could not be inferred
+   * To be used after executing the resolveType() method
+   *
+   * @param fsPath
+   * @returns an array of suggestions
+   */
+  private getSuggestionsForUnresolvedTypes(fsPath: string): string[] {
+    const parsedMetaXml = parseMetadataXml(fsPath);
+    const metaSuffix = parsedMetaXml?.suffix;
+    // Finds close matches for meta suffixes
+    // Examples: https://regex101.com/r/vbRjwy/1
+    const closeMetaSuffix = new RegExp(/.+\.([^.-]+)(?:-.*)?\.xml/).exec(basename(fsPath));
+
+    let guesses;
+
+    if (metaSuffix) {
+      guesses = this.registry.guessTypeBySuffix(metaSuffix)
+    } else if (!metaSuffix && closeMetaSuffix) {
+      guesses = this.registry.guessTypeBySuffix(closeMetaSuffix[1]);
+    } else {
+      guesses = this.registry.guessTypeBySuffix(extName(fsPath));
+    }
+
+    // If guesses were found, format an array of strings to be passed to SfError's actions
+    return guesses && guesses.length > 0
+      ? [
+          messages.getMessage('suggest_type_header', [ basename(fsPath) ]),
+          ...guesses.map((guess) =>
+            messages.getMessage('suggest_type_did_you_mean', [
+              guess.suffixGuess,
+              (metaSuffix || closeMetaSuffix) ? '-meta.xml' : '',
+              guess.metadataTypeGuess.name,
+            ])
+          ),
+          '', // A blank line makes this much easier to read (it doesn't seem to be possible to start a markdown message entry with a newline)
+          messages.getMessage('suggest_type_more_suggestions'),
+        ]
+      : [];
+  }
+
   /**
    * Whether or not a directory that represents a single component should be resolved as one,
    * or if it should be walked for additional components.

diff --git a/test/client/metadataApiDeploy.test.ts b/test/client/metadataApiDeploy.test.ts
@@ -82,7 +82,7 @@ describe('MetadataApiDeploy', () => {
 
       it('should save the temp directory if the environment variable is set', async () => {
         try {
-          process.env.SFDX_MDAPI_TEMP_DIR = 'test';
+          process.env.SF_MDAPI_TEMP_DIR = 'test';
           const components = new ComponentSet([matchingContentFile.COMPONENT]);
           const { operation, convertStub, deployStub } = await stubMetadataDeploy($$, testOrg, {
             components,
@@ -95,7 +95,7 @@ describe('MetadataApiDeploy', () => {
           expect(deployStub.firstCall.args[0]).to.equal(zipBuffer);
           expect(getString(convertStub.secondCall.args[2], 'outputDirectory', '')).to.equal('test');
         } finally {
-          delete process.env.SFDX_MDAPI_TEMP_DIR;
+          delete process.env.SF_MDAPI_TEMP_DIR;
         }
       });
 

diff --git a/test/client/metadataApiRetrieve.test.ts b/test/client/metadataApiRetrieve.test.ts
@@ -303,7 +303,7 @@ describe('MetadataApiRetrieve', () => {
 
       it('should save the temp directory if the environment variable is set', async () => {
         try {
-          process.env.SFDX_MDAPI_TEMP_DIR = 'test';
+          process.env.SF_MDAPI_TEMP_DIR = 'test';
           const toRetrieve = new ComponentSet([COMPONENT]);
           const { operation, convertStub } = await stubMetadataRetrieve($$, testOrg, {
             toRetrieve,
@@ -317,7 +317,7 @@ describe('MetadataApiRetrieve', () => {
 
           expect(getString(convertStub.secondCall.args[2], 'outputDirectory', '')).to.equal('test');
         } finally {
-          delete process.env.SFDX_MDAPI_TEMP_DIR;
+          delete process.env.SF_MDAPI_TEMP_DIR;
         }
       });