Move formatter to SQLX from API (dataform-co#878)

* Move formatter to SQLX from API * Removed deprecated format from API * Swapped from fs to fs-extra * Added enable non literal fs filename to tslint exceptions for sqlx * Fixed CLI dependency
rumbin · Jul 13, 2020 · 7d59b7d · 7d59b7d
1 parent b8c0cd7
commit 7d59b7d
Show file tree

Hide file tree

Showing 10 changed files with 134 additions and 126 deletions.
diff --git a/api/index.ts b/api/index.ts
@@ -1,7 +1,6 @@
 import { build, Builder } from "df/api/commands/build";
 import { compile } from "df/api/commands/compile";
 import * as credentials from "df/api/commands/credentials";
-import * as format from "df/api/commands/format";
 import { init } from "df/api/commands/init";
 import { install } from "df/api/commands/install";
 import { prune } from "df/api/commands/prune";
@@ -25,6 +24,5 @@ export {
   validateSchedulesFileIfExists,
   Runner,
   Builder,
-  format,
   prune
 };
diff --git a/cli/BUILD b/cli/BUILD
@@ -9,6 +9,7 @@ ts_library(
         "//api",
         "//core",
         "//protos:ts",
+        "//sqlx",
         "@npm//@types/analytics-node",
         "@npm//@types/glob",
         "@npm//@types/long",

diff --git a/cli/index.ts b/cli/index.ts
@@ -4,7 +4,7 @@ import * as path from "path";
 import yargs from "yargs";
 
 import * as chokidar from "chokidar";
-import { build, compile, credentials, format, init, install, run, table, test } from "df/api";
+import { build, compile, credentials, init, install, run, table, test } from "df/api";
 import { CREDENTIALS_FILENAME } from "df/api/commands/credentials";
 import * as dbadapters from "df/api/dbadapters";
 import { prettyJsonStringify } from "df/api/utils";
@@ -35,6 +35,7 @@ import { actuallyResolve, assertPathExists, compiledGraphHasErrors } from "df/cl
 import { createYargsCli, INamedOption } from "df/cli/yargswrapper";
 import { supportsCancel, WarehouseType } from "df/core/adapters";
 import { dataform } from "df/protos/ts";
+import { formatFile } from "df/sqlx/format";
 
 const RECOMPILE_DELAY = 500;
 
@@ -591,7 +592,7 @@ export function runCli() {
           const results = await Promise.all(
             filenames.map(async filename => {
               try {
-                await format.formatFile(path.resolve(argv["project-dir"], filename), {
+                await formatFile(path.resolve(argv["project-dir"], filename), {
                   overwriteFile: true
                 });
                 return {

diff --git a/sqlx/BUILD b/sqlx/BUILD
@@ -7,8 +7,12 @@ ts_library(
     srcs = glob(["**/*.ts"]),
     deps = [
         "//:modules-fix",
+        "//common/errors",
         "@npm//@types/moo",
         "@npm//@types/node",
+        "@npm//@types/js-beautify",
+        "@npm//js-beautify",
+        "@npm//sql-formatter",
         "@npm//moo",
     ],
 )
diff --git a/api/commands/format.ts → sqlx/format.ts b/api/commands/format.ts → sqlx/format.ts
diff --git a/api/sql_formatter.d.ts → sqlx/sql_formatter.d.ts b/api/sql_formatter.d.ts → sqlx/sql_formatter.d.ts
diff --git a/sqlx/tslint.json b/sqlx/tslint.json
@@ -1,6 +1,7 @@
 {
   "extends": ["../tslint.json"],
   "rules": {
-    "tsr-detect-possible-timing-attacks": false
+    "tsr-detect-possible-timing-attacks": false,
+    "tsr-detect-non-literal-fs-filename": false
   }
 }
diff --git a/tests/api/api.spec.ts b/tests/api/api.spec.ts
@@ -1,13 +1,11 @@
 import { assert, config, expect } from "chai";
 import Long from "long";
-import * as path from "path";
 import { anyString, anything, instance, mock, verify, when } from "ts-mockito";
 
-import { Builder, credentials, format, prune, query, Runner } from "df/api";
+import { Builder, credentials, prune, query, Runner } from "df/api";
 import { computeAllTransitiveInputs } from "df/api/commands/build";
 import { IDbAdapter } from "df/api/dbadapters";
 import { BigQueryDbAdapter } from "df/api/dbadapters/bigquery";
-import { actionsByTarget } from "df/api/utils/graphs";
 import { sleep, sleepUntil } from "df/common/promises";
 import { dataform } from "df/protos/ts";
 import { suite, test } from "df/testing";
@@ -1295,124 +1293,6 @@ postOps`
       expect(result.actions[0].tasks[0].errorMessage).to.match(/cancelled/);
     });
   });
-
-  suite("formatter", () => {
-    test("correctly formats simple.sqlx", async () => {
-      expect(await format.formatFile(path.resolve("examples/formatter/definitions/simple.sqlx")))
-        .eql(`config {
-  type: "view",
-  tags: ["tag1", "tag2"]
-}
-
-js {
-  const foo =
-    jsFunction("table");
-}
-
-select
-  1
-from
-  \${
-    ref({
-      schema: "df_integration_test",
-      name: "sample_data"
-    })
-  }
-`);
-    });
-
-    test("correctly formats multiple_queries.sqlx", async () => {
-      expect(
-        await format.formatFile(
-          path.resolve("examples/formatter/definitions/multiple_queries.sqlx")
-        )
-      ).eql(`js {
-  var tempTable = "yay"
-  const colname = "column";
-
-  let finalTableName = 'dkaodihwada';
-}
-
-drop something
-
----
-
-alter table
-  \${tempTable} rename to \${finalTableName}
-
----
-
-SELECT
-  SUM(IF (session_start_event, 1, 0)) AS session_index
-`);
-    });
-
-    test("correctly formats bigquery_regexps.sqlx", async () => {
-      expect(
-        await format.formatFile(
-          path.resolve("examples/formatter/definitions/bigquery_regexps.sqlx")
-        )
-      ).eql(`config {
-  type: "operation",
-  tags: ["tag1", "tag2"]
-}
-
-select
-  CAST(
-    REGEXP_EXTRACT("", r'^/([0-9]+)\\'\\"/.*') AS INT64
-  ) AS id,
-  CAST(
-    REGEXP_EXTRACT("", r"^/([0-9]+)\\"\\'/.*") AS INT64
-  ) AS id2,
-  IFNULL (
-    regexp_extract('', r'\\a?query=([^&]+)&*'),
-    regexp_extract('', r'\\a?q=([^&]+)&*')
-  ) AS id3,
-  regexp_extract('bar', r'bar') as ID4
-from
-  \${ref("dab")}
-where
-  sample = 100
-`);
-    });
-
-    test("correctly formats comments.sqlx", async () => {
-      expect(await format.formatFile(path.resolve("examples/formatter/definitions/comments.sqlx")))
-        .eql(`config {
-  type: "test",
-}
-
-SELECT
-  MAX(
-    (
-      SELECT
-        SUM(IF(track.event = "event_viewed_project_with_connection", 1, 0))
-      FROM
-        UNNEST(records)
-    )
-  ) > 0 as created_project,
-  /* multi line
-  comment      */
-  2 as foo
-
-input "something" {
-  select
-    1 as test
-    /* something */
-    /* something
-    else      */
-    -- and another thing
-}
-`);
-    });
-    test("Backslashes within regex don't cause 'r' prefix to separate.", async () => {
-      expect(await format.formatFile(path.resolve("examples/formatter/definitions/regex.sqlx")))
-        .equal(`select
-  regexp_extract("", r'abc\\de\\'fg select * from self()'),
-  'bar'
-`);
-    });
-  });
 });
 
 function cleanTiming(runResult: dataform.IRunResult) {

diff --git a/tests/sqlx/BUILD b/tests/sqlx/BUILD
@@ -3,6 +3,10 @@ load("//testing:index.bzl", "ts_test_suite")
 ts_test_suite(
     name = "tests",
     srcs = glob(["**/*.ts"]),
+    data = [
+        "//examples/formatter:files",
+        "//examples/formatter:node_modules",
+    ],
     deps = [
         "//sqlx",
         "//testing",

diff --git a/tests/sqlx/format.spec.ts b/tests/sqlx/format.spec.ts
@@ -0,0 +1,119 @@
+import { expect } from "chai";
+import * as path from "path";
+
+import { formatFile } from "df/sqlx/format";
+import { suite, test } from "df/testing";
+
+suite("@dataform/sqlx", () => {
+  suite("formatter", () => {
+    test("correctly formats simple.sqlx", async () => {
+      expect(await formatFile(path.resolve("examples/formatter/definitions/simple.sqlx")))
+        .eql(`config {
+  type: "view",
+  tags: ["tag1", "tag2"]
+}
+
+js {
+  const foo =
+    jsFunction("table");
+}
+
+select
+  1
+from
+  \${
+    ref({
+      schema: "df_integration_test",
+      name: "sample_data"
+    })
+  }
+`);
+    });
+
+    test("correctly formats multiple_queries.sqlx", async () => {
+      expect(await formatFile(path.resolve("examples/formatter/definitions/multiple_queries.sqlx")))
+        .eql(`js {
+  var tempTable = "yay"
+  const colname = "column";
+
+  let finalTableName = 'dkaodihwada';
+}
+
+drop something
+
+---
+
+alter table
+  \${tempTable} rename to \${finalTableName}
+
+---
+
+SELECT
+  SUM(IF (session_start_event, 1, 0)) AS session_index
+`);
+    });
+
+    test("correctly formats bigquery_regexps.sqlx", async () => {
+      expect(await formatFile(path.resolve("examples/formatter/definitions/bigquery_regexps.sqlx")))
+        .eql(`config {
+  type: "operation",
+  tags: ["tag1", "tag2"]
+}
+
+select
+  CAST(
+    REGEXP_EXTRACT("", r'^/([0-9]+)\\'\\"/.*') AS INT64
+  ) AS id,
+  CAST(
+    REGEXP_EXTRACT("", r"^/([0-9]+)\\"\\'/.*") AS INT64
+  ) AS id2,
+  IFNULL (
+    regexp_extract('', r'\\a?query=([^&]+)&*'),
+    regexp_extract('', r'\\a?q=([^&]+)&*')
+  ) AS id3,
+  regexp_extract('bar', r'bar') as ID4
+from
+  \${ref("dab")}
+where
+  sample = 100
+`);
+    });
+
+    test("correctly formats comments.sqlx", async () => {
+      expect(await formatFile(path.resolve("examples/formatter/definitions/comments.sqlx")))
+        .eql(`config {
+  type: "test",
+}
+
+SELECT
+  MAX(
+    (
+      SELECT
+        SUM(IF(track.event = "event_viewed_project_with_connection", 1, 0))
+      FROM
+        UNNEST(records)
+    )
+  ) > 0 as created_project,
+  /* multi line
+  comment      */
+  2 as foo
+
+input "something" {
+  select
+    1 as test
+    /* something */
+    /* something
+    else      */
+    -- and another thing
+}
+`);
+    });
+    test("Backslashes within regex don't cause 'r' prefix to separate.", async () => {
+      expect(await formatFile(path.resolve("examples/formatter/definitions/regex.sqlx")))
+        .equal(`select
+  regexp_extract("", r'abc\\de\\'fg select * from self()'),
+  'bar'
+`);
+    });
+  });
+});