parseCompilerLogOutput crashes when first error is graphql deprecation warning

[AlexMoutonNoble]

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 |   .......

[zth]

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.

[AlexMoutonNoble]

@zth I can make a PR if you have a preference for a fix?

[zth]

@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?

[AlexMoutonNoble]

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: @.***>

[zth]

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! 😄

[zth]

@AlexMoutonNoble did you end up having a look at this?

[AlexMoutonNoble]

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: @.***>

[aspeddro]

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)

[AlexMoutonNoble]

@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

[cristianoc]

The robust solution consists of avoiding parsing altogether. Compiler log should be a dedicated json format. Reanalyze, for example, already does that.

[AlexMoutonNoble]

@cristianoc is there an issue on the compiler for that already?

[cristianoc]

@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