Skip to content

Commit 868f3d8

Browse files
authored
Merge pull request #281 from exoego/fix-196
Fix #196: Do not instrument default parameter for Scala.js
2 parents d0c89c3 + 17df646 commit 868f3d8

File tree

2 files changed

+117
-0
lines changed

2 files changed

+117
-0
lines changed

scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,14 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt
107107
private var options: ScoverageOptions = new ScoverageOptions()
108108
private var coverageFilter: CoverageFilter = AllCoverageFilter
109109

110+
private val isScalaJsEnabled: Boolean = {
111+
try {
112+
rootMirror.getClassIfDefined("scala.scalajs.js.Any") != NoSymbol
113+
} catch {
114+
case _: Throwable => false
115+
}
116+
}
117+
110118
def setOptions(options: ScoverageOptions): Unit = {
111119
this.options = options
112120
coverageFilter = new RegexCoverageFilter(options.excludedPackages, options.excludedFiles, options.excludedSymbols)
@@ -215,6 +223,10 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt
215223
if (tree.pos.isDefined && !isStatementIncluded(tree.pos)) {
216224
coverage.add(statement.copy(ignored = true))
217225
tree
226+
} else if (isUndefinedParameterInScalaJs(tree.symbol)) {
227+
coverage.add(statement.copy(ignored = true))
228+
statementIds.decrementAndGet()
229+
tree
218230
} else {
219231
coverage.add(statement)
220232

@@ -225,6 +237,86 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt
225237
}
226238
}
227239

240+
// Copied from
241+
// https://github.com/scala-js/scala-js/blob/4619d906baef7feb5d0b6d555d5b33044669434e/compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala#L2696-L2721
242+
private def isJSDefaultParam(sym: Symbol): Boolean = {
243+
if (isCtorDefaultParam(sym)) {
244+
isJSCtorDefaultParam(sym)
245+
} else {
246+
sym.hasFlag(reflect.internal.Flags.DEFAULTPARAM) &&
247+
isJSType(sym.owner) && {
248+
/* If this is a default parameter accessor on a
249+
* non-native JS class, we need to know if the method for which we
250+
* are the default parameter is exposed or not.
251+
* We do this by removing the $default suffix from the method name,
252+
* and looking up a member with that name in the owner.
253+
* Note that this does not work for local methods. But local methods
254+
* are never exposed.
255+
* Further note that overloads are easy, because either all or none
256+
* of them are exposed.
257+
*/
258+
def isAttachedMethodExposed = {
259+
val methodName = nme.defaultGetterToMethod(sym.name)
260+
val ownerMethod = sym.owner.info.decl(methodName)
261+
ownerMethod.filter(isExposed).exists
262+
}
263+
264+
!isNonNativeJSClass(sym.owner) || isAttachedMethodExposed
265+
}
266+
}
267+
}
268+
269+
private lazy val JSTypeAnnot = rootMirror.getRequiredClass("scala.scalajs.js.annotation.internal.JSType")
270+
private lazy val ExposedJSMemberAnnot = rootMirror.getRequiredClass("scala.scalajs.js.annotation.internal.ExposedJSMember")
271+
private lazy val JSNativeAnnotation = rootMirror.getRequiredClass("scala.scalajs.js.native")
272+
273+
private def isJSType(sym: Symbol): Boolean =
274+
sym.hasAnnotation(JSTypeAnnot)
275+
276+
def isNonNativeJSClass(sym: Symbol): Boolean =
277+
!sym.isTrait && isJSType(sym) && !sym.hasAnnotation(JSNativeAnnotation)
278+
279+
private def isExposed(sym: Symbol): Boolean = {
280+
!sym.isBridge && {
281+
if (sym.isLazy)
282+
sym.isAccessor && sym.accessed.hasAnnotation(ExposedJSMemberAnnot)
283+
else
284+
sym.hasAnnotation(ExposedJSMemberAnnot)
285+
}
286+
}
287+
288+
private def isJSCtorDefaultParam(sym: Symbol) = {
289+
isCtorDefaultParam(sym) &&
290+
isJSType(patchedLinkedClassOfClass(sym.owner))
291+
}
292+
293+
private def patchedLinkedClassOfClass(sym: Symbol): Symbol = {
294+
/* Work around a bug of scalac with linkedClassOfClass where package
295+
* objects are involved (the companion class would somehow exist twice
296+
* in the scope, making an assertion fail in Symbol.suchThat).
297+
* Basically this inlines linkedClassOfClass up to companionClass,
298+
* then replaces the `suchThat` by a `filter` and `head`.
299+
*/
300+
val flatOwnerInfo = {
301+
// inline Symbol.flatOwnerInfo because it is protected
302+
if (sym.needsFlatClasses)
303+
sym.info
304+
sym.owner.rawInfo
305+
}
306+
val result = flatOwnerInfo.decl(sym.name).filter(_ isCoDefinedWith sym)
307+
if (!result.isOverloaded) result
308+
else result.alternatives.head
309+
}
310+
311+
private def isCtorDefaultParam(sym: Symbol) = {
312+
sym.hasFlag(reflect.internal.Flags.DEFAULTPARAM) &&
313+
sym.owner.isModuleClass &&
314+
nme.defaultGetterToMethod(sym.name) == nme.CONSTRUCTOR
315+
}
316+
317+
def isUndefinedParameterInScalaJs(sym: Symbol): Boolean = {
318+
isScalaJsEnabled && sym != null && isJSDefaultParam(sym)
319+
}
228320
def isClassIncluded(symbol: Symbol): Boolean = coverageFilter.isClassIncluded(symbol.fullNameString)
229321
def isFileIncluded(source: SourceFile): Boolean = coverageFilter.isFileIncluded(source)
230322
def isStatementIncluded(pos: Position): Boolean = coverageFilter.isLineIncluded(pos)
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package scoverage
2+
3+
import org.scalatest.{ BeforeAndAfterEachTestData, FunSuite, OneInstancePerTest }
4+
5+
/**
6+
* https://github.com/scoverage/scalac-scoverage-plugin/issues/196
7+
*/
8+
class PluginCoverageScalaJsTest
9+
extends FunSuite
10+
with OneInstancePerTest
11+
with BeforeAndAfterEachTestData
12+
with MacroSupport {
13+
14+
test("scoverage should ignore default undefined parameter") {
15+
val compiler = ScoverageCompiler.default
16+
compiler.compileCodeSnippet(
17+
"""import scala.scalajs.js
18+
|
19+
|object JSONHelper {
20+
| def toJson(value: String): String = js.JSON.stringify(value)
21+
|}""".stripMargin)
22+
assert(!compiler.reporter.hasErrors)
23+
compiler.assertNMeasuredStatements(4)
24+
}
25+
}

0 commit comments

Comments
 (0)