mtkopone / scct

Scala Code Coverage Tool
http://mtkopone.github.com/scct/
Apache License 2.0
129 stars 39 forks source link

Call to Position.source is unsafe #41

Open copumpkin opened 11 years ago

copumpkin commented 11 years ago

I'd normally submit a patch but can't do so easily from where I am right now, so for now I'll just report the bug.

In https://github.com/mtkopone/scct/blob/master/src/main/scala/reaktor/scct/ScctInstrumentPlugin.scala#L238, you call .source on the position. It looks like for some reason, someone decided in the scala compiler that using Option on source was too much bother (without updating the comment), so converted .source to return a value or throw an exception by default. The contract now appears to check that .isDefined is true on the position before calling .source on it. I have no clue why someone would drop compiler-guaranteed safety in favor of behavior like this that developers might miss (especially when the documentation is wrong) as indeed happened in scct.

Anyway, a simple replacement would be just to check .isDefined before calling .source, and emit "" if the source isn't defined.

If I were to guess, the reason I encountered this is due to another compiler plugin I'm using that is inserting code programmatically. Presumably, when scct traverses the generated code, it encounters the autogenerated code that has no corresponding file position, and I get a nice NotImplementedException during compilation.

For reference, the scalac commit in question is https://github.com/scala/scala/commit/db045cb8ddbd725fc545da5296bf0cdd722c20ce#L35L26