Skip to content

fix: keep spaces around - operator in dialects with dashed identifiers#953

Open
sarathfrancis90 wants to merge 1 commit into
sql-formatter-org:masterfrom
sarathfrancis90:fix-dense-minus-bigquery-identifier
Open

fix: keep spaces around - operator in dialects with dashed identifiers#953
sarathfrancis90 wants to merge 1 commit into
sql-formatter-org:masterfrom
sarathfrancis90:fix-dense-minus-bigquery-identifier

Conversation

@sarathfrancis90

@sarathfrancis90 sarathfrancis90 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

With denseOperators: true on BigQuery, a subtraction like a - b comes out as a-b. That's not just ugly: BigQuery allows dashes inside identifiers, so the densed output re-parses as a single identifier a-b, and format(format(sql)) no longer matches format(sql). a - foo(y) is worse — it becomes a-foo (y), turning the expression into an identifier followed by a call.

The fix is to leave the - operator spaced in dialects that allow dashed identifiers (currently just BigQuery). I derived that flag from the existing identChars.dashes tokenizer option so there's a single source of truth. Numeric cases like 1 - 2 were already safe (the - is parsed into the literal), so this only affects the identifier case.

The shared dense-operator test in operators.ts was actually asserting the broken foo-bar output for BigQuery; I scoped that to the dashed-identifier dialects and added a BigQuery regression test.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed formatting of the - operator in dialects that support dashes in identifiers, such as BigQuery. When dense operator formatting is enabled, spaces around the - operator are now properly preserved to prevent expressions from being incorrectly merged into dashed identifiers.

In BigQuery (and any dialect allowing dashes inside identifiers) the
denseOperators option turned "a - b" into "a-b", which then re-parses
as a single dashed identifier - corrupting the query. Skip densing the
"-" operator for those dialects so the output round-trips.
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e3b36fa7-e8ec-4e96-8e6c-d7df6cadcf9f

📥 Commits

Reviewing files that changed from the base of the PR and between f3707ba and 68e8f71.

📒 Files selected for processing (4)
  • src/dialect.ts
  • src/formatter/ExpressionFormatter.ts
  • test/bigquery.test.ts
  • test/features/operators.ts

📝 Walkthrough

Walkthrough

The PR adds support for dialect-specific handling of the minus operator to prevent accidental merging of a - b into dashed identifiers in dialects like BigQuery. A new identifierDashes flag flows from tokenizer configuration through dialect setup and into operator formatting logic, with supporting test coverage.

Changes

Identifier dashes operator spacing

Layer / File(s) Summary
Type contract and configuration definitions
src/formatter/ExpressionFormatter.ts, test/features/operators.ts
ProcessedDialectFormatOptions interface adds identifierDashes boolean flag; OperatorsConfig test type gains corresponding optional field to document dialect support for dashed identifiers.
Dialect configuration propagation
src/dialect.ts
dialectFromOptions and processDialectFormatOptions compute and propagate identifierDashes derived from tokenizerOptions.identChars?.dashes into format options.
Operator formatting with identifier dashes
src/formatter/ExpressionFormatter.ts
formatOperator special-cases the - operator when identifierDashes is enabled, preserving surrounding spaces before falling through to standard dense operator logic.
Test coverage and validation
test/bigquery.test.ts, test/features/operators.ts
BigQuery test enables identifierDashes: true and adds regression test verifying space preservation around - in dense mode; operators test harness conditionally validates expected behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A dash that dares to dash with pride,
Must keep its spaces, side by side,
Lest a - b become a-b,
A single name where two should be!
The formatter learns dialects deep,
To guard the - in BigQuery's keep. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving spaces around the minus operator in dialects supporting dashed identifiers, which directly addresses the core fix in the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant