From 2ee5a3b82ed15cb0848c42cd902f91c43049a77d Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 12 Apr 2024 15:25:08 -0400 Subject: [PATCH 1/5] Start adding incorrect test for init before required --- ...stCheck_Function_DefaultBeforeRequired.txt | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt diff --git a/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt b/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt new file mode 100644 index 000000000..c687891b1 --- /dev/null +++ b/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt @@ -0,0 +1,85 @@ +# https://jsr.io/@scope/a/meta.json +{"versions": { "1.0.0": {} } } + +# https://jsr.io/@scope/a/1.0.0_meta.json +{ "exports": { ".": "./mod.ts" } } + +# https://jsr.io/@scope/a/1.0.0/mod.ts +// this can be called like `test(undefined, 1)` +export function test(value: number[] = [], value2: number) { +} + +export class Test { + constructor(value: number[] = [], value2: number) { + } + + method(value: number[] = [], value2: number) { + } +} + +export class TestParamProp { + constructor(public value: number[] = [], public value2: number) { + } +} + +# mod.ts +import 'jsr:@scope/a' + +# output +{ + "roots": [ + "file:///mod.ts" + ], + "modules": [ + { + "kind": "esm", + "dependencies": [ + { + "specifier": "jsr:@scope/a", + "code": { + "specifier": "jsr:@scope/a", + "span": { + "start": { + "line": 0, + "character": 7 + }, + "end": { + "line": 0, + "character": 21 + } + } + } + } + ], + "size": 22, + "mediaType": "TypeScript", + "specifier": "file:///mod.ts" + }, + { + "kind": "esm", + "size": 246, + "mediaType": "TypeScript", + "specifier": "https://jsr.io/@scope/a/1.0.0/mod.ts" + } + ], + "redirects": { + "jsr:@scope/a": "https://jsr.io/@scope/a/1.0.0/mod.ts" + }, + "packages": { + "@scope/a": "@scope/a@1.0.0" + } +} + +Fast check https://jsr.io/@scope/a/1.0.0/mod.ts: + {} + export function test(value?: number[], value2: number): void {} + export class Test { + constructor(value?: number[], value2: number){} + method(value?: number[], value2: number): void {} + } + --- DTS --- + export declare function test(value?: number[], value2: number): void; + export declare class Test { + constructor(value?: number[], value2: number); + method(value?: number[], value2: number): void; + } From 383e095ff2213520aad47f323c1d48dec7a03126 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 12 Apr 2024 18:36:49 -0400 Subject: [PATCH 2/5] update --- .../jsr/FastCheck_Function_DefaultBeforeRequired.txt | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt b/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt index c687891b1..eba4661c5 100644 --- a/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt +++ b/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt @@ -57,7 +57,7 @@ import 'jsr:@scope/a' }, { "kind": "esm", - "size": 246, + "size": 350, "mediaType": "TypeScript", "specifier": "https://jsr.io/@scope/a/1.0.0/mod.ts" } @@ -77,9 +77,19 @@ Fast check https://jsr.io/@scope/a/1.0.0/mod.ts: constructor(value?: number[], value2: number){} method(value?: number[], value2: number): void {} } + export class TestParamProp { + declare value: number[]; + declare value2: number; + constructor(value?: number[], value2: number){} + } --- DTS --- export declare function test(value?: number[], value2: number): void; export declare class Test { constructor(value?: number[], value2: number); method(value?: number[], value2: number): void; } + export declare class TestParamProp { + value: number[]; + value2: number; + constructor(value?: number[], value2: number); + } From 41aacda0b4ecadb71b8f7c2064fd8255a86428cb Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 12 Apr 2024 19:54:02 -0400 Subject: [PATCH 3/5] Fix issue. --- src/fast_check/transform.rs | 100 ++++++++++++++++-- .../function_default_before_required.txt} | 17 +-- 2 files changed, 100 insertions(+), 17 deletions(-) rename tests/specs/graph/{jsr/FastCheck_Function_DefaultBeforeRequired.txt => fast_check/function_default_before_required.txt} (74%) diff --git a/src/fast_check/transform.rs b/src/fast_check/transform.rs index fb186dbc5..4c225417d 100644 --- a/src/fast_check/transform.rs +++ b/src/fast_check/transform.rs @@ -769,10 +769,19 @@ impl<'a> FastCheckTransformer<'a> { } } - for param in &mut n.params { + let optional_start_index = + ParamsOptionalStartIndex::build(n.params.iter().map(|p| match p { + ParamOrTsParamProp::Param(p) => &p.pat, + // should have been converted to a param + ParamOrTsParamProp::TsParamProp(_) => unreachable!(), + })); + for (i, param) in n.params.iter_mut().enumerate() { match param { ParamOrTsParamProp::Param(param) => { - self.handle_param_pat(&mut param.pat)?; + self.handle_param_pat( + &mut param.pat, + optional_start_index.is_optional_at_index(i), + )?; param.decorators.clear(); } ParamOrTsParamProp::TsParamProp(_) => { @@ -1021,9 +1030,13 @@ impl<'a> FastCheckTransformer<'a> { })); } } - - for param in &mut n.params { - self.handle_param_pat(&mut param.pat)?; + let optional_start_index = + ParamsOptionalStartIndex::build(n.params.iter().map(|p| &p.pat)); + for (i, param) in n.params.iter_mut().enumerate() { + self.handle_param_pat( + &mut param.pat, + optional_start_index.is_optional_at_index(i), + )?; param.decorators.clear(); } @@ -1105,8 +1118,10 @@ impl<'a> FastCheckTransformer<'a> { n.is_async = false; } - for pat in &mut n.params { - self.handle_param_pat(pat)?; + let optional_start_index = ParamsOptionalStartIndex::build(n.params.iter()); + for (i, pat) in n.params.iter_mut().enumerate() { + self + .handle_param_pat(pat, optional_start_index.is_optional_at_index(i))?; } Ok(()) @@ -1115,7 +1130,28 @@ impl<'a> FastCheckTransformer<'a> { fn handle_param_pat( &mut self, pat: &mut Pat, + is_optional: bool, ) -> Result<(), Vec> { + fn convert_optional_ident_to_nullable_type(ident: &mut BindingIdent) { + ident.optional = false; + if let Some(type_ann) = ident.type_ann.take() { + let mut types = Vec::with_capacity(2); + types.push(type_ann.type_ann); + types.push(Box::new(ts_keyword_type( + TsKeywordTypeKind::TsUndefinedKeyword, + ))); + ident.type_ann = Some(Box::new(TsTypeAnn { + span: type_ann.span, + type_ann: Box::new(TsType::TsUnionOrIntersectionType( + TsUnionOrIntersectionType::TsUnionType(TsUnionType { + span: DUMMY_SP, + types, + }), + )), + })); + } + } + match pat { Pat::Ident(ident) => { if ident.type_ann.is_none() { @@ -1134,7 +1170,11 @@ impl<'a> FastCheckTransformer<'a> { span: DUMMY_SP, type_ann: Box::new(t), })); - ident.id.optional = true; + if !is_optional { + convert_optional_ident_to_nullable_type(ident); + } else { + ident.id.optional = true; + } *pat = Pat::Ident((*ident).clone()); } None => { @@ -1152,7 +1192,11 @@ impl<'a> FastCheckTransformer<'a> { } } } else { - ident.id.optional = true; + if !is_optional { + convert_optional_ident_to_nullable_type(ident); + } else { + ident.id.optional = true; + } *pat = Pat::Ident((*ident).clone()); } } @@ -1851,6 +1895,44 @@ fn infer_simple_type_from_type(t: &TsType) -> Option { } } +/// Holds when the first optional parameter occurs. +struct ParamsOptionalStartIndex(Option); + +impl ParamsOptionalStartIndex { + pub fn build<'a>(param_pats: impl Iterator) -> Self { + fn is_param_pat_optional(pat: &Pat) -> bool { + match pat { + Pat::Ident(ident) => ident.optional, + Pat::Array(a) => a.optional, + Pat::Object(o) => o.optional, + Pat::Assign(_) | Pat::Rest(_) => true, + Pat::Invalid(_) | Pat::Expr(_) => false, + } + } + + let mut optional_start_index = None; + for (i, pat) in param_pats.enumerate() { + if is_param_pat_optional(pat) { + if optional_start_index.is_none() { + optional_start_index = Some(i); + } + } else { + optional_start_index = None; + } + } + + Self(optional_start_index) + } + + pub fn is_optional_at_index(&self, index: usize) -> bool { + self + .0 + .as_ref() + .map(|start_index| index >= *start_index) + .unwrap_or(false) + } +} + fn is_expr_ident_or_member_idents(expr: &Expr) -> bool { match expr { Expr::Ident(_) => true, diff --git a/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt b/tests/specs/graph/fast_check/function_default_before_required.txt similarity index 74% rename from tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt rename to tests/specs/graph/fast_check/function_default_before_required.txt index eba4661c5..39f3dec8f 100644 --- a/tests/specs/graph/jsr/FastCheck_Function_DefaultBeforeRequired.txt +++ b/tests/specs/graph/fast_check/function_default_before_required.txt @@ -18,6 +18,7 @@ export class Test { } export class TestParamProp { + // the type of `value` property should be `number[]` constructor(public value: number[] = [], public value2: number) { } } @@ -72,24 +73,24 @@ import 'jsr:@scope/a' Fast check https://jsr.io/@scope/a/1.0.0/mod.ts: {} - export function test(value?: number[], value2: number): void {} + export function test(value: number[] | undefined, value2: number): void {} export class Test { - constructor(value?: number[], value2: number){} - method(value?: number[], value2: number): void {} + constructor(value: number[] | undefined, value2: number){} + method(value: number[] | undefined, value2: number): void {} } export class TestParamProp { declare value: number[]; declare value2: number; - constructor(value?: number[], value2: number){} + constructor(value: number[] | undefined, value2: number){} } --- DTS --- - export declare function test(value?: number[], value2: number): void; + export declare function test(value: number[] | undefined, value2: number): void; export declare class Test { - constructor(value?: number[], value2: number); - method(value?: number[], value2: number): void; + constructor(value: number[] | undefined, value2: number); + method(value: number[] | undefined, value2: number): void; } export declare class TestParamProp { value: number[]; value2: number; - constructor(value?: number[], value2: number); + constructor(value: number[] | undefined, value2: number); } From 33a4f5eb3ec7f7c84f558aeb4f7b02d6ca66db86 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 12 Apr 2024 19:55:56 -0400 Subject: [PATCH 4/5] Clippy --- src/fast_check/transform.rs | 39 ++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/fast_check/transform.rs b/src/fast_check/transform.rs index 4c225417d..fdfc0a213 100644 --- a/src/fast_check/transform.rs +++ b/src/fast_check/transform.rs @@ -1132,26 +1132,6 @@ impl<'a> FastCheckTransformer<'a> { pat: &mut Pat, is_optional: bool, ) -> Result<(), Vec> { - fn convert_optional_ident_to_nullable_type(ident: &mut BindingIdent) { - ident.optional = false; - if let Some(type_ann) = ident.type_ann.take() { - let mut types = Vec::with_capacity(2); - types.push(type_ann.type_ann); - types.push(Box::new(ts_keyword_type( - TsKeywordTypeKind::TsUndefinedKeyword, - ))); - ident.type_ann = Some(Box::new(TsTypeAnn { - span: type_ann.span, - type_ann: Box::new(TsType::TsUnionOrIntersectionType( - TsUnionOrIntersectionType::TsUnionType(TsUnionType { - span: DUMMY_SP, - types, - }), - )), - })); - } - } - match pat { Pat::Ident(ident) => { if ident.type_ann.is_none() { @@ -1731,6 +1711,25 @@ fn replacement_return_value(ty: &TsType) -> Option> { } } +// convert `ident?: string` to `ident: string | undefined` +fn convert_optional_ident_to_nullable_type(ident: &mut BindingIdent) { + ident.optional = false; + if let Some(type_ann) = ident.type_ann.take() { + ident.type_ann = Some(Box::new(TsTypeAnn { + span: type_ann.span, + type_ann: Box::new(TsType::TsUnionOrIntersectionType( + TsUnionOrIntersectionType::TsUnionType(TsUnionType { + span: DUMMY_SP, + types: vec![ + type_ann.type_ann, + Box::new(ts_keyword_type(TsKeywordTypeKind::TsUndefinedKeyword)), + ], + }), + )), + })); + } +} + fn prefix_ident(ident: &mut Ident, prefix: &str) { ident.sym = format!("{}{}", prefix, ident.sym).into(); } From c7f93cc6e1494c69ac9eb87fbe1e4be64d901ea4 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 12 Apr 2024 20:00:41 -0400 Subject: [PATCH 5/5] Fix. --- src/fast_check/transform.rs | 2 +- .../specs/graph/fast_check/function_default_before_required.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fast_check/transform.rs b/src/fast_check/transform.rs index fdfc0a213..c60a73729 100644 --- a/src/fast_check/transform.rs +++ b/src/fast_check/transform.rs @@ -1711,7 +1711,7 @@ fn replacement_return_value(ty: &TsType) -> Option> { } } -// convert `ident?: string` to `ident: string | undefined` +// Converts `ident?: string` to `ident: string | undefined` fn convert_optional_ident_to_nullable_type(ident: &mut BindingIdent) { ident.optional = false; if let Some(type_ann) = ident.type_ann.take() { diff --git a/tests/specs/graph/fast_check/function_default_before_required.txt b/tests/specs/graph/fast_check/function_default_before_required.txt index 39f3dec8f..d45891ccd 100644 --- a/tests/specs/graph/fast_check/function_default_before_required.txt +++ b/tests/specs/graph/fast_check/function_default_before_required.txt @@ -58,7 +58,7 @@ import 'jsr:@scope/a' }, { "kind": "esm", - "size": 350, + "size": 405, "mediaType": "TypeScript", "specifier": "https://jsr.io/@scope/a/1.0.0/mod.ts" }