scoverage / scalac-scoverage-plugin

Scoverage Scala Code Coverage Core Libs
https://github.com/scoverage
Apache License 2.0
423 stars 126 forks source link

`Found a dangling UndefinedParam` on Scala.js #447

Closed armanbilge closed 2 years ago

armanbilge commented 2 years ago
[error] /home/runner/work/observe/observe/modules/web/client/src/main/scala/observe/web/client/handlers/WebSocketHandler.scala:40:7: Found a dangling UndefinedParam at Position(https://raw.githubusercontent.com/gemini-hlsw/observe/4c95fb1dd6e860604d1446f05b30eab0165f6d22/modules/web/client/src/main/scala/observe/web/client/handlers/WebSocketHandler.scala,104,52). This is likely due to a bad interaction between a macro or a compiler plugin and the Scala.js compiler plugin. If you hit this, please let us know.

https://github.com/gemini-hlsw/observe/runs/5059573540?check_suite_focus=true#step:7:402

Thanks for the encouraging message to open an issue! :)

armanbilge commented 2 years ago

Alas, I have run into this again in another project https://github.com/buntec/scala-js-snabbdom/pull/34#issuecomment-1140416940

ckipp01 commented 2 years ago

If you hit this, please let us know. Thanks for the encouraging message to open an issue! :)

Thanks for the report! I actually think this is emitted from ScalaJS, not scoverage. I have no idea just glancing at this, to be honest I'm not sure what a dangling UndefinedParam is.

armanbilge commented 2 years ago

See https://github.com/scoverage/scalac-scoverage-plugin/issues/196 for a similar issue, which was closed by https://github.com/scoverage/scalac-scoverage-plugin/pull/281. So probably this needs a similar sort of fix.

armanbilge commented 2 years ago

Ok, I isolated it one problematic line.

https://github.com/buntec/scala-js-snabbdom/blob/f1a3f58b934e739cce72157f01aaed1ac7d0eeac/snabbdom/src/main/scala/snabbdom/modules/Styles.scala#L88

elm.asInstanceOf[dom.HTMLElement].style.setProperty(name, cur)

Where HTMLElement comes from https://github.com/scala-js/scala-js-dom/

armanbilge commented 2 years ago

Isolated down to the setProperty call, which is defined here: https://github.com/scala-js/scala-js-dom/blob/883d3d944e5a6909551dfe256681b22eb8b33f93/dom/src/main/scala/org/scalajs/dom/CSSStyleDeclaration.scala#L183

I reckon it's the default parameter again, just like was supposedly solved by https://github.com/scoverage/scalac-scoverage-plugin/pull/281. Maybe another case?

armanbilge commented 2 years ago

Oh hang on, maybe it's a regression. The test is being ignored!

https://github.com/scoverage/scalac-scoverage-plugin/blob/1d404ca3f12d00c75820da2783158f9eee82e97c/plugin/src/test/scala/scoverage/PluginCoverageScalaJsTest.scala#L9

armanbilge commented 2 years ago

Became ignored when dropping 2.11 in https://github.com/scoverage/scalac-scoverage-plugin/commit/5d3c897bedf344ff7ca45857bc125e75fb8f0e56, idk why though.

armanbilge commented 2 years ago

I used a workaround in https://github.com/buntec/scala-js-snabbdom/pull/34 to get coverage working with JSDOM, which is cool. But would be great to re-enable this test and fix the regression!

ckipp01 commented 2 years ago

I used a workaround in buntec/scala-js-snabbdom#34 to get coverage working with JSDOM, which is cool. But would be great to re-enable this test and fix the regression!

I'm glad you found a workaround :). I'll be pretty honest, I'm a bit hands off with scoverage at the moment, and don't intend on putting much time into it, just enough to keep it alive and published, so I probably won't be diving into this. However, if anyone is interested in contributing, I'm more than happy to review.

armanbilge commented 2 years ago

@ckipp01 yeah, no worries 😁

I have half a mind to dig into this, but what would be very helpful is a pointer how to run the scoverage compiler in the test suite with Scala.js and the Scala.js stdlib on its classpath. Nevermind, I've figured this out :)

It's not really obvious to me how the test was accomplishing that before. Thanks! https://github.com/scoverage/scalac-scoverage-plugin/blob/1d404ca3f12d00c75820da2783158f9eee82e97c/plugin/src/test/scala/scoverage/PluginCoverageScalaJsTest.scala#L7-L19

cornim commented 1 year ago

I seem to be having this issue with scoverage 2.0.7.

See https://github.com/scala-js/scala-js/issues/4825 for details.