Skip to content
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.

Sign up for GitHub

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

Jump to bottom

#27 use fully-qualified class names to avoid ambiguity in reports #133

Merged
merged 1 commit into from
Dec 2, 2015
Merged

#27 use fully-qualified class names to avoid ambiguity in reports #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions scalac-scoverage-plugin/src/main/scala/scoverage/Location.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@ package scoverage

import scala.tools.nsc.Global

/**
* @param packageName the name of the enclosing package
* @param className the name of the closest enclosing class
* @param fullClassName the fully qualified name of the closest enclosing class
*/
case class Location(packageName: String,
className: String,
topLevelClass: String,
fullClassName: String,
classType: ClassType,
method: String,
sourcePath: String) extends java.io.Serializable {
val fqn = (packageName + ".").replace("<empty>.", "") + className
}
sourcePath: String) extends java.io.Serializable

object Location {

Expand All @@ -31,8 +34,10 @@ object Location {
else ClassType.Class
}

def topLevelClass(s: global.Symbol): String = {
s.enclosingTopLevelClass.nameString
def fullClassName(s: global.Symbol): String = {
// anon functions are enclosed in proper classes.
if (s.enclClass.isAnonymousFunction || s.enclClass.isAnonymousClass) fullClassName(s.owner)
else s.enclClass.fullNameString
}

def enclosingMethod(s: global.Symbol): String = {
Expand All @@ -51,7 +56,7 @@ object Location {
Location(
packageName(symbol),
className(symbol),
topLevelClass(symbol),
fullClassName(symbol),
classType(symbol),
enclosingMethod(symbol),
sourcePath(symbol))
Expand Down
10 changes: 5 additions & 5 deletions scalac-scoverage-plugin/src/main/scala/scoverage/Serializer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ object Serializer {
<classType>
{stmt.location.classType.toString}
</classType>
<topLevelClass>
{stmt.location.topLevelClass}
</topLevelClass>
<fullClassName>
{stmt.location.fullClassName}
</fullClassName>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that the XML format used here is internal to scoverage, so can be changed without BC concerns

<method>
{stmt.location.method}
</method>
Expand Down Expand Up @@ -93,7 +93,7 @@ object Serializer {
val branch = (node \ "branch").text.toBoolean
val _package = (node \ "package").text
val _class = (node \ "class").text
val topLevelClass = (node \ "topLevelClass").text
val fullClassName = (node \ "fullClassName").text
val method = (node \ "method").text
val path = (node \ "path").text
val treeName = (node \ "treeName").text
Expand All @@ -109,7 +109,7 @@ object Serializer {
case _ => ClassType.Class
}
Statement(source,
Location(_package, _class, topLevelClass, classType, method, path),
Location(_package, _class, fullClassName, classType, method, path),
id,
start,
end,
Expand Down
17 changes: 14 additions & 3 deletions scalac-scoverage-plugin/src/main/scala/scoverage/coverage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ trait PackageBuilders {

trait ClassBuilders {
def statements: Iterable[Statement]
def classes = statements.groupBy(_.location.className).map(arg => MeasuredClass(arg._1, arg._2))
def classes = statements.groupBy(_.location.fullClassName).map(arg => MeasuredClass(arg._1, arg._2))
def classCount: Int = classes.size
}

Expand All @@ -74,12 +74,23 @@ case class MeasuredMethod(name: String, statements: Iterable[Statement]) extends
override def ignoredStatements: Iterable[Statement] = Seq()
}

case class MeasuredClass(name: String, statements: Iterable[Statement])
case class MeasuredClass(fullClassName: String, statements: Iterable[Statement])
extends CoverageMetrics with MethodBuilders {

def source: String = statements.head.source
def simpleName = name.split('.').last
def loc = statements.map(_.line).max

/**
* The class name for display is the FQN minus the package,
* for example "com.a.Foo.Bar.Baz" should display as "Foo.Bar.Baz"
* and "com.a.Foo" should display as "Foo".
*
* This is used in the class lists in the package and overview pages.
*/
def displayClassName = statements.headOption.map(_.location).map { location =>
location.fullClassName.stripPrefix(location.packageName + ".")
}.getOrElse(fullClassName)

override def ignoredStatements: Iterable[Statement] = Seq()
}

Expand Down
2 changes: 1 addition & 1 deletion scalac-scoverage-plugin/src/main/scala/scoverage/report/CoberturaXmlWriter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class CoberturaXmlWriter(sourceDirectories: Seq[File], outputDir: File) extends
}

def klass(klass: MeasuredClass): Node = {
<class name={klass.name}
<class name={klass.fullClassName}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the Cobertura XML format requires here.
Is this change going to be a problem?

Do you have access to a Cobertura formatted XML report to check?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just checked it (a.b.c - package names, Foo.Boo.Moo - nested objects):

  • Maven Java project with Cobertura
<class name="a.b.c.Foo$Boo$Moo" filename="a/b/c/Foo.java" line-rate="1.0" branch-rate="1.0" complexity="1.0">
    <methods>
        <method name="hello" signature="()Ljava/lang/String;" line-rate="1.0" branch-rate="1.0" complexity="0">
            ...
        </method>
        ...
</class>

Maven Scala project with scalac-scoverage-plugin 1.1.1

<class name="Moo" filename="a/b/c/Foo.scala" line-rate="1.00" branch-rate="1.00" complexity="0">
    <methods>
        <method name="a.b.c/Moo/hello" signature="()V" line-rate="1.00" branch-rate="1.00">
            ...
        </method>
        ...
</class>

Maven Scala project with scalac-scoverage-plugin 1.2.0 with your PR applied (current master):

<class name="a.b.c.Foo.Boo" filename="a/b/c/Foo.scala" line-rate="0.00" branch-rate="0.00" complexity="0">
    <methods>
        <method name="a.b.c/Moo/hello" signature="()V" line-rate="1.00" branch-rate="1.00">
            ...
        </method>
        ...
</class>

So now is better, but not ideal. There should be dollar signs in class name. You did not change method name, but it could be fixed with class name fix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested on Java class:

package a.b.c;

public class Foo
{
  public String hello()
  {
    return "Hello Foo";
  }

  static class Boo
  {
    public String hello()
    {
      return "Hello Foo.Boo";
    }


    static class Moo
    {
      public String hello()
      {
        return "Hello Foo.Boo.Moo";
      }

    }
  }
}

and Scala class:

package a.b.c

object Foo
{
  def hello: String = "Hello Foo"

  object Boo
  {
    def hello: String = "Hello Foo.Boo"

    object Moo
    {
      def hello: String = "Hello Foo.Boo.Moo"
    }
  }
}

filename={relativeSource(klass.source).replace(File.separator, "/")}
line-rate={format(klass.statementCoverage)}
branch-rate={format(klass.branchCoverage)}
Expand Down
9 changes: 4 additions & 5 deletions scalac-scoverage-plugin/src/main/scala/scoverage/report/ScoverageHtmlWriter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import scala.xml.Node
class ScoverageHtmlWriter(sourceDirectories: Seq[File], outputDir: File) extends BaseReportWriter(sourceDirectories, outputDir) {

def this (sourceDirectory: File, outputDir: File) {
this(Seq(sourceDirectory), outputDir);
this(Seq(sourceDirectory), outputDir)
}

def write(coverage: Coverage): Unit = {
Expand Down Expand Up @@ -193,7 +193,7 @@ class ScoverageHtmlWriter(sourceDirectories: Seq[File], outputDir: File) extends
</tr>
</thead>
<tbody>
{classes.toSeq.sortBy(_.simpleName) map classRow}
{classes.toSeq.sortBy(_.fullClassName) map classRow}
</tbody>
</table>
}
Expand All @@ -217,11 +217,10 @@ class ScoverageHtmlWriter(sourceDirectories: Seq[File], outputDir: File) extends
val statement0f = Math.round(klass.statementCoveragePercent).toInt.toString
val branch0f = Math.round(klass.branchCoveragePercent).toInt.toString

val simpleClassName = klass.name.split('.').last
<tr>
<td>
<a href={filename}>
{simpleClassName}
{klass.displayClassName}
</a>
</td>
<td>
Expand Down Expand Up @@ -331,7 +330,7 @@ class ScoverageHtmlWriter(sourceDirectories: Seq[File], outputDir: File) extends
{coverage.risks(limit).map(klass =>
<tr>
<td>
{klass.simpleName}
{klass.displayClassName}
</td>
<td>
{klass.loc.toString}
Expand Down
4 changes: 2 additions & 2 deletions scalac-scoverage-plugin/src/main/scala/scoverage/report/ScoverageXmlReader.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ object ScoverageXmlReader {
val pkg = node \ "@package"
val classname = node \ "@class"
val classType = node \ "@class-type"
val topLevelClass = node \ "@top-level-class"
val fullClassName = node \ "@full-class-name"
val method = node \ "@method"
val start = node \ "@start"
val end = node \ "@end"
Expand All @@ -35,7 +35,7 @@ object ScoverageXmlReader {

val location = Location(pkg.text,
classname.text,
topLevelClass.text,
fullClassName.text,
ClassType fromString classType.text,
method.text,
source.text)
Expand Down
8 changes: 4 additions & 4 deletions scalac-scoverage-plugin/src/main/scala/scoverage/report/ScoverageXmlWriter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import scala.xml.{Node, PrettyPrinter}
class ScoverageXmlWriter(sourceDirectories: Seq[File], outputDir: File, debug: Boolean) extends BaseReportWriter(sourceDirectories, outputDir) {

def this (sourceDir: File, outputDir: File, debug: Boolean) {
this(Seq(sourceDir), outputDir, debug);
this(Seq(sourceDir), outputDir, debug)
}

def write(coverage: Coverage): Unit = {
Expand All @@ -37,7 +37,7 @@ class ScoverageXmlWriter(sourceDirectories: Seq[File], outputDir: File, debug: B
<statement package={stmt.location.packageName}
class={stmt.location.className}
class-type={stmt.location.classType.toString}
top-level-class={stmt.location.topLevelClass}
full-class-name={stmt.location.fullClassName}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that the XML format used here is internal to scoverage, so can be changed without BC concerns

source={stmt.source}
method={stmt.location.method}
start={stmt.start.toString}
Expand All @@ -54,7 +54,7 @@ class ScoverageXmlWriter(sourceDirectories: Seq[File], outputDir: File, debug: B
<statement package={stmt.location.packageName}
class={stmt.location.className}
class-type={stmt.location.classType.toString}
top-level-class={stmt.location.topLevelClass}
full-class-name={stmt.location.fullClassName}
source={stmt.source}
method={stmt.location.method}
start={stmt.start.toString}
Expand All @@ -79,7 +79,7 @@ class ScoverageXmlWriter(sourceDirectories: Seq[File], outputDir: File, debug: B
}

private def klass(klass: MeasuredClass): Node = {
<class name={klass.name}
<class name={klass.fullClassName}
filename={relativeSource(klass.source)}
statement-count={klass.statementCount.toString}
statements-invoked={klass.invokedStatementCount.toString}
Expand Down
20 changes: 10 additions & 10 deletions scalac-scoverage-plugin/src/test/scala/scoverage/CoberturaXmlWriterTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,29 @@ class CoberturaXmlWriterTest extends FunSuite with BeforeAndAfter with OneInstan

val coverage = scoverage.Coverage()
coverage
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "A", ClassType.Object, "create", canonicalPath("a.scala")),
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "com.sksamuel.scoverage.A", ClassType.Object, "create", canonicalPath("a.scala")),
1, 2, 3, 12, "", "", "", false, 3))
coverage
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "A", ClassType.Object, "create2", canonicalPath("a.scala")),
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "com.sksamuel.scoverage.A", ClassType.Object, "create2", canonicalPath("a.scala")),
2, 2, 3, 16, "", "", "", false, 3))
coverage
.add(Statement(canonicalPath("b.scala"), Location("com.sksamuel.scoverage2", "B", "B", ClassType.Object, "retrieve", canonicalPath("b.scala")),
.add(Statement(canonicalPath("b.scala"), Location("com.sksamuel.scoverage2", "B", "com.sksamuel.scoverage2.B", ClassType.Object, "retrieve", canonicalPath("b.scala")),
3, 2, 3, 21, "", "", "", false, 0))
coverage
.add(Statement(canonicalPath("b.scala"),
Location("com.sksamuel.scoverage2", "B", "B", ClassType.Object, "retrieve2", canonicalPath("b.scala")),
4, 2, 3, 9, "", "", "", false, 3))
coverage
.add(Statement(canonicalPath("c.scala"), Location("com.sksamuel.scoverage3", "C", "C", ClassType.Object, "update", canonicalPath("c.scala")),
.add(Statement(canonicalPath("c.scala"), Location("com.sksamuel.scoverage3", "C", "com.sksamuel.scoverage3.C", ClassType.Object, "update", canonicalPath("c.scala")),
5, 2, 3, 66, "", "", "", true, 3))
coverage
.add(Statement(canonicalPath("c.scala"), Location("com.sksamuel.scoverage3", "C", "C", ClassType.Object, "update2", canonicalPath("c.scala")),
.add(Statement(canonicalPath("c.scala"), Location("com.sksamuel.scoverage3", "C", "com.sksamuel.scoverage3.C", ClassType.Object, "update2", canonicalPath("c.scala")),
6, 2, 3, 6, "", "", "", true, 3))
coverage
.add(Statement(canonicalPath("d.scala"), Location("com.sksamuel.scoverage4", "D", "D", ClassType.Object, "delete", canonicalPath("d.scala")),
.add(Statement(canonicalPath("d.scala"), Location("com.sksamuel.scoverage4", "D", "com.sksamuel.scoverage4.D", ClassType.Object, "delete", canonicalPath("d.scala")),
7, 2, 3, 4, "", "", "", false, 0))
coverage
.add(Statement(canonicalPath("d.scala"), Location("com.sksamuel.scoverage4", "D", "D", ClassType.Object, "delete2", canonicalPath("d.scala")),
.add(Statement(canonicalPath("d.scala"), Location("com.sksamuel.scoverage4", "D", "com.sksamuel.scoverage4.D", ClassType.Object, "delete2", canonicalPath("d.scala")),
8, 2, 3, 14, "", "", "", false, 0))

val writer = new CoberturaXmlWriter(sourceRoot, dir)
Expand Down Expand Up @@ -87,13 +87,13 @@ class CoberturaXmlWriterTest extends FunSuite with BeforeAndAfter with OneInstan

val coverage = Coverage()
coverage
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "A", ClassType.Object, "create", canonicalPath("a.scala")),
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "com.sksamuel.scoverage.A", ClassType.Object, "create", canonicalPath("a.scala")),
1, 2, 3, 12, "", "", "", false))
coverage
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "A", ClassType.Object, "create2", canonicalPath("a.scala")),
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "com.sksamuel.scoverage.A", ClassType.Object, "create2", canonicalPath("a.scala")),
2, 2, 3, 16, "", "", "", true))
coverage
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "A", ClassType.Object, "create3", canonicalPath("a.scala")),
.add(Statement(canonicalPath("a.scala"), Location("com.sksamuel.scoverage", "A", "com.sksamuel.scoverage.A", ClassType.Object, "create3", canonicalPath("a.scala")),
3, 3, 3, 20, "", "", "", true, 1))

val writer = new CoberturaXmlWriter(sourceRoot, dir)
Expand Down
2 changes: 1 addition & 1 deletion scalac-scoverage-plugin/src/test/scala/scoverage/CoverageAggregatorTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class CoverageAggregatorTest extends FreeSpec with Matchers {
val source = canonicalPath("com/scoverage/class.scala")
val location = Location("com.scoverage.foo",
"ServiceState",
"Service",
"com.scoverage.foo.Service.ServiceState",
ClassType.Trait,
"methlab",
source)
Expand Down
Loading