microsoft / powerquery-parser

A parser for the Power Query / M formula language, written in TypeScript
MIT License
112 stars 26 forks source link

Multi Sections, SectionAccessExpression parser error #155

Closed UliPlabst closed 4 years ago

UliPlabst commented 4 years ago

I just tried to parse some examples from the power query section specification and it appears they cannot be parsed.

Expected behavior As sections are not excluded in specifications.md it would appear that they are supported

Actual behavior For example this code

section Section1; 
A = "Hello";      
B = A + " world";  
K = Section1!A + 2;

results in an Error: ."Error: Expected to find a semicolon <';'>, but a exclamation mark <'!'> was found instead
It only works without the section access expression.

Also this code

section Section1; 
A = "Hello";      

section Section2;
B = "HELLO";

results in error "Expected to find a identifier, but a keyword <'section'> was found instead". Only one section works, multiple sections result in this error.

To Reproduce I took the above code examples and ran the formatter with:

import { Task, DefaultSettings, ResultKind, Ast } from '@microsoft/powerquery-parser';
let parsed = Task.tryLexParse(DefaultSettings, code);

Additional context I use Version 0.1.50-5, I tried to pull the latest version but there I cannot parse any document. Just in case you're also interested in that error:

TypeError: Cannot read property 'tryRead' of undefined
    at tryParse (dir\node_modules\@microsoft\powerquery-parser\lib\task.js:25:34)
    at Object.tryLexParse (dir\node_modules\@microsoft\powerquery-parser\lib\task.js:97:24)
    at Object.<anonymous> (dir\src\formatter.ts:60:21)
    at Generator.next (<anonymous>)
    at dir\node_modules\tslib\tslib.js:110:75
    at new Promise (<anonymous>)
    at Object.__awaiter (dir\node_modules\tslib\tslib.js:106:16)
    at Object.format (dir\dist\formatter.js:49:20)
    at Object.<anonymous> (dir\src\test\debug.ts:22:1)
    at Module._compile (internal/modules/cjs/loader.js:1155:14)
JordanBoltonMN commented 4 years ago

Section Access Expression and Multiple Sections: The parser follows the official language specification, which the Syntactic grammar section (which the parser is based around) it makes no mention of multiple sections. This is why SectionAccessExpression and multiple sections don't parse.

From our experience most people don't deal with multiple sections so it hasn't been a priority to enable parsing for them. However, I don't think it would be too difficult to add that in. Once I'm back in office I'll look into it.

Can't parse any document Just to confirm, you're saying if you pull the latest version (0.1.54) then you can't parse any document? When I do a fresh repo of the latest version all of the tests work fine. Is your code snippet throwing the tryRead error? I tried using your snippet but the only issue for me was with Ast, which was moved into the Language export.

The only usage of "tryRead" is with the newly created IParserUtils file, but I don't see how that would be causing an error in the code snippet.

bgribaudo commented 4 years ago

This is an interesting one. :-) The rules in the consolidated grammar section don't allow multiple sections but the specification section on sections contains an example showing a document containing two sections (whew! that is a lot of uses of sections in one sentence).

Related: https://github.com/MicrosoftDocs/powerquery-docs/issues/16

UliPlabst commented 4 years ago

regarding Can't parse any document.
I checked out the repo again and ran the example, which worked. Then I made a folder diff between the folder between the parser folder I was using and the new clone, which yielded absolutely no source differences. I found out that the type error was caused because tsc did not produce any output in my old powerquery-parser folder. Since there was a lib directory from a previous build I did not notice and thought that the build succeeded (no errors what so ever occurred during tsc). I tried to find the difference and ended up copying all folders except the src folder from the new clone to the old one. And the folder diff showed no differences in the src dir whatsoever. Still npm run build did nothing. This problem feels really strange and I ended up just doing a totally fresh clone and building from there and it's now working. I really wonder why it wasn't working. Maybe something was corrupted in my node_modules/typescript.
regarding Section Access Expression and Multiple Sections:
One of my users asked me to support section document for my formatter so it would be nice if that makes it into the ast.

JordanBoltonMN commented 4 years ago

@UliPlabst do you know if any one else other than Curt (a fellow Microsoft employee) that has run into this issue? If it's just him then I'd say he'd rather me prioritize other work, but if it's effecting customers then I'd like to make sure this is on my short term TODO list.

UliPlabst commented 4 years ago

@JordanBoltonMN No, I don't know about anybody else that ran into this. I just encountered this issue because somebody created an issue for my m-formatter because he wanted to auto format a query that contained sections. Since the section page examples show such documents I was looking into implementing sections into my formatter.
It would be Ok for me if this is in your backlog, since I don't think a lot of people are using documents with multiple sections right now. On the other hand I think the contradiction of the examples in the section documentation to the actual spec that does not allow multiple sections should probably be adressed.

mattmasson commented 4 years ago

FWIW - none of the current Power Query experiences (including the VS SDK) generate M documents with multiple sections and/or use section access expressions.

tonyhallett commented 4 years ago

Power Bi will error with multiple sections in a document. I have added a comment through "Is this page helpful?" m-spec-sections.

Section access expressions are used in the Power Bi Desktop source code, so there are people out there that might want it !

mattmasson commented 4 years ago

Section access expressions are used in the Power Bi Desktop source code, so there are people out there that might want it !

@tonyhallett could you provide an example? I'm not aware of any place where Power Query uses section access expressions. Power Query always uses a single section/document.

tonyhallett commented 4 years ago

@mattmasson I don't have my computer to hand but off the top of my head. power bi has some built in modules generated from a string. There were section access expressions to the Table module for example. If you have access to the source code look at Modules.GetCoreModules(). There is also the DependencyVisitor class which probably deals with these dependencies.

As stated this is internal and your normal developer will not be able to use such syntax.

JordanBoltonMN commented 4 years ago

Feel free to disagree and re-open the issue, but for now I'm going to close this issue as technically multiple sections aren't in the grammar and their usage are an edge case. I'll open a new issue up specifically to add SectionAccessExpression.