Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make formatting in the CLI use the same SQL formatter as BigQuery #1741

Merged
merged 4 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Update to Google SQL formatter
  • Loading branch information
Ekrekr committed May 22, 2024
commit 4cbd00e0522caeb9e974482368c85230ed724d48
2 changes: 1 addition & 1 deletion cli/api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ ts_library(
"@npm//deepmerge",
"@npm//fs-extra",
"@npm//glob",
"@npm//google-sql-syntax-ts",
"@npm//js-beautify",
"@npm//js-yaml",
"@npm//promise-pool-executor",
"@npm//protobufjs",
"@npm//semver",
"@npm//tmp",
"@npm//sql-formatter",
],
)
11 changes: 11 additions & 0 deletions contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@ bazel build cli

The projects folder here is not built as it requires an environment file, which can be provided from the team.

### Add New NPM Dependencies

Global yarn installations will throw errors when installing packages, instead you should use:

```bash
$ bazel run @nodejs//:yarn add ...
```

Additionally, installed NPM dependencies need to be added to the `deps` of `ts_library` rules by
prefixing them with `@npm//...`.

## The Contribution Process

1. Decide on what you'd like to contribute. The majority of open-source contributions come from:
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"estraverse": "^5.1.0",
"fs-extra": "^9.0.0",
"glob": "^10.3.3",
"google-sql-syntax-ts": "^1.0.1",
"grpc": "^1.24.0",
"js-beautify": "^1.10.2",
"js-yaml": "^4.1.0",
Expand All @@ -56,7 +57,6 @@
"rollup": "^2.7.3",
"rollup-plugin-dts": "^1.4.0",
"semver": "^7.5.2",
"sql-formatter": "^12.2.1",
"tarjan-graph": "^2.0.0",
"tmp": "^0.2.0",
"ts-loader": "^5.3.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/@dataform/cli/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ externals = [
"deepmerge",
"fs-extra",
"glob",
"google-sql-syntax-ts",
"js-beautify",
"js-yaml",
"moo",
Expand All @@ -39,7 +40,6 @@ externals = [
"protobufjs",
"readline-sync",
"semver",
"sql-formatter",
"tarjan-graph",
"tmp",
"typeid-js",
Expand Down
2 changes: 1 addition & 1 deletion sqlx/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ ts_library(
"//common/errors",
"@npm//@types/js-beautify",
"@npm//@types/node",
"@npm//google-sql-syntax-ts",
"@npm//js-beautify",
"@npm//sql-formatter",
"@npm//typeid-js",
],
)
Expand Down
25 changes: 5 additions & 20 deletions sqlx/format.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from "fs";
import * as jsBeautify from "js-beautify";
import * as sqlFormatter from "sql-formatter";
import { QueryFormatter, GoogleSqlDefinition } from "google-sql-syntax-ts";
import { promisify } from "util";

import { ErrorWithCause } from "df/common/errors/errors";
Expand All @@ -13,7 +13,7 @@ const JS_BEAUTIFY_OPTIONS = {
max_preserve_newlines: 2
};

const MAX_SQL_FORMAT_ATTEMPTS = 5;
const queryFormatter = new QueryFormatter(new GoogleSqlDefinition());

