microsoft / vscode-powerquery

Visual Studio Code extension for Power Query / M.
MIT License
88 stars 18 forks source link

Argument count error for #table using a list of column names #105

Closed migueesc123 closed 3 years ago

migueesc123 commented 3 years ago

image Repro steps:

  1. Create a new table where the first argument is a list of columns such as {"Column1", "Column2"} and complete the rest of the code
  2. Note how the first argument of the constructor appears to be incorrect according to the editor
ninmonkey commented 3 years ago

What is your environment?

Were you using an unsaved file? Sometimes I get errors, it just might be when I create a new tab, and never save it.

If I have a saved file I get no errors

Test Query

let
    miguel = #table(
        {"Column1"},
        { {1} }
    ),
    matt = Table.AddColumn(
        miguel,
        "New",
        each Number.Acos([Column1]),
        type any
    )
in
    matt

Environment

PowerQuery.vscode-powerquery@0.1.21
1.60.0-insider - efaa9c304bba61bdce8b0d39db5df94ce3a53fb6 - x64

Generated by:

🐒> code --list-extensions --show-versions | Select-String 'powerquery'
    (code --version) -join ' - '
ninmonkey commented 3 years ago

output_logging_20210901T205429_12-Power Query.log Edit: After further testing it seems like the linter gets stuck. It seems like an event isn't firing. But if you close and reopen it, then it says it's valid. I have extensions that modify inlay warnings, so it's possible it's related to that. vscode-parse-event - 2021-09-01 -- octree - 64

I am able to reproduce it consistently.

The log has a few places where exceptions are thrown

[Error - 8:56:06 PM] validateDocument err: {
    "kind": "Error",
    "error": {
        "innerError": {
            "innerError": {
                "kind": "Error",
                "error": {
                    "innerError": {
                        "invariantBroken": "nodeId doesn't have a parent",
                        "maybeDetails": {
                            "nodeId": 96
                        }
                    }
                }
            }
        }
    }
}
JDCAce commented 3 years ago

I get a similar or identical error whenever the parser (linter? I don't know the terminology) attempts to use a previously defined variable which it couldn't determine the type of. For clarity, I do not have any extensions that modify inlay warnings, to my knowledge. To fix the issue, the variable's type must be explicitly cast, using as either during its declaration (see Note 1 below) or its use. Unfortunately, it brings up two more errors, both of which are type errors as well:

image

Here's the code with no errors. I'm not sure how to create a GIF showing @ninmonkey's reproduction steps, but following the same steps does not produce any errors with this code:

image

Note 1: In this case, the unknown variable is a value inside a column. You can explicitly cast 1 as a number, but that doesn't fix the issue. I assume this is because the entire column needs to be cast, and as far as I'm aware that cannot be done using as anywhere in #table; instead, Table.TransformColumnTypes() would be needed.

JDCAce commented 3 years ago

To state something I simply implied in my last post, this is not an error with the #table parsing/linting. The problem arises with the usage of that table (or values in that table). The most reliable way of determining where the error occurs is the following:

Hovering the mouse cursor over each value being assigned (i.e., those to the left of any assignment operators =) will reveal a normal pop-up box for every assigned variable before the expression containing the error. Hovering over the variable containing the error will be the first variable that does not produce a hover popup box. In the example above, the first assigned variable that doesn't produce the popup is matt because the error occurs in the right-hand side of that assignment expression. This process may be used even before any error has been reported by VS Code (i.e., when there are no red squiggles). This indicates the error is there before @ninmonkey's comma-3 trick is employed. (Coincidentally, I use the same method of putting a digit after a comma when I want to force the red squiggles to appear.)

PS – I apologize for what I assume must be confusing terminology in my posts. I largely taught myself Power Query and M, and I haven't learned all the proper terminology, so I use terms borrowed from different technologies that I know.

ninmonkey commented 3 years ago

TL;DR

Parsing / linting

That's a good point, I thought some of my tests were errors, but some were type warnings. It should split the severity like:

Type Severity
powerquery(Error.Parse) Error
powerquery(Error.InvokeArgumentTypeMismatch) Warning

Like optional nullable parameters should show warning, verses the error severity, like this: image

Type Errors

problem arises with the usage of that table... pop up blank

At first I thought #table() specifically isn't detecting the return type, so coercing fixed it. (The other table constructors are working), so I added it to #86 . But TIL the specs say #table returns type any. So that part was operating as expected.

terminology

parser/linter: I've been kinda loose with that terminology. There is a PQ Parser, PQ Language Service, and this extension.

For data types, the language itself, only has primitive types. That's any of these:

any, binary, date, datetime, datetimezone, duration, function, list, logical, none, null, number, record, table, text, time, type

Then there's ascribed types, like Int64.Type or Decimal.Type, which add metadata to values that functions can use. Both are type number. Try Table.Schema( someTable ) and check out the columns Kind and TypeName

How to type line 10

Line 10 error I haven't figured out completely yet.

Here's how I'd add typing. I omitted as table on purpose.

let
    miguel = #table(
        type table[Num = Int64.Type],
        { {13} }
    ),
    /*
    this next step is mostly redundant -- because I declared a table type (ie: schema)
    But -- TransformColumnTypes actually transforms the data, rather than ascribing the type
    It uses the the current locale (or set it as the last arg) 
    for converting text to numbers/dates/times/etc 
    */    
    miguel_typed  = Table.TransformColumnTypes(
        miguel, { {"Num", Int64.Type} }
    ),
    FinalTable = Table.AddColumn(
        miguel_typed,
        "New",        
        (row as record) as number =>
            Number.Acos( row[Num] ),
            Int64.Type
    )
in
    FinalTable

I never use each with a Table.AddColumn, because each is synatic sugar to declare a function that returns type any. But the column's type uses that function's return type, to set the column type. That, plus the 4th argument.

each [Num] + 10
// evaluates as
(row as any) as any => row[Num] + 10

This last part might be hard for statically detecting types image

Resources on Power Query

Note: Power Query and M are names for the same language.

JordanBoltonMN commented 3 years ago

There's several compounding issues with this problem.

  1. There was a bug where function arguments weren't being validated correctly for null literals. That should be fixed as of today.
  2. The metadata that drives function helpers reports constructors (eg. #table) as returning the any type.
  3. Specialized function stubs need to be written in order for functions like Table.AddColumn so it knows the first argument will get piped into the function of the third argument (note: each BLAH is short hand for creating a special type of function)
  4. I discovered an issue with static analysis where certain expressions like field-projection-expression will throw an error if inside an each-expression. I've been working on that issue some today and hopefully will get time to finish it by end of day tomorrow.
migueesc123 commented 3 years ago

gj @JordanBoltonMN ! my scenario works now :)

I'm not entirely sure what the rest of the replies mean if i'm being honest with you, so i'm unsure if there are other hidden bugs identified by @JDCAce or @ninmonkey, so i'll leave this issue open and let you be the judge if you wish to close it as I no longer have the issues statement in my initial post.

Thx again and keep up the good work!

JordanBoltonMN commented 3 years ago

Glad it's working for you. I'll be closing this issue as I created a bug in another repo/layer - https://github.com/microsoft/powerquery-language-services/issues/131