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

feat(fmt): support SQL #26750

Merged
merged 3 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ serde_repr.workspace = true
sha2.workspace = true
shell-escape = "=0.1.5"
spki = { version = "0.7", features = ["pem"] }
# NOTE(bartlomieju): for now using github URL, because 0.3.2 with important fixes hasn't been released yet.
sqlformat = { git = "https://github.com/shssoichiro/sqlformat-rs.git", rev = "827d639" }
strsim = "0.11.1"
tar.workspace = true
tempfile.workspace = true
Expand Down
27 changes: 25 additions & 2 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ pub struct FmtFlags {
pub no_semicolons: Option<bool>,
pub watch: Option<WatchFlags>,
pub unstable_component: bool,
pub unstable_sql: bool,
}

impl FmtFlags {
Expand Down Expand Up @@ -2291,7 +2292,7 @@ Ignore formatting a file by adding an ignore comment at the top of the file:
.value_parser([
"ts", "tsx", "js", "jsx", "md", "json", "jsonc", "css", "scss",
"sass", "less", "html", "svelte", "vue", "astro", "yml", "yaml",
"ipynb",
"ipynb", "sql"
])
.help_heading(FMT_HEADING).requires("files"),
)
Expand Down Expand Up @@ -2410,6 +2411,14 @@ Ignore formatting a file by adding an ignore comment at the top of the file:
.help_heading(FMT_HEADING)
.hide(true),
)
.arg(
Arg::new("unstable-sql")
.long("unstable-sql")
.help("Enable formatting SQL files.")
.value_parser(FalseyValueParser::new())
.action(ArgAction::SetTrue)
.help_heading(FMT_HEADING),
)
})
}

