From 87beacaa4573e9784c9146d0fb525ac12c8eca7c Mon Sep 17 00:00:00 2001 From: Victor Lanvin Date: Wed, 4 Sep 2024 08:21:44 -0700 Subject: [PATCH] Report bad map types Summary: Report approximated map types. The error depends on whether the ill-formed association is required or optional. This is done by adding an invalid form to the module stub during expansion containing the location of the ill-formed association. Reporting is hidden behind variable `EQWALIZER_REPORT_BAD_MAPS` which defaults to `false`. Reviewed By: ilya-klyuchnikov Differential Revision: D62180614 fbshipit-source-id: ad163fa95222efd5f0635da4eb39c3238a82b718 --- crates/eqwalizer/src/ast/expand.rs | 94 ++++++++++++------- crates/eqwalizer/src/lib.rs | 4 + crates/types_db/src/eqwalizer/ext_types.rs | 16 ++++ crates/types_db/src/eqwalizer/form.rs | 7 ++ .../src/eqwalizer/invalid_diagnostics.rs | 1 + 5 files changed, 90 insertions(+), 32 deletions(-) diff --git a/crates/eqwalizer/src/ast/expand.rs b/crates/eqwalizer/src/ast/expand.rs index 4be9d35c06..0253d7d084 100644 --- a/crates/eqwalizer/src/ast/expand.rs +++ b/crates/eqwalizer/src/ast/expand.rs @@ -42,6 +42,7 @@ use elp_types_db::eqwalizer::form::ExternalRecField; use elp_types_db::eqwalizer::form::ExternalTypeDecl; use elp_types_db::eqwalizer::form::InvalidForm; use elp_types_db::eqwalizer::form::InvalidFunSpec; +use elp_types_db::eqwalizer::form::InvalidMapType; use elp_types_db::eqwalizer::form::InvalidRecDecl; use elp_types_db::eqwalizer::form::InvalidTypeDecl; use elp_types_db::eqwalizer::form::TypeDecl; @@ -69,12 +70,13 @@ use crate::ast; struct Expander<'d> { module: SmolStr, project_id: ProjectId, + invalids: Vec, db: &'d dyn EqwalizerASTDatabase, } impl Expander<'_> { fn expand_fun_spec( - &self, + &mut self, fun_spec: ExternalFunSpec, ) -> Result { match self.expand_cfts(fun_spec.types) { @@ -88,13 +90,13 @@ impl Expander<'_> { } fn expand_cfts( - &self, + &mut self, cfts: Vec, ) -> Result, Invalid> { cfts.into_iter().map(|cft| self.expand_cft(cft)).collect() } - fn expand_cft(&self, cft: ConstrainedFunType) -> Result { + fn expand_cft(&mut self, cft: ConstrainedFunType) -> Result { let ft = { if cft.constraints.is_empty() { cft.ty @@ -134,7 +136,10 @@ impl Expander<'_> { }) } - fn expand_callback(&self, cb: ExternalCallback) -> Result { + fn expand_callback( + &mut self, + cb: ExternalCallback, + ) -> Result { match self.expand_cfts(cb.types) { Ok(types) => Ok(ExternalCallback { types, ..cb }), Err(te) => Err(InvalidFunSpec { @@ -146,7 +151,7 @@ impl Expander<'_> { } fn expand_type_decl( - &self, + &mut self, decl: ExternalTypeDecl, ) -> Result { let result = self @@ -163,7 +168,7 @@ impl Expander<'_> { } fn expand_opaque_decl( - &self, + &mut self, decl: ExternalOpaqueDecl, ) -> Result { let result = self @@ -180,7 +185,7 @@ impl Expander<'_> { } fn validate_type_vars( - &self, + &mut self, pos: &ast::Pos, body: &ExtType, params: &Vec, @@ -194,7 +199,7 @@ impl Expander<'_> { } fn check_repeated_type_param( - &self, + &mut self, pos: &ast::Pos, params: &Vec, ) -> Result<(), Invalid> { @@ -211,7 +216,10 @@ impl Expander<'_> { Ok(()) } - fn check_multiply_constrained_type_var(&self, cft: &ConstrainedFunType) -> Result<(), Invalid> { + fn check_multiply_constrained_type_var( + &mut self, + cft: &ConstrainedFunType, + ) -> Result<(), Invalid> { let mut names = FxHashSet::default(); let vars: Vec<&SmolStr> = cft.constraints.iter().map(|c| &c.t_var).collect(); for name in vars { @@ -229,7 +237,7 @@ impl Expander<'_> { } fn check_unbound_type_var( - &self, + &mut self, pos: &ast::Pos, params: &[SmolStr], ty_var: &VarExtType, @@ -244,7 +252,10 @@ impl Expander<'_> { } } - fn expand_rec_decl(&self, decl: ExternalRecDecl) -> Result { + fn expand_rec_decl( + &mut self, + decl: ExternalRecDecl, + ) -> Result { let fields = decl .fields .into_iter() @@ -260,11 +271,11 @@ impl Expander<'_> { } } - fn expand_types(&self, ts: Vec) -> Result, Invalid> { + fn expand_types(&mut self, ts: Vec) -> Result, Invalid> { ts.into_iter().map(|t| self.expand_type(t)).collect() } - fn expand_type(&self, t: ExtType) -> Result { + fn expand_type(&mut self, t: ExtType) -> Result { match t { ExtType::LocalExtType(ty) => { let id = RemoteId { @@ -341,7 +352,16 @@ impl Expander<'_> { tys: self.expand_types(ty.tys)?, })), ExtType::MapExtType(ty) => { - if ty.props.iter().any(|prop| !prop.is_ok()) { + if let Some(invalid_prop) = ty.props.iter().find(|prop| !prop.is_ok()) { + let location = invalid_prop.location().clone(); + let invalid_form = InvalidForm::InvalidMapType(InvalidMapType { + location: location.clone(), + te: Invalid::BadMapKey(BadMapKey { + location, + required: invalid_prop.required(), + }), + }); + self.invalids.push(invalid_form); Ok(ExtType::MapExtType(MapExtType { location: ty.location.clone(), props: vec![ExtProp::OptExtProp(OptExtProp { @@ -391,28 +411,29 @@ impl Expander<'_> { } } - fn expand_prop(&self, prop: ExtProp) -> Result { + fn expand_prop(&mut self, prop: ExtProp) -> Result { match prop { ExtProp::ReqExtProp(prop) => Ok(ExtProp::ReqExtProp(ReqExtProp { location: prop.location, key: self.expand_type(prop.key)?, tp: self.expand_type(prop.tp)?, })), - ExtProp::ReqBadExtProp(prop) => Err(Invalid::BadMapKey(BadMapKey { - location: prop.location, - })), ExtProp::OptExtProp(prop) => Ok(ExtProp::OptExtProp(OptExtProp { location: prop.location, key: self.expand_type(prop.key)?, tp: self.expand_type(prop.tp)?, })), - ExtProp::OptBadExtProp(prop) => Err(Invalid::BadMapKey(BadMapKey { - location: prop.location, + prop => Err(Invalid::BadMapKey(BadMapKey { + location: prop.location().clone(), + required: prop.required(), })), } } - fn expand_refined_record_field(&self, field: RefinedField) -> Result { + fn expand_refined_record_field( + &mut self, + field: RefinedField, + ) -> Result { Ok(RefinedField { label: field.label, ty: self.expand_type(field.ty)?, @@ -420,7 +441,7 @@ impl Expander<'_> { } fn expand_all_constraints( - &self, + &mut self, ts: Vec, sub: &FxHashMap, stack: &FxHashSet, @@ -431,7 +452,7 @@ impl Expander<'_> { } fn expand_constraints( - &self, + &mut self, t: ExtType, sub: &FxHashMap, stack: &FxHashSet, @@ -479,7 +500,16 @@ impl Expander<'_> { tys: self.expand_all_constraints(ty.tys, sub, stack)?, })), ExtType::MapExtType(ty) => { - if ty.props.iter().any(|prop| !prop.is_ok()) { + if let Some(invalid_prop) = ty.props.iter().find(|prop| !prop.is_ok()) { + let location = invalid_prop.location().clone(); + let invalid_form = InvalidForm::InvalidMapType(InvalidMapType { + location: location.clone(), + te: Invalid::BadMapKey(BadMapKey { + location, + required: invalid_prop.required(), + }), + }); + self.invalids.push(invalid_form); Ok(ExtType::MapExtType(MapExtType { location: ty.location.clone(), props: vec![ExtProp::OptExtProp(OptExtProp { @@ -537,7 +567,7 @@ impl Expander<'_> { } fn expand_prop_constraint( - &self, + &mut self, prop: ExtProp, sub: &FxHashMap, stack: &FxHashSet, @@ -548,22 +578,20 @@ impl Expander<'_> { key: self.expand_constraints(prop.key, sub, stack)?, tp: self.expand_constraints(prop.tp, sub, stack)?, })), - ExtProp::ReqBadExtProp(prop) => Err(Invalid::BadMapKey(BadMapKey { - location: prop.location, - })), ExtProp::OptExtProp(prop) => Ok(ExtProp::OptExtProp(OptExtProp { location: prop.location, key: self.expand_constraints(prop.key, sub, stack)?, tp: self.expand_constraints(prop.tp, sub, stack)?, })), - ExtProp::OptBadExtProp(prop) => Err(Invalid::BadMapKey(BadMapKey { - location: prop.location, + prop => Err(Invalid::BadMapKey(BadMapKey { + location: prop.location().clone(), + required: prop.required(), })), } } fn expand_refined_record_field_constraint( - &self, + &mut self, field: RefinedField, sub: &FxHashMap, stack: &FxHashSet, @@ -574,7 +602,7 @@ impl Expander<'_> { }) } - fn expand_rec_field(&self, field: ExternalRecField) -> Result { + fn expand_rec_field(&mut self, field: ExternalRecField) -> Result { let tp = { if let Some(tp) = field.tp { Some(self.expand_type(tp)?) @@ -603,6 +631,7 @@ impl StubExpander<'_> { ) -> StubExpander<'d> { let expander = Expander { module: module.clone(), + invalids: vec![], db, project_id, }; @@ -801,6 +830,7 @@ impl StubExpander<'_> { | ExternalForm::TypingAttribute(_) => Ok(()), })?; self.add_extra_types(); + self.stub.invalid_forms.append(&mut self.expander.invalids); Ok(()) } } diff --git a/crates/eqwalizer/src/lib.rs b/crates/eqwalizer/src/lib.rs index c3c8b9b772..0b43ac34d5 100644 --- a/crates/eqwalizer/src/lib.rs +++ b/crates/eqwalizer/src/lib.rs @@ -66,6 +66,7 @@ pub struct EqwalizerConfig { pub fault_tolerance: Option, pub occurrence_typing: Option, pub clause_coverage: Option, + pub report_bad_maps: Option, } impl EqwalizerConfig { fn set_cmd_env(&self, cmd: &mut CommandProxy<'_>) { @@ -75,6 +76,8 @@ impl EqwalizerConfig { .map(|cfg| cmd.env("EQWALIZER_EQWATER", cfg.to_string())); self.clause_coverage .map(|cfg| cmd.env("EQWALIZER_CLAUSE_COVERAGE", cfg.to_string())); + self.report_bad_maps + .map(|cfg| cmd.env("EQWALIZER_REPORT_BAD_MAPS", cfg.to_string())); } pub fn default_test() -> EqwalizerConfig { @@ -82,6 +85,7 @@ impl EqwalizerConfig { fault_tolerance: Some(false), occurrence_typing: Some(true), clause_coverage: Some(false), + report_bad_maps: Some(false), } } } diff --git a/crates/types_db/src/eqwalizer/ext_types.rs b/crates/types_db/src/eqwalizer/ext_types.rs index 21d5dc9c0f..9dc04268f8 100644 --- a/crates/types_db/src/eqwalizer/ext_types.rs +++ b/crates/types_db/src/eqwalizer/ext_types.rs @@ -291,6 +291,22 @@ impl ExtProp { ExtProp::ReqBadExtProp(_) | ExtProp::OptBadExtProp(_) => false, } } + + pub fn location(&self) -> &eqwalizer::Pos { + match self { + ExtProp::ReqExtProp(p) => &p.location, + ExtProp::ReqBadExtProp(p) => &p.location, + ExtProp::OptExtProp(p) => &p.location, + ExtProp::OptBadExtProp(p) => &p.location, + } + } + + pub fn required(&self) -> bool { + match self { + ExtProp::ReqExtProp(_) | ExtProp::ReqBadExtProp(_) => true, + ExtProp::OptExtProp(_) | ExtProp::OptBadExtProp(_) => false, + } + } } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] diff --git a/crates/types_db/src/eqwalizer/form.rs b/crates/types_db/src/eqwalizer/form.rs index 5449e91791..bc20330e46 100644 --- a/crates/types_db/src/eqwalizer/form.rs +++ b/crates/types_db/src/eqwalizer/form.rs @@ -48,6 +48,7 @@ pub enum InvalidForm { InvalidFunSpec(InvalidFunSpec), InvalidRecDecl(InvalidRecDecl), InvalidConvertTypeInRecDecl(InvalidConvertTypeInRecDecl), + InvalidMapType(InvalidMapType), } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] @@ -281,3 +282,9 @@ pub struct InvalidConvertTypeInRecDecl { pub name: SmolStr, pub te: Invalid, } + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] +pub struct InvalidMapType { + pub location: eqwalizer::Pos, + pub te: Invalid, +} diff --git a/crates/types_db/src/eqwalizer/invalid_diagnostics.rs b/crates/types_db/src/eqwalizer/invalid_diagnostics.rs index c416fa83c8..9233b22f69 100644 --- a/crates/types_db/src/eqwalizer/invalid_diagnostics.rs +++ b/crates/types_db/src/eqwalizer/invalid_diagnostics.rs @@ -97,4 +97,5 @@ pub struct AliasWithNonCovariantParam { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] pub struct BadMapKey { pub location: eqwalizer::Pos, + pub required: bool, }