Skip to content

improve collection type inference#16530

Merged
132ikl merged 1 commit intonushell:mainfrom
sgvictorino:fix-collection-types
Sep 24, 2025
Merged

improve collection type inference#16530
132ikl merged 1 commit intonushell:mainfrom
sgvictorino:fix-collection-types

Conversation

@sgvictorino
Copy link
Copy Markdown
Contributor

Closes #16421

Release notes summary - What our users need to know

Fixes order-dependent table type inference bugs and introduces similar type widening for lists:

def get_type [] { scope variables | where name == "$foo" | first | get type }

let foo = [ [bar]; [ { baz: 1 } ], [ { baz: 1, extra: true } ] ]
get_type # table<bar: record<baz: int, extra: bool>> -> table<bar: record<baz: int>>

let foo = [ [bar]; [ { baz: 1, extra: true } ], [ { baz: 1 } ] ]
get_type # table<bar: any> -> table<bar: record<baz: int>>

let foo = [ { baz: 1 }, { baz: 1, extra: true } ]
get_type # list<any> -> list<record<baz: int>>

@github-actions github-actions bot added the A:parser Issues related to parsing label Aug 28, 2025
@132ikl
Copy link
Copy Markdown
Member

132ikl commented Aug 28, 2025

awesome!! i'll take a look when i get a chance

@132ikl 132ikl self-requested a review August 28, 2025 12:14
Copy link
Copy Markdown
Member

@Bahex Bahex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think putting this logic into a method like Type::widen(self, other: Self) -> Self would be a little neater.

Other than that I have no complaints 👍

# Description

Fixes order-dependent table type inference bugs and introduces similar
type widening for lists:

```nushell
def get_type [] { scope variables | where name == "$foo" | first | get type }

let foo = [ [bar]; [ { baz: 1 } ], [ { baz: 1, extra: true } ] ]
get_type # table<bar: record<baz: int, extra: bool>> -> table<bar: record<baz: int>>

let foo = [ [bar]; [ { baz: 1, extra: true } ], [ { baz: 1 } ] ]
get_type # table<bar: any> -> table<bar: record<baz: int>>

let foo = [ { baz: 1 }, { baz: 1, extra: true } ]
get_type # list<any> -> list<record<baz: int>>
```
} else if other.is_subtype_of(&self) {
self
} else {
Type::Any
Copy link
Copy Markdown
Member

@132ikl 132ikl Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be a good spot to improve in the future. for example, if self and other are lists of different types then we can widen to list<any>. it looks like you are handling this for list expressions already but it would be good to generalize it as well

@132ikl
Copy link
Copy Markdown
Member

132ikl commented Sep 24, 2025

thanks! sorry it took so long on this one, i forgot about it 😢

@132ikl 132ikl merged commit d54de7d into nushell:main Sep 24, 2025
16 checks passed
@132ikl 132ikl added notes:ready The "Release notes summary" section of this PR is ready to be included in our release notes. notes:fixes Include the release notes summary in the "Bug fixes" section labels Sep 24, 2025
@github-actions github-actions bot added this to the v0.108.0 milestone Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:parser Issues related to parsing notes:fixes Include the release notes summary in the "Bug fixes" section notes:ready The "Release notes summary" section of this PR is ready to be included in our release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parser incorrectly infers type of table column with multiple different types

3 participants