From b630b09c7ed8c55ea010b0ea588327c5e2dcde2b Mon Sep 17 00:00:00 2001
From: Pete Bacon Darwin <pete@bacondarwin.com>
Date: Mon, 25 Jan 2021 21:26:06 +0000
Subject: [PATCH] fix(compiler-cli): use `Map` rather than `object` for map of
 partial linkers (#40563)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, we were naïvely checking whether a function name was a partial linker
declaration call by testing the map of linkers with `linkers[name]`. Since
`linkers` was a plain object, it also matched function names like `toString`!

This has been refactored as a `Map` to avoid the problem.

PR Close #40563
---
 .../partial_linker_selector.ts                | 35 +++++++++++--------
 .../partial_linker_selector_spec.ts           |  6 ++++
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_linker_selector.ts b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_linker_selector.ts
index 81e494ecf5..461e2522f9 100644
--- a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_linker_selector.ts
+++ b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_linker_selector.ts
@@ -19,6 +19,11 @@ export const ɵɵngDeclareDirective = 'ɵɵngDeclareDirective';
 export const ɵɵngDeclareComponent = 'ɵɵngDeclareComponent';
 export const declarationFunctions = [ɵɵngDeclareDirective, ɵɵngDeclareComponent];
 
+interface LinkerRange<TExpression> {
+  range: string;
+  linker: PartialLinker<TExpression>;
+}
+
 /**
  * A helper that selects the appropriate `PartialLinker` for a given declaration.
  *
@@ -35,7 +40,7 @@ export const declarationFunctions = [ɵɵngDeclareDirective, ɵɵngDeclareCompon
  * allows the linker to work on local builds effectively.
  */
 export class PartialLinkerSelector<TStatement, TExpression> {
-  private readonly linkers: Record<string, {range: string, linker: PartialLinker<TExpression>}[]>;
+  private readonly linkers: Map<string, LinkerRange<TExpression>[]>;
 
   constructor(
       environment: LinkerEnvironment<TStatement, TExpression>, sourceUrl: AbsoluteFsPath,
@@ -47,7 +52,7 @@ export class PartialLinkerSelector<TStatement, TExpression> {
    * Returns true if there are `PartialLinker` classes that can handle functions with this name.
    */
   supportsDeclaration(functionName: string): boolean {
-    return this.linkers[functionName] !== undefined;
+    return this.linkers.has(functionName);
   }
 
   /**
@@ -55,10 +60,10 @@ export class PartialLinkerSelector<TStatement, TExpression> {
    * Throws an error if there is none.
    */
   getLinker(functionName: string, version: string): PartialLinker<TExpression> {
-    const versions = this.linkers[functionName];
-    if (versions === undefined) {
+    if (!this.linkers.has(functionName)) {
       throw new Error(`Unknown partial declaration function ${functionName}.`);
     }
+    const versions = this.linkers.get(functionName)!;
     for (const {range, linker} of versions) {
       if (satisfies(version, range, {includePrerelease: true})) {
         return linker;
@@ -71,21 +76,21 @@ export class PartialLinkerSelector<TStatement, TExpression> {
 
   private createLinkerMap(
       environment: LinkerEnvironment<TStatement, TExpression>, sourceUrl: AbsoluteFsPath,
-      code: string) {
+      code: string): Map<string, LinkerRange<TExpression>[]> {
     const partialDirectiveLinkerVersion1 = new PartialDirectiveLinkerVersion1(sourceUrl, code);
     const partialComponentLinkerVersion1 = new PartialComponentLinkerVersion1(
         environment, createGetSourceFile(sourceUrl, code, environment.sourceFileLoader), sourceUrl,
         code);
 
-    return {
-      [ɵɵngDeclareDirective]: [
-        {range: '0.0.0-PLACEHOLDER', linker: partialDirectiveLinkerVersion1},
-        {range: '>=11.1.0-next.1', linker: partialDirectiveLinkerVersion1},
-      ],
-      [ɵɵngDeclareComponent]: [
-        {range: '0.0.0-PLACEHOLDER', linker: partialComponentLinkerVersion1},
-        {range: '>=11.1.0-next.1', linker: partialComponentLinkerVersion1},
-      ],
-    };
+    const linkers = new Map<string, LinkerRange<TExpression>[]>();
+    linkers.set(ɵɵngDeclareDirective, [
+      {range: '0.0.0-PLACEHOLDER', linker: partialDirectiveLinkerVersion1},
+      {range: '>=11.1.0-next.1', linker: partialDirectiveLinkerVersion1},
+    ]);
+    linkers.set(ɵɵngDeclareComponent, [
+      {range: '0.0.0-PLACEHOLDER', linker: partialComponentLinkerVersion1},
+      {range: '>=11.1.0-next.1', linker: partialComponentLinkerVersion1},
+    ]);
+    return linkers;
   }
 }
diff --git a/packages/compiler-cli/linker/test/file_linker/partial_linkers/partial_linker_selector_spec.ts b/packages/compiler-cli/linker/test/file_linker/partial_linkers/partial_linker_selector_spec.ts
index f8d524881a..773f4867fa 100644
--- a/packages/compiler-cli/linker/test/file_linker/partial_linkers/partial_linker_selector_spec.ts
+++ b/packages/compiler-cli/linker/test/file_linker/partial_linkers/partial_linker_selector_spec.ts
@@ -44,6 +44,12 @@ describe('PartialLinkerSelector', () => {
          expect(selector.supportsDeclaration('ɵɵngDeclareComponent')).toBe(true);
          expect(selector.supportsDeclaration('$foo')).toBe(false);
        });
+
+    it('should return false for methods on `Object`', () => {
+      const selector = new PartialLinkerSelector(
+          environment, fs.resolve('/some/path/to/file.js'), 'some file contents');
+      expect(selector.supportsDeclaration('toString')).toBe(false);
+    });
   });
 
   describe('getLinker()', () => {