Skip to content

Commit

Permalink
feat: skip warning for searches (#428)
Browse files Browse the repository at this point in the history
  • Loading branch information
morgante authored Jul 20, 2024
1 parent 4141c07 commit 5923f64
Show file tree
Hide file tree
Showing 12 changed files with 367 additions and 82 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ grit-util = { path = "../grit-util" }
marzano-core = { path = "../core", features = [
"non_wasm",
], default-features = false }
grit-pattern-matcher = { path = "../grit-pattern-matcher" }
marzano-gritmodule = { path = "../gritmodule" }
marzano-language = { path = "../language", features = ["finder"] }
marzano-lsp = { path = "../lsp" }
Expand Down
36 changes: 18 additions & 18 deletions crates/cli/src/commands/apply_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use tracing::span;
#[allow(unused_imports)]
use tracing_opentelemetry::OpenTelemetrySpanExt as _;

use grit_pattern_matcher::has_rewrite;
use grit_util::Position;
use indicatif::MultiProgress;
use marzano_core::api::{AllDone, AllDoneReason, AnalysisLog, MatchResult};
Expand Down Expand Up @@ -336,24 +337,6 @@ pub(crate) async fn run_apply_pattern(
#[cfg(feature = "grit_tracing")]
stdlib_download_span.exit();

let warn_uncommitted =
!arg.dry_run && !arg.force && has_uncommitted_changes(cwd.clone()).await;
if warn_uncommitted {
let term = console::Term::stderr();
if !term.is_term() {
bail!("Error: Untracked changes detected. Grit will not proceed with rewriting files in non-TTY environments unless '--force' is used. Please commit all changes or use '--force' to override this safety check.");
}

let proceed = flushable_unwrap!(emitter, Confirm::new()
.with_prompt("Your working tree currently has untracked changes and Grit will rewrite files in place. Do you want to proceed?")
.default(false)
.interact_opt());

if proceed != Some(true) {
return Ok(());
}
}

#[cfg(feature = "grit_tracing")]
let grit_file_discovery = span!(tracing::Level::INFO, "grit_file_discovery",).entered();

Expand Down Expand Up @@ -547,6 +530,23 @@ pub(crate) async fn run_apply_pattern(
.unwrap();
}

let warn_uncommitted = !arg.dry_run && !arg.force && has_uncommitted_changes(cwd.clone()).await;
if warn_uncommitted && has_rewrite(&compiled.pattern, &compiled.pattern_definitions) {
let term = console::Term::stderr();
if !term.is_term() {
bail!("Error: Untracked changes detected. Grit will not proceed with rewriting files in non-TTY environments unless '--force' is used. Please commit all changes or use '--force' to override this safety check.");
}

let proceed = flushable_unwrap!(emitter, Confirm::new()
.with_prompt("Your working tree currently has untracked changes and Grit will rewrite files in place. Do you want to proceed?")
.default(false)
.interact_opt());

if proceed != Some(true) {
return Ok(());
}
}

let processed = AtomicI32::new(0);

let mut emitter = par_apply_pattern(
Expand Down
37 changes: 37 additions & 0 deletions crates/cli_bin/tests/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2659,6 +2659,43 @@ fn tty_behavior() -> Result<()> {
Ok(())
}

/// If there's no rewrite, the warning can be skipped.
#[test]
fn no_search_warning() -> Result<()> {
let (_temp_dir, dir) = get_fixture("yaml_padding", true)?;

// Init an empty git repo
let mut git_init_cmd = Command::new("git");
git_init_cmd.arg("init").current_dir(dir.clone());
let output = git_init_cmd.output()?;
assert!(output.status.success(), "Git init failed");

// from the tempdir as cwd, run marzano apply
let mut apply_cmd = get_test_cmd()?;
apply_cmd.current_dir(dir.clone());
apply_cmd
.arg("apply")
.arg("--language=yaml")
.arg("`stuff: good`")
.arg("file.yaml");

let output = apply_cmd.output()?;
let stdout = String::from_utf8(output.stdout)?;
let stderr = String::from_utf8(output.stderr)?;
println!("stdout: {:?}", stdout);
println!("stderr: {:?}", stderr);

// Expect it to fail
assert!(output.status.success(), "Command should have passed");

assert!(!stderr.contains("Untracked changes detected."));

assert!(stdout.contains("stuff: good"));
assert!(stdout.contains("Processed 1 files and found 1 matches"));

Ok(())
}

#[test]
fn apply_stdin() -> Result<()> {
let (_temp_dir, fixture_dir) = get_fixture("limit_files", false)?;
Expand Down
143 changes: 143 additions & 0 deletions crates/core/src/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ fn find_child_tree_definition(

#[cfg(test)]
mod tests {
use grit_pattern_matcher::has_rewrite;
use marzano_language::target_language::TargetLanguage;

use crate::pattern_compiler::src_to_problem_libs;

use super::*;
use std::collections::BTreeMap;

Expand Down Expand Up @@ -250,4 +255,142 @@ mod tests {
let decided = is_async(&parsed.root_node(), &libs, &mut parser).unwrap();
assert!(decided);
}

#[test]
fn test_is_rewrite() {
let pattern_src = r#"
`console.log` => `console.error`
"#
.to_string();
let libs = BTreeMap::new();
let problem = src_to_problem_libs(
pattern_src.to_string(),
&libs,
TargetLanguage::default(),
None,
None,
None,
None,
)
.unwrap()
.problem;

println!("problem: {:?}", problem);

assert!(has_rewrite(&problem.pattern, &[]));
}

#[test]
fn test_is_not_rewrite() {
let pattern_src = r#"
`console.log($msg)` where {
$msg <: not contains `foo`
}
"#
.to_string();
let libs = BTreeMap::new();
let problem = src_to_problem_libs(
pattern_src.to_string(),
&libs,
TargetLanguage::default(),
None,
None,
None,
None,
)
.unwrap()
.problem;

println!("problem: {:?}", problem);

assert!(!has_rewrite(&problem.pattern, &[]));
}

#[test]
fn test_is_rewrite_with_pattern_call() {
let pattern_src = r#"
pattern pattern_with_rewrite() {
`console.log($msg)` => `console.error($msg)`
}
pattern_with_rewrite()
"#
.to_string();
let libs = BTreeMap::new();
let problem = src_to_problem_libs(
pattern_src.to_string(),
&libs,
TargetLanguage::default(),
None,
None,
None,
None,
)
.unwrap()
.problem;

println!("problem: {:?}", problem);

assert!(has_rewrite(&problem.pattern, &problem.pattern_definitions));
}

#[test]
fn test_is_rewrite_with_yaml() {
let pattern_src = r#"
language yaml
or {
`- $item` where {
$item => `nice: car
second: detail`
}
}
"#
.to_string();
let libs = BTreeMap::new();
let problem = src_to_problem_libs(
pattern_src.to_string(),
&libs,
TargetLanguage::default(),
None,
None,
None,
None,
)
.unwrap()
.problem;

println!("problem: {:?}", problem);

assert!(has_rewrite(&problem.pattern, &problem.pattern_definitions));
}

#[test]
fn test_is_rewrite_with_insert() {
let pattern_src = r#"
language yaml
or {
`- $item` where {
$item += `good stuff`
}
}
"#
.to_string();
let libs = BTreeMap::new();
let problem = src_to_problem_libs(
pattern_src.to_string(),
&libs,
TargetLanguage::default(),
None,
None,
None,
None,
)
.unwrap()
.problem;

println!("problem: {:?}", problem);

assert!(has_rewrite(&problem.pattern, &problem.pattern_definitions));
}
}
6 changes: 5 additions & 1 deletion crates/core/src/ast_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::{
problem::MarzanoQueryContext,
};
use anyhow::{anyhow, Result};
use grit_pattern_matcher::pattern::PatternDefinition;
use grit_pattern_matcher::{
binding::Binding,
pattern::{
Expand All @@ -29,7 +30,10 @@ impl ASTNode {
impl AstNodePattern<MarzanoQueryContext> for ASTNode {
const INCLUDES_TRIVIA: bool = false;

fn children(&self) -> Vec<PatternOrPredicate<MarzanoQueryContext>> {
fn children<'a>(
&'a self,
_definitions: &'a [PatternDefinition<MarzanoQueryContext>],
) -> Vec<PatternOrPredicate<'a, MarzanoQueryContext>> {
self.args
.iter()
.map(|a| PatternOrPredicate::Pattern(&a.2))
Expand Down
4 changes: 2 additions & 2 deletions crates/core/src/pattern_compiler/not_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl NodeCompiler for NotCompiler {
.ok_or_else(|| anyhow!("missing pattern of patternNot"))?;
let range = pattern.range();
let pattern = PatternCompiler::from_node(&pattern, context)?;
if pattern.iter().any(|p| {
if pattern.iter(&[]).any(|p| {
matches!(
p,
PatternOrPredicate::Pattern(Pattern::Rewrite(_))
Expand Down Expand Up @@ -59,7 +59,7 @@ impl NodeCompiler for PrNotCompiler {
.ok_or_else(|| anyhow!("predicateNot missing predicate"))?;
let range = not.range();
let not = PredicateCompiler::from_node(&not, context)?;
if not.iter().any(|p| {
if not.iter(&[]).any(|p| {
matches!(
p,
PatternOrPredicate::Pattern(Pattern::Rewrite(_))
Expand Down
2 changes: 1 addition & 1 deletion crates/core/src/problem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub struct Problem {
pub hash: [u8; 32],
pub name: Option<String>,
pub(crate) variables: VariableLocations,
pub(crate) pattern_definitions: Vec<PatternDefinition<MarzanoQueryContext>>,
pub pattern_definitions: Vec<PatternDefinition<MarzanoQueryContext>>,
pub(crate) predicate_definitions: Vec<PredicateDefinition<MarzanoQueryContext>>,
pub(crate) function_definitions: Vec<GritFunctionDefinition<MarzanoQueryContext>>,
pub(crate) foreign_function_definitions: Vec<ForeignFunctionDefinition>,
Expand Down
39 changes: 39 additions & 0 deletions crates/grit-pattern-matcher/src/analysis.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use crate::{
context::QueryContext,
pattern::{Pattern, PatternDefinition, PatternOrPredicate, Predicate},
};

/// Determine if a provided pattern has a rewrite anywhere inside of it
///
/// Note this does not yet walk inside predicates and function calls
pub fn has_rewrite<Q: QueryContext>(
current_pattern: &Pattern<Q>,
definitions: &[PatternDefinition<Q>],
) -> bool {
for pattern in current_pattern.iter(definitions) {
if matches!(pattern, PatternOrPredicate::Pattern(Pattern::Rewrite(_))) {
return true;
}
if matches!(
pattern,
PatternOrPredicate::Predicate(Predicate::Rewrite(_))
) {
return true;
}
if matches!(
pattern,
PatternOrPredicate::Predicate(Predicate::Accumulate(_))
) {
return true;
}
// match pattern {
// PatternOrPredicate::Pattern(p) => {
// println!("Check {}", p.name());
// }
// PatternOrPredicate::Predicate(p) => {
// println!("Check {}", p.name());
// }
// }
}
false
}
3 changes: 3 additions & 0 deletions crates/grit-pattern-matcher/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! GritQL engine. There's the [`pattern::Matcher`] trait that's implemented by
//! the patterns, which implements the matching logic.

pub mod analysis;
pub mod binding;
pub mod constant;
pub mod constants;
Expand All @@ -13,3 +14,5 @@ pub mod errors;
pub mod file_owners;
pub mod intervals;
pub mod pattern;

pub use analysis::has_rewrite;
6 changes: 5 additions & 1 deletion crates/grit-pattern-matcher/src/pattern/ast_node_pattern.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::{
iter_pattern::PatternOrPredicate,
patterns::{Matcher, PatternName},
PatternDefinition,
};
use crate::context::QueryContext;

Expand All @@ -12,7 +13,10 @@ pub trait AstNodePattern<Q: QueryContext>:
/// Trivia is useful for being able to re-print an AST, but not all parsers support collecting it.
const INCLUDES_TRIVIA: bool;

fn children(&self) -> Vec<PatternOrPredicate<Q>>;
fn children<'a>(
&'a self,
definitions: &'a [PatternDefinition<Q>],
) -> Vec<PatternOrPredicate<'a, Q>>;

fn matches_kind_of(&self, node: &Q::Node<'_>) -> bool;
}
Expand Down
Loading

0 comments on commit 5923f64

Please sign in to comment.