Expand Down Expand Up @@ -4634,6 +4643,7 @@ fn fmt_parse(
let prose_wrap = matches.remove_one::<String>("prose-wrap");
let no_semicolons = matches.remove_one::<bool>("no-semicolons");
let unstable_component = matches.get_flag("unstable-component");
let unstable_sql = matches.get_flag("unstable-sql");

flags.subcommand = DenoSubcommand::Fmt(FmtFlags {
check: matches.get_flag("check"),
Expand All @@ -4646,6 +4656,7 @@ fn fmt_parse(
no_semicolons,
watch: watch_arg_parse(matches)?,
unstable_component,
unstable_sql,
});
Ok(())
}
Expand Down Expand Up @@ -6565,6 +6576,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: false,
unstable_sql: false,
watch: Default::default(),
}),
..Flags::default()
Expand All @@ -6588,6 +6600,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: false,
unstable_sql: false,
watch: Default::default(),
}),
..Flags::default()
Expand All @@ -6611,6 +6624,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: false,
unstable_sql: false,
watch: Default::default(),
}),
..Flags::default()
Expand All @@ -6634,6 +6648,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: false,
unstable_sql: false,
watch: Some(Default::default()),
}),
..Flags::default()
Expand All @@ -6648,7 +6663,8 @@ mod tests {
"--unstable-css",
"--unstable-html",
"--unstable-component",
"--unstable-yaml"
"--unstable-yaml",
"--unstable-sql"
]);
assert_eq!(
r.unwrap(),
Expand All @@ -6666,6 +6682,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: true,
unstable_sql: true,
watch: Some(WatchFlags {
hmr: false,
no_clear_screen: true,
Expand Down Expand Up @@ -6700,6 +6717,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: false,
unstable_sql: false,
watch: Some(Default::default()),
}),
..Flags::default()
Expand All @@ -6723,6 +6741,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: false,
unstable_sql: false,
watch: Default::default(),
}),
config_flag: ConfigFlag::Path("deno.jsonc".to_string()),
Expand Down Expand Up @@ -6754,6 +6773,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: false,
unstable_sql: false,
watch: Some(Default::default()),
}),
config_flag: ConfigFlag::Path("deno.jsonc".to_string()),
Expand Down Expand Up @@ -6790,6 +6810,7 @@ mod tests {
prose_wrap: Some("never".to_string()),
no_semicolons: Some(true),
unstable_component: false,
unstable_sql: false,
watch: Default::default(),
}),
..Flags::default()
Expand Down Expand Up @@ -6820,6 +6841,7 @@ mod tests {
prose_wrap: None,
no_semicolons: Some(false),
unstable_component: false,
unstable_sql: false,
watch: Default::default(),
}),
..Flags::default()
Expand All @@ -6845,6 +6867,7 @@ mod tests {
prose_wrap: None,
no_semicolons: None,
unstable_component: false,
unstable_sql: false,
watch: Default::default(),
}),
ext: Some("html".to_string()),
Expand Down
4 changes: 4 additions & 0 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ impl BenchOptions {
#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
pub struct UnstableFmtOptions {
pub component: bool,
pub sql: bool,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -322,6 +323,7 @@ impl FmtOptions {
options: resolve_fmt_options(fmt_flags, fmt_config.options),
unstable: UnstableFmtOptions {
component: unstable.component || fmt_flags.unstable_component,
sql: unstable.sql || fmt_flags.unstable_sql,
},
files: fmt_config.files,
}
Expand Down Expand Up @@ -1319,6 +1321,7 @@ impl CliOptions {
let workspace = self.workspace();
UnstableFmtOptions {
component: workspace.has_unstable("fmt-component"),
sql: workspace.has_unstable("fmt-sql"),
}
}

Expand Down Expand Up @@ -1667,6 +1670,7 @@ impl CliOptions {
"byonm",
"bare-node-builtins",
"fmt-component",
"fmt-sql",
])
.collect();

Expand Down
3 changes: 3 additions & 0 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,9 @@ impl Inner {
component: config_data
.map(|d| d.unstable.contains("fmt-component"))
.unwrap_or(false),
sql: config_data
.map(|d| d.unstable.contains("fmt-sql"))
.unwrap_or(false),
};
let document = document.clone();
move || {
Expand Down
1 change: 1 addition & 0 deletions cli/schemas/config-file.v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@
"ffi",
"fs",
"fmt-component",
"fmt-sql",
"http",
"kv",
"net",
Expand Down
64 changes: 63 additions & 1 deletion cli/tools/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ fn format_markdown(
| "njk"
| "yml"
| "yaml"
| "sql"
) {
// It's important to tell dprint proper file extension, otherwise
// it might parse the file twice.
Expand Down Expand Up @@ -301,6 +302,13 @@ fn format_markdown(
}
}
"yml" | "yaml" => format_yaml(text, fmt_options),
"sql" => {
if unstable_options.sql {
format_sql(text, fmt_options)
} else {
Ok(None)
}
}
_ => {
let mut codeblock_config =
get_resolved_typescript_config(fmt_options);
Expand Down Expand Up @@ -503,7 +511,48 @@ pub fn format_html(
})
}

/// Formats a single TS, TSX, JS, JSX, JSONC, JSON, MD, or IPYNB file.
pub fn format_sql(
file_text: &str,
fmt_options: &FmtOptionsConfig,
) -> Result<Option<String>, AnyError> {
let ignore_file = file_text
.lines()
.take_while(|line| line.starts_with("--"))
.any(|line| {
line
.strip_prefix("--")
.unwrap()
.trim()
.starts_with("deno-fmt-ignore-file")
});

if ignore_file {
return Ok(None);
}

let mut formatted_str = sqlformat::format(
file_text,
&sqlformat::QueryParams::None,
&sqlformat::FormatOptions {
ignore_case_convert: None,
indent: if fmt_options.use_tabs.unwrap_or_default() {
sqlformat::Indent::Tabs
} else {
sqlformat::Indent::Spaces(fmt_options.indent_width.unwrap_or(2))
},
// leave one blank line between queries.
lines_between_queries: 2,
uppercase: Some(true),
Comment on lines +544 to +545
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be beneficial to control this options?
For me it would be a nice to have to control the uppercase, as i like my sql to be lowercase, but all good if not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for the first pass we shouldn't have it; we also have no "knobs" for controlling options for HTML/CSS/YAML formatters for now.

},
);

// Add single new line to the end of file.
formatted_str.push('\n');

Ok(Some(formatted_str))
}

/// Formats a single TS, TSX, JS, JSX, JSONC, JSON, MD, IPYNB or SQL file.
pub fn format_file(
file_path: &Path,
file_text: &str,
Expand Down Expand Up @@ -538,6 +587,13 @@ pub fn format_file(
format_file(file_path, &file_text, fmt_options, unstable_options, None)
},
),
"sql" => {
if unstable_options.sql {
format_sql(file_text, fmt_options)
} else {
Ok(None)
}
}
_ => {
let config = get_resolved_typescript_config(fmt_options);
dprint_plugin_typescript::format_text(
Expand Down Expand Up @@ -1209,6 +1265,7 @@ fn is_supported_ext_fmt(path: &Path) -> bool {
| "yml"
| "yaml"
| "ipynb"
| "sql"
)
})
}
Expand Down Expand Up @@ -1269,6 +1326,11 @@ mod test {
assert!(is_supported_ext_fmt(Path::new("foo.yaml")));
assert!(is_supported_ext_fmt(Path::new("foo.YaML")));
assert!(is_supported_ext_fmt(Path::new("foo.ipynb")));
assert!(is_supported_ext_fmt(Path::new("foo.sql")));
assert!(is_supported_ext_fmt(Path::new("foo.Sql")));
assert!(is_supported_ext_fmt(Path::new("foo.sQl")));
assert!(is_supported_ext_fmt(Path::new("foo.sqL")));
assert!(is_supported_ext_fmt(Path::new("foo.SQL")));
}

#[test]
Expand Down
Loading