Skip to content

Commit

Permalink
Default methods are not abstract
Browse files Browse the repository at this point in the history
  • Loading branch information
oowekyala committed Jan 5, 2019
1 parent 45c8f50 commit dc7e023
Show file tree
Hide file tree
Showing 5 changed files with 198 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,13 @@ public boolean isPublic() {
* Returns true if this method is abstract, so doesn't
* declare a body. Interface members are
* implicitly abstract, whether they declare the
* {@code abstract} modifier or not.
* {@code abstract} modifier or not. Default interface
* methods are not abstract though, consistently with the
* standard reflection API.
*/
@Override
public boolean isAbstract() {
return isInterfaceMember() || super.isAbstract();
return isInterfaceMember() && !isDefault() || super.isAbstract();
}


Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package net.sourceforge.pmd.lang.java.ast

import io.kotlintest.should
import io.kotlintest.specs.FunSpec
import net.sourceforge.pmd.lang.ast.test.shouldBe
import net.sourceforge.pmd.lang.java.ast.JavaVersion.Companion.Earliest
import net.sourceforge.pmd.lang.java.ast.JavaVersion.Companion.Latest
import net.sourceforge.pmd.lang.java.ast.JavaVersion.J1_8
import net.sourceforge.pmd.lang.java.ast.JavaVersion.J9

class ASTMethodDeclarationTest : FunSpec({

// notes about dsl:
// * testGroup generates one test per "should" assertion that
// uses a node matcher, which is nice to know which one failed
// (without explicitly giving them each a specific name)
// * the it::isPublic syntax allows including the property name in the error message in case of failure

testGroup("Non-private interfaces members should be public", javaVersions = Earliest..Latest) {

genClassHeader = "interface Bar"

"int foo();" should matchDeclaration<ASTMethodDeclaration>(ignoreChildren = true) {
it::isPublic shouldBe true
it::isPrivate shouldBe false
it::isSyntacticallyPublic shouldBe false
}

"public int kk();" should matchDeclaration<ASTMethodDeclaration>(ignoreChildren = true) {
it::isPublic shouldBe true
it::isPrivate shouldBe false
it::isSyntacticallyPublic shouldBe true
}

"int FOO = 0;" should matchDeclaration<ASTFieldDeclaration>(ignoreChildren = true) {
it::isPublic shouldBe true
it::isPrivate shouldBe false
it::isSyntacticallyPublic shouldBe false
}

"public int FOO = 0;" should matchDeclaration<ASTFieldDeclaration>(ignoreChildren = true) {
it::isPublic shouldBe true
it::isPrivate shouldBe false
it::isSyntacticallyPublic shouldBe true
}
}

parserTest("Private methods in interface should be private", J9..Latest) {

"private int de() { return 1; }" should matchDeclaration<ASTMethodDeclaration>(ignoreChildren = true) {
it::isPublic shouldBe false
it::isPrivate shouldBe true
it::isSyntacticallyPublic shouldBe false
}

}

testGroup("Non-default methods in interfaces should be abstract", javaVersions = J1_8..Latest) {

genClassHeader = "interface Bar"

"int bar();" should matchDeclaration<ASTMethodDeclaration>(ignoreChildren = true) {
it::isDefault shouldBe false
it::isAbstract shouldBe true
}

"abstract int bar();" should matchDeclaration<ASTMethodDeclaration>(ignoreChildren = true) {
it::isDefault shouldBe false
it::isAbstract shouldBe true
}

}

parserTest("Default methods in interfaces should not be abstract", javaVersions = J1_8..Latest) {

genClassHeader = "interface Bar"

"default int kar() { return 1; } " should matchDeclaration<ASTMethodDeclaration>(ignoreChildren = true) {
it::isDefault shouldBe true
it::isAbstract shouldBe false
}

// default abstract is an invalid combination of modifiers so we won't encounter it in real analysis
}

})
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import net.sourceforge.pmd.lang.ast.test.*
import net.sourceforge.pmd.lang.java.ParserTstUtil
import io.kotlintest.should as kotlintestShould


/**
* Represents the different Java language versions.
*/
Expand Down Expand Up @@ -129,7 +128,7 @@ fun AbstractFunSpec.testGroup(name: String,
class GroupTestCtx(private val funspec: AbstractFunSpec, private val groupName: String, javaVersion: JavaVersion) : ParserTestCtx(javaVersion) {

infix fun String.should(matcher: Matcher<String>) {
funspec.parserTest("$groupName: '$this'") {
funspec.parserTest("$groupName: '$this'", javaVersion = javaVersion) {
this@should kotlintestShould matcher
}
}
Expand Down Expand Up @@ -173,10 +172,12 @@ class GroupTestCtx(private val funspec: AbstractFunSpec, private val groupName:
* @property javaVersion The java version that will be used for parsing.
* @property importedTypes Types to import at the beginning of parsing contexts
* @property otherImports Other imports, without the `import` and semicolon
* @property genClassHeader Header of the enclosing class used in parsing contexts like parseExpression, etc. E.g. "class Foo"
*/
open class ParserTestCtx(val javaVersion: JavaVersion = JavaVersion.Latest,
val importedTypes: MutableList<Class<*>> = mutableListOf(),
val otherImports: MutableList<String> = mutableListOf()) {
val otherImports: MutableList<String> = mutableListOf(),
var genClassHeader: String = "class Foo") {

/** Imports to add to the top of the parsing contexts. */
internal val imports: List<String>
Expand Down Expand Up @@ -218,6 +219,25 @@ open class ParserTestCtx(val javaVersion: JavaVersion = JavaVersion.Latest,
noinline nodeSpec: NWrapper<N>.() -> Unit) =
makeMatcher(TypeParsingCtx(this), ignoreChildren, nodeSpec)

/**
* Returns a String matcher that parses the node using [parseToplevelDeclaration] with
* type param [N], then matches it against the [nodeSpec] using [matchNode].
*/
inline fun <reified N : ASTAnyTypeDeclaration> matchToplevelType(
ignoreChildren: Boolean = false,
noinline nodeSpec: NWrapper<N>.() -> Unit) =
makeMatcher(TopLevelTypeDeclarationParsingCtx(this), ignoreChildren, nodeSpec)

/**
* Returns a String matcher that parses the node using [parseBodyDeclaration] with
* type param [N], then matches it against the [nodeSpec] using [matchNode].
*
* Note that the enclosing type declaration can be customized by changing [genClassHeader].
*/
inline fun <reified N : Node> matchDeclaration(
ignoreChildren: Boolean = false,
noinline nodeSpec: NWrapper<N>.() -> Unit) = makeMatcher(EnclosedDeclarationParsingCtx(this), ignoreChildren, nodeSpec)


/**
* Expect a parse exception to be thrown by [block].
Expand All @@ -239,6 +259,11 @@ open class ParserTestCtx(val javaVersion: JavaVersion = JavaVersion.Latest,

fun parseAstType(type: String): ASTType = TypeParsingCtx(this).parseNode(type)

fun parseToplevelAnyTypeDeclaration(type: String): ASTAnyTypeDeclaration = TopLevelTypeDeclarationParsingCtx(this).parseNode(type)

fun parseBodyDeclaration(type: String): ASTAnyTypeBodyDeclaration = EnclosedDeclarationParsingCtx(this).parseNode(type)

// reified shorthands, fetching the node

inline fun <reified N : Node> parseExpression(expr: String): N = ExpressionParsingCtx(this).parseAndFind(expr)

Expand All @@ -247,6 +272,9 @@ open class ParserTestCtx(val javaVersion: JavaVersion = JavaVersion.Latest,

inline fun <reified N : Node> parseType(type: String): N = TypeParsingCtx(this).parseAndFind(type)

inline fun <reified N : Node> parseToplevelDeclaration(decl: String): N = TopLevelTypeDeclarationParsingCtx(this).parseAndFind(decl)

inline fun <reified N : Node> parseDeclaration(decl: String): N = EnclosedDeclarationParsingCtx(this).parseAndFind(decl)

companion object {

Expand All @@ -261,11 +289,7 @@ open class ParserTestCtx(val javaVersion: JavaVersion = JavaVersion.Latest,
*/
fun <N : Node> Node.findFirstNodeOnStraightLine(klass: Class<N>): N? {
return when {
klass.isInstance(this) -> {
@Suppress("UNCHECKED_CAST")
val n = this as N
n
}
klass.isInstance(this) -> klass.cast(this)
this.numChildren == 1 -> getChild(0).findFirstNodeOnStraightLine(klass)
else -> null
}
Expand Down Expand Up @@ -326,7 +350,7 @@ open class ParserTestCtx(val javaVersion: JavaVersion = JavaVersion.Latest,
override fun getTemplate(construct: String): String =
"""
${ctx.imports.joinToString(separator = "\n")}
class Foo {
${ctx.genClassHeader} {
{
Object o = $construct;
}
Expand All @@ -342,7 +366,7 @@ open class ParserTestCtx(val javaVersion: JavaVersion = JavaVersion.Latest,
override fun getTemplate(construct: String): String =
"""
${ctx.imports.joinToString(separator = "\n")}
class Foo {
${ctx.genClassHeader} {
{
$construct
}
Expand All @@ -353,11 +377,34 @@ open class ParserTestCtx(val javaVersion: JavaVersion = JavaVersion.Latest,
override fun retrieveNode(acu: ASTCompilationUnit): ASTBlockStatement = acu.getFirstDescendantOfType(ASTBlockStatement::class.java)
}

class EnclosedDeclarationParsingCtx(ctx: ParserTestCtx) : NodeParsingCtx<ASTAnyTypeBodyDeclaration>("enclosed declaration", ctx) {

override fun getTemplate(construct: String): String = """
${ctx.imports.joinToString(separator = "\n")}
${ctx.genClassHeader} {
$construct
}
""".trimIndent()

override fun retrieveNode(acu: ASTCompilationUnit): ASTAnyTypeBodyDeclaration =
acu.getFirstDescendantOfType(ASTAnyTypeBodyDeclaration::class.java)!!
}

class TopLevelTypeDeclarationParsingCtx(ctx: ParserTestCtx) : NodeParsingCtx<ASTAnyTypeDeclaration>("top-level declaration", ctx) {

override fun getTemplate(construct: String): String = """
${ctx.imports.joinToString(separator = "\n")}
$construct
""".trimIndent()

override fun retrieveNode(acu: ASTCompilationUnit): ASTAnyTypeDeclaration = acu.getFirstDescendantOfType(ASTAnyTypeDeclaration::class.java)!!
}

class TypeParsingCtx(ctx: ParserTestCtx) : NodeParsingCtx<ASTType>("type", ctx) {
override fun getTemplate(construct: String): String =
"""
${ctx.imports.joinToString(separator = "\n")}
class Foo {
${ctx.genClassHeader} {
$construct foo;
}
""".trimIndent()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package net.sourceforge.pmd.lang.ast.test

import io.kotlintest.Matcher
import kotlin.reflect.KCallable
import io.kotlintest.shouldBe as ktShouldBe

/**
* Extension to add the name of a property to error messages.
*
* @see [shouldBe].
*/
infix fun <N, V : N> KCallable<N>.shouldEqual(expected: V?) =
assertWrapper(this, expected) { n, v -> n ktShouldBe v }

private fun <N, V> assertWrapper(callable: KCallable<N>, right: V, asserter: (N, V) -> Unit) {

fun formatName() = "::" + callable.name.removePrefix("get").decapitalize()

val value: N = try {
callable.call()
} catch (e: Exception) {
throw RuntimeException("Couldn't fetch value for property ${formatName()}", e)
}

try {
asserter(value, right)
} catch (e: AssertionError) {

if (e.message?.contains("expected:") == true) {
// the exception has no path, let's add one
throw AssertionError(e.message!!.replace("expected:", "expected property ${formatName()} to be"))
}

throw e
}
}

/**
* Extension to add the name of the property to error messages.
* Use with double colon syntax, eg `it::isIntegerLiteral shouldBe true`.
* For properties synthesized from Java getters starting with "get", you
* have to use the name of the getter instead of that of the generated
* property (with the get prefix).
*
* If this conflicts with [io.kotlintest.shouldBe], use the equivalent [shouldEqual]
*
*/
infix fun <N, V : N> KCallable<N>.shouldBe(expected: V?) = this.shouldEqual(expected)

infix fun <T> KCallable<T>.shouldBe(expected: Matcher<T>) = assertWrapper(this, expected) { n, v -> n ktShouldBe v }

0 comments on commit dc7e023

Please sign in to comment.