Skip to content

Commit

Permalink
Report bad map types
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@VLanvin @facebook-github-bot
VLanvin authored and facebook-github-bot committed Sep 4, 2024
1 parent c96bfcb commit 87beaca
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 32 deletions.
94 changes: 62 additions & 32 deletions crates/eqwalizer/src/ast/expand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -69,12 +70,13 @@ use crate::ast;
struct Expander<'d> {
module: SmolStr,
project_id: ProjectId,
invalids: Vec<InvalidForm>,
db: &'d dyn EqwalizerASTDatabase,
}

impl Expander<'_> {
fn expand_fun_spec(
&self,
&mut self,
fun_spec: ExternalFunSpec,
) -> Result<ExternalFunSpec, InvalidFunSpec> {
match self.expand_cfts(fun_spec.types) {
Expand All @@ -88,13 +90,13 @@ impl Expander<'_> {
}

fn expand_cfts(
&self,
&mut self,
cfts: Vec<ConstrainedFunType>,
) -> Result<Vec<ConstrainedFunType>, Invalid> {
cfts.into_iter().map(|cft| self.expand_cft(cft)).collect()
}

fn expand_cft(&self, cft: ConstrainedFunType) -> Result<ConstrainedFunType, Invalid> {
fn expand_cft(&mut self, cft: ConstrainedFunType) -> Result<ConstrainedFunType, Invalid> {
let ft = {
if cft.constraints.is_empty() {
cft.ty
Expand Down Expand Up @@ -134,7 +136,10 @@ impl Expander<'_> {
})
}

fn expand_callback(&self, cb: ExternalCallback) -> Result<ExternalCallback, InvalidFunSpec> {
fn expand_callback(
&mut self,
cb: ExternalCallback,
) -> Result<ExternalCallback, InvalidFunSpec> {
match self.expand_cfts(cb.types) {
Ok(types) => Ok(ExternalCallback { types, ..cb }),
Err(te) => Err(InvalidFunSpec {
Expand All @@ -146,7 +151,7 @@ impl Expander<'_> {
}

fn expand_type_decl(
&self,
&mut self,
decl: ExternalTypeDecl,
) -> Result<ExternalTypeDecl, InvalidTypeDecl> {
let result = self
Expand All @@ -163,7 +168,7 @@ impl Expander<'_> {
}

fn expand_opaque_decl(
&self,
&mut self,
decl: ExternalOpaqueDecl,
) -> Result<ExternalOpaqueDecl, InvalidTypeDecl> {
let result = self
Expand All @@ -180,7 +185,7 @@ impl Expander<'_> {
}

fn validate_type_vars(
&self,
&mut self,
pos: &ast::Pos,
body: &ExtType,
params: &Vec<SmolStr>,
Expand All @@ -194,7 +199,7 @@ impl Expander<'_> {
}

fn check_repeated_type_param(
&self,
&mut self,
pos: &ast::Pos,
params: &Vec<SmolStr>,
) -> Result<(), Invalid> {
Expand All @@ -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 {
Expand All @@ -229,7 +237,7 @@ impl Expander<'_> {
}

fn check_unbound_type_var(
&self,
&mut self,
pos: &ast::Pos,
params: &[SmolStr],
ty_var: &VarExtType,
Expand All @@ -244,7 +252,10 @@ impl Expander<'_> {
}
}

fn expand_rec_decl(&self, decl: ExternalRecDecl) -> Result<ExternalRecDecl, InvalidRecDecl> {
fn expand_rec_decl(
&mut self,
decl: ExternalRecDecl,
) -> Result<ExternalRecDecl, InvalidRecDecl> {
let fields = decl
.fields
.into_iter()
Expand All @@ -260,11 +271,11 @@ impl Expander<'_> {
}
}

fn expand_types(&self, ts: Vec<ExtType>) -> Result<Vec<ExtType>, Invalid> {
fn expand_types(&mut self, ts: Vec<ExtType>) -> Result<Vec<ExtType>, Invalid> {
ts.into_iter().map(|t| self.expand_type(t)).collect()
}

fn expand_type(&self, t: ExtType) -> Result<ExtType, Invalid> {
fn expand_type(&mut self, t: ExtType) -> Result<ExtType, Invalid> {
match t {
ExtType::LocalExtType(ty) => {
let id = RemoteId {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -391,36 +411,37 @@ impl Expander<'_> {
}
}

fn expand_prop(&self, prop: ExtProp) -> Result<ExtProp, Invalid> {
fn expand_prop(&mut self, prop: ExtProp) -> Result<ExtProp, Invalid> {
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<RefinedField, Invalid> {
fn expand_refined_record_field(
&mut self,
field: RefinedField,
) -> Result<RefinedField, Invalid> {
Ok(RefinedField {
label: field.label,
ty: self.expand_type(field.ty)?,
})
}

fn expand_all_constraints(
&self,
&mut self,
ts: Vec<ExtType>,
sub: &FxHashMap<SmolStr, ExtType>,
stack: &FxHashSet<SmolStr>,
Expand All @@ -431,7 +452,7 @@ impl Expander<'_> {
}

fn expand_constraints(
&self,
&mut self,
t: ExtType,
sub: &FxHashMap<SmolStr, ExtType>,
stack: &FxHashSet<SmolStr>,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -537,7 +567,7 @@ impl Expander<'_> {
}

fn expand_prop_constraint(
&self,
&mut self,
prop: ExtProp,
sub: &FxHashMap<SmolStr, ExtType>,
stack: &FxHashSet<SmolStr>,
Expand All @@ -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<SmolStr, ExtType>,
stack: &FxHashSet<SmolStr>,
Expand All @@ -574,7 +602,7 @@ impl Expander<'_> {
})
}

fn expand_rec_field(&self, field: ExternalRecField) -> Result<ExternalRecField, Invalid> {
fn expand_rec_field(&mut self, field: ExternalRecField) -> Result<ExternalRecField, Invalid> {
let tp = {
if let Some(tp) = field.tp {
Some(self.expand_type(tp)?)
Expand Down Expand Up @@ -603,6 +631,7 @@ impl StubExpander<'_> {
) -> StubExpander<'d> {
let expander = Expander {
module: module.clone(),
invalids: vec![],
db,
project_id,
};
Expand Down Expand Up @@ -801,6 +830,7 @@ impl StubExpander<'_> {
| ExternalForm::TypingAttribute(_) => Ok(()),
})?;
self.add_extra_types();
self.stub.invalid_forms.append(&mut self.expander.invalids);
Ok(())
}
}
4 changes: 4 additions & 0 deletions crates/eqwalizer/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ pub struct EqwalizerConfig {
pub fault_tolerance: Option<bool>,
pub occurrence_typing: Option<bool>,
pub clause_coverage: Option<bool>,
pub report_bad_maps: Option<bool>,
}
impl EqwalizerConfig {
fn set_cmd_env(&self, cmd: &mut CommandProxy<'_>) {
Expand All @@ -75,13 +76,16 @@ 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 {
EqwalizerConfig {
fault_tolerance: Some(false),
occurrence_typing: Some(true),
clause_coverage: Some(false),
report_bad_maps: Some(false),
}
}
}
Expand Down
16 changes: 16 additions & 0 deletions crates/types_db/src/eqwalizer/ext_types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
7 changes: 7 additions & 0 deletions crates/types_db/src/eqwalizer/form.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub enum InvalidForm {
InvalidFunSpec(InvalidFunSpec),
InvalidRecDecl(InvalidRecDecl),
InvalidConvertTypeInRecDecl(InvalidConvertTypeInRecDecl),
InvalidMapType(InvalidMapType),
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -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,
}
1 change: 1 addition & 0 deletions crates/types_db/src/eqwalizer/invalid_diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,5 @@ pub struct AliasWithNonCovariantParam {
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
pub struct BadMapKey {
pub location: eqwalizer::Pos,
pub required: bool,
}

0 comments on commit 87beaca

Please sign in to comment.