Skip to content

Conversation

olafurpg
Copy link
Contributor

@olafurpg olafurpg commented Apr 20, 2023

Previously, scip-typescript didn't infer correct relationships between the object literel property and interface property in return statements like below:

interface Configuration {
  property: number
}

export function returnStatement(): Configuration {
  return {
    property: 42,
  }
}

This commit fixes this specific issue. This commit also adds a test case for a situation where scip-typescript doesn't infer the relationship today but tsserver does. We should fix that in the future.

Test plan

See updated snapshot tests.

…tement

Previously, scip-typescript didn't infer correct relationships between
the object literel property and interface property in return statements
like below:

```ts
interface Configuration {
  property: number
}

export function returnStatement(): Configuration {
  return {
    property: 42,
  }
}
```

This commit fixes this specific issue. This commit also adds a test case
for a situation where scip-typescript doesn't infer the relationship
today but tsserver does. We should fix that in the future.
@olafurpg olafurpg requested a review from keynmol April 20, 2023 08:29
@olafurpg olafurpg changed the title Infer symnbol relationship for object literals property in return statement Infer symbol relationship for object literals property in return statement Apr 20, 2023
@olafurpg
Copy link
Contributor Author

Realized the previous logic failed as soon as you didn't have a toplevel return statement. I have a half-baked idea how to clean up the logic so that we pass down the inferred type while traversing the AST avoiding the need to look up parents (which is typically a sign of a broken abstraction IME).

@olafurpg olafurpg merged commit b3bb125 into main Apr 20, 2023
@olafurpg olafurpg deleted the olafurpg/return-infer branch April 20, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants