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.

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

Removed overloads are missing, not incompatible #362

Merged
merged 2 commits into from
Jul 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,28 @@ private[analyze] abstract class BaseMethodChecker extends Checker[MethodInfo, Cl

protected val rules = Seq(AccessModifier, FinalModifier, AbstractModifier, JavaStatic)

protected def check(method: MethodInfo, in: Iterator[MethodInfo]): Option[Problem] = {
val meths = in.filter(method.paramsCount == _.paramsCount).toList
protected def check(method: MethodInfo, newclazz: ClassInfo, methsLookup: ClassInfo => Iterator[MethodInfo]): Option[Problem] = {
val meths = methsLookup(newclazz).filter(method.paramsCount == _.paramsCount).toList // newmeths
if (meths.isEmpty)
Some(DirectMissingMethodProblem(method))
else {
meths.find(m => m.descriptor == method.descriptor && m.signature == method.signature) match {
case Some(found) => checkRules(rules)(method, found)
case None => meths.filter(method.matchesType(_)) match {
case Nil => Some(IncompatibleMethTypeProblem(method, uniques(meths)))
case filtered @ first :: _ => filtered.find(_.tpe.resultType == method.tpe.resultType) match {
case None => Some(IncompatibleResultTypeProblem(method, first))
case None =>
val filtered = meths.filter(method.matchesType(_))
filtered.find(_.tpe.resultType == method.tpe.resultType) match {
case Some(found) => Some(IncompatibleSignatureProblem(method, found))
case None =>
val oldmethsDescriptors = methsLookup(method.owner).map(_.descriptor).toSet
if (meths.forall(newmeth => oldmethsDescriptors.contains(newmeth.descriptor)))
Some(DirectMissingMethodProblem(method))
else {
filtered match {
case Nil => Some(IncompatibleMethTypeProblem(method, uniques(meths)))
case first :: _ => Some(IncompatibleResultTypeProblem(method, first))
}
}
}
}
}
}
}
Expand All @@ -35,9 +43,9 @@ private[analyze] class ClassMethodChecker extends BaseMethodChecker {
if (method.nonAccessible)
None
else if (method.isDeferred)
super.check(method, inclazz.lookupMethods(method.bytecodeName))
super.check(method, inclazz, _.lookupMethods(method.bytecodeName))
else
super.check(method, inclazz.lookupClassMethods(method.bytecodeName))
super.check(method, inclazz, _.lookupClassMethods(method.bytecodeName))
Copy link
Contributor

Choose a reason for hiding this comment

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

The lookup function is used for old and new, I wonder if there is a case when we get a misleading error message if a method is removed and only exists in a parent trait in the new version.

Generally, maybe can you explain why a different lookup function is used if meth.isDeferred?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I can't. It's a part of the code that I don't understand and just try to preserve the existing semantics.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for your pragmatism

}
}

Expand All @@ -48,7 +56,7 @@ private[analyze] class TraitMethodChecker extends BaseMethodChecker {
else if (method.owner.hasStaticImpl(method))
checkStaticImplMethod(method, inclazz)
else
super.check(method, inclazz.lookupMethods(method.bytecodeName))
super.check(method, inclazz, _.lookupMethods(method.bytecodeName))
}

private def checkStaticImplMethod(method: MethodInfo, inclazz: ClassInfo) = {
Expand All @@ -66,7 +74,7 @@ private[analyze] class TraitMethodChecker extends BaseMethodChecker {
// otherwise we check the all concrete trait methods and report
// either that the method is missing or that no method with the
// same signature exists. Either way, we expect that a problem is reported!
val prob = super.check(method, inclazz.lookupConcreteTraitMethods(method.bytecodeName))
val prob = super.check(method, inclazz, _.lookupConcreteTraitMethods(method.bytecodeName))
assert(prob.isDefined)
prob
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
method foo(java.lang.Object)java.lang.String in class A does not have a correspondent in new version
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
final class A {
def foo(s: String): String = s
def foo(o: Object): String = o.toString
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
final class A {
def foo(s: String): String = s
}