From 18fbf2797ab5d26a1a2a3b1f7906aab94320acdc Mon Sep 17 00:00:00 2001
From: "Micael Levi L. Cavalcante" <mllc@icomp.ufam.edu.br>
Date: Fri, 1 Sep 2023 09:20:03 -0400
Subject: [PATCH] feat(core): disambiguate string-based providers on error
 message

---
 .../e2e/optional-factory-provider-dep.spec.ts |  8 ++---
 packages/core/errors/messages.ts              | 34 ++++++++++++-------
 .../core/test/errors/test/messages.spec.ts    | 22 ++++++++++++
 3 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/integration/injector/e2e/optional-factory-provider-dep.spec.ts b/integration/injector/e2e/optional-factory-provider-dep.spec.ts
index d82f05136d4..22566f1d4c0 100644
--- a/integration/injector/e2e/optional-factory-provider-dep.spec.ts
+++ b/integration/injector/e2e/optional-factory-provider-dep.spec.ts
@@ -118,14 +118,14 @@ describe('Optional factory provider deps', () => {
       } catch (err) {
         expect(err).to.be.instanceOf(UnknownDependenciesException);
         expect(err.message).to
-          .equal(`Nest can't resolve dependencies of the POSSIBLY_MISSING_DEP (?). Please make sure that the argument MISSING_DEP at index [0] is available in the RootTestModule context.
+          .equal(`Nest can't resolve dependencies of the POSSIBLY_MISSING_DEP (?). Please make sure that the argument "MISSING_DEP" at index [0] is available in the RootTestModule context.
 
 Potential solutions:
 - Is RootTestModule a valid NestJS module?
-- If MISSING_DEP is a provider, is it part of the current RootTestModule?
-- If MISSING_DEP is exported from a separate @Module, is that module imported within RootTestModule?
+- If "MISSING_DEP" is a provider, is it part of the current RootTestModule?
+- If "MISSING_DEP" is exported from a separate @Module, is that module imported within RootTestModule?
   @Module({
-    imports: [ /* the Module containing MISSING_DEP */ ]
+    imports: [ /* the Module containing "MISSING_DEP" */ ]
   })
 `);
       }
diff --git a/packages/core/errors/messages.ts b/packages/core/errors/messages.ts
index ba33e4f8506..0791f2967ea 100644
--- a/packages/core/errors/messages.ts
+++ b/packages/core/errors/messages.ts
@@ -23,20 +23,31 @@ const getInstanceName = (instance: unknown): string => {
 };
 
 /**
- * Returns the name of the dependency
+ * Returns the name of the dependency.
  * Tries to get the class name, otherwise the string value
- * (= injection token). As fallback it returns '+'
+ * (= injection token). As fallback to any falsy value for `dependency`, it
+ * returns `fallbackValue`
  * @param dependency The name of the dependency to be displayed
+ * @param fallbackValue The fallback value if the dependency is falsy
+ * @param disambiguated Whether dependency's name is disambiguated with double quotes
  */
-const getDependencyName = (dependency: InjectorDependency): string =>
+const getDependencyName = (
+  dependency: InjectorDependency | undefined,
+  fallbackValue: string,
+  disambiguated = true,
+): string =>
   // use class name
   getInstanceName(dependency) ||
   // use injection token (symbol)
   (isSymbol(dependency) && dependency.toString()) ||
   // use string directly
-  (dependency as string) ||
+  (dependency
+    ? disambiguated
+      ? `"${dependency as string}"`
+      : (dependency as string)
+    : undefined) ||
   // otherwise
-  '+';
+  fallbackValue;
 
 /**
  * Returns the name of the module
@@ -54,14 +65,9 @@ export const UNKNOWN_DEPENDENCIES_MESSAGE = (
   unknownDependencyContext: InjectorDependencyContext,
   module: Module,
 ) => {
-  const {
-    index,
-    name = 'dependency',
-    dependencies,
-    key,
-  } = unknownDependencyContext;
+  const { index, name, dependencies, key } = unknownDependencyContext;
   const moduleName = getModuleName(module);
-  const dependencyName = getDependencyName(name);
+  const dependencyName = getDependencyName(name, 'dependency');
 
   const potentialSolutions =
     // If module's name is well defined
@@ -90,7 +96,9 @@ Potential solutions:
     message += `. Please make sure that the "${key.toString()}" property is available in the current context.${potentialSolutions}`;
     return message;
   }
-  const dependenciesName = (dependencies || []).map(getDependencyName);
+  const dependenciesName = (dependencies || []).map(dependencyName =>
+    getDependencyName(dependencyName, '+', false),
+  );
   dependenciesName[index] = '?';
 
   message += ` (`;
diff --git a/packages/core/test/errors/test/messages.spec.ts b/packages/core/test/errors/test/messages.spec.ts
index de51aa101fd..df4a74c5b26 100644
--- a/packages/core/test/errors/test/messages.spec.ts
+++ b/packages/core/test/errors/test/messages.spec.ts
@@ -57,6 +57,28 @@ describe('Error Messages', () => {
 
       expect(actualMessage).to.equal(expectedResult);
     });
+    it('should display the provide token as double-quoted string for string-based tokens', () => {
+      const expectedResult =
+        stringCleaner(`Nest can't resolve dependencies of the CatService (?). Please make sure that the argument "FooRepository" at index [0] is available in the current context.
+
+      Potential solutions:
+      - If "FooRepository" is a provider, is it part of the current Module?
+      - If "FooRepository" is exported from a separate @Module, is that module imported within Module?
+      @Module({
+      imports: [ /* the Module containing "FooRepository" */ ]
+      })
+      `);
+
+      const actualMessage = stringCleaner(
+        new UnknownDependenciesException('CatService', {
+          index: 0,
+          dependencies: ['FooRepository'],
+          name: 'FooRepository',
+        }).message,
+      );
+
+      expect(actualMessage).to.equal(expectedResult);
+    });
     it('should display the function name', () => {
       const expectedResult =
         stringCleaner(`Nest can't resolve dependencies of the CatService (?, CatFunction). Please make sure that the argument dependency at index [0] is available in the current context.