search

com-lihaoyi / fastparse

Writing Fast Parsers Fast in Scala
https://com-lihaoyi.github.io/fastparse
MIT License
1.09k stars 164 forks source link

Nowarn fix for `Whitespace` macro #310

Closed He-Pin closed 4 months ago

He-Pin commented 4 months ago

Motivation: refs: https://github.com/com-lihaoyi/fastparse/issues/285 with reading: https://www.scala-lang.org/2021/01/12/configuring-and-suppressing-warnings.html

Modification:

  1. I updated some depenecies,both scala and etc
  2. because of https://github.com/com-lihaoyi/mill/issues/3167, I have to comments out the native modules locally
  3. add -Xlint:unused to reproduce the problem
  4. updated some tests when 3 is enabled.
  5. remove 2.11 related code
  6. as Scala 2.12.x now supports the nowarn , so I changed the code to use the @nowarn annotation directly.
  7. the root cause is the annotation in WhiteSpace macro

Result: I think the issue is fixed now, and as I'm using Fastparse within a Java project, so I was not knowing this issue. All tests passed locally.

He-Pin commented 4 months ago

@fanf Would you like to give this a try? I mean locally.

lihaoyi commented 4 months ago

The PR looks good to me, once you get CI green I can merge it and close out the bounty

He-Pin commented 4 months ago

@lihaoyi Would you like give it another round of run, thanks.

He-Pin commented 4 months ago

@lihaoyi The CI is happy now:)

Note: in Scala 2.12, I added two mina filters where the nowarn is the scala nowarn now.

lihaoyi commented 4 months ago

@He-Pin can we leave the original nowarn in place but deprecated? That way we can avoid adding the mima filters

He-Pin commented 4 months ago

@lihaoyi OK, I think that would be better.

He-Pin commented 4 months ago

@lihaoyi I have updated and removed the added mima.

lihaoyi commented 4 months ago

@He-Pin not sure if I'm missing something, but where is the new version of ScalacParser used in the PR?

He-Pin commented 4 months ago

@lihaoyi Because I updated the Scala 2.13.x to 2.13.14 in this PR, and with https://github.com/scala/scala/pull/10406 , the additional actions: List[CodeAction] parameter is added, so when do cross compiling with Scala 2.12 and Scala 2.13, it will raise an error override nothing without this change in the scalaparse tests.

lihaoyi commented 4 months ago

@He-Pin got it, looks good! Can send me details at haoyi.sg@gmail.com and I'll close out the bounty

He-Pin commented 4 months ago

thanks, I just implement a jsonpath (https://www.rfc-editor.org/rfc/rfc9535#name-collected-abnf-grammars) with Fastparse at work, it works great.