stc icon indicating copy to clipboard operation
stc copied to clipboard

Change error code for non-iterable based on the target

Open kdy1 opened this issue 2 years ago • 6 comments

https://github.com/dudykr/stc/blob/8b5fad3b580e2a4e2a4a75b0e3f45b75a8cc3f63/crates/stc_ts_type_checker/tests/conformance/es6/destructuring/destructuringAssignabilityCheck.ts#L5

We should report TS2461 instead of TS2345 for the line above

kdy1 avatar Feb 01 '23 14:02 kdy1

I think an issue is that get_iterator_inner https://github.com/dudykr/stc/blob/8ea5b02590550cdd7d1e6cbf53a54bc719be99a2/crates/stc_ts_file_analyzer/src/analyzer/expr/array.rs#L605 is never called, which by running the conformance tests on the microsoft/TypeScript repo I figured should be called for this to be handled correctly. I couldn't figure out why but I suspect this might be relevant:

I modified the fail macro to log to and from and got back (for to)

Tuple(Tuple { span: Span { lo: BytePos(3), hi: BytePos(5), ctxt: #0 }, elems: [], metadata: TupleMetadata { prevent_tuple_to_array: false, common: CommonTypeMetadata { implicit:
  │ false, infected_by_this_in_object_literal: false, prevent_converting_to_children: false, contains_infer_type: false, prevent_complex_simplification: false, resolved_from_var: false,
  │ prevent_generalization: false, destructure_key: DestructureId(1) } }, tracker: Tracker { _priv: () } })

which to me is weird because I expected an array type but I dont understand stc well enough to know for sure. If it should have been an array type then that explains why get_iterator_inner is never called.

I also had the following logged:

[crates/stc_ts_file_analyzer/src/analyzer/types/mod.rs:93] &ty = Arc(
    Freezed {
        ty: Function(
            Function {
                span: Span {
                    lo: BytePos(
                        2,
                    ),
                    hi: BytePos(
                        11,
                    ),
                    ctxt: #0,
                },
                type_params: None,
                params: [
                    FnParam {
                        span: Span {
                            lo: BytePos(
                                3,
                            ),
                            hi: BytePos(
                                5,
                            ),
                            ctxt: #0,
                        },
                        required: true,
                        pat: Array(
                            RArrayPat {
                                node_id: NodeId(
                                    6,
                                ),
                                span: Span {
                                    lo: BytePos(
                                        3,
                                    ),
                                    hi: BytePos(
                                        5,
                                    ),
                                    ctxt: #0,
                                },
                                elems: [],
                                optional: false,
                                type_ann: None,
                            },
                        ),
                        ty: Arc(
                            Freezed {
                                ty: Tuple(
                                    Tuple {
                                        span: Span {
                                            lo: BytePos(
                                                3,
                                            ),
                                            hi: BytePos(
                                                5,
                                            ),
                                            ctxt: #0,
                                        },
                                        elems: [],
                                        metadata: TupleMetadata {
                                            prevent_tuple_to_array: false,
                                            common: CommonTypeMetadata {
                                                implicit: false,
                                                infected_by_this_in_object_literal: false,
                                                prevent_converting_to_children: false,
                                                contains_infer_type: false,
                                                prevent_complex_simplification: false,
                                                resolved_from_var: false,
                                                prevent_generalization: false,
                                                destructure_key: DestructureId(
                                                    1,
                                                ),
                                            },
                                        },
                                        tracker: Tracker {
                                            _priv: (),
                                        },
                                    },
                                ),
                            },
                        ),
                    },
                ],
                ret_ty: Keyword(
                    KeywordType {
                        span: Span {
                            lo: BytePos(
                                10,
                            ),
                            hi: BytePos(
                                11,
                            ),
                            ctxt: #0,
                        },
                        kind: TsNumberKeyword,
                        metadata: KeywordTypeMetadata {
                            common: CommonTypeMetadata {
                                implicit: false,
                                infected_by_this_in_object_literal: false,
                                prevent_converting_to_children: false,
                                contains_infer_type: false,
                                prevent_complex_simplification: false,
                                resolved_from_var: false,
                                prevent_generalization: false,
                                destructure_key: DestructureId(
                                    0,
                                ),
                            },
                        },
                        tracker: Tracker {
                            _priv: (),
                        },
                    },
                ),
                metadata: FunctionMetadata {
                    common: CommonTypeMetadata {
                        implicit: false,
                        infected_by_this_in_object_literal: false,
                        prevent_converting_to_children: false,
                        contains_infer_type: false,
                        prevent_complex_simplification: false,
                        resolved_from_var: false,
                        prevent_generalization: false,
                        destructure_key: DestructureId(
                            0,
                        ),
                    },
                },
                tracker: Tracker {
                    _priv: (),
                },
            },
        ),
    },
)

which to me it seems weird that pat is an Array but ty is a Tuple but again I dont know if this is useful.

I hope this might be a bit helpful for anyone looking to tackle the issue.

awareness481 avatar Feb 05 '23 10:02 awareness481

I think it being Tuple is the correct behavior, but not sure. Maybe we should use Widen on the type. Widen will convert a Tuple to an Array if it's allowed. (i.e. no as const)

kdy1 avatar Feb 05 '23 11:02 kdy1

I think it being Tuple is the correct behavior, but not sure. Maybe we should use Widen on the type. Widen will convert a Tuple to an Array if it's allowed. (i.e. no as const)

Excuse my ignorance, but if you mean [] should be a tuple I think that's not correct..? At least based on https://ts-ast-viewer.com/#code/BTDaF0EoAIF4D5oAZLAN4F9IG4g

awareness481 avatar Feb 05 '23 14:02 awareness481

Oh.. Seems like tuples should be widened on IIFE?

kdy1 avatar Feb 05 '23 18:02 kdy1

I think you're right that they should be widened, at least thats what I understood from stepping through the ts compiler, but I couldn't tell if it's because there is an IIFE. I was suspecting it might be because there's no type param, but I couldn't find when/why TS decides to widen a type.

awareness481 avatar Feb 06 '23 19:02 awareness481

https://github.com/dudykr/stc/pull/661#issuecomment-1426180321

awareness481 : Edit: I'm not sure if it's problematic that this error does not come from get_iterator_inner like in tsc but I couldn't figure out a way to do it from there kdy1 : I'm not sure at the moment, too. It should be investigated

This is an issue that is far from over. And I don't think it's GFI to investigate what happens afterward.

I'd like to ask you for your thoughts. @kdy1

sunrabbit123 avatar Aug 23 '23 15:08 sunrabbit123