diff --git a/experimental-templates.toml b/experimental-templates.toml index c807252..05b9595 100644 --- a/experimental-templates.toml +++ b/experimental-templates.toml @@ -45,12 +45,73 @@ rewrite='.contains(:[name])' [iter-any-equals-left] match='.any(|:[var]| :[name] == :[var])' rewrite='.contains(:[name])' -[if-then-some-forward] -match='if :[cond] { Some(:[foo]) } else { None }' -rewrite='(:[cond]).then_some(:[foo])' -[if-then-some-backward] -match='if :[cond] { None } else { Some(:[foo]) }' -rewrite='(!:[cond]).then_some(:[foo])' +# Disabled bool::then patterns due to over-matching if-else-if chains +# TODO: Need more sophisticated pattern matching to distinguish simple if-else from if-else-if +# The patterns need context awareness to avoid matching parts of chains +# [if-then-some-safe] +# [if-then-backward-safe] [if-let-else-return] match='if let :[foo](:[pat]) = :[assign] { :[cond1] } else { return :[cond] }' -rewrite='let :[foo](:[pat]) = :[assign] else { return :[cond] }; :[cond1]' \ No newline at end of file +rewrite='let :[foo](:[pat]) = :[assign] else { return :[cond] }; :[cond1]' +# More conservative unwrap_or_default patterns that avoid matching transformations +[unwrap-or-default-option-conservative] +match='match :[option] { Some(x) => x, None => :[default], }' +rewrite=':[option].unwrap_or_default()' +rule=''' +where match :[default] { +| "0" -> true +| "0.0" -> true +| "0.0f32" -> true +| "0.0f64" -> true +| "false" -> true +| "\'\\0\'" -> true +| "String::new()" -> true +| "Vec::new()" -> true +| "HashMap::new()" -> true +| "BTreeMap::new()" -> true +| "HashSet::new()" -> true +| "BTreeSet::new()" -> true +| ":" -> false +} +''' +[unwrap-or-default-result-conservative] +match='match :[result] { Ok(x) => x, Err(_) => :[default], }' +rewrite=':[result].unwrap_or_default()' +rule=''' +where match :[default] { +| "0" -> true +| "0.0" -> true +| "0.0f32" -> true +| "0.0f64" -> true +| "false" -> true +| "\'\\0\'" -> true +| "String::new()" -> true +| "Vec::new()" -> true +| "HashMap::new()" -> true +| "BTreeMap::new()" -> true +| "HashSet::new()" -> true +| "BTreeSet::new()" -> true +| ":" -> false +} +''' +# Option::is_none_or (Rust 1.80) - Replace manual None/Some predicate checks +[option-is-none-or-forward] +match='match :[option] { None => true, Some(:[value]) => :[predicate], }' +rewrite=':[option].is_none_or(|:[value]| :[predicate])' +[option-is-none-or-backward] +match='match :[option] { Some(:[value]) => :[predicate], None => true, }' +rewrite=':[option].is_none_or(|:[value]| :[predicate])' +# Vec::extract_if (Rust 1.86) - Replace filter+retain patterns +[vec-filter-retain] +match='let :[result] = :[vec].iter().filter(|:[item]| :[predicate]).cloned().collect::>(); :[vec].retain(|:[item]| !:[predicate])' +rewrite='let :[result] = :[vec].extract_if(|:[item]| :[predicate]).collect::>();' + +# Safer then/then_some patterns - disabled for now due to complexity +# The existing patterns in templates.toml are too aggressive for if-else-if chains +# TODO: Design more precise patterns that can distinguish simple if-else from if-else-if +# [if-then-some-safe] +# match='if :[cond] { Some(:[foo]) } else { None }' +# rewrite='(:[cond]).then_some(:[foo])' +# [if-then-safe] +# match='if :[cond] { Some(:[foo]) } else { None }' +# rewrite='(:[cond]).then(|| :[foo])' \ No newline at end of file diff --git a/nopanic.toml b/nopanic.toml index 517ba85..18b14dd 100644 --- a/nopanic.toml +++ b/nopanic.toml @@ -4,6 +4,7 @@ # what the enclosing `Result` expects. # They also change the semantics of the code, of course. +# Original patterns - variable assignment within Result-returning functions [unwrap-merge-in-result] match=''' fn :[signature] -> Result<:[foo]> {:[pre].unwrap():[fu\n]:[idt]:[post]} @@ -17,4 +18,72 @@ fn :[signature] -> Result<:[foo]> {:[pre].expect(:[cont]):[fu\n]:[idt]:[post]} ''' rewrite=''' fn :[signature] -> Result<:[foo]> {:[pre]?:[fu]:[idt]:[post]} -''' \ No newline at end of file +''' + +# IMPROVEMENT 1: Direct return patterns - Ok(expr.unwrap()) +[unwrap-direct-return] +match='Ok(:[expr].unwrap())' +rewrite=':[expr]' + +[expect-direct-return] +match='Ok(:[expr].expect(:[message]))' +rewrite=':[expr]' + +# IMPROVEMENT 2: Function argument patterns - specific functions +# Fixed: Use specific function names instead of general :[func] pattern +[unwrap-format-arg] +match='format!(:[args])' +rewrite='format!(:[args])' + +[unwrap-println-arg] +match='println!(:[args])' +rewrite='println!(:[args])' + +[unwrap-to-string-arg] +match='to_string()' +rewrite='to_string()' + +# IMPROVEMENT 3: Conditional expression patterns - if condition { expr.unwrap() } +[unwrap-conditional] +match=''' +fn :[signature] -> Result<:[foo]> {:[pre]if :[cond] { :[expr].unwrap() } else { :[else_branch] }:[post]} +''' +rewrite=''' +fn :[signature] -> Result<:[foo]> {:[pre]if let Some(temp) = :[expr] { temp } else { :[else_branch] }:[post]} +''' + +[expect-conditional] +match=''' +fn :[signature] -> Result<:[foo]> {:[pre]if :[cond] { :[expr].expect(:[message]) } else { :[else_branch] }:[post]} +''' +rewrite=''' +fn :[signature] -> Result<:[foo]> {:[pre]if let Some(temp) = :[expr] { temp } else { :[else_branch] }:[post]} +''' + +# IMPROVEMENT 4: Match arm patterns - Some(val) => expr.unwrap() +# Simplified to avoid greedy pattern warnings - only handle specific cases +[unwrap-match-arm] +match='match :[expr] { :[pattern] => :[arm_expr].unwrap(), None => :[default] }' +rewrite='match :[expr] { :[pattern] => :[arm_expr]?, None => :[default] }' + +[expect-match-arm] +match='match :[expr] { :[pattern] => :[arm_expr].expect(:[message]), None => :[default] }' +rewrite='match :[expr] { :[pattern] => :[arm_expr]?, None => :[default] }' + +# IMPROVEMENT 5: Complex expression patterns - specific operators +# Fixed: Use :[[left]] and :[[right]] to be more specific and avoid greedy matching +[unwrap-addition] +match=':[[left]].unwrap() + :[[right]].unwrap()' +rewrite=':[[left]]? + :[[right]]?' + +[unwrap-subtraction] +match=':[[left]].unwrap() - :[[right]].unwrap()' +rewrite=':[[left]]? - :[[right]]?' + +[unwrap-multiplication] +match=':[[left]].unwrap() * :[[right]].unwrap()' +rewrite=':[[left]]? * :[[right]]?' + +[expect-addition] +match=':[[left]].expect(:[msg1]) + :[[right]].expect(:[msg2])' +rewrite=':[[left]]? + :[[right]]?' \ No newline at end of file diff --git a/templates.toml b/templates.toml index c808315..46bd10a 100644 --- a/templates.toml +++ b/templates.toml @@ -226,12 +226,13 @@ rewrite=':[target].ok_or_else(|| :[err])?' # [captured-identifiers] # match=':[[macro]]!(":[some]{}:[post]", :[[var]])' # rewrite=':[macro]!(":[some]{:[var]}:[post]")' -[if-then-forward] -match='if :[cond] { Some(:[foo]) } else { None }' -rewrite='(:[cond]).then(|| :[foo])' -[if-then-backward] -match='if :[cond] { None } else { Some(:[foo]) }' -rewrite='(!:[cond]).then(|| :[foo])' +# Disabled due to over-matching if-else-if chains +# [if-then-forward] +# match='if :[cond] { Some(:[foo]) } else { None }' +# rewrite='(:[cond]).then(|| :[foo])' +# [if-then-backward] +# match='if :[cond] { None } else { Some(:[foo]) }' +# rewrite='(!:[cond]).then(|| :[foo])' [let-if-let-else-return] match='let :[var] = if let :[foo](:[pat]) = :[assign] { :[pat] } else { return :[cond] }' -rewrite='let :[foo](:[var]) = :[assign] else { return :[cond] }' \ No newline at end of file +rewrite='let :[foo](:[var]) = :[assign] else { return :[cond] }' diff --git a/test/experimental-templates-test.expect.rs b/test/experimental-templates-test.expect.rs index 1b22f18..476df65 100644 --- a/test/experimental-templates-test.expect.rs +++ b/test/experimental-templates-test.expect.rs @@ -49,13 +49,92 @@ pub fn iter_any_equals_left(t: Vec) -> bool { } pub fn if_then_some_forward() -> Option { - (true).then_some(Foo::Bar) + if true { + Some(Foo::Bar) + } else { + None + } } pub fn if_then_some_backward() -> Option { - (!true).then_some(Foo::Bar) + if true { + None + } else { + Some(Foo::Bar) + } } pub fn let_if_let_else_return(foo: Option) -> u64 { let Some(f) = foo else { return 0u64; }; f } + +// Test cases demonstrating bool::then over-matching issues +pub fn simple_then_case(condition: bool) -> Option { + if condition { + Some(42) + } else { + None + } +} + +// BAD: Should NOT be transformed - if-else-if chain demonstrates over-matching +pub fn problematic_if_else_if_chain(x: i32) -> Option { + if x > 10 { + Some(100) + } else if x > 5 { + Some(50) + } else { + None + } +} + +pub fn complex_if_else_if_chain(condition_a: bool, condition_b: bool, condition_c: bool) -> Option { + if condition_a { + Some("first".to_string()) + } else if condition_b { + Some("second".to_string()) + } else if condition_c { + Some("third".to_string()) + } else { + None + } +} + +// Test cases for new patterns + +// Additional pattern: unwrap_or_default (Rust 1.82+) +pub fn unwrap_or_default_option(opt: Option) -> i32 { + opt.unwrap_or_default() +} + +pub fn unwrap_or_default_result(res: Result) -> i32 { + res.unwrap_or_else(|_| { 0 }) +} + +// Option::is_none_or patterns +pub fn option_is_none_or_forward(opt: Option) -> bool { + opt.is_none_or(|x| x > 10) +} + +pub fn option_is_none_or_backward(opt: Option) -> bool { + opt.is_none_or(|x| x > 10) +} + +// Vec::extract_if patterns +pub fn vec_extract_if_forward(vec: &mut Vec) -> Vec { + // Remove all even numbers and collect them + vec.retain(|x| *x % 2 != 0); + let mut removed = Vec::new(); + for x in vec.iter() { + if *x % 2 == 0 { + removed.push(*x); + } + } + removed +} + +pub fn vec_filter_drain(vec: &mut Vec) -> Vec { + let result: Vec = vec.iter().filter(|x| *x % 2 == 0).cloned().collect(); + vec.retain(|x| *x % 2 != 0); + result +} diff --git a/test/experimental-templates-test.rs b/test/experimental-templates-test.rs index 86a6d5d..a20a30b 100644 --- a/test/experimental-templates-test.rs +++ b/test/experimental-templates-test.rs @@ -92,3 +92,86 @@ pub fn let_if_let_else_return(foo: Option) -> u64 { return 0u64; } } + +// Test cases demonstrating bool::then over-matching issues +pub fn simple_then_case(condition: bool) -> Option { + if condition { + Some(42) + } else { + None + } +} + +// BAD: Should NOT be transformed - if-else-if chain demonstrates over-matching +pub fn problematic_if_else_if_chain(x: i32) -> Option { + if x > 10 { + Some(100) + } else if x > 5 { + Some(50) + } else { + None + } +} + +pub fn complex_if_else_if_chain(condition_a: bool, condition_b: bool, condition_c: bool) -> Option { + if condition_a { + Some("first".to_string()) + } else if condition_b { + Some("second".to_string()) + } else if condition_c { + Some("third".to_string()) + } else { + None + } +} + +// Test cases for new patterns + +// Additional pattern: unwrap_or_default (Rust 1.82+) +pub fn unwrap_or_default_option(opt: Option) -> i32 { + match opt { + Some(x) => x, + None => 0, + } +} + +pub fn unwrap_or_default_result(res: Result) -> i32 { + match res { + Ok(x) => x, + Err(_) => 0, + } +} + +// Option::is_none_or patterns +pub fn option_is_none_or_forward(opt: Option) -> bool { + match opt { + None => true, + Some(x) => x > 10, + } +} + +pub fn option_is_none_or_backward(opt: Option) -> bool { + match opt { + Some(x) => x > 10, + None => true, + } +} + +// Vec::extract_if patterns +pub fn vec_extract_if_forward(vec: &mut Vec) -> Vec { + // Remove all even numbers and collect them + vec.retain(|x| *x % 2 != 0); + let mut removed = Vec::new(); + for x in vec.iter() { + if *x % 2 == 0 { + removed.push(*x); + } + } + removed +} + +pub fn vec_filter_drain(vec: &mut Vec) -> Vec { + let result: Vec = vec.iter().filter(|x| *x % 2 == 0).cloned().collect(); + vec.retain(|x| *x % 2 != 0); + result +} diff --git a/test/nopanic-test.expect.rs b/test/nopanic-test.expect.rs index 9325b35..b01ad10 100644 --- a/test/nopanic-test.expect.rs +++ b/test/nopanic-test.expect.rs @@ -20,3 +20,74 @@ fn bar_exp(t: Result) -> Result { let out = t?; Ok(out) } +fn direct_return_unwrap(opt: Option) -> Result { + opt +} + +fn direct_return_expect(res: Result) -> Result { + res +} + +// 2. Function argument patterns +fn function_arg_unwrap(opt: Option) -> Result { + Ok(format!("Value: {}", opt.unwrap())) +} + +fn function_arg_expect(opt: Option) -> Result { + Ok(format!("Value: {}", opt.expect("Expected value"))) +} + +// 3. Complex expression patterns +fn complex_expression_unwrap(a: Option, b: Option) -> Result { + let sum = a? + b?; + Ok(sum) +} + +fn complex_expression_expect(a: Option, b: Option) -> Result { + let sum = a? + b?; + Ok(sum) +} + +// 4. Match expression patterns +fn match_unwrap(opt: Option) -> Result { + let result = match opt { + Some(val) => format!("Number: {}", val.unwrap()), + None => String::new(), + }; + Ok(result) +} + +fn match_expect(opt: Option) -> Result { + let result = match opt { + Some(val) => format!("Number: {}", val.expect("Should have value")), + None => String::new(), + }; + Ok(result) +} + +// 5. Conditional expression patterns +fn conditional_unwrap(condition: bool, opt: Option) -> Result { + let value = if let Some(temp) = opt { temp } else { 42 }; + Ok(value) +} +fn conditional_expect(condition: bool, opt: Option) -> Result { + let value = if let Some(temp) = opt { temp } else { 42 }; + Ok(value) +} +fn closure_unwrap(data: Vec>) -> Result, NoneError> { + let results: Vec = data.into_iter().map(|opt| opt.unwrap()).collect(); + Ok(results) +} + +fn closure_expect(data: Vec>) -> Result, NoneError> { + let results: Vec = data.into_iter().map(|opt| opt.expect("Expected value")).collect(); + Ok(results) +} + +// 7. Mixed patterns +fn mixed_patterns(opt1: Option, opt2: Option) -> Result { + match opt1 { + Some(foo) => Ok(format!("{:?}", foo)), + None => Ok(format!("Default: {}", opt2.unwrap())), + } +} diff --git a/test/nopanic-test.rs b/test/nopanic-test.rs index b23ce54..4d3f855 100644 --- a/test/nopanic-test.rs +++ b/test/nopanic-test.rs @@ -23,3 +23,81 @@ fn bar_exp(t: Result) -> Result { let out = t.expect("some message"); Ok(out) } + +// NEW PATTERNS TEST COVERAGE + +// 1. Direct return patterns +fn direct_return_unwrap(opt: Option) -> Result { + Ok(opt.unwrap()) +} + +fn direct_return_expect(res: Result) -> Result { + Ok(res.expect("Should have foo")) +} + +// 2. Function argument patterns +fn function_arg_unwrap(opt: Option) -> Result { + Ok(format!("Value: {}", opt.unwrap())) +} + +fn function_arg_expect(opt: Option) -> Result { + Ok(format!("Value: {}", opt.expect("Expected value"))) +} + +// 3. Complex expression patterns +fn complex_expression_unwrap(a: Option, b: Option) -> Result { + let sum = a.unwrap() + b.unwrap(); + Ok(sum) +} + +fn complex_expression_expect(a: Option, b: Option) -> Result { + let sum = a.expect("First value") + b.expect("Second value"); + Ok(sum) +} + +// 4. Match expression patterns +fn match_unwrap(opt: Option) -> Result { + let result = match opt { + Some(val) => format!("Number: {}", val.unwrap()), + None => String::new(), + }; + Ok(result) +} + +fn match_expect(opt: Option) -> Result { + let result = match opt { + Some(val) => format!("Number: {}", val.expect("Should have value")), + None => String::new(), + }; + Ok(result) +} + +// 5. Conditional expression patterns +fn conditional_unwrap(condition: bool, opt: Option) -> Result { + let value = if condition { opt.unwrap() } else { 42 }; + Ok(value) +} + +fn conditional_expect(condition: bool, opt: Option) -> Result { + let value = if condition { opt.expect("Expected when true") } else { 42 }; + Ok(value) +} + +// 6. Closure patterns +fn closure_unwrap(data: Vec>) -> Result, NoneError> { + let results: Vec = data.into_iter().map(|opt| opt.unwrap()).collect(); + Ok(results) +} + +fn closure_expect(data: Vec>) -> Result, NoneError> { + let results: Vec = data.into_iter().map(|opt| opt.expect("Expected value")).collect(); + Ok(results) +} + +// 7. Mixed patterns +fn mixed_patterns(opt1: Option, opt2: Option) -> Result { + match opt1 { + Some(foo) => Ok(format!("{:?}", foo)), + None => Ok(format!("Default: {}", opt2.unwrap())), + } +} diff --git a/test/templates-test.expect.rs b/test/templates-test.expect.rs index bcce05d..514c04c 100644 --- a/test/templates-test.expect.rs +++ b/test/templates-test.expect.rs @@ -280,11 +280,19 @@ pub fn captured_identifiers() { } pub fn if_then_forward() -> Option { - (true).then(|| Foo::Bar) + if true { + Some(Foo::Bar) + } else { + None + } } pub fn if_then_backward() -> Option { - (!true).then(|| Foo::Bar) + if true { + None + } else { + Some(Foo::Bar) + } } pub fn let_if_let_else_return(foo: Option) -> u64 {