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: implement grit format command #595

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
561ce1d
feat: implement proof of concept of format command
Arian8j2 Jan 1, 2025
03d22b2
refactor: move format for loop body to different function
Arian8j2 Jan 2, 2025
7b49f63
feat: add tests for markdown and grit files (yaml currently has issues)
Arian8j2 Jan 2, 2025
fc431ec
feat: add yaml support
Arian8j2 Jan 2, 2025
7f767a4
feat: make file path bold in print
Arian8j2 Jan 2, 2025
3b0de9e
feat: change format command help message
Arian8j2 Jan 3, 2025
5761689
refactor: better explanation of grit_code_with_yaml_indent
Arian8j2 Jan 3, 2025
203de80
refactor: rewrite file using hunks instead of find and replace
Arian8j2 Jan 3, 2025
18e038f
feat: use better and consistent way of formating yaml files
Arian8j2 Jan 3, 2025
ea82025
chore: change some comments and variable names
Arian8j2 Jan 3, 2025
5ae0ebd
fix: resolves not being sorted currectly
Arian8j2 Jan 3, 2025
b744fdd
refactor: resolv todo
Arian8j2 Jan 4, 2025
f9b05e9
refactor: rename some functions and variables
Arian8j2 Jan 4, 2025
5e697ad
chore: fix clippy warnings
Arian8j2 Jan 4, 2025
1807c3c
refactor: better format .grit files
Arian8j2 Jan 4, 2025
570ff55
revert: grit range pointing at pattern definition
Arian8j2 Jan 4, 2025
828a2b5
feat: unwrap instead of returning error when it's safe
Arian8j2 Jan 4, 2025
3dcc719
chore: format yaml.rs
Arian8j2 Jan 4, 2025
9a13522
chore: remove todo question that i need to ask in review
Arian8j2 Jan 4, 2025
1c11a9f
refactor: return some errors instead of unwrapping
Arian8j2 Jan 7, 2025
e97fba8
chore: fix typos
Arian8j2 Jan 7, 2025
54076c7
feat: format files in parallel
Arian8j2 Jan 7, 2025
b049607
refactor: remove useless reference
Arian8j2 Jan 7, 2025
a977855
test: remove format_patterns test that snapshots the stdout
Arian8j2 Jan 7, 2025
f593bdd
test: add test for not parsable patterns
Arian8j2 Jan 7, 2025
9e7c24b
feat: use grit pattern to find and replace grit code in yaml
Arian8j2 Jan 10, 2025
0af5bf0
chore: fix comment
Arian8j2 Jan 11, 2025
409dcb3
refactor: format_yaml_file take patterns by reference
Arian8j2 Jan 11, 2025
7bb2622
chore: fix clippy warnings
Arian8j2 Jan 11, 2025
692673d
refactor: remove usesless clones
Arian8j2 Jan 11, 2025
ac8a0a8
refactor: apply hunk changes in reverse to make our life easier
Arian8j2 Jan 11, 2025
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
799 changes: 654 additions & 145 deletions Cargo.lock

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ anyhow = { version = "1.0.70" }
clap = { version = "4.1.13", features = ["derive"] }
indicatif = { version = "0.17.5" }
# Do *NOT* upgrade beyond 1.0.171 until https://github.com/serde-rs/serde/issues/2538 is fixed
Copy link
Author

@Arian8j2 Arian8j2 Jan 4, 2025

Choose a reason for hiding this comment

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

@morgante Is this still important because biome depends on serde version 1.0.217?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's avoid changing this in the same PR if we can.

Copy link
Author

@Arian8j2 Arian8j2 Jan 7, 2025

Choose a reason for hiding this comment

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

after looking deeper into this, i found out that the main branch doesn't even use serde version under 1.0.171 😄, cargo lock of main branch shows:

[[package]]
name = "serde"
version = "1.0.208"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cff085d2cb684faa248efb494c39b68e522822ac0de72ccf08109abde717cfb2"
dependencies = [
 "serde_derive",
]

even in the very first commit 79a5365, cargo lock was:

[[package]]
name = "serde"
version = "1.0.197"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3fb1c873e1b9b056a4dc4c0c198b24c3ffa059243875552b2bd0933b1aee4ce2"
dependencies = [
 "serde_derive",
]

the current cargo toml is:

serde = { version = "1.0.164", features = ["derive"] }

this allows versions >= 1.0.164, < 2.0.0 according to the cargo book, for exact 1.0.164 version the toml should looked like:

serde = { version = "=1.0.164", features = ["derive"] }

