fix: sql-formatter (#1152)

* fix: sql-formmater * remove originalStatementText * try catch on whole query
pinterest · Feb 8, 2023 · efdbd7b · efdbd7b
1 parent 4734e01
commit efdbd7b
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 56 deletions.
diff --git a/querybook/webapp/components/QueryEditor/QueryEditor.tsx b/querybook/webapp/components/QueryEditor/QueryEditor.tsx
@@ -233,11 +233,11 @@ export const QueryEditor: React.FC<
         );
 
         const formatQuery = useCallback(
-            (
-                options: ISQLFormatOptions = {
-                    silent: false,
-                }
-            ) => {
+            (options: ISQLFormatOptions) => {
+                options = {
+                    silent: false, // default false to throw format errors
+                    ...options,
+                };
                 if (editorRef.current) {
                     const indentWithTabs =
                         editorRef.current.getOption('indentWithTabs');

diff --git a/querybook/webapp/lib/sql-helper/sql-formatter.ts b/querybook/webapp/lib/sql-helper/sql-formatter.ts
@@ -64,7 +64,6 @@ interface IProcessedStatement {
     statementText: string;
     idToTemplateTag: Record<string, string>;
     firstKeyWord: IToken;
-    originalStatementText: string;
 }
 
 function tokenizeAndFormatQuery(
@@ -139,10 +138,6 @@ function processStatements(
                 statementText,
                 idToTemplateTag,
                 firstKeyWord,
-                originalStatementText: query.slice(
-                    statementRange[0],
-                    statementRange[1] + 1
-                ),
             };
         }
     );
@@ -158,39 +153,19 @@ function formatEachStatement(
     language: string,
     options: ISQLFormatOptions
 ) {
-    const safeSQLFormat = (text: string, unformattedText: string) => {
-        try {
-            return sqlFormat(text, {
-                tabWidth: options.tabWidth,
-                language: getLanguageForSqlFormatter(language),
-                useTabs: options.useTabs,
-            });
-        } catch (e) {
-            if (options.silent) {
-                return unformattedText;
-            } else {
-                throw e;
-            }
-        }
-    };
-
-    const formattedStatements: string[] = statements.map(
-        ({
-            firstKeyWord,
-            statementText,
-            idToTemplateTag,
-            originalStatementText,
-        }) => {
+    return statements.map(
+        ({ firstKeyWord, statementText, idToTemplateTag }) => {
             // Use standard formatter to format
-            let formattedStatement = originalStatementText;
+            let formattedStatement = statementText;
             if (
                 firstKeyWord &&
                 allowedStatement.has(firstKeyWord.text.toLocaleLowerCase())
             ) {
-                formattedStatement = safeSQLFormat(
-                    statementText,
-                    originalStatementText
-                );
+                formattedStatement = sqlFormat(statementText, {
+                    tabWidth: options.tabWidth,
+                    language: getLanguageForSqlFormatter(language),
+                    useTabs: options.useTabs,
+                });
             }
 
             for (const [id, templateTag] of Object.entries(idToTemplateTag)) {
@@ -203,8 +178,6 @@ function formatEachStatement(
             return formattedStatement;
         }
     );
-
-    return formattedStatements;
 }
 
 export function format(
@@ -223,22 +196,27 @@ export function format(
         ...options,
     };
 
-    const { processedStatements, newLineBetweenStatement } = processStatements(
-        query,
-        language,
-        options
-    );
-    const formattedStatements = formatEachStatement(
-        processedStatements,
-        language,
-        options
-    );
-
-    return formattedStatements.reduce(
-        (acc, statement, index) =>
-            acc + '\n'.repeat(newLineBetweenStatement[index]) + statement,
-        ''
-    );
+    try {
+        const { processedStatements, newLineBetweenStatement } =
+            processStatements(query, language, options);
+        const formattedStatements = formatEachStatement(
+            processedStatements,
+            language,
+            options
+        );
+
+        return formattedStatements.reduce(
+            (acc, statement, index) =>
+                acc + '\n'.repeat(newLineBetweenStatement[index]) + statement,
+            ''
+        );
+    } catch (e) {
+        if (options.silent) {
+            return query;
+        } else {
+            throw e;
+        }
+    }
 }
 
 // Override according to https://github.com/sql-formatter-org/sql-formatter/blob/master/docs/language.md