Skip to content

Commit

Permalink
fix(ts/analyzer): Fix variance issue (#135)
Browse files Browse the repository at this point in the history
**Description:**

 - Allow assignment of void in function return values.
 - Fix variance handling.

**Related issue:**

 - Closes #136.
  • Loading branch information
kdy1 authored Oct 29, 2022
1 parent d62bfa2 commit 77e9e7e
Show file tree
Hide file tree
Showing 16 changed files with 170 additions and 64 deletions.
35 changes: 11 additions & 24 deletions crates/stc_ts_file_analyzer/src/analyzer/assign/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,13 @@ impl Analyzer<'_, '_> {
_ => (r_params, r_ret_ty),
};

// TypeScript functions are bivariant if strict_function_types is false.
if !self.env.rule().strict_function_types {
if self.assign_params(data, opts, &l_params, &r_params).is_ok() {
return Ok(());
}
}

// () => void
//
// is assignable to
Expand All @@ -386,7 +393,7 @@ impl Analyzer<'_, '_> {
//
// So we check for length first.
if r_params.len() != 0 {
self.assign_params(data, opts, l_params, &r_params)
self.assign_params(data, opts, &r_params, l_params)
.context(
"tried to assign parameters of a function to parameters of another function",
)?;
Expand Down Expand Up @@ -740,29 +747,9 @@ impl Analyzer<'_, '_> {
l_ty.make_clone_cheap();
r_ty.make_clone_cheap();

let reverse = !opts.for_overload
&& match (l_ty.normalize_instance(), r_ty.normalize_instance()) {
(Type::Union(..), Type::Union(..)) => false,

(Type::Function(..), Type::Function(..)) => false,

(
Type::Function(..) | Type::Constructor(..) | Type::Class(..),
Type::TypeLit(..) | Type::Interface(..),
) => false,

(_, Type::Union(..)) => true,

_ => true,
};

let res = if reverse {
self.assign_with_opts(data, AssignOpts { ..opts }, &r_ty, &l_ty)
.context("tried to assign the type of a parameter to another (reversed)")
} else {
self.assign_with_opts(data, AssignOpts { ..opts }, &l_ty, &r_ty)
.context("tried to assign the type of a parameter to another")
};
let res = self
.assign_with_opts(data, AssignOpts { ..opts }, &l_ty, &r_ty)
.context("tried to assign the type of a parameter to another");

res.convert_err(|err| match &err {
Error::MissingFields { span, .. } => Error::SimpleAssignFailed {
Expand Down
2 changes: 2 additions & 0 deletions crates/stc_ts_file_analyzer/src/analyzer/assign/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ pub(crate) struct AssignOpts {

/// If `true`, assignment will success if rhs is `void`.
/// [None] means `false`.
///
/// This is [Option] because it's required.
pub allow_assignment_of_void: Option<bool>,

/// If `true`, assignment will success if lhs is `void`.
Expand Down
13 changes: 11 additions & 2 deletions crates/stc_ts_file_analyzer/src/analyzer/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use swc_common::Spanned;
use swc_ecma_ast::TsKeywordTypeKind;

use crate::{
analyzer::{pat::PatMode, Analyzer, Ctx, ScopeKind},
analyzer::{assign::AssignOpts, pat::PatMode, Analyzer, Ctx, ScopeKind},
ty::TypeExt,
validator,
validator::ValidateWith,
Expand Down Expand Up @@ -152,7 +152,16 @@ impl Analyzer<'_, '_> {
if let Some(ref declared) = declared_ret_ty {
let span = inferred_return_type.span();
if let Some(ref inferred) = inferred_return_type {
child.assign(span, &mut Default::default(), declared, inferred)?;
child.assign_with_opts(
&mut Default::default(),
AssignOpts {
span,
allow_assignment_of_void: Some(true),
..Default::default()
},
declared,
inferred,
)?;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ impl Analyzer<'_, '_> {
&declared,
ret_ty,
)
.context("tried to assign return type")
.report(&mut self.storage);
}
}
Expand Down
2 changes: 2 additions & 0 deletions crates/stc_ts_file_analyzer/src/analyzer/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ fn get_env() -> Env {

Env::simple(
Rule {
strict_null_checks: true,
strict_function_types: true,
..Default::default()
},
EsVersion::latest(),
Expand Down
1 change: 1 addition & 0 deletions crates/stc_ts_file_analyzer/tests/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ fn get_env() -> Env {

Env::simple(
Rule {
strict_function_types: true,
..Default::default()
},
EsVersion::latest(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/1.ts:6:12
|
6 | return r;
| ^
|
= note: (x: T) => T

warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/1.ts:10:15
|
10 | var r1b = foo((x) => 1, (x) => ''); // {} => {}
| ^^^^^^^^
|
= note: (x: any) => 1

warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/1.ts:10:25
|
10 | var r1b = foo((x) => 1, (x) => ''); // {} => {}
| ^^^^^^^^^
|
= note: (x: any) => ''

warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/1.ts:10:11
|
10 | var r1b = foo((x) => 1, (x) => ''); // {} => {}
| ^^^
|
= note: <T>(a: (x: T) => T, b: (x: T) => T) => (x: T) => T

warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/1.ts:10:11
|
10 | var r1b = foo((x) => 1, (x) => ''); // {} => {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: (x: unknown) => unknown

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// When a function expression is inferentially typed (section 4.9.3) and a type assigned to a parameter in that expression references type parameters for which inferences are being made,
// the corresponding inferred type arguments to become fixed and no further candidate inferences are made for them.

function foo<T>(a: (x: T) => T, b: (x: T) => T) {
var r: (x: T) => T;
return r;
}

//var r1 = foo((x: number) => 1, (x: string) => ''); // error
var r1b = foo((x) => 1, (x) => ''); // {} => {}

export { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/2.ts:6:12
|
6 | return r;
| ^
|
= note: (x: T) => T

warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/2.ts:10:14
|
10 | var r2 = foo((x: Object) => null, (x: string) => ''); // Object => Object
| ^^^^^^^^^^^^^^^^^^^
|
= note: (x: Object) => null

warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/2.ts:10:35
|
10 | var r2 = foo((x: Object) => null, (x: string) => ''); // Object => Object
| ^^^^^^^^^^^^^^^^^
|
= note: (x: string) => ''

warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/2.ts:10:10
|
10 | var r2 = foo((x: Object) => null, (x: string) => ''); // Object => Object
| ^^^
|
= note: <T>(a: (x: T) => T, b: (x: T) => T) => (x: T) => T

warning: Type
--> $DIR/tests/pass/exprs/call/genericCallWithGenericSignatureArguments/2.ts:10:10
|
10 | var r2 = foo((x: Object) => null, (x: string) => ''); // Object => Object
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: (x: Object) => Object

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// When a function expression is inferentially typed (section 4.9.3) and a type assigned to a parameter in that expression references type parameters for which inferences are being made,
// the corresponding inferred type arguments to become fixed and no further candidate inferences are made for them.

function foo<T>(a: (x: T) => T, b: (x: T) => T) {
var r: (x: T) => T;
return r;
}

//var r1 = foo((x: number) => 1, (x: string) => ''); // error
var r2 = foo((x: Object) => null, (x: string) => ''); // Object => Object

export { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
warning: Type
--> $DIR/tests/pass/types/union/unionAndIntersectionInference3/1.ts:7:9
|
7 | throw com;
| ^^^
|
= note: () => (Iterator<S, U, R> | AsyncIterator<S, U, R>)

warning: Type
--> $DIR/tests/pass/types/union/unionAndIntersectionInference3/1.ts:6:98
|
6 | export const g: <U, R, S>(com: () => Iterator<S, U, R> | AsyncIterator<S, U, R>) => Promise<U> = async <U, R, S>(com: () => Iterator<S, U, R> | AsyncIterator<S, U, R>): Promise<U> => {
| __________________________________________________________________________________________________^
7 | | throw com;
8 | | };
| |_^
|
= note: <U, R, S>(com: () => (Iterator<S, U, R> | AsyncIterator<S, U, R>)) => Promise<U>

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// @strict: true
// @target: esnext

// Repros from #32247

export const g: <U, R, S>(com: () => Iterator<S, U, R> | AsyncIterator<S, U, R>) => Promise<U> = async <U, R, S>(com: () => Iterator<S, U, R> | AsyncIterator<S, U, R>): Promise<U> => {
throw com;
};

Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ warning: Type
4 | let f = !!true ? x : y; // (x?: 'hello') => void
| ^^^^^^^^^^^^^^
|
= note: (x?: 'hello') => void
= note: ((x: (string | undefined)) => void | (x?: 'hello') => void)

warning: Type
--> $DIR/tests/pass/types/union/unionTypeReduction2/5.ts:5:5
|
5 | f();
| ^
|
= note: (x?: 'hello') => void
= note: ((x: (string | undefined)) => void | (x?: 'hello') => void)

warning: Type
--> $DIR/tests/pass/types/union/unionTypeReduction2/5.ts:5:5
Expand All @@ -60,7 +60,7 @@ warning: Type
6 | f('hello');
| ^
|
= note: (x?: 'hello') => void
= note: ((x: (string | undefined)) => void | (x?: 'hello') => void)

warning: Type
--> $DIR/tests/pass/types/union/unionTypeReduction2/5.ts:6:5
Expand Down
Loading

0 comments on commit 77e9e7e

Please sign in to comment.