rescript-vscode
rescript-vscode copied to clipboard
parseCompilerLogOutput crashes when first error is graphql deprecation warning
Hi Folks. Found a case here. given a codebase using rescript-apollo, with a deprecated field warning, the parser will fail its "File " branch based on the next line not being Warning, and end up in its last case where it attempts to push a follow up line to an empty parsedDiagnostics array.
I can see some pushback on the apollo output here maybe, but would it be better to make the "File " branch more permissive, or allow a new parsed diagnostic to be created there in that non-empty-line case?
Thanks Alex
File "/Users/alexmouton/Documents/code/noble/web/src/packs/reactor/PackCapabilityRunSweepUpdateHf.res", line 45, characters 8-26:
45 | ........entDataRecordId
45 | .....
Warning 22 [preprocessor]: Field "parentDataRecordId" has been deprecated. Reason: Prefer parent_data_record
File "/Users/alexmouton/Documents/code/noble/web/src/packs/reactor/PackCapabilityRunSweepUpdateHf.res", line 42, characters 40-53:
42 | ........................................ectedInput
42 | .......
Good catch! I'll soon be working on adding a test suite for a few of these things, I'll keep you posted here so we can add this as a test case at a later point.
@zth I can make a PR if you have a preference for a fix?
@AlexMoutonNoble sorry, I blanked on this! Turns out I won't be doing a test setup just now, so if you feel like taking a stab at fixing this and PR:ing, that'd be great!
Do you want me to give some background/try and guide you to where to fix it?
I remember it being pretty clearcut but happy to hear any guidance you have that will make the PR go easier
On Thu, May 5, 2022 at 7:14 AM Gabriel Nordeborn @.***> wrote:
@AlexMoutonNoble https://github.com/AlexMoutonNoble sorry, I blanked on this! Turns out I won't be doing a test setup just now, so if you feel like taking a stab at fixing this and PR:ing, that'd be great!
Do you want me to give some background/try and guide you to where to fix it?
— Reply to this email directly, view it on GitHub https://github.com/rescript-lang/rescript-vscode/issues/386#issuecomment-1118534829, or unsubscribe https://github.com/notifications/unsubscribe-auth/APRNFBWR4LFYXEFOBIJNFTTVIPCVBANCNFSM5S2JVXZQ . You are receiving this because you were mentioned.Message ID: @.***>
Yeah, re-reading your initial details it looks like you already have a clear idea of what's needed. Feel free to ping me otherwise! 😄
@AlexMoutonNoble did you end up having a look at this?
I havnt yet but still have it in mind
On Sun, Jul 24, 2022 at 9:19 AM Gabriel Nordeborn @.***> wrote:
@AlexMoutonNoble https://github.com/AlexMoutonNoble did you end up having a look at this?
— Reply to this email directly, view it on GitHub https://github.com/rescript-lang/rescript-vscode/issues/386#issuecomment-1193350602, or unsubscribe https://github.com/notifications/unsubscribe-auth/APRNFBR2DR3O4V67CJHZEU3VVVUJ5ANCNFSM5S2JVXZQ . You are receiving this because you were mentioned.Message ID: @.***>
I got this error here is the log
cat lib/bs/.compiler.log
#Start(1660943070627)
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/fragmentsUsage/Fragments.res", lines 11-13, characters 6-7:
11 | ......Todos {
| ...TodoItem
| }
13 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/clientUsage/PromiseChaining.res", lines 17-21, characters 6-7:
17 | ......os: allTodos {
| id
| text
| completed
| }
21 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/clientUsage/ClientBasics.res", lines 18-22, characters 6-7:
18 | ......os: allTodos {
| id
| text
| completed
| }
22 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/docs/Docs.res", lines 7-11, characters 6-7:
7 | ......os: allTodos {
| id
| text
| completed
| }
11 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/fragmentsUsage/Query_Fragments.res", lines 6-9, characters 6-7:
6 | ......os: allTodos {
| # This references the TodoItem fragment definition module above!
| ...TodoItem
| }
9 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/hooksUsage/Mutation.res", lines 18-22, characters 6-7:
18 | ......os: allTodos {
| id
| completed
| text
| }
22 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/hooksUsage/Query_Lazy.res", lines 3-7, characters 6-7:
3 | ......os: allTodos {
| id
| text
| completed
| }
7 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/hooksUsage/Query_OverlySimple.res", lines 3-7, characters 4-5:
3 | ....os: allTodos {
| id
| text
| completed
| }
7 | .
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/hooksUsage/Query_SubscribeToMore.res", lines 5-9, characters 6-7:
5 | ......os: allTodos {
| id
| completed
| text
| }
9 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
File "/home/pedro/Desktop/rescript-apollo-client/EXAMPLES/src/hooksUsage/Query_Typical.res", lines 4-8, characters 6-7:
4 | ......os: allTodos {
| id
| text
| completed
| }
8 | ...
Warning 22 [preprocessor]: Field "allTodos" has been deprecated. Reason: null
#Done(1660943070848)
@zth is this a moment to write this parser a little more formally? Im unclear about what types of errors can include code snippets, for example.
https://github.com/AlexMoutonNoble/rescript-vscode/tree/alexm/first-error
The robust solution consists of avoiding parsing altogether. Compiler log should be a dedicated json format. Reanalyze, for example, already does that.
@cristianoc is there an issue on the compiler for that already?
@cristianoc is there an issue on the compiler for that already?
There's no issue yet -- can be created. Meanwhile, here's how Reanalyze does it:
- on the TS side: https://github.com/rescript-lang/rescript-vscode/blob/master/client/src/commands/code_analysis.ts#L23
- generation: https://github.com/rescript-lang/rescript-vscode/blob/master/analysis/reanalyze/src/EmitJson.ml#L6