or for under 1.0.171 it should looked like this:

serde = { version = "< 1.0.171", features = ["derive"] }

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok this is fine then.

Copy link
Author

@Arian8j2 Arian8j2 Jan 7, 2025

Choose a reason for hiding this comment

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

ok then do you want me to remove that comment to avoid confusion later?

serde = { version = "1.0.164", features = ["derive"] }
serde = { version = "1.0.217", features = ["derive"] }
serde_json = { version = "1.0.96" }
uuid = { version = "1.1", features = ["v4", "serde"] }
tokio = { version = "1", features = ["full"] }
Expand Down Expand Up @@ -88,6 +88,8 @@ tracing-subscriber = { version = "0.3", default-features = false, optional = tru
tracing-log = { version = "0.2.0", optional = true }

fs-err = { version = "2.11.0" }
biome_grit_parser = { git = "https://github.com/biomejs/biome" }
biome_grit_formatter = { git = "https://github.com/biomejs/biome" }

[target.'cfg(not(windows))'.dependencies]
openssl = { version = "0.10", features = ["vendored"] }
Expand Down
244 changes: 244 additions & 0 deletions crates/cli/src/commands/format.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
use crate::{
resolver::{resolve_from_cwd, GritModuleResolver, Source},
ux::{format_diff, DiffString},
};
use anyhow::{anyhow, bail, ensure, Context, Result};
use biome_grit_formatter::context::GritFormatOptions;
use clap::Args;
use colored::Colorize;
use marzano_core::api::MatchResult;
use marzano_gritmodule::{config::ResolvedGritDefinition, parser::PatternFileExt};
use marzano_util::{rich_path::RichFile, runtime::ExecutionContext};
use rayon::iter::{IntoParallelIterator, ParallelIterator};
use serde::Serialize;
use std::collections::BTreeMap;

#[derive(Args, Debug, Serialize, Clone)]
pub struct FormatArgs {
/// Write formats to file instead of just showing them
#[clap(long)]
pub write: bool,
}

pub async fn run_format(arg: &FormatArgs) -> Result<()> {
let (resolved, _) = resolve_from_cwd(&Source::Local).await?;

let file_path_to_resolved = group_resolved_patterns_by_group(resolved);
let mut results = file_path_to_resolved
.into_par_iter()
.map(|(file_path, resolved_patterns)| {
let result = format_file_resolved_patterns(&file_path, resolved_patterns, arg.clone());
(file_path, result)
})
.collect::<Vec<_>>();

// sort outputs to ensure consistent stdout output
// also avoid using sort_by_key to prevent additional cloning of file_path
results.sort_by(|(file_path, _), (other_file_path, _)| file_path.cmp(other_file_path));

for (file_path, result) in results {
match result {
Err(error) => eprintln!("couldn't format '{}': {error:?}", file_path),
Ok(Some(diff)) => println!("{}:\n{}", file_path.bold(), diff),
Ok(None) => (), // `args.write` is true or file is already formated
}
}
Ok(())
}

fn group_resolved_patterns_by_group(
resolved: Vec<ResolvedGritDefinition>,
) -> Vec<(String, Vec<ResolvedGritDefinition>)> {
resolved.into_iter().fold(Vec::new(), |mut acc, resolved| {
let file_path = &resolved.config.path;
if let Some((_, resolved_patterns)) = acc
.iter_mut()
.find(|(resolv_file_path, _)| resolv_file_path == file_path)
{
resolved_patterns.push(resolved);
} else {
acc.push((file_path.clone(), vec![resolved]));
}
acc
})
}

fn format_file_resolved_patterns(
file_path: &str,
patterns: Vec<ResolvedGritDefinition>,
arg: FormatArgs,
) -> Result<Option<DiffString>> {
let first_pattern = patterns
.first()
.ok_or_else(|| anyhow!("patterns is empty"))?;
let first_pattern_raw_data = first_pattern
.config
.raw
.as_ref()
.ok_or_else(|| anyhow!("pattern doesn't have raw data"))?;
let old_file_content = &first_pattern_raw_data.content;

let new_file_content = match first_pattern_raw_data.format {
PatternFileExt::Yaml => format_yaml_file(&patterns, old_file_content)?,
PatternFileExt::Grit => format_grit_code(old_file_content)?,
PatternFileExt::Md => {
let hunks = patterns
.iter()
.map(format_pattern_as_hunk_changes)
.collect::<Result<Vec<HunkChange>>>()?;
apply_hunk_changes(old_file_content, hunks)
}
};

if &new_file_content == old_file_content {
return Ok(None);
}

if arg.write {
std::fs::write(file_path, new_file_content).with_context(|| "could not write to file")?;
Ok(None)
} else {
Ok(Some(format_diff(old_file_content, &new_file_content)))
}
}

/// bubble clause that finds a grit pattern with name "\<pattern_name\>" in yaml and
/// replaces it's body to "\<new_body\>", `format_yaml_file` uses this pattern to replace
/// pattern bodies with formatted ones
const YAML_REPLACE_BODY_PATERN: &str = r#"
Copy link
Contributor

@morgante morgante Jan 11, 2025

Choose a reason for hiding this comment

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

Composing a string then parsing it is going to be error prone / run into escaping issues.

Better to just build the pattern directly in Rust.

Copy link
Author

@Arian8j2 Arian8j2 Jan 11, 2025

Choose a reason for hiding this comment

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

Composing a string then parsing it is going to be error prone / run into escaping issues.

Yeah, i was thinking about inserting variable to runtime and use it in the pattern, But couldn't find how to do it

Better to just build the pattern directly in Rust.

That's gonna be hard :), Is there any place that i can see examples of this?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's gonna be hard :), Is there any place that i can see examples of this?

Unfortunately don't have many examples today, you would have to build it up via the different pattern structs. I'm probably ok merging this as-is.

Copy link
Author

@Arian8j2 Arian8j2 Jan 11, 2025

Choose a reason for hiding this comment

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

Is escaping just double quote ok? if so yeah i think it can be merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah double quotes should be sufficient.

bubble file($body) where {
$body <: contains block_mapping(items=$items) where {
$items <: within `patterns: $_`,
$items <: contains `name: $name`,
$name <: "<pattern_name>",
$items <: contains `body: $yaml_body`,
$new_body = "<new_body>",
$yaml_body => $new_body
},
}
"#;

/// format each pattern and use gritql pattern to match and rewrite
fn format_yaml_file(patterns: &[ResolvedGritDefinition], file_content: &str) -> Result<String> {
let bubbles = patterns
.iter()
.map(|pattern| {
let formatted_body = format_grit_code(&pattern.body)
.with_context(|| format!("could not format '{}'", pattern.name()))?;
let bubble = YAML_REPLACE_BODY_PATERN
.replace("<pattern_name>", pattern.name())
.replace("<new_body>", &format_yaml_body_code(&formatted_body));
Ok(bubble)
})
.collect::<Result<Vec<_>>>()?
.join(",\n");
let pattern_body = format!("language yaml\nsequential{{ {bubbles} }}");
apply_grit_rewrite(file_content, &pattern_body)
}

fn format_yaml_body_code(input: &str) -> String {
// yaml body still needs two indentation to look good
let body_with_prefix = prefix_lines(input, &" ".repeat(2));
let escaped_body = body_with_prefix.replace("\"", "\\\"");
// body: |
// escaped_body
format!("|\n{escaped_body}")
}

fn prefix_lines(input: &str, prefix: &str) -> String {
input
.lines()
.map(|line| {
if line.is_empty() {
line.to_owned()
} else {
format!("{prefix}{line}")
}
})
.collect::<Vec<_>>()
.join("\n")
}

fn apply_grit_rewrite(input: &str, pattern: &str) -> Result<String> {
let resolver = GritModuleResolver::new();
let rich_pattern = resolver.make_pattern(pattern, None)?;

let compiled = rich_pattern
.compile(&BTreeMap::new(), None, None, None)
.map(|cr| cr.problem)
.with_context(|| "could not compile pattern")?;

let rich_file = RichFile::new(String::new(), input.to_owned());
let runtime = ExecutionContext::default();
for result in compiled.execute_file(&rich_file, &runtime) {
if let MatchResult::Rewrite(rewrite) = result {
let content = rewrite
.rewritten
.content
.ok_or_else(|| anyhow!("rewritten content is empty"))?;
return Ok(content);
}
}
bail!("no rewrite result after applying grit pattern")
}

fn format_pattern_as_hunk_changes(pattern: &ResolvedGritDefinition) -> Result<HunkChange> {
let formatted_grit_code = format_grit_code(&pattern.body)?;
let body_range = pattern
.config
.range
.ok_or_else(|| anyhow!("pattern doesn't have config range"))?;
Ok(HunkChange {
starting_byte: body_range.start_byte as usize,
ending_byte: body_range.end_byte as usize,
new_content: formatted_grit_code,
})
}

/// format grit code using `biome`
fn format_grit_code(source: &str) -> Result<String> {
let parsed = biome_grit_parser::parse_grit(source);
ensure!(
parsed.diagnostics().is_empty(),
"biome couldn't parse: {}",
parsed
.diagnostics()
.iter()
.map(|diag| diag.message.to_string())
.collect::<Vec<_>>()
.join("\n")
);

let options = GritFormatOptions::default();
let doc = biome_grit_formatter::format_node(options, &parsed.syntax())
.with_context(|| "biome couldn't format")?;
Ok(doc.print()?.into_code())
}

/// Represent a hunk of text that needs to be changed
#[derive(Debug)]
struct HunkChange {
starting_byte: usize,
ending_byte: usize,
new_content: String,
}

/// returns a new string that applies hunk changes
fn apply_hunk_changes(input: &str, mut hunks: Vec<HunkChange>) -> String {
if hunks.is_empty() {
return input.to_string();
}
hunks.sort_by_key(|hunk| hunk.starting_byte);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you're sorting it, just reverse the order (apply last to first) and use replace_range.

Copy link
Author

Choose a reason for hiding this comment

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

Nice idea, Done

let mut buffer = String::new();
let mut last_ending_byte = 0;
for (index, hunk) in hunks.iter().enumerate() {
buffer.push_str(&input[last_ending_byte..hunk.starting_byte]);
buffer.push_str(&hunk.new_content);
last_ending_byte = hunk.ending_byte;

if index == hunks.len() - 1 {
buffer.push_str(&input[last_ending_byte..]);
}
}
buffer
}
Arian8j2 marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions crates/cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub(crate) mod patterns_list;
pub(crate) mod patterns_test;
pub(crate) mod plumbing;
pub(crate) mod version;
pub(crate) mod format;

#[cfg(feature = "workflows_v2")]
pub(crate) mod apply_migration;
Expand Down Expand Up @@ -84,6 +85,7 @@ use indicatif_log_bridge::LogWrapper;
use init::InitArgs;
use install::InstallArgs;
use list::ListArgs;
use format::{run_format, FormatArgs};
use log::LevelFilter;
use lsp::LspArgs;
use marzano_messenger::emit::ApplyDetails;
Expand Down Expand Up @@ -164,6 +166,8 @@ pub enum Commands {
Plumbing(PlumbingArgs),
/// Display version information about the CLI and agents
Version(VersionArgs),
/// Format grit files under current directory
Format(FormatArgs),
/// Generate documentation for the Grit CLI (internal use only)
#[cfg(feature = "docgen")]
#[clap(hide = true)]
Expand Down Expand Up @@ -204,6 +208,7 @@ impl fmt::Display for Commands {
},
Commands::Plumbing(_) => write!(f, "plumbing"),
Commands::Version(_) => write!(f, "version"),
Commands::Format(_) => write!(f, "format"),
#[cfg(feature = "docgen")]
Commands::Docgen(_) => write!(f, "docgen"),
#[cfg(feature = "server")]
Expand Down Expand Up @@ -439,6 +444,7 @@ async fn run_command(_use_tracing: bool) -> Result<()> {
run_plumbing(arg, multi, &mut apply_details, app.format_flags).await
}
Commands::Version(arg) => run_version(arg).await,
Commands::Format(arg) => run_format(&arg).await,
#[cfg(feature = "docgen")]
Commands::Docgen(arg) => run_docgen(arg).await,
#[cfg(feature = "server")]
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/src/ux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ impl fmt::Display for DiffString {
}
}

fn format_diff(expected: &str, actual: &str) -> DiffString {
pub fn format_diff(expected: &str, actual: &str) -> DiffString {
let mut output = String::new();
let diff = TextDiff::from_lines(expected, actual);

Expand Down
21 changes: 21 additions & 0 deletions crates/cli_bin/fixtures/unformatted_patterns/.grit/grit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
version: 0.0.1
patterns:
- name: aspect_ratio_yaml
description: Yaml version of aspect_ratio.md
body: |
language css
`a { $props }` where {
$props <: contains `aspect-ratio: $x`
}
- file: ./others/test_move_import.md

- name: some_json_pattern
body: |
language json
`account: $val` where {
$val <: contains `password: $password`,
$password => raw`hidden`
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
private: true
tags: [private]
---
```grit
language js
`sanitizeFilePath` as $s where {
move_import(`sanitizeFilePath`, `'@getgrit/universal'`)
}
```
Loading