Skip to content

Found a dangling UndefinedParam on Scala.js #447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
armanbilge opened this issue Feb 3, 2022 · 11 comments · Fixed by #464 or #466
Closed

Found a dangling UndefinedParam on Scala.js #447

armanbilge opened this issue Feb 3, 2022 · 11 comments · Fixed by #464 or #466

Comments

@armanbilge
Copy link
Contributor

[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
Copy link
Contributor Author

Alas, I have run into this again in another project buntec/scala-js-snabbdom#34 (comment)

@ckipp01
Copy link
Member

ckipp01 commented May 29, 2022

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
Copy link
Contributor Author

See #196 for a similar issue, which was closed by #281. So probably this needs a similar sort of fix.

@armanbilge
Copy link
Contributor Author

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
Copy link
Contributor Author

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 #281. Maybe another case?

@armanbilge
Copy link
Contributor Author

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

test("scoverage should ignore default undefined parameter".ignore) {

@armanbilge
Copy link
Contributor Author

Became ignored when dropping 2.11 in 5d3c897, idk why though.

@armanbilge
Copy link
Contributor Author

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!

@ckipp01
Copy link
Member

ckipp01 commented May 30, 2022

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
Copy link
Contributor Author

armanbilge commented May 30, 2022

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

class PluginCoverageScalaJsTest extends FunSuite with MacroSupport {
test("scoverage should ignore default undefined parameter".ignore) {
val compiler = ScoverageCompiler.default
compiler.compileCodeSnippet(
"""import scala.scalajs.js
|
|object JSONHelper {
| def toJson(value: String): String = js.JSON.stringify(value)
|}""".stripMargin
)
assert(!compiler.reporter.hasErrors)
compiler.assertNMeasuredStatements(4)

@cornim
Copy link

cornim commented Mar 7, 2023

I seem to be having this issue with scoverage 2.0.7.

See scala-js/scala-js#4825 for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants