search

broadinstitute / wdl4s

Scala bindings for WDL
3 stars 5 forks source link

Prevent the witness from sometimes committing perjury in cwl.CommandL… #211

Closed curoli closed 7 years ago

curoli commented 7 years ago

Prevent the witness from sometimes committing perjury in cwl.CommandLineTool, the same way it was done before for cwl.Workflow.

danbills commented 7 years ago

Please attempt a unit test that shows that this enforces CommandLineTool's class to always be "CommandLineTool."

I don't think it's possible, as it's not enforced when declared this way.

I'm looking at your loam stream repo to try to figure out what is going on.

curoli commented 7 years ago

I don't know how to make Unit tests about type safety, because I can't put stuff into unit tests that doesn't compile.

But let's see:

CommandLineTool() // compiles - good CommandLineTool(class = "Hello") // does not compile - good CommandLineTool(class = "Hello".narrow) // does not compile - good CommandLineTool(class = "CommandLineTool") // does not compile - I think that's fine CommandLineTool(class = "CommandLineTool",narrow) // does not compile - I think that's fine CommandLineTool(class = CommandLineTool.class) // compiles - good CommandLineTool(class = Workflow.class) // does not compile - good

That "CommandLineTool" and "CommandLineTool".narrow might look less than ideal, but I think it's OK, because using CommandLineTool.class is better anyway. Besides, Shapeless calls these things singletons, so by design, you're not supposed to create a second instance of it, right?

With asInstanceOf[ClassType], you can assign anything, of course, but that's by design of asInstanceOf.

The remaining problem is that the Shapeless Witness is broken. The same code sometimes works in one place but not in another for no apparent reason. No amount of clever coding or testing can make us safe until Shapeless fixes it.

danbills commented 7 years ago

closing this as fix was applied in #214