From 87f70527b2428ec50f9c013f7963aa085367d44b Mon Sep 17 00:00:00 2001 From: exoego Date: Tue, 8 Oct 2019 10:31:26 +0900 Subject: [PATCH 1/2] Fix 196 --- .../src/main/scala/scoverage/plugin.scala | 17 +++++++++++++ .../scoverage/PluginCoverageScalaJsTest.scala | 25 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 scalac-scoverage-plugin/src/test/scala/scoverage/PluginCoverageScalaJsTest.scala diff --git a/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala b/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala index 1aa04d13..43649868 100644 --- a/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala +++ b/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala @@ -107,6 +107,14 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt private var options: ScoverageOptions = new ScoverageOptions() private var coverageFilter: CoverageFilter = AllCoverageFilter + private val isScalaJsEnabled: Boolean = { + try { + rootMirror.getClassIfDefined("scala.scalajs.js.Any") != NoSymbol + } catch { + case _: Throwable => false + } + } + def setOptions(options: ScoverageOptions): Unit = { this.options = options coverageFilter = new RegexCoverageFilter(options.excludedPackages, options.excludedFiles, options.excludedSymbols) @@ -215,6 +223,10 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt if (tree.pos.isDefined && !isStatementIncluded(tree.pos)) { coverage.add(statement.copy(ignored = true)) tree + } else if (isUndefinedParameterInScalaJs(tree.symbol)) { + coverage.add(statement.copy(ignored = true)) + statementIds.decrementAndGet() + tree } else { coverage.add(statement) @@ -225,6 +237,11 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt } } + def isUndefinedParameterInScalaJs(symbol: Symbol): Boolean = { + isScalaJsEnabled && symbol != null && symbol.isSynthetic && symbol.isMethod && + symbol.nameString.contains("$default$") && + symbol.tpe.resultType.annotations.exists(_.symbol.nameString == "uncheckedVariance") + } def isClassIncluded(symbol: Symbol): Boolean = coverageFilter.isClassIncluded(symbol.fullNameString) def isFileIncluded(source: SourceFile): Boolean = coverageFilter.isFileIncluded(source) def isStatementIncluded(pos: Position): Boolean = coverageFilter.isLineIncluded(pos) diff --git a/scalac-scoverage-plugin/src/test/scala/scoverage/PluginCoverageScalaJsTest.scala b/scalac-scoverage-plugin/src/test/scala/scoverage/PluginCoverageScalaJsTest.scala new file mode 100644 index 00000000..64c64215 --- /dev/null +++ b/scalac-scoverage-plugin/src/test/scala/scoverage/PluginCoverageScalaJsTest.scala @@ -0,0 +1,25 @@ +package scoverage + +import org.scalatest.{ BeforeAndAfterEachTestData, FunSuite, OneInstancePerTest } + +/** + * https://github.com/scoverage/scalac-scoverage-plugin/issues/196 + */ +class PluginCoverageScalaJsTest + extends FunSuite + with OneInstancePerTest + with BeforeAndAfterEachTestData + with MacroSupport { + + test("scoverage should ignore default undefined parameter") { + 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) + } +} From 17df646266059ad7d44e35317bd2acb1727d5dd7 Mon Sep 17 00:00:00 2001 From: exoego Date: Tue, 17 Mar 2020 21:06:19 +0900 Subject: [PATCH 2/2] Use the same logic how scala.js compiler identify default parameter --- .../src/main/scala/scoverage/plugin.scala | 83 ++++++++++++++++++- 1 file changed, 79 insertions(+), 4 deletions(-) diff --git a/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala b/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala index 43649868..9d97b4da 100644 --- a/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala +++ b/scalac-scoverage-plugin/src/main/scala/scoverage/plugin.scala @@ -237,10 +237,85 @@ class ScoverageInstrumentationComponent(val global: Global, extraAfterPhase: Opt } } - def isUndefinedParameterInScalaJs(symbol: Symbol): Boolean = { - isScalaJsEnabled && symbol != null && symbol.isSynthetic && symbol.isMethod && - symbol.nameString.contains("$default$") && - symbol.tpe.resultType.annotations.exists(_.symbol.nameString == "uncheckedVariance") + // Copied from + // https://github.com/scala-js/scala-js/blob/4619d906baef7feb5d0b6d555d5b33044669434e/compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala#L2696-L2721 + private def isJSDefaultParam(sym: Symbol): Boolean = { + if (isCtorDefaultParam(sym)) { + isJSCtorDefaultParam(sym) + } else { + sym.hasFlag(reflect.internal.Flags.DEFAULTPARAM) && + isJSType(sym.owner) && { + /* If this is a default parameter accessor on a + * non-native JS class, we need to know if the method for which we + * are the default parameter is exposed or not. + * We do this by removing the $default suffix from the method name, + * and looking up a member with that name in the owner. + * Note that this does not work for local methods. But local methods + * are never exposed. + * Further note that overloads are easy, because either all or none + * of them are exposed. + */ + def isAttachedMethodExposed = { + val methodName = nme.defaultGetterToMethod(sym.name) + val ownerMethod = sym.owner.info.decl(methodName) + ownerMethod.filter(isExposed).exists + } + + !isNonNativeJSClass(sym.owner) || isAttachedMethodExposed + } + } + } + + private lazy val JSTypeAnnot = rootMirror.getRequiredClass("scala.scalajs.js.annotation.internal.JSType") + private lazy val ExposedJSMemberAnnot = rootMirror.getRequiredClass("scala.scalajs.js.annotation.internal.ExposedJSMember") + private lazy val JSNativeAnnotation = rootMirror.getRequiredClass("scala.scalajs.js.native") + + private def isJSType(sym: Symbol): Boolean = + sym.hasAnnotation(JSTypeAnnot) + + def isNonNativeJSClass(sym: Symbol): Boolean = + !sym.isTrait && isJSType(sym) && !sym.hasAnnotation(JSNativeAnnotation) + + private def isExposed(sym: Symbol): Boolean = { + !sym.isBridge && { + if (sym.isLazy) + sym.isAccessor && sym.accessed.hasAnnotation(ExposedJSMemberAnnot) + else + sym.hasAnnotation(ExposedJSMemberAnnot) + } + } + + private def isJSCtorDefaultParam(sym: Symbol) = { + isCtorDefaultParam(sym) && + isJSType(patchedLinkedClassOfClass(sym.owner)) + } + + private def patchedLinkedClassOfClass(sym: Symbol): Symbol = { + /* Work around a bug of scalac with linkedClassOfClass where package + * objects are involved (the companion class would somehow exist twice + * in the scope, making an assertion fail in Symbol.suchThat). + * Basically this inlines linkedClassOfClass up to companionClass, + * then replaces the `suchThat` by a `filter` and `head`. + */ + val flatOwnerInfo = { + // inline Symbol.flatOwnerInfo because it is protected + if (sym.needsFlatClasses) + sym.info + sym.owner.rawInfo + } + val result = flatOwnerInfo.decl(sym.name).filter(_ isCoDefinedWith sym) + if (!result.isOverloaded) result + else result.alternatives.head + } + + private def isCtorDefaultParam(sym: Symbol) = { + sym.hasFlag(reflect.internal.Flags.DEFAULTPARAM) && + sym.owner.isModuleClass && + nme.defaultGetterToMethod(sym.name) == nme.CONSTRUCTOR + } + + def isUndefinedParameterInScalaJs(sym: Symbol): Boolean = { + isScalaJsEnabled && sym != null && isJSDefaultParam(sym) } def isClassIncluded(symbol: Symbol): Boolean = coverageFilter.isClassIncluded(symbol.fullNameString) def isFileIncluded(source: SourceFile): Boolean = coverageFilter.isFileIncluded(source)