Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Toolip Shows type none for Functions With Return Type as text #228

Open
ninmonkey opened this issue Jun 6, 2024 · 0 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@ninmonkey
Copy link
Contributor

ninmonkey commented Jun 6, 2024

After experimenting more, I think

  • it ignores the return type of the function itself. even if it's any.
  • instead the type resolution seems to be determined based on the final expression in the function, overriding method return type ?

Based on this thread #206 , I tried both settings. It did not appear to change anything.

{   "powerquery.general.mode": "SDK",
   "powerquery.general.mode": "Power Query" }
// like it determines this as any
= () as text => "str" as any

// and this as type text
= () as any => "str" as text

Expected behavior

For some of these cases I think the return type is guaranteed to be text but display as none instead.

Example = () as text => Text.Combine( {"a"} )

// tooltip: [let-variable] Example: none

image

I thought if the parser didn't know the return type of Text.Combine, the function assert forces it to be either text or errors ?

1] If it can't resolve it my function, should it say any instead? Because the function is using a return type that's not type none, so type none would not be possible in this case?

2] Is this correct? I thought a function with a return type declared, guaranteed the return value is compatible using a type assertion on the final value?

If I explicitly assert it twice, then it displays text

Example = () as text => 
    Text.Combine( {"a"} ) as text

I think it's some interaction with the standard library definitions

One reason is because of this case. I tried asserting the return type as any
But the parser was able to tell adding two string literals has must be type text

    // text 🟢
    Any = () as any => "foo " & "bar",
    // text 🟢
    AnySum = () => Any(),

I was able to break the return type when the value was assert as any
Maybe the type resolution doesn't use function return types at all? It's resolving the type of the final value, instead?

    // none ⛔
    AnyExplicit2 = () as text => "foo " & "bar" as any,
    // none ⛔
    AnyExp2 = () => AnyExplicit2(),

Actual behavior

Pq ⁞ Bug Requires Double Assert - VsCode-PowerQuery  ⁞ 2024-06 ⁞ High quality low color ⁞ i0

Is this a different repo ?

Nothing in the log stood out. Is it just the showing results from the Language Server ?
Everything is a TRACE
There are zero matches if you search it with the regex: info|warning|debug|error
So maybe this should be filed in a different repository ?

Long log: 2024-06-05-PowerQueryExtension-hoverTests.log

To Reproduce

I started a new vscode window -> new tab -> set language powerquery

Then pasted this script
The tooltip for Text.Combine has a return type

[library function] Text.Combine: (texts: list, separator: optional nullable text) => text
let
    // text 🟢
    GetText4 = () as text =>
        "sdfd" & "23",

    // none ⛔
    GetText = (texts as list) as text =>
        Text.Combine( {"a"} ),

    // text 🟢
    GetText2 = (texts as list) as text =>
        Text.Combine( {"a"} ) as text,

    // none ⛔
    GetText3 = (texts as list) as text =>
        "prefix" & Text.Combine( {"a"} ),

    // none ⛔
    GetText3B = (texts as list) as text =>
        ("prefix" as text) & Text.Combine( {"a"} ),

    // text 🟢
    GetText3C = (texts as list) as text =>
        "prefix"  & Text.Combine( {"a"} ) as text
in
    ...

Environment

- extension: 'PowerQuery.vscode-powerquery'
  version: '0.1.60'
- extension: 'PowerQuery.vscode-powerquery-sdk'
  version: '0.4.0'
- name: 'vscode'
  version: '1.89.1 dc96b837cf6bb4af9cd736aa3af08cf8279f7685 x64'

Settings.json

{
    "powerquery.diagnostics.typeStrategy": "Extended",
    "powerquery.trace.server": "verbose", 
    "powerquery.general.mode": "SDK",
}

I don't think the SDK engine is involved. It logged 1 line total.

@ninmonkey ninmonkey added the bug Something isn't working label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants