Update @dataform/core to never return a graph with non-unique action names. (#1366)

* Update @dataform/core to never return a graph with non-unique action names.

* Include a compilation error for each removed action.

* Rename a shadowed variable

* Increment DF_VERSION

Author: deeasu

core/session.ts

@@ -416,19 +416,10 @@ export class Session {
       [].concat(compiledGraph.declarations.map(declaration => declaration.target))
     );
 
-    const standardActions = [].concat(
-      compiledGraph.tables,
-      compiledGraph.assertions,
-      compiledGraph.operations,
-      compiledGraph.declarations
-    );
-
-    this.checkActionNameUniqueness(standardActions);
+    this.removeNonUniqueActionsFromCompiledGraph(compiledGraph);
 
     this.checkTestNameUniqueness(compiledGraph.tests);
 
-    this.checkCanonicalTargetUniqueness(standardActions);
-
     this.checkTableConfigValidity(compiledGraph.tables);
 
     this.checkCircularity(
@@ -574,41 +565,6 @@ export class Session {
     });
   }
 
-  private checkActionNameUniqueness(actions: IActionProto[]) {
-    const allNames: string[] = [];
-    actions.forEach(action => {
-      const name = targetAsReadableString(action.target);
-      if (allNames.includes(name)) {
-        this.compileError(
-          new Error(
-            `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations`
-          ),
-          action.fileName,
-          action.target
-        );
-      }
-      allNames.push(name);
-    });
-  }
-
-  private checkCanonicalTargetUniqueness(actions: IActionProto[]) {
-    const allCanonicalTargets = new StringifiedSet<dataform.ITarget>(targetStringifier);
-    actions.forEach(action => {
-      if (allCanonicalTargets.has(action.canonicalTarget)) {
-        this.compileError(
-          new Error(
-            `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n"${JSON.stringify(
-              action.canonicalTarget
-            )}"`
-          ),
-          action.fileName,
-          action.target
-        );
-      }
-      allCanonicalTargets.add(action.canonicalTarget);
-    });
-  }
-
   private checkTableConfigValidity(tables: dataform.ITable[]) {
     tables.forEach(table => {
       // type
@@ -866,6 +822,65 @@ export class Session {
       );
     });
   }
+
+  private removeNonUniqueActionsFromCompiledGraph(compiledGraph: dataform.CompiledGraph) {
+    function getNonUniqueTargets(targets: dataform.ITarget[]): StringifiedSet<dataform.ITarget> {
+      const allTargets = new StringifiedSet<dataform.ITarget>(targetStringifier);
+      const nonUniqueTargets = new StringifiedSet<dataform.ITarget>(targetStringifier);
+  
+      targets.forEach(target => {
+        if (allTargets.has(target)) {
+          nonUniqueTargets.add(target);
+        }
+        allTargets.add(target);
+      });
+  
+      return nonUniqueTargets;
+    }
+
+    const actions = [].concat(
+      compiledGraph.tables,
+      compiledGraph.assertions,
+      compiledGraph.operations,
+      compiledGraph.declarations
+    );
+
+    const nonUniqueActionsTargets = getNonUniqueTargets(actions.map(action => action.target));
+    const nonUniqueActionsCanonicalTargets = getNonUniqueTargets(actions.map(action => action.canonicalTarget));
+
+    const isUniqueAction = (action: IActionProto) => {
+      const isNonUniqueTarget = nonUniqueActionsTargets.has(action.target);
+      const isNonUniqueCanonicalTarget = nonUniqueActionsCanonicalTargets.has(action.canonicalTarget);
+
+      if (isNonUniqueTarget) {
+        this.compileError(
+          new Error(
+            `Duplicate action name detected. Names within a schema must be unique across tables, declarations, assertions, and operations`
+          ),
+          action.fileName,
+          action.target
+        );
+      }
+      if (isNonUniqueCanonicalTarget) {
+        this.compileError(
+          new Error(
+            `Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n"${JSON.stringify(
+              action.canonicalTarget
+            )}"`
+          ),
+          action.fileName,
+          action.target
+        );
+      }
+
+      return !isNonUniqueTarget && !isNonUniqueCanonicalTarget;
+    }
+
+    compiledGraph.tables = compiledGraph.tables.filter(isUniqueAction);
+    compiledGraph.operations = compiledGraph.operations.filter(isUniqueAction);
+    compiledGraph.declarations = compiledGraph.declarations.filter(isUniqueAction);
+    compiledGraph.assertions = compiledGraph.assertions.filter(isUniqueAction);
+  }
 }
 
 function declaresDataset(type: string, hasOutput?: boolean) {

tests/core/core.spec.ts

@@ -310,9 +310,9 @@ suite("@dataform/core", () => {
         schema: "schema"
       });
       const graph = session.compile();
-      expect(graph.graphErrors.compilationErrors.map(error => error.message)).deep.equals([
+      expect(graph.graphErrors.compilationErrors.map(error => error.message)).deep.equals(Array(2).fill(
         'Duplicate canonical target detected. Canonical targets must be unique across tables, declarations, assertions, and operations:\n"{"schema":"schema","name":"view","database":"database"}"'
-      ]);
+      ));
     });
 
     test("validation_type_incremental", () => {
@@ -967,7 +967,32 @@ suite("@dataform/core", () => {
         cGraph.graphErrors.compilationErrors.filter(item =>
           item.message.match(/Duplicate action name/)
         ).length
-      ).greaterThan(0);
+      ).equals(2);
+    });
+
+    test("duplicate actions in compiled graph", () => {
+      const session = new Session(path.dirname(__filename), TestConfigs.redshift);
+      session.publish("a")
+      session.publish("a");
+      session.publish("b"); // unique action
+      session.publish("c")
+      
+      session.operate("a")
+      session.operate("d") // unique action
+      session.operate("e") // unique action
+
+      session.declare({ name: "a" })
+      session.declare({ name: "f" }) // unique action
+      session.declare({ name: "g" })
+
+      session.assert("c")
+      session.assert("g")
+
+      const cGraph = session.compile();
+ 
+      expect(
+        [].concat(cGraph.tables, cGraph.assertions, cGraph.operations, cGraph.declarations).length
+      ).equals(4)
     });
 
     test("same action names in different schemas (ambiguity)", () => {
@@ -995,7 +1020,7 @@ suite("@dataform/core", () => {
         cGraph.graphErrors.compilationErrors.filter(item =>
           item.message.match(/Duplicate action name detected. Names within a schema must be unique/)
         ).length
-      ).greaterThan(0);
+      ).equals(2);
     });
 
     test("same action names in different schemas", () => {

version.bzl

@@ -1,3 +1,3 @@
 # NOTE: If you change the format of this line, you must change the bash command
 # in /scripts/publish to extract the version string correctly.
-DF_VERSION = "2.0.0"
+DF_VERSION = "2.0.1"