export function format(text: string, fileExtension: string) {
try {
Expand Down Expand Up @@ -73,7 +73,7 @@ function formatSqlx(node: SyntaxTreeNode, indent = "") {
[placeholderId: string]: SyntaxTreeNode | string;
} = {};
const unformattedPlaceholderSql = stripUnformattableText(sqlxStatement, placeholders).join("");
const formattedPlaceholderSql = formatSql(unformattedPlaceholderSql);
const formattedPlaceholderSql = queryFormatter.formatQuery(unformattedPlaceholderSql);
return formatEveryLine(
replacePlaceholders(formattedPlaceholderSql, placeholders),
line => `${indent}${line}`
Expand Down Expand Up @@ -161,8 +161,8 @@ function stripUnformattableText(
return placeholderId;
}
case SyntaxTreeNodeType.SQL_COMMENT: {
// sql-formatter knows how to format comments (as long as they keep to a single line);
// give it a hint.
// google-sql-syntax-ts knows how to format comments (as long as they keep to a single
// line); give it a hint.
const commentPlaceholderId = part.concatenate().startsWith("--")
? `--${placeholderId}`
: `/*${placeholderId}*/`;
Expand Down Expand Up @@ -207,21 +207,6 @@ function formatJavaScript(text: string) {
return jsBeautify.js(text, JS_BEAUTIFY_OPTIONS);
}

function formatSql(text: string) {
let formatted = sqlFormatter.format(text, { language: "bigquery" }) as string;
// Unfortunately sql-formatter does not always produce final formatted output (even on plain SQL) in a single pass.
for (let attempts = 0; attempts < MAX_SQL_FORMAT_ATTEMPTS; attempts++) {
const newFormatted = sqlFormatter.format(formatted, { language: "bigquery" }) as string;
if (newFormatted === formatted) {
return newFormatted;
}
formatted = newFormatted;
}
throw new Error(
`SQL formatter was unable to determine final formatted form within ${MAX_SQL_FORMAT_ATTEMPTS} attempts. Original text: ${text}`
);
}

function formatPlaceholderInSqlx(
placeholderId: string,
placeholderSyntaxNode: SyntaxTreeNode,
Expand Down
82 changes: 18 additions & 64 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1686,11 +1686,6 @@ diffie-hellman@^5.0.0:
miller-rabin "^4.0.0"
randombytes "^2.0.0"

discontinuous-range@1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/discontinuous-range/-/discontinuous-range-1.0.0.tgz#e38331f0844bba49b9a9cb71c771585aab1bc65a"
integrity sha512-c68LpLbO+7kP/b1Hr1qs8/BJ09F5khZGTxqxZuhzxpmwJKOgRFHJWIb9/KmqnqHhLdO55aOxFH/EGBvUQbL/RQ==

dom-serializer@0:
version "0.2.2"
resolved "https://registry.yarnpkg.com/dom-serializer/-/dom-serializer-0.2.2.tgz#1afb81f533717175d478655debc5e332d9f9bb51"
Expand Down Expand Up @@ -2529,6 +2524,13 @@ google-p12-pem@^3.0.3:
dependencies:
node-forge "^0.10.0"

google-sql-syntax-ts@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/google-sql-syntax-ts/-/google-sql-syntax-ts-1.0.1.tgz#219c20626481e1359307a5429b6585bf6a405c88"
integrity sha512-gHdfepEo5+fB3HSvpp+CS0C9OivFtetX3eQm503mxaUqt7LFTJoS5HGtT2CCVebxd7VfUjL7/LFgVuknTr1UDA==
dependencies:
monaco-editor "^0.44.0"

graceful-fs@^4.1.11, graceful-fs@^4.1.15:
version "4.2.11"
resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.11.tgz#4183e4e8bf08bb6e05bbb2f7d2e0c8f712ca40e3"
Expand Down Expand Up @@ -3553,6 +3555,11 @@ mkdirp@^1.0.3, mkdirp@^1.0.4:
resolved "https://registry.yarnpkg.com/mkdirp/-/mkdirp-1.0.4.tgz#3eb5ed62622756d79a5f0e2a221dfebad75c2f7e"
integrity "sha1-PrXtYmInVteaXw4qIh3+utdcL34= sha512-vVqVZQyf3WLx2Shd0qJ9xuvqgAyKPLAiqITEtqW0oIUjzo3PePDd6fW9iFz30ef7Ysp/oiWqbhszeGWW2T6Gzw=="

monaco-editor@^0.44.0:
version "0.44.0"
resolved "https://registry.yarnpkg.com/monaco-editor/-/monaco-editor-0.44.0.tgz#3c0fe3655923bbf7dd647057302070b5095b6c59"
integrity sha512-5SmjNStN6bSuSE5WPT2ZV+iYn1/yI9sd4Igtk23ChvqB7kDk9lZbB9F5frsuvpB+2njdIeGGFf2G4gbE6rCC9Q==

moo@^0.5.0:
version "0.5.1"
resolved "https://registry.yarnpkg.com/moo/-/moo-0.5.1.tgz#7aae7f384b9b09f620b6abf6f74ebbcd1b65dbc4"
Expand Down Expand Up @@ -3612,16 +3619,6 @@ nanomatch@^1.2.9:
snapdragon "^0.8.1"
to-regex "^3.0.1"

nearley@^2.20.1:
version "2.20.1"
resolved "https://registry.yarnpkg.com/nearley/-/nearley-2.20.1.tgz#246cd33eff0d012faf197ff6774d7ac78acdd474"
integrity sha512-+Mc8UaAebFzgV+KpI5n7DasuuQCHA89dmwm7JXw3TV43ukfNQ9DnBH3Mdb2g/I4Fdxc26pwimBWvjIw0UAILSQ==
dependencies:
commander "^2.19.0"
moo "^0.5.0"
railroad-diagrams "^1.0.0"
randexp "0.4.6"

neo-async@^2.5.0, neo-async@^2.6.1:
version "2.6.2"
resolved "https://registry.yarnpkg.com/neo-async/-/neo-async-2.6.2.tgz#b4aafb93e3aeb2d8174ca53cf163ab7d7308305f"
Expand Down Expand Up @@ -4211,19 +4208,6 @@ querystring@0.2.0:
resolved "https://registry.yarnpkg.com/querystring/-/querystring-0.2.0.tgz#b209849203bb25df820da756e747005878521620"
integrity "sha1-sgmEkgO7Jd+CDadW50cAWHhSFiA= sha512-X/xY82scca2tau62i9mDyU9K+I+djTMUsvwf7xnUX5GLvVzgJybOJf4Y6o9Zx3oJK/LSXg5tTZBjwzqVPaPO2g=="

railroad-diagrams@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/railroad-diagrams/-/railroad-diagrams-1.0.0.tgz#eb7e6267548ddedfb899c1b90e57374559cddb7e"
integrity sha512-cz93DjNeLY0idrCNOH6PviZGRN9GJhsdm9hpn1YCS879fj4W+x5IFJhhkRZcwVgMmFF7R82UA/7Oh+R8lLZg6A==

randexp@0.4.6:
version "0.4.6"
resolved "https://registry.yarnpkg.com/randexp/-/randexp-0.4.6.tgz#e986ad5e5e31dae13ddd6f7b3019aa7c87f60ca3"
integrity sha512-80WNmd9DA0tmZrw9qQa62GPPWfuXJknrmVmLcxvq4uZBdYqb1wYoKTmnlGUchvVWe0XiLupYkBoXVOxz3C8DYQ==
dependencies:
discontinuous-range "1.0.0"
ret "~0.1.10"

randombytes@^2.0.0, randombytes@^2.0.1, randombytes@^2.0.5, randombytes@^2.1.0:
version "2.1.0"
resolved "https://registry.yarnpkg.com/randombytes/-/randombytes-2.1.0.tgz#df6f84372f0270dc65cdf6291349ab7a473d4f2a"
Expand Down Expand Up @@ -4724,14 +4708,6 @@ sprintf-js@~1.0.2:
resolved "https://registry.yarnpkg.com/sprintf-js/-/sprintf-js-1.0.3.tgz#04e6926f662895354f3dd015203633b857297e2c"
integrity "sha1-BOaSb2YolTVPPdAVIDYzuFcpfiw= sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g=="

sql-formatter@^12.2.1:
version "12.2.1"
resolved "https://registry.yarnpkg.com/sql-formatter/-/sql-formatter-12.2.1.tgz#6e048fea63c06f7f10371066502e4eb163967e17"
integrity sha512-rvNfY+g+mv3vxzqdDrsLVjhe/UZohYdCqUqyTK/JI8oXxnW4ObcldlclqUoL55SWQs8azEfFf66XfPBi6tkjfQ==
dependencies:
argparse "^2.0.1"
nearley "^2.20.1"

sshpk@^1.7.0:
version "1.16.1"
resolved "https://registry.yarnpkg.com/sshpk/-/sshpk-1.16.1.tgz#fb661c0bef29b39db40769ee39fa70093d6f6877"
Expand Down Expand Up @@ -4802,16 +4778,8 @@ stream-shift@^1.0.0:
resolved "https://registry.yarnpkg.com/stream-shift/-/stream-shift-1.0.0.tgz#d5c752825e5367e786f78e18e445ea223a155952"
integrity "sha1-1cdSgl5TZ+eG944Y5EXqIjoVWVI= sha512-Afuc4BKirbx0fwm9bKOehZPG01DJkm/4qbklw4lo9nMPqd2x0kZTLcgwQUXdGiPPY489l3w8cQ5xEEAGbg8ACQ=="

"string-width-cjs@npm:string-width@^4.2.0":
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
dependencies:
emoji-regex "^8.0.0"
is-fullwidth-code-point "^3.0.0"
strip-ansi "^6.0.1"

"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.2.3:
"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.2.3:
name string-width-cjs
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
Expand Down Expand Up @@ -4861,7 +4829,8 @@ string_decoder@~1.1.1:
dependencies:
safe-buffer "~5.1.0"

"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.1:
name strip-ansi-cjs
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
Expand All @@ -4882,13 +4851,6 @@ strip-ansi@^6.0.0:
dependencies:
ansi-regex "^5.0.0"

strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
dependencies:
ansi-regex "^5.0.1"

strip-ansi@^7.0.1:
version "7.1.0"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45"
Expand Down Expand Up @@ -5571,7 +5533,8 @@ worker-farm@^1.7.0:
dependencies:
errno "~0.1.7"

"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
name wrap-ansi-cjs
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
Expand All @@ -5589,15 +5552,6 @@ wrap-ansi@^5.1.0:
string-width "^3.0.0"
strip-ansi "^5.0.0"

wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
dependencies:
ansi-styles "^4.0.0"
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^8.1.0:
version "8.1.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"
Expand Down