From 6a7d080b782f0cf41acb2bf58fa080a19e9c5d1c Mon Sep 17 00:00:00 2001 From: Rene Saarsoo Date: Mon, 13 Nov 2023 20:48:27 +0200 Subject: [PATCH] Throw error when commaPosition or tabulateAlias option used --- README.md | 1 - docs/commaPosition.md | 66 --------- src/FormatOptions.ts | 3 - src/formatter/Formatter.ts | 13 +- src/formatter/formatCommaPositions.ts | 106 --------------- src/index.ts | 1 - src/sqlFormatter.ts | 1 - src/validateConfig.ts | 12 +- static/index.html | 8 -- static/index.js | 3 - test/behavesLikeSqlFormatter.ts | 2 - test/options/commaPosition.ts | 185 -------------------------- test/sqlFormatter.test.ts | 12 ++ 13 files changed, 19 insertions(+), 394 deletions(-) delete mode 100644 docs/commaPosition.md delete mode 100644 src/formatter/formatCommaPositions.ts delete mode 100644 test/options/commaPosition.ts diff --git a/README.md b/README.md index 6b96555ccf..b41da467ba 100644 --- a/README.md +++ b/README.md @@ -153,7 +153,6 @@ All fields are optional and all fields that are not specified will be filled wit - [**`identifierCase`**](docs/identifierCase.md) uppercases or lowercases identifiers. (**experimental!**) - [**`indentStyle`**](docs/indentStyle.md) defines overall indentation style. - [**`logicalOperatorNewline`**](docs/logicalOperatorNewline.md) newline before or after boolean operator (AND, OR, XOR). -- [**`commaPosition`**](docs/commaPosition.md) where to place the comma in column lists (**deprecated!**). - [**`expressionWidth`**](docs/expressionWidth.md) maximum number of characters in parenthesized expressions to be kept on single line. - [**`linesBetweenQueries`**](docs/linesBetweenQueries.md) how many newlines to insert between queries. - [**`denseOperators`**](docs/denseOperators.md) packs operators densely without spaces. diff --git a/docs/commaPosition.md b/docs/commaPosition.md deleted file mode 100644 index 8300f828aa..0000000000 --- a/docs/commaPosition.md +++ /dev/null @@ -1,66 +0,0 @@ -# commaPosition (deprected) - -Defines where to place commas in lists of columns. - -**Warning:** This feature is known to be buggy. Use at your own risk. See [#505][bug]. -It will be removed in the next major version. - -## Options - -- `"after"` (default) places comma at the end of line. -- `"before"` places comma at the start of line. -- `"tabular"` aligns commas in a column at the end of line. - -Caveats: - -`"before"` style does not work when tabs are used for indentation. - -### after - -``` -SELECT - p.first_name AS fname, - p.last_name AS lname, - YEAR() - p.birth_year AS age, - p.occupation AS job -FROM - persons -GROUP BY - age, - fname, - lname -``` - -### before - -``` -SELECT - p.first_name AS name -, p.last_name AS lname -, YEAR() - p.birth_year AS age -, p.occupation AS job -FROM - persons -GROUP BY - age -, fname -, lname -``` - -### tabular - -``` -SELECT - p.first_name name , - p.last_name lname , - YEAR() - p.birth_year age, - p.occupation AS job -FROM - persons -GROUP BY - age , - fname, - lname -``` - -[bug]: https://github.com/sql-formatter-org/sql-formatter/issues/505 diff --git a/src/FormatOptions.ts b/src/FormatOptions.ts index 83470b8d50..7cf102759a 100644 --- a/src/FormatOptions.ts +++ b/src/FormatOptions.ts @@ -8,8 +8,6 @@ export type KeywordCase = 'preserve' | 'upper' | 'lower'; export type IdentifierCase = 'preserve' | 'upper' | 'lower'; -export type CommaPosition = 'before' | 'after' | 'tabular'; - export type LogicalOperatorNewline = 'before' | 'after'; export interface FormatOptions { @@ -19,7 +17,6 @@ export interface FormatOptions { identifierCase: IdentifierCase; indentStyle: IndentStyle; logicalOperatorNewline: LogicalOperatorNewline; - commaPosition: CommaPosition; expressionWidth: number; linesBetweenQueries: number; denseOperators: boolean; diff --git a/src/formatter/Formatter.ts b/src/formatter/Formatter.ts index ad5288fabd..8f10f87791 100644 --- a/src/formatter/Formatter.ts +++ b/src/formatter/Formatter.ts @@ -6,7 +6,6 @@ import { createParser } from '../parser/createParser.js'; import { StatementNode } from '../parser/ast.js'; import { Dialect } from '../dialect.js'; -import formatCommaPositions from './formatCommaPositions.js'; import ExpressionFormatter from './ExpressionFormatter.js'; import Layout, { WS } from './Layout.js'; import Indentation from './Indentation.js'; @@ -31,9 +30,7 @@ export default class Formatter { public format(query: string): string { const ast = this.parse(query); const formattedQuery = this.formatAst(ast); - const finalQuery = this.postFormat(formattedQuery); - - return finalQuery.trimEnd(); + return formattedQuery.trimEnd(); } private parse(query: string): StatementNode[] { @@ -63,12 +60,4 @@ export default class Formatter { } return layout.toString(); } - - private postFormat(query: string): string { - if (this.cfg.commaPosition === 'before' || this.cfg.commaPosition === 'tabular') { - query = formatCommaPositions(query, this.cfg.commaPosition, indentString(this.cfg)); - } - - return query; - } } diff --git a/src/formatter/formatCommaPositions.ts b/src/formatter/formatCommaPositions.ts deleted file mode 100644 index c148029418..0000000000 --- a/src/formatter/formatCommaPositions.ts +++ /dev/null @@ -1,106 +0,0 @@ -import { CommaPosition } from '../FormatOptions.js'; -import { maxLength } from '../utils.js'; - -const PRECEDING_WHITESPACE_REGEX = /^\s+/u; - -/** - * Handles comma placement - either before, after or tabulated - */ -export default function formatCommaPositions( - query: string, - commaPosition: CommaPosition, - indent: string -): string { - return groupCommaDelimitedLines(query.split('\n')) - .flatMap(commaLines => { - if (commaLines.length === 1) { - return commaLines; - } else if (commaPosition === 'tabular') { - return formatTabular(commaLines); - } else if (commaPosition === 'before') { - return formatBefore(commaLines, indent); - } else { - throw new Error(`Unexpected commaPosition: ${commaPosition}`); - } - }) - .join('\n'); -} - -/** - * Given lines like this: - * - * [ - * 'SELECT', - * ' foo,', - * ' bar, --comment', - * ' baz', - * 'FROM' - * ] - * - * Returns groups like this: - * - * [ - * ['SELECT'], - * [' foo,', ' bar, --comment', ' baz'], - * ['FROM'] - * ] - */ -function groupCommaDelimitedLines(lines: string[]): string[][] { - const groups: string[][] = []; - for (let i = 0; i < lines.length; i++) { - const group = [lines[i]]; - // when line ends with comma, - // gather together all following lines that also end with comma, - // plus one (which doesn't end with comma) - while (lines[i].match(/.*,(\s*(--.*)?$)/)) { - i++; - group.push(lines[i]); - } - groups.push(group); - } - return groups; -} - -// makes all lines the same length by appending spaces before comma -function formatTabular(commaLines: string[]): string[] { - const commaPosition = maxLength(trimTrailingComments(commaLines)) - 1; - return commaLines.map((line, i) => { - if (i === commaLines.length - 1) { - return line; // do not add comma for last item - } - return indentComma(line, commaPosition); - }); -} - -function indentComma(line: string, commaPosition: number) { - const [, code, comment] = line.match(/^(.*?),(\s*--.*)?$/) || []; - - const spaces = ' '.repeat(commaPosition - code.length); - return `${code}${spaces},${comment ?? ''}`; -} - -function formatBefore(commaLines: string[], indent: string): string[] { - return trimTrailingCommas(commaLines).map((line, i) => { - if (i === 0) { - return line; // do not add comma for first item - } - const [whitespace] = line.match(PRECEDING_WHITESPACE_REGEX) || ['']; - return ( - removeLastIndent(whitespace, indent) + - indent.replace(/ {2}$/, ', ') + // add comma to the end of last indent - line.trimStart() - ); - }); -} - -function removeLastIndent(whitespace: string, indent: string): string { - return whitespace.replace(new RegExp(indent + '$'), ''); -} - -function trimTrailingCommas(lines: string[]): string[] { - return lines.map(line => line.replace(/,(\s*(--.*)?$)/, '$1')); -} - -function trimTrailingComments(lines: string[]): string[] { - return lines.map(line => line.replace(/\s*--.*/, '')); -} diff --git a/src/index.ts b/src/index.ts index c3866d669e..8f17bad72b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -17,7 +17,6 @@ export type { IndentStyle, KeywordCase, IdentifierCase, - CommaPosition, LogicalOperatorNewline, FormatOptions, } from './FormatOptions.js'; diff --git a/src/sqlFormatter.ts b/src/sqlFormatter.ts index 7533cf9d8a..51e4cf0488 100644 --- a/src/sqlFormatter.ts +++ b/src/sqlFormatter.ts @@ -44,7 +44,6 @@ const defaultOptions: FormatOptions = { identifierCase: 'preserve', indentStyle: 'standard', logicalOperatorNewline: 'before', - commaPosition: 'after', expressionWidth: 50, linesBetweenQueries: 1, denseOperators: false, diff --git a/src/validateConfig.ts b/src/validateConfig.ts index caa4efbfeb..72419beec4 100644 --- a/src/validateConfig.ts +++ b/src/validateConfig.ts @@ -16,6 +16,12 @@ export function validateConfig(cfg: FormatOptions): FormatOptions { if ('aliasAs' in cfg) { throw new ConfigError('aliasAs config is no more supported.'); } + if ('commaPosition' in cfg) { + throw new ConfigError('commaPosition config is no more supported.'); + } + if ('tabulateAlias' in cfg) { + throw new ConfigError('tabulateAlias config is no more supported.'); + } if (cfg.expressionWidth <= 0) { throw new ConfigError( @@ -23,12 +29,6 @@ export function validateConfig(cfg: FormatOptions): FormatOptions { ); } - if (cfg.commaPosition === 'before' && cfg.useTabs) { - throw new ConfigError( - 'commaPosition: before does not work when tabs are used for indentation.' - ); - } - if (cfg.params && !validateParams(cfg.params)) { // eslint-disable-next-line no-console console.warn('WARNING: All "params" option values should be strings.'); diff --git a/static/index.html b/static/index.html index 69aa855dbd..252bb3d9c0 100644 --- a/static/index.html +++ b/static/index.html @@ -98,14 +98,6 @@

Options

-
- - -
{ const keywordCase = document.getElementById('keywordCase'); const indentStyle = document.getElementById('indentStyle'); const logicalOperatorNewline = document.getElementById('logicalOperatorNewline'); - const commaPosition = document.getElementById('commaPosition'); const expressionWidth = document.getElementById('expressionWidth'); const lineBetweenQueries = document.getElementById('lineBetweenQueries'); const denseOperators = document.getElementById('denseOperators'); @@ -37,7 +36,6 @@ const attachFormat = () => { indentStyle: indentStyle.options[indentStyle.selectedIndex].value, logicalOperatorNewline: logicalOperatorNewline.options[logicalOperatorNewline.selectedIndex].value, - commaPosition: commaPosition.options[commaPosition.selectedIndex].value, expressionWidth: expressionWidth.value, lineBetweenQueries: lineBetweenQueries.value, denseOperators: denseOperators.checked, @@ -72,7 +70,6 @@ const attachFormat = () => { keywordCase, indentStyle, logicalOperatorNewline, - commaPosition, expressionWidth, lineBetweenQueries, denseOperators, diff --git a/test/behavesLikeSqlFormatter.ts b/test/behavesLikeSqlFormatter.ts index 4542107d2d..25f990c6ea 100644 --- a/test/behavesLikeSqlFormatter.ts +++ b/test/behavesLikeSqlFormatter.ts @@ -11,7 +11,6 @@ import supportsExpressionWidth from './options/expressionWidth.js'; import supportsKeywordCase from './options/keywordCase.js'; import supportsIdentifierCase from './options/identifierCase.js'; import supportsIndentStyle from './options/indentStyle.js'; -import supportsCommaPosition from './options/commaPosition.js'; import supportsLinesBetweenQueries from './options/linesBetweenQueries.js'; import supportsNewlineBeforeSemicolon from './options/newlineBeforeSemicolon.js'; import supportsLogicalOperatorNewline from './options/logicalOperatorNewline.js'; @@ -34,7 +33,6 @@ export default function behavesLikeSqlFormatter(format: FormatFn) { supportsLinesBetweenQueries(format); supportsExpressionWidth(format); supportsNewlineBeforeSemicolon(format); - supportsCommaPosition(format); supportsLogicalOperatorNewline(format); supportsParamTypes(format); supportsWindowFunctions(format); diff --git a/test/options/commaPosition.ts b/test/options/commaPosition.ts deleted file mode 100644 index 1d707687a1..0000000000 --- a/test/options/commaPosition.ts +++ /dev/null @@ -1,185 +0,0 @@ -import { expect } from '@jest/globals'; -import dedent from 'dedent-js'; - -import { FormatFn } from '../../src/sqlFormatter.js'; - -export default function supportsCommaPosition(format: FormatFn) { - it('defaults to comma after column', () => { - const result = format( - 'SELECT alpha , MAX(beta) , delta AS d ,epsilon FROM gamma GROUP BY alpha , delta, epsilon' - ); - expect(result).toBe( - dedent` - SELECT - alpha, - MAX(beta), - delta AS d, - epsilon - FROM - gamma - GROUP BY - alpha, - delta, - epsilon - ` - ); - }); - - describe('commaPosition: before', () => { - it('adds comma before column', () => { - const result = format( - 'SELECT alpha, MAX(beta), delta AS d, epsilon FROM gamma GROUP BY alpha, delta, epsilon', - { commaPosition: 'before' } - ); - expect(result).toBe( - dedent` - SELECT - alpha - , MAX(beta) - , delta AS d - , epsilon - FROM - gamma - GROUP BY - alpha - , delta - , epsilon - ` - ); - }); - - it('handles comments after commas', () => { - const result = format( - `SELECT alpha, --comment1 - MAX(beta), --comment2 - delta AS d, epsilon --comment3`, - { commaPosition: 'before' } - ); - expect(result).toBe( - dedent` - SELECT - alpha --comment1 - , MAX(beta) --comment2 - , delta AS d - , epsilon --comment3 - ` - ); - }); - - it('works with larger indent', () => { - const result = format( - 'SELECT alpha, MAX(beta), delta AS d, epsilon FROM gamma GROUP BY alpha, delta, epsilon', - { commaPosition: 'before', tabWidth: 4 } - ); - expect(result).toBe( - dedent` - SELECT - alpha - , MAX(beta) - , delta AS d - , epsilon - FROM - gamma - GROUP BY - alpha - , delta - , epsilon - ` - ); - }); - - // This style is fundamentally incompatible with tabs - it('throws error when tabs used for indentation', () => { - expect(() => { - format('SELECT alpha, MAX(beta), delta AS d, epsilon', { - commaPosition: 'before', - useTabs: true, - }); - }).toThrowErrorMatchingInlineSnapshot( - `"commaPosition: before does not work when tabs are used for indentation."` - ); - }); - }); - - describe('commaPosition: tabular', () => { - it('aligns commas to a column', () => { - const result = format( - 'SELECT alpha, MAX(beta), delta AS d, epsilon FROM gamma GROUP BY alpha, delta, epsilon', - { commaPosition: 'tabular' } - ); - expect(result).toBe( - dedent` - SELECT - alpha , - MAX(beta) , - delta AS d, - epsilon - FROM - gamma - GROUP BY - alpha , - delta , - epsilon - ` - ); - }); - - it('handles comments after commas', () => { - const result = format( - `SELECT alpha, --comment1 - beta,--comment2 - delta, epsilon,--comment3 - iota --comment4`, - { commaPosition: 'tabular' } - ); - expect(result).toBe( - dedent` - SELECT - alpha , --comment1 - beta , --comment2 - delta , - epsilon, --comment3 - iota --comment4 - ` - ); - }); - - it('is not effected by indent size', () => { - const result = format( - 'SELECT alpha, MAX(beta), delta AS d, epsilon FROM gamma GROUP BY alpha, delta, epsilon', - { commaPosition: 'tabular', tabWidth: 6 } - ); - expect(result).toBe( - dedent` - SELECT - alpha , - MAX(beta) , - delta AS d, - epsilon - FROM - gamma - GROUP BY - alpha , - delta , - epsilon - ` - ); - }); - - it('handles tabs', () => { - const result = format('SELECT alpha, MAX(beta), delta AS d, epsilon', { - commaPosition: 'tabular', - useTabs: true, - }); - expect(result).toBe( - dedent` - SELECT - \talpha , - \tMAX(beta) , - \tdelta AS d, - \tepsilon - ` - ); - }); - }); -} diff --git a/test/sqlFormatter.test.ts b/test/sqlFormatter.test.ts index 3efd3b06b3..d63bb03b79 100644 --- a/test/sqlFormatter.test.ts +++ b/test/sqlFormatter.test.ts @@ -55,6 +55,18 @@ describe('sqlFormatter', () => { }).toThrow('aliasAs config is no more supported.'); }); + it('throws error when tabulateAlias config option used', () => { + expect(() => { + format('SELECT *', { tabulateAlias: false } as any); + }).toThrow('tabulateAlias config is no more supported.'); + }); + + it('throws error when commaPosition config option used', () => { + expect(() => { + format('SELECT *', { commaPosition: 'before' } as any); + }).toThrow('commaPosition config is no more supported.'); + }); + describe('formatDialect()', () => { it('allows passing Dialect config object as a dialect parameter', () => { expect(formatDialect('SELECT [foo], `bar`;', { dialect: sqlite })).toBe(dedent`