Skip to content

Commit

Permalink
fix: uppercase=false does not lowercase the query
Browse files Browse the repository at this point in the history
  • Loading branch information
wugeer committed Sep 16, 2024
2 parents c68c0a4 + cf4eee9 commit 26e74a3
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 65 deletions.
77 changes: 43 additions & 34 deletions .github/workflows/sqlformat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,48 +14,57 @@ jobs:
strategy:
matrix:
conf:
- minimum
- latest-stable
- latest-beta
- latest-nightly
include:
- conf: minimum
toolchain: 1.62.0
- conf: latest-stable
toolchain: stable
- conf: latest-beta
toolchain: beta
- conf: latest-nightly
toolchain: nightly
steps:
- uses: actions/checkout@v2
- name: Install ${{ matrix.toolchain }}
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: ${{ matrix.toolchain }}
override: true
components: clippy, rustfmt
- name: Cache cargo registry
uses: actions/cache@v1
with:
path: ~/.cargo/registry/cache
key: ${{ runner.os }}-${{ matrix.conf }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }}
restore-keys: |
${{ runner.os }}-${{ matrix.conf }}-cargo-registry-
- name: Run rustfmt
if: matrix.toolchain == 'stable'
uses: actions-rs/cargo@v1
with:
command: fmt
args: -- --check
- name: Run clippy
if: matrix.toolchain == 'stable'
uses: actions-rs/clippy-check@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
args: -- -D warnings
- name: Run tests
run: cargo test
- name: Build benchmarks
if: matrix.toolchain == 'stable'
run: cargo bench --no-run
- name: Build docs
run: cargo doc --no-deps
- uses: actions/checkout@v2
- name: Install ${{ matrix.toolchain }}
uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: ${{ matrix.toolchain }}
override: true
components: clippy, rustfmt
- name: Cache cargo registry
uses: actions/cache@v1
with:
path: ~/.cargo/registry/cache
key: ${{ runner.os }}-${{ matrix.conf }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }}
restore-keys: |
${{ runner.os }}-${{ matrix.conf }}-cargo-registry-
- name: Run rustfmt
if: matrix.toolchain == 'stable'
uses: actions-rs/cargo@v1
with:
command: fmt
args: -- --check
- name: Run clippy
if: matrix.toolchain == 'stable'
uses: actions-rs/clippy-check@v1
with:
token: ${{ secrets.GITHUB_TOKEN }}
args: -- -D warnings
# FIXME: criterion and its dependencies require a newer version than 1.62, but it is only used for benchmarks.
# Is there a way to not have criterion built when we run tests?
- name: Run cargo check
if: matrix.toolchain == '1.62.0'
run: cargo check
- name: Run tests
if: matrix.toolchain != '1.62.0'
run: cargo test
- name: Build benchmarks
if: matrix.toolchain == 'stable'
run: cargo bench --no-run
- name: Build docs
run: cargo doc --no-deps
11 changes: 10 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
### Version 0.2.6

- fix: ON UPDATE with two many blank formatted incorrectly (#46)
- fix: `EXCEPT` not handled well
- fix: REFERENCES xyz ON UPDATE .. causes formatter to treat the remaining as an UPDATE statement
- fix: Escaped strings formatted incorrectly
- fix: RETURNING is not placed on a new line
- fix: fix the issue of misaligned comments after formatting (#40)

### Version 0.2.4

- Remove `itertools` dependency [#34](https://github.com/shssoichiro/sqlformat-rs/pull/34)
Expand Down Expand Up @@ -26,7 +35,7 @@
- Fix extra spaces in string escaping [#13](https://github.com/shssoichiro/sqlformat-rs/pull/13)
- Fix panic on overflowing integer [#14](https://github.com/shssoichiro/sqlformat-rs/pull/14)
- Bump Rust edition to 2021
- This is technically a breaking change as it bumps the minimum Rust version to 1.56
- This is technically a breaking change as it bumps the minimum Rust version to 1.56

### Version 0.1.8

Expand Down
8 changes: 5 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
[package]
name = "sqlformat"
version = "0.2.4"
version = "0.2.6"
authors = ["Josh Holmer <[email protected]>"]
edition = "2021"
rust-version = "1.56"
rust-version = "1.62"
license = "MIT OR Apache-2.0"
homepage = "https://github.com/shssoichiro/sqlformat-rs"
repository = "https://github.com/shssoichiro/sqlformat-rs"
Expand All @@ -15,9 +15,11 @@ categories = ["development-tools"]
[dependencies]
nom = "7.0.0"
unicode_categories = "0.1.1"
once_cell = "1"
regex = "=1.6"

[dev-dependencies]
criterion = "0.5"
criterion = "0.4"
indoc = "2.0"

[[bench]]
Expand Down
2 changes: 0 additions & 2 deletions benches/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ fn issue_633(c: &mut Criterion) {
const SIZE: usize = 1000;

pub struct UserData {
pub id: i64,
pub first_name: String,
pub last_name: String,
pub address: String,
Expand All @@ -87,7 +86,6 @@ fn issue_633(c: &mut Criterion) {

fn sample() -> UserData {
UserData {
id: -1,
first_name: "FIRST_NAME".to_string(),
last_name: "LAST_NAME".to_string(),
address: "SOME_ADDRESS".to_string(),
Expand Down
57 changes: 41 additions & 16 deletions src/formatter.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use regex::Regex;
use std::borrow::Cow;

use crate::indentation::Indentation;
Expand All @@ -6,12 +7,40 @@ use crate::params::Params;
use crate::tokenizer::{Token, TokenKind};
use crate::{FormatOptions, QueryParams};

use once_cell::sync::Lazy;

static RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"(?i)^(--|/\*)\s*fmt\s*:\s*(off|on)").unwrap());

pub(crate) fn check_fmt_off(s: &str) -> Option<bool> {
RE.captures(s)?
.get(2)
.map(|matched| matched.as_str().eq_ignore_ascii_case("off"))
}

pub(crate) fn format(tokens: &[Token<'_>], params: &QueryParams, options: FormatOptions) -> String {
let mut formatter = Formatter::new(tokens, params, options);
let mut formatted_query = String::new();
let mut is_fmt_enabled = true;
let mut is_prev_token_fmt_switch = false;
for (index, token) in tokens.iter().enumerate() {
if is_prev_token_fmt_switch {
is_prev_token_fmt_switch = false;
continue;
}
if matches!(token.kind, TokenKind::LineComment | TokenKind::BlockComment) {
if let Some(is_fmt_off) = check_fmt_off(token.value) {
is_fmt_enabled = !is_fmt_off;
is_prev_token_fmt_switch = true;
continue;
}
}
formatter.index = index;

if !is_fmt_enabled {
formatter.format_no_change(token, &mut formatted_query);
continue;
}

if token.kind == TokenKind::Whitespace {
// ignore (we do our own whitespace formatting)
} else if token.kind == TokenKind::LineComment {
Expand Down Expand Up @@ -79,16 +108,7 @@ impl<'a> Formatter<'a> {
self.next_token(1).map_or(false, |current_token| {
current_token.kind == TokenKind::Whitespace
&& self.next_token(2).map_or(false, |next_token| {
matches!(
next_token.kind,
TokenKind::Number
| TokenKind::String
| TokenKind::Word
| TokenKind::ReservedTopLevel
| TokenKind::ReservedTopLevelNoIndent
| TokenKind::ReservedNewline
| TokenKind::Reserved
)
!matches!(next_token.kind, TokenKind::Operator)
})
});

Expand Down Expand Up @@ -136,13 +156,14 @@ impl<'a> Formatter<'a> {
}

fn format_with_spaces(&self, token: &Token<'_>, query: &mut String) {
let value = if token.kind == TokenKind::Reserved {
self.format_reserved_word(token.value)
if token.kind == TokenKind::Reserved {
let value = self.equalize_whitespace(&self.format_reserved_word(token.value));
query.push_str(&value);
query.push(' ');
} else {
Cow::Borrowed(token.value)
query.push_str(token.value);
query.push(' ');
};
query.push_str(&value);
query.push(' ');
}

// Opening parentheses increase the block indent level and start a new line
Expand Down Expand Up @@ -249,7 +270,7 @@ impl<'a> Formatter<'a> {
}

fn trim_spaces_end(&self, query: &mut String) {
query.truncate(query.trim_end_matches(|c| c == ' ' || c == '\t').len());
query.truncate(query.trim_end_matches([' ', '\t']).len());
}

fn trim_all_spaces_end(&self, query: &mut String) {
Expand Down Expand Up @@ -315,4 +336,8 @@ impl<'a> Formatter<'a> {
None
}
}

fn format_no_change(&self, token: &Token<'_>, query: &mut String) {
query.push_str(token.value);
}
}
94 changes: 86 additions & 8 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,14 @@ pub enum Indent {
Tabs,
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, Default)]
pub enum QueryParams {
Named(Vec<(String, String)>),
Indexed(Vec<String>),
#[default]
None,
}

impl Default for QueryParams {
fn default() -> Self {
QueryParams::None
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -1621,7 +1616,7 @@ mod tests {
#[test]
fn it_recognizes_on_update_clause() {
let input = indoc!(
"CREATE TABLE a (b integer REFERENCES c (id) ON UPDATE RESTRICT, other integer);"
"CREATE TABLE a (b integer REFERENCES c (id) ON UPDATE RESTRICT, other integer);"
);
let options = FormatOptions::default();
let expected = indoc!(
Expand Down Expand Up @@ -1659,6 +1654,89 @@ mod tests {
assert_eq!(format(input, &QueryParams::None, options), expected);
}

#[test]
fn it_recognizes_fmt_off() {
let input = indoc!(
"SELECT * FROM sometable
WHERE
-- comment test here
-- fmt: off
first_key.second_key = 1
-- json:first_key.second_key = 1
-- fmt: on
AND
-- fm1t: off
first_key.second_key = 1
-- json:first_key.second_key = 1
-- fmt:on"
);
let options = FormatOptions {
indent: Indent::Spaces(4),
..Default::default()
};
let expected = indoc!(
"
SELECT
*
FROM
sometable
WHERE
-- comment test here
first_key.second_key = 1
-- json:first_key.second_key = 1
AND
-- fm1t: off
first_key.second_key = 1
-- json:first_key.second_key = 1"
);

assert_eq!(format(input, &QueryParams::None, options), expected);
}

#[test]
fn it_converts_keywords_to_lowercase_when_option_passed_in() {
let input = "select distinct * frOM foo left join bar WHERe cola > 1 and colb = 3";
let options = FormatOptions {
uppercase: Some(false),
..FormatOptions::default()
};
let expected = indoc!(
"
select
distinct *
from
foo
left join bar
where
cola > 1
and colb = 3"
);

assert_eq!(format(input, &QueryParams::None, options), expected);
}

#[test]
fn it_converts_keywords_nothing_when_no_option_passed_in() {
let input = "select distinct * frOM foo left join bar WHERe cola > 1 and colb = 3";
let options = FormatOptions {
uppercase: None,
..FormatOptions::default()
};
let expected = indoc!(
"
select
distinct *
frOM
foo
left join bar
WHERe
cola > 1
and colb = 3"
);

assert_eq!(format(input, &QueryParams::None, options), expected);
}

#[test]
fn it_converts_keywords_to_lowercase_when_option_passed_in() {

Check failure on line 1741 in src/lib.rs

View workflow job for this annotation

GitHub Actions / build-test-unix (latest-stable)

the name `it_converts_keywords_to_lowercase_when_option_passed_in` is defined multiple times
let input = "select distinct * frOM foo left join bar WHERe cola > 1 and colb = 3";
Expand Down
3 changes: 2 additions & 1 deletion src/tokenizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,8 @@ fn get_plain_reserved_two_token(input: &str) -> IResult<&str, Token<'_>> {
terminated(tag("ON UPDATE"), end_of_word),
))(&uc_input);
if let Ok((_, token)) = result {
let input_end_pos = token.len();
let final_word = token.split(' ').last().unwrap();
let input_end_pos = input.to_ascii_uppercase().find(final_word).unwrap() + final_word.len();
let (token, input) = input.split_at(input_end_pos);
Ok((
input,
Expand Down

0 comments on commit 26e74a3

Please sign in to comment.