aboutsummaryrefslogtreecommitdiff
path: root/crates/ide_assists
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-05-22 13:26:40 +0100
committerGitHub <[email protected]>2021-05-22 13:26:40 +0100
commit7d81e40e36a7b5451785e224a2f16822834f29a3 (patch)
treeed5ab1b896294963128bc3f34589634c83d812b6 /crates/ide_assists
parent3cfe2d0a5d663d29c3d196f9d16e91964780792a (diff)
parentd5c96672aac6fd4ac25474814879af2c04bd798b (diff)
Merge #8901
8901: fix: `fill_match_arms` hangs on a tuple of large enums r=matklad a=iDawer + Lazy computation of missing arms. + Convenience function to test lazy computation: `ide_assists::tests::check_assist_unresolved`. Fixes #8835 Co-authored-by: Dawer <[email protected]>
Diffstat (limited to 'crates/ide_assists')
-rw-r--r--crates/ide_assists/src/handlers/fill_match_arms.rs81
-rw-r--r--crates/ide_assists/src/tests.rs21
2 files changed, 75 insertions, 27 deletions
diff --git a/crates/ide_assists/src/handlers/fill_match_arms.rs b/crates/ide_assists/src/handlers/fill_match_arms.rs
index 58b001050..97435f021 100644
--- a/crates/ide_assists/src/handlers/fill_match_arms.rs
+++ b/crates/ide_assists/src/handlers/fill_match_arms.rs
@@ -1,4 +1,4 @@
1use std::iter; 1use std::iter::{self, Peekable};
2 2
3use either::Either; 3use either::Either;
4use hir::{Adt, HasSource, ModuleDef, Semantics}; 4use hir::{Adt, HasSource, ModuleDef, Semantics};
@@ -63,50 +63,61 @@ pub(crate) fn fill_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option<
63 63
64 let module = ctx.sema.scope(expr.syntax()).module()?; 64 let module = ctx.sema.scope(expr.syntax()).module()?;
65 65
66 let missing_arms: Vec<MatchArm> = if let Some(enum_def) = resolve_enum_def(&ctx.sema, &expr) { 66 let mut missing_pats: Peekable<Box<dyn Iterator<Item = ast::Pat>>> = if let Some(enum_def) =
67 resolve_enum_def(&ctx.sema, &expr)
68 {
67 let variants = enum_def.variants(ctx.db()); 69 let variants = enum_def.variants(ctx.db());
68 70
69 let mut variants = variants 71 let missing_pats = variants
70 .into_iter() 72 .into_iter()
71 .filter_map(|variant| build_pat(ctx.db(), module, variant)) 73 .filter_map(|variant| build_pat(ctx.db(), module, variant))
72 .filter(|variant_pat| is_variant_missing(&top_lvl_pats, variant_pat)) 74 .filter(|variant_pat| is_variant_missing(&top_lvl_pats, variant_pat));
73 .map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block())) 75
74 .map(|it| it.clone_for_update()) 76 let missing_pats: Box<dyn Iterator<Item = _>> = if Some(enum_def)
75 .collect::<Vec<_>>(); 77 == FamousDefs(&ctx.sema, Some(module.krate())).core_option_Option().map(lift_enum)
76 if Some(enum_def)
77 == FamousDefs(&ctx.sema, Some(module.krate()))
78 .core_option_Option()
79 .map(|x| lift_enum(x))
80 { 78 {
81 // Match `Some` variant first. 79 // Match `Some` variant first.
82 cov_mark::hit!(option_order); 80 cov_mark::hit!(option_order);
83 variants.reverse() 81 Box::new(missing_pats.rev())
84 } 82 } else {
85 variants 83 Box::new(missing_pats)
84 };
85 missing_pats.peekable()
86 } else if let Some(enum_defs) = resolve_tuple_of_enum_def(&ctx.sema, &expr) { 86 } else if let Some(enum_defs) = resolve_tuple_of_enum_def(&ctx.sema, &expr) {
87 let mut n_arms = 1;
88 let variants_of_enums: Vec<Vec<ExtendedVariant>> = enum_defs
89 .into_iter()
90 .map(|enum_def| enum_def.variants(ctx.db()))
91 .inspect(|variants| n_arms *= variants.len())
92 .collect();
93
87 // When calculating the match arms for a tuple of enums, we want 94 // When calculating the match arms for a tuple of enums, we want
88 // to create a match arm for each possible combination of enum 95 // to create a match arm for each possible combination of enum
89 // values. The `multi_cartesian_product` method transforms 96 // values. The `multi_cartesian_product` method transforms
90 // Vec<Vec<EnumVariant>> into Vec<(EnumVariant, .., EnumVariant)> 97 // Vec<Vec<EnumVariant>> into Vec<(EnumVariant, .., EnumVariant)>
91 // where each tuple represents a proposed match arm. 98 // where each tuple represents a proposed match arm.
92 enum_defs 99
100 // A number of arms grows very fast on even a small tuple of large enums.
101 // We skip the assist beyond an arbitrary threshold.
102 if n_arms > 256 {
103 return None;
104 }
105 let missing_pats = variants_of_enums
93 .into_iter() 106 .into_iter()
94 .map(|enum_def| enum_def.variants(ctx.db()))
95 .multi_cartesian_product() 107 .multi_cartesian_product()
108 .inspect(|_| cov_mark::hit!(fill_match_arms_lazy_computation))
96 .map(|variants| { 109 .map(|variants| {
97 let patterns = 110 let patterns =
98 variants.into_iter().filter_map(|variant| build_pat(ctx.db(), module, variant)); 111 variants.into_iter().filter_map(|variant| build_pat(ctx.db(), module, variant));
99 ast::Pat::from(make::tuple_pat(patterns)) 112 ast::Pat::from(make::tuple_pat(patterns))
100 }) 113 })
101 .filter(|variant_pat| is_variant_missing(&top_lvl_pats, variant_pat)) 114 .filter(|variant_pat| is_variant_missing(&top_lvl_pats, variant_pat));
102 .map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block())) 115 (Box::new(missing_pats) as Box<dyn Iterator<Item = _>>).peekable()
103 .map(|it| it.clone_for_update())
104 .collect()
105 } else { 116 } else {
106 return None; 117 return None;
107 }; 118 };
108 119
109 if missing_arms.is_empty() { 120 if missing_pats.peek().is_none() {
110 return None; 121 return None;
111 } 122 }
112 123
@@ -117,6 +128,9 @@ pub(crate) fn fill_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option<
117 target, 128 target,
118 |builder| { 129 |builder| {
119 let new_match_arm_list = match_arm_list.clone_for_update(); 130 let new_match_arm_list = match_arm_list.clone_for_update();
131 let missing_arms = missing_pats
132 .map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block()))
133 .map(|it| it.clone_for_update());
120 134
121 let catch_all_arm = new_match_arm_list 135 let catch_all_arm = new_match_arm_list
122 .arms() 136 .arms()
@@ -167,13 +181,13 @@ fn does_pat_match_variant(pat: &Pat, var: &Pat) -> bool {
167 } 181 }
168} 182}
169 183
170#[derive(Eq, PartialEq, Clone)] 184#[derive(Eq, PartialEq, Clone, Copy)]
171enum ExtendedEnum { 185enum ExtendedEnum {
172 Bool, 186 Bool,
173 Enum(hir::Enum), 187 Enum(hir::Enum),
174} 188}
175 189
176#[derive(Eq, PartialEq, Clone)] 190#[derive(Eq, PartialEq, Clone, Copy)]
177enum ExtendedVariant { 191enum ExtendedVariant {
178 True, 192 True,
179 False, 193 False,
@@ -185,7 +199,7 @@ fn lift_enum(e: hir::Enum) -> ExtendedEnum {
185} 199}
186 200
187impl ExtendedEnum { 201impl ExtendedEnum {
188 fn variants(&self, db: &RootDatabase) -> Vec<ExtendedVariant> { 202 fn variants(self, db: &RootDatabase) -> Vec<ExtendedVariant> {
189 match self { 203 match self {
190 ExtendedEnum::Enum(e) => { 204 ExtendedEnum::Enum(e) => {
191 e.variants(db).into_iter().map(|x| ExtendedVariant::Variant(x)).collect::<Vec<_>>() 205 e.variants(db).into_iter().map(|x| ExtendedVariant::Variant(x)).collect::<Vec<_>>()
@@ -266,7 +280,9 @@ fn build_pat(db: &RootDatabase, module: hir::Module, var: ExtendedVariant) -> Op
266mod tests { 280mod tests {
267 use ide_db::helpers::FamousDefs; 281 use ide_db::helpers::FamousDefs;
268 282
269 use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target}; 283 use crate::tests::{
284 check_assist, check_assist_not_applicable, check_assist_target, check_assist_unresolved,
285 };
270 286
271 use super::fill_match_arms; 287 use super::fill_match_arms;
272 288
@@ -1045,4 +1061,19 @@ fn foo(t: Test) {
1045}"#, 1061}"#,
1046 ); 1062 );
1047 } 1063 }
1064
1065 #[test]
1066 fn lazy_computation() {
1067 // Computing a single missing arm is enough to determine applicability of the assist.
1068 cov_mark::check_count!(fill_match_arms_lazy_computation, 1);
1069 check_assist_unresolved(
1070 fill_match_arms,
1071 r#"
1072enum A { One, Two, }
1073fn foo(tuple: (A, A)) {
1074 match $0tuple {};
1075}
1076"#,
1077 );
1078 }
1048} 1079}
diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs
index 1739302bf..6a9231e07 100644
--- a/crates/ide_assists/src/tests.rs
+++ b/crates/ide_assists/src/tests.rs
@@ -65,6 +65,12 @@ pub(crate) fn check_assist_not_applicable(assist: Handler, ra_fixture: &str) {
65 check(assist, ra_fixture, ExpectedResult::NotApplicable, None); 65 check(assist, ra_fixture, ExpectedResult::NotApplicable, None);
66} 66}
67 67
68/// Check assist in unresolved state. Useful to check assists for lazy computation.
69#[track_caller]
70pub(crate) fn check_assist_unresolved(assist: Handler, ra_fixture: &str) {
71 check(assist, ra_fixture, ExpectedResult::Unresolved, None);
72}
73
68#[track_caller] 74#[track_caller]
69fn check_doc_test(assist_id: &str, before: &str, after: &str) { 75fn check_doc_test(assist_id: &str, before: &str, after: &str) {
70 let after = trim_indent(after); 76 let after = trim_indent(after);
@@ -101,6 +107,7 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) {
101 107
102enum ExpectedResult<'a> { 108enum ExpectedResult<'a> {
103 NotApplicable, 109 NotApplicable,
110 Unresolved,
104 After(&'a str), 111 After(&'a str),
105 Target(&'a str), 112 Target(&'a str),
106} 113}
@@ -115,7 +122,11 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label:
115 let sema = Semantics::new(&db); 122 let sema = Semantics::new(&db);
116 let config = TEST_CONFIG; 123 let config = TEST_CONFIG;
117 let ctx = AssistContext::new(sema, &config, frange); 124 let ctx = AssistContext::new(sema, &config, frange);
118 let mut acc = Assists::new(&ctx, AssistResolveStrategy::All); 125 let resolve = match expected {
126 ExpectedResult::Unresolved => AssistResolveStrategy::None,
127 _ => AssistResolveStrategy::All,
128 };
129 let mut acc = Assists::new(&ctx, resolve);
119 handler(&mut acc, &ctx); 130 handler(&mut acc, &ctx);
120 let mut res = acc.finish(); 131 let mut res = acc.finish();
121 132
@@ -163,8 +174,14 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label:
163 let range = assist.target; 174 let range = assist.target;
164 assert_eq_text!(&text_without_caret[range], target); 175 assert_eq_text!(&text_without_caret[range], target);
165 } 176 }
177 (Some(assist), ExpectedResult::Unresolved) => assert!(
178 assist.source_change.is_none(),
179 "unresolved assist should not contain source changes"
180 ),
166 (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"), 181 (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"),
167 (None, ExpectedResult::After(_)) | (None, ExpectedResult::Target(_)) => { 182 (None, ExpectedResult::After(_))
183 | (None, ExpectedResult::Target(_))
184 | (None, ExpectedResult::Unresolved) => {
168 panic!("code action is not applicable") 185 panic!("code action is not applicable")
169 } 186 }
170 (None, ExpectedResult::NotApplicable) => (), 187 (None, ExpectedResult::NotApplicable) => (),