Regression? - Trailing comment causes parsing to die

[bgribaudo]

Per https://github.com/microsoft/vscode-powerquery-sdk/issues/194#issuecomment-1351394978, the fix for #194 was to ship in the 2.112.x version of the SdkTools package.

I'm running 2.114.4 and a .query.pq file containing just //hi still crashes pqtest. :-( Interestingly, the exception message is different than before.

To reproduce, follow the repo steps given in #194.

(I don't have access to the reopen button on this project, so am raising this in a new issue instead of reopening #194.)

[1:35:44 PM]    [Info]  [Task found] c:\...\.vscode\extensions\powerquery.vscode-powerquery-sdk-0.2.2-win32-x64\.nuget\Microsoft.PowerQuery.SdkTools.2.114.4\tools\PQTest.exe run-test
                --extension "c:\...\Test\bin\AnyCPU\Debug\Test.mez"
                --queryFile "c:\...\Test\Test.query.pq"
                --prettyPrint
[1:35:44 PM]    [Error] [Task exited abnormally] pqtest run-test pid(53024) exit(3762504530) stderr: 
Unhandled Exception: System.InvalidOperationException: Unable to parse the document
   at PQTest.Common.Utilities.Parse(String mashupDocument)
   at PQTest.Common.Utilities.GetDocumentKind(String code)
   at PQTest.Common.QueryFileAnalyzer.PerformAnalysis()
   at PQTest.Common.Commands.PrepareTests(Configuration configuration)
   at PQTest.Common.Commands.<>c__DisplayClass14_0.<RunTestBattery>b__0()
   at PQTest.Common.Commands.ConfigureMashupEnvironmentAndRun(Configuration configuration, Action action)
   at PQTest.Console.Program.RunTestBatteryCommand(CommandLineApplication app, CommandLineApplication command, Action`1 invoke)
   at Microsoft.Extensions.CommandLineUtils.CommandLineApplication.Execute(String[] args)
   at PQTest.Console.Program.Main(String[] args)

[albertli-msft]

Hi @bgribaudo

Someone was cramping poor al's bandwith in the past few weeks, sorry for the delayed response.

For this case, I think we could regard it by design, b/c it failed at the place when we try to parse the .query.pq who missed one concrete starting literal token. Thus, it honestly bubbles up the message: Unable to parse the document. And that's the expected behavior.

Maybe we could regard this as a chance to enhance this error message for a better experience. Like, we could turn it into something like we missed the body of the query specified?

What do you think @mattmasson? 😀

[bgribaudo]

HI @albertli-msft,

Thank you for all the work you have put into this SDK. I use it quite frequently—thank you!!

Interestingly, the same "unable to parse the document" exception occurs when the file ends in a single-line comment even if the .query.pq file also contains a valid token.

Example:

1
// a

In the case of the comment-only file, I agree that a nicer error message could be nice. However, in the case of the above, shouldn't it parse and evaluate successfully? :-)

[albertli-msft]

Understood, I will look into it.

[bgribaudo]

Thanks, Al!

[albertli-msft]

Fixed in 2.117.*

[bgribaudo]

Hi @albertli-msft,

Hope you are doing well! Do you know when 2.117.* will be released? Thanks!

[mattmasson]

This should be resolved in the 0.3.1 release (sdktools 2.127.2).

[bgribaudo]

This appears partially resolved. :-)

Example case 2 works as expected (no longer results in an error).

However, example case 1 now returns a different error. I'll open a new issue.