Skip to content

Commit

Permalink
fix: improve analysis type checks. (#209)
Browse files Browse the repository at this point in the history
  • Loading branch information
@peterhuene
peterhuene authored Oct 4, 2024
1 parent 341a348 commit b1439df
Show file tree
Hide file tree
Showing 9 changed files with 506 additions and 351 deletions.
449 changes: 227 additions & 222 deletions Gauntlet.toml
View file

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions wdl-analysis/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

* Improved type calculations in function calls and when determining common
types in certain expressions ([#209](https://github.com/stjude-rust-labs/wdl/pull/209)).
* Treat a coercion to `T?` for a function argument of type `T` as a preference
over any other coercion ([#199](https://github.com/stjude-rust-labs/wdl/pull/199)).
* Fix the signature of `select_first` such that it is monomorphic ([#199](https://github.com/stjude-rust-labs/wdl/pull/199)).
Expand Down
14 changes: 5 additions & 9 deletions wdl-analysis/src/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,30 +373,26 @@ pub fn call_input_type_mismatch(
)
}

/// Creates a "element type mismatch" diagnostic.
pub fn element_type_mismatch(
/// Creates a "no common type" diagnostic.
pub fn no_common_type(
types: &Types,
kind: &str,
expected: Type,
expected_span: Span,
actual: Type,
actual_span: Span,
) -> Diagnostic {
Diagnostic::error(format!(
"type mismatch: expected every {kind} to be type `{expected}`, but found type `{actual}`",
"type mismatch: a type common to both type `{expected}` and type `{actual}` does not exist",
expected = expected.display(types),
actual = actual.display(types)
))
.with_label(
format!(
"this {kind} is type `{actual}`",
actual = actual.display(types)
),
format!("this is type `{actual}`", actual = actual.display(types)),
actual_span,
)
.with_label(
format!(
"this {kind} is type `{expected}`",
"this is type `{expected}`",
expected = expected.display(types)
),
expected_span,
Expand Down
201 changes: 155 additions & 46 deletions wdl-analysis/src/stdlib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,22 +163,33 @@ impl GenericType {
}

/// Infers any type parameters from the generic type.
fn infer_type_parameters(&self, types: &Types, ty: Type, params: &mut TypeParameters<'_>) {
fn infer_type_parameters(
&self,
types: &Types,
ty: Type,
params: &mut TypeParameters<'_>,
ignore_constraints: bool,
) {
match self {
Self::Parameter(name) | Self::UnqualifiedParameter(name) => {
// Verify the type satisfies any constraint
let (param, _) = params.get(name).expect("should have parameter");
if let Some(constraint) = param.constraint() {
if !constraint.satisfied(types, ty) {
return;

if !ignore_constraints {
if let Some(constraint) = param.constraint() {
if !constraint.satisfied(types, ty) {
return;
}
}
}

params.set_inferred_type(name, ty);
}
Self::Array(array) => array.infer_type_parameters(types, ty, params),
Self::Pair(pair) => pair.infer_type_parameters(types, ty, params),
Self::Map(map) => map.infer_type_parameters(types, ty, params),
Self::Array(array) => {
array.infer_type_parameters(types, ty, params, ignore_constraints)
}
Self::Pair(pair) => pair.infer_type_parameters(types, ty, params, ignore_constraints),
Self::Map(map) => map.infer_type_parameters(types, ty, params, ignore_constraints),
}
}

Expand Down Expand Up @@ -312,14 +323,33 @@ impl GenericArrayType {
}

/// Infers any type parameters from the generic type.
fn infer_type_parameters(&self, types: &Types, ty: Type, params: &mut TypeParameters<'_>) {
if let Type::Compound(ty) = ty {
if !ty.is_optional() {
fn infer_type_parameters(
&self,
types: &Types,
ty: Type,
params: &mut TypeParameters<'_>,
ignore_constraints: bool,
) {
match ty {
Type::Union => {
self.element_type.infer_type_parameters(
types,
Type::Union,
params,
ignore_constraints,
);
}
Type::Compound(ty) if !ty.is_optional() => {
if let CompoundTypeDef::Array(ty) = types.type_definition(ty.definition()) {
self.element_type
.infer_type_parameters(types, ty.element_type(), params);
self.element_type.infer_type_parameters(
types,
ty.element_type(),
params,
ignore_constraints,
);
}
}
_ => {}
}
}

Expand Down Expand Up @@ -408,16 +438,45 @@ impl GenericPairType {
}

/// Infers any type parameters from the generic type.
fn infer_type_parameters(&self, types: &Types, ty: Type, params: &mut TypeParameters<'_>) {
if let Type::Compound(ty) = ty {
if !ty.is_optional() {
fn infer_type_parameters(
&self,
types: &Types,
ty: Type,
params: &mut TypeParameters<'_>,
ignore_constraints: bool,
) {
match ty {
Type::Union => {
self.first_type.infer_type_parameters(
types,
Type::Union,
params,
ignore_constraints,
);
self.second_type.infer_type_parameters(
types,
Type::Union,
params,
ignore_constraints,
);
}
Type::Compound(ty) if !ty.is_optional() => {
if let CompoundTypeDef::Pair(ty) = types.type_definition(ty.definition()) {
self.first_type
.infer_type_parameters(types, ty.first_type(), params);
self.second_type
.infer_type_parameters(types, ty.second_type(), params);
self.first_type.infer_type_parameters(
types,
ty.first_type(),
params,
ignore_constraints,
);
self.second_type.infer_type_parameters(
types,
ty.second_type(),
params,
ignore_constraints,
);
}
}
_ => {}
}
}

Expand Down Expand Up @@ -498,16 +557,41 @@ impl GenericMapType {
}

/// Infers any type parameters from the generic type.
fn infer_type_parameters(&self, types: &Types, ty: Type, params: &mut TypeParameters<'_>) {
if let Type::Compound(ty) = ty {
if !ty.is_optional() {
fn infer_type_parameters(
&self,
types: &Types,
ty: Type,
params: &mut TypeParameters<'_>,
ignore_constraints: bool,
) {
match ty {
Type::Union => {
self.key_type
.infer_type_parameters(types, Type::Union, params, ignore_constraints);
self.value_type.infer_type_parameters(
types,
Type::Union,
params,
ignore_constraints,
);
}
Type::Compound(ty) if !ty.is_optional() => {
if let CompoundTypeDef::Map(ty) = types.type_definition(ty.definition()) {
self.key_type
.infer_type_parameters(types, ty.key_type(), params);
self.value_type
.infer_type_parameters(types, ty.value_type(), params);
self.key_type.infer_type_parameters(
types,
ty.key_type(),
params,
ignore_constraints,
);
self.value_type.infer_type_parameters(
types,
ty.value_type(),
params,
ignore_constraints,
);
}
}
_ => {}
}
}

Expand Down Expand Up @@ -671,9 +755,15 @@ impl FunctionalType {
}

/// Infers any type parameters if the type is generic.
fn infer_type_parameters(&self, types: &Types, ty: Type, params: &mut TypeParameters<'_>) {
fn infer_type_parameters(
&self,
types: &Types,
ty: Type,
params: &mut TypeParameters<'_>,
ignore_constraints: bool,
) {
if let Self::Generic(generic) = self {
generic.infer_type_parameters(types, ty, params);
generic.infer_type_parameters(types, ty, params, ignore_constraints);
}
}

Expand Down Expand Up @@ -914,10 +1004,15 @@ impl FunctionSignature {
/// signature.
///
/// Returns the collection of type parameters.
fn infer_type_parameters(&self, types: &Types, arguments: &[Type]) -> TypeParameters<'_> {
fn infer_type_parameters(
&self,
types: &Types,
arguments: &[Type],
ignore_constraints: bool,
) -> TypeParameters<'_> {
let mut parameters = TypeParameters::new(&self.type_parameters);
for (parameter, argument) in self.parameters.iter().zip(arguments.iter()) {
parameter.infer_type_parameters(types, *argument, &mut parameters);
parameter.infer_type_parameters(types, *argument, &mut parameters, ignore_constraints);
}

parameters
Expand Down Expand Up @@ -950,7 +1045,7 @@ impl FunctionSignature {

// Ensure the argument types are correct for the function
let mut coerced = false;
let type_parameters = self.infer_type_parameters(types, arguments);
let type_parameters = self.infer_type_parameters(types, arguments, false);
for (i, (parameter, argument)) in self.parameters.iter().zip(arguments.iter()).enumerate() {
match parameter.realize(types, &type_parameters) {
Some(ty) => {
Expand Down Expand Up @@ -1128,26 +1223,40 @@ impl Function {
}
}

/// Gets the return type of the function.
/// Realizes the return type of the function without constraints.
///
/// Returns `None` if the function return type cannot be statically
/// determined.
/// This is typically called after a failure to bind a function so that the
/// return type can be calculated despite the failure.
///
/// This may occur for functions with a generic return type or if the
/// function is polymorphic and not every overload of the function has the
/// same return type.
pub fn ret(&self, types: &Types) -> Option<Type> {
/// As such, it attempts to realize any type parameters without constraints,
/// as an unsatisfied constraint likely caused the bind failure.
pub fn realize_unconstrained_return_type(&self, types: &mut Types, arguments: &[Type]) -> Type {
match self {
Self::Monomorphic(f) => f.signature.ret.concrete_type(),
Self::Monomorphic(f) => {
let type_parameters = f.signature.infer_type_parameters(types, arguments, true);
f.signature
.ret()
.realize(types, &type_parameters)
.unwrap_or(Type::Union)
}
Self::Polymorphic(f) => {
let ty = f.signatures[0].ret.concrete_type()?;
for signature in f.signatures.iter().skip(1) {
if signature.ret.concrete_type()?.type_eq(types, &ty) {
return None;
let mut ty = None;

// For polymorphic functions, the calculated return type must be the same for
// each overload
for signature in &f.signatures {
let type_parameters = signature.infer_type_parameters(types, arguments, true);
let ret_ty = signature
.ret()
.realize(types, &type_parameters)
.unwrap_or(Type::Union);

if !ty.get_or_insert(ret_ty).type_eq(types, &ret_ty) {
return Type::Union;
}
}

Some(ty)
ty.unwrap_or(Type::Union)
}
}
}
Expand Down Expand Up @@ -2802,7 +2911,7 @@ mod test {
let ty = f
.bind(&mut types, &[Type::Union])
.expect("bind should succeed");
assert_eq!(ty.display(&types).to_string(), "Union");
assert_eq!(ty.display(&types).to_string(), "Array[Union]");

// Check for a Map[String, String]
let ty = types.add_map(MapType::new(
Expand Down Expand Up @@ -2859,7 +2968,7 @@ mod test {
let ty = f
.bind(&mut types, &[Type::Union])
.expect("bind should succeed");
assert_eq!(ty.display(&types).to_string(), "Union");
assert_eq!(ty.display(&types).to_string(), "Array[Union]");

// Check for a Array[Array[String]?] -> Array[Array[String]]
let array_string = types
Expand Down
5 changes: 3 additions & 2 deletions wdl-analysis/src/stdlib/constraints.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ impl Constraint for OptionalTypeConstraint {
}

fn satisfied(&self, _: &Types, ty: Type) -> bool {
ty.is_optional()
// For the purpose of the constraint, treat `Union` as optional
ty == Type::Union || ty.is_optional()
}
}

Expand Down Expand Up @@ -258,7 +259,7 @@ mod test {
assert!(!constraint.satisfied(&types, PrimitiveTypeKind::Directory.into()));
assert!(!constraint.satisfied(&types, Type::Object));
assert!(constraint.satisfied(&types, Type::OptionalObject));
assert!(!constraint.satisfied(&types, Type::Union));
assert!(constraint.satisfied(&types, Type::Union));

let ty = types.add_array(ArrayType::new(PrimitiveType::optional(
PrimitiveTypeKind::Boolean,
Expand Down
Loading

0 comments on commit b1439df

Please sign in to comment.