Permalink
Browse files

Ticket Re #1000741 raised the question of whether we needed an open e…

…ditor to check if the MethodBinding passed to

the ScalaMethodVerifierProvider is indeed abstract (the reported NPE exception was not generating any issue, it was
simply creating lot of noise in the Eclipse Error Log view).

The answer is no, and I updated the code accordingly. Now, we check if the MethodBinding belongs to a trait only
if the MethodBinding is declared in a Scala file (we look at the file's extension to infer this). That makes much
more sense and allowed to re-enable all tests in jcompiler package.

Any exception occuring in this code is still being logger in the Error Log view, that because I believe that now
the code should work as expected (and if it doesn't, I'd prefer not to be silent so that the issue is reported and
we can look into it).

Added also one test to check that the case of a trait providing the implementation of a method declared in a Java
interface is correctly handled.

Scala entitites with a {{{ protected }}} access modifier are now exposed to JDT as {{{ public }}}. This emulates
the Scala compiler behavior. Fix #1000751.

Some Scala types (e.g., AnyRef, Any, Array, ...) with no generic signature were not correctly converted in the
ScalaMethodVerifierProvider. Fix #1000752.

Integrated Iulian's feedback (pull request #42).

Resolved Conflicts:

	org.scala-ide.sdt.core/src/scala/tools/eclipse/jcompiler/ScalaMethodVerifierProvider.scala
  • Loading branch information...
1 parent 168fecc commit beb28ca12357658d56777d59806f98e6adacab09 @dotta dotta committed Nov 16, 2011
Showing with 337 additions and 28 deletions.
  1. +64 −1 org.scala-ide.sdt.core.tests/src/scala/tools/eclipse/jcompiler/AbstractMethodVerifierTest.scala
  2. +7 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000660/ScalaTest.java
  3. +3 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000741/AFoo.scala
  4. +4 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000741/FooImpl.java
  5. +5 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000741/IFoo.java
  6. +5 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000741/TFoo.scala
  7. +3 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000751/J.java
  8. +9 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000751/Top.scala
  9. +3 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752/J.java
  10. +7 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752/Top.scala
  11. +3 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_1/J.java
  12. +7 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_1/Top.scala
  13. +57 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_2/Actor.scala
  14. +45 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_2/ActorRef.scala
  15. +3 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_2/Future.scala
  16. +7 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_2/JavaAPITestActor.java
  17. +43 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_2/UntypedActor.scala
  18. +3 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_3/J.java
  19. +9 −0 org.scala-ide.sdt.core.tests/test-workspace/jcompiler/src/t1000752_3/Top.scala
  20. +2 −4 org.scala-ide.sdt.core/src/scala/tools/eclipse/javaelements/ScalaJavaMapper.scala
  21. +48 −23 org.scala-ide.sdt.core/src/scala/tools/eclipse/jcompiler/ScalaMethodVerifierProvider.scala
@@ -50,13 +50,39 @@ object AbstractMethodVerifierTest extends TestProjectSetup("jcompiler") {
}
}
}
+
+ def verifyThat(expectedProblem: String) = new {
+ def is = new {
+ def reported = {
+ //when
+ val unit = compilationUnit(path2unit)
+
+ val owner = new WorkingCopyOwner() {
+ override def getProblemRequestor(unit: ICompilationUnit): IProblemRequestor = {
+ new IProblemRequestor {
+ override def acceptProblem(problem: IProblem) {
+ //verify
+ assertEquals(expectedProblem, problem.toString())
+ }
+ def beginReporting() {}
+ def endReporting() {}
+ def isActive(): Boolean = true
+ }
+ }
+ }
+
+ //then
+ // this will trigger the java reconciler so that the problems will be reported in the ProblemReporter
+ unit.getWorkingCopy(owner, new NullProgressMonitor)
+ }
+ }
+ }
}
private def no: VerificationMode = never()
private def one: VerificationMode = times(1)
}
-@Ignore
class AbstractMethodVerifierTest {
import AbstractMethodVerifierTest._
@@ -89,4 +115,41 @@ class AbstractMethodVerifierTest {
def javaClassExtendingAbstractScalaClassWithMixedDeferredAndConcreteMembersWithSameSignature_JavaEditorShouldNotReportErrorsForUnimplementedDeferredMethod_t1000670_2() {
whenOpening("t1000670_2/JFoo.java").verifyThat(no).errors.are.reported
}
+
+ @Test
+ def scalaMethodVerifierProvider_isNotExecutedOnJavaSources_t1000660() {
+ val expectedProblem = "Pb(400) The type ScalaTest must implement the inherited abstract method Runnable.run()"
+
+ whenOpening("t1000660/ScalaTest.java").verifyThat(expectedProblem).is.reported
+ }
+
+ @Test
+ def t1000741() {
+ whenOpening("t1000741/FooImpl.java").verifyThat(no).errors.are.reported
+ }
+
+ @Test
+ def protectedScalaEntitiesNeedToBeExposedToJDTAsPublic_t1000751() {
+ whenOpening("t1000751/J.java").verifyThat(no).errors.are.reported
+ }
+
+ @Test
+ def scalaAnyRefNeedToBeMappedIntoJavaLangObject_ToBeCorrectlyExposedToJDT_1000752() {
+ whenOpening("t1000752/J.java").verifyThat(no).errors.are.reported
+ }
+
+ @Test
+ def scalaArrayNeedToBeMappedIntoJavaArray_ToBeCorrectlyExposedToJDT_t1000752_1() {
+ whenOpening("t1000752_1/J.java").verifyThat(no).errors.are.reported
+ }
+
+ @Test
+ def simplifiedExampleFromAkkaSources_ThatWasCausingWrongErrorsToBeReportedInTheJavaEditor_t1000752_2() {
+ whenOpening("t1000752_2/JavaAPITestActor.java").verifyThat(no).errors.are.reported
+ }
+
+ @Test
+ def wideningAccessModifiersOfInherithedMethod_t1000752_3() {
+ whenOpening("t1000752_3/J.java").verifyThat(no).errors.are.reported
+ }
}
@@ -0,0 +1,7 @@
+package t1000660;
+
+public class ScalaTest implements Runnable {
+ // The Runnable interface contains one abstract method `run`. Therefore,
+ // we expect an error to be reported because we don't provide the implementation
+ // of the method.
+}
@@ -0,0 +1,3 @@
+package t1000741
+
+abstract class AFoo extends TFoo
@@ -0,0 +1,4 @@
+package t1000741;
+
+
+public class FooImpl extends AFoo implements IFoo {}
@@ -0,0 +1,5 @@
+package t1000741;
+
+public interface IFoo {
+ void foo();
+}
@@ -0,0 +1,5 @@
+package t1000741
+
+trait TFoo extends IFoo {
+ def foo() {}
+}
@@ -0,0 +1,3 @@
+package t1000751;
+
+public class J extends ATop {}
@@ -0,0 +1,9 @@
+package t1000751
+
+trait Top {
+ protected def foo
+}
+
+abstract class ATop extends Top {
+ final protected def foo {}
+}
@@ -0,0 +1,3 @@
+package t1000752;
+
+public class J extends ATop {}
@@ -0,0 +1,7 @@
+package t1000752
+
+trait Top {
+ def loggable(self: AnyRef) {}
+}
+
+abstract class ATop extends Top {}
@@ -0,0 +1,3 @@
+package t1000752_1;
+
+public class J extends ATop {}
@@ -0,0 +1,7 @@
+package t1000752_1
+
+trait Top {
+ def loggable(self: Array[String]) {}
+}
+
+abstract class ATop extends Top {}
@@ -0,0 +1,57 @@
+package t1000752_2
+
+object Actor {
+ type Receive = PartialFunction[Any, Unit]
+}
+
+trait Actor {
+ import Actor._
+
+ type Receive = Actor.Receive
+
+ trait MessageDispatcher
+
+ def loggable(self: AnyRef)(r: Receive): Receive = null
+
+ def someSelf: Some[ActorRef with ScalaActorRef] = Some(null)
+
+ def optionSelf: Option[ActorRef with ScalaActorRef] = someSelf
+
+ implicit def self = someSelf.get
+
+ @inline
+ final def sender: ActorRef = null
+
+ def receiveTimeout: Option[Long] = None
+
+ def receiveTimeout_=(timeout: Option[Long]) {}
+
+ def children: Iterable[ActorRef] = null
+
+ def dispatcher: MessageDispatcher = null
+
+ protected def receive: Receive
+
+ def preStart() {}
+
+ def postStop() {}
+
+ def preRestart(reason: Throwable, message: Option[Any]) { postStop() }
+
+ def postRestart(reason: Throwable) { preStart() }
+
+ def unhandled(message: Any) {}
+
+ def become(behavior: Receive, discardOld: Boolean = true) { }
+
+ def unbecome() { }
+
+ def watch(subject: ActorRef): ActorRef = self startsMonitoring subject
+
+ def unwatch(subject: ActorRef): ActorRef = self stopsMonitoring subject
+
+ private[this] final def apply(msg: Any) {}
+
+ private val processingBehavior = receive
+}
+
@@ -0,0 +1,45 @@
+package t1000752_2
+
+abstract class ActorRef extends Serializable {
+ scalaRef: ScalaActorRef
+
+ def name: String
+
+ def address: String
+
+ def compareTo(other: ActorRef) = this.address compareTo other.address
+
+ def tell(msg: Any): Unit = this.!(msg)
+
+ def tell(msg: Any, sender: ActorRef): Unit
+
+ def ask(message: AnyRef, timeout: Long): Future[AnyRef] = ?(message, timeout).asInstanceOf[Future[AnyRef]]
+
+ def forward(message: Any) = tell(message, null)
+
+ def suspend(): Unit
+
+ def resume(): Unit
+
+ def stop(): Unit
+
+ def isShutdown: Boolean
+
+ def startsMonitoring(subject: ActorRef): ActorRef //TODO FIXME REMOVE THIS
+
+ def stopsMonitoring(subject: ActorRef): ActorRef //TODO FIXME REMOVE THIS
+
+ override def equals(that: Any): Boolean = {
+ that.isInstanceOf[ActorRef] &&
+ that.asInstanceOf[ActorRef].address == address
+ }
+
+ override def toString = "Actor[%s]".format(address)
+}
+
+trait ScalaActorRef { ref: ActorRef
+ def !(message: Any)(implicit sender: ActorRef = null): Unit = ref.tell(message, sender)
+ def ?(message: Any)(implicit timeout: Any): Future[Any]
+ def ?(message: Any, timeout: Any)(implicit ignore: Int = 0): Future[Any] = ?(message)(timeout)
+}
+
@@ -0,0 +1,3 @@
+package t1000752_2
+
+trait Future[+T]
@@ -0,0 +1,7 @@
+package t1000752_2;
+
+public class JavaAPITestActor extends UntypedActor {
+ public void onReceive(Object msg) {
+ getSender().tell("got it!");
+ }
+}
@@ -0,0 +1,43 @@
+package t1000752_2
+
+abstract class UntypedActor extends Actor {
+ trait Procedure[+T]
+
+ @throws(classOf[Exception])
+ def onReceive(message: Any): Unit
+
+ def getSelf(): ActorRef = self
+
+ def getSender(): ActorRef = sender
+
+ def getReceiveTimeout: Option[Long] = receiveTimeout
+
+ def setReceiveTimeout(timeout: Long): Unit = receiveTimeout = Some(timeout)
+
+ def getChildren(): java.lang.Iterable[ActorRef] = {
+ null
+ }
+
+ def getDispatcher(): MessageDispatcher = dispatcher
+
+ def become(behavior: Procedure[Any]): Unit = become(behavior, false)
+
+ def become(behavior: Procedure[Any], discardOld: Boolean): Unit =
+ super.become(null, discardOld)
+
+ override def preStart() {}
+
+ override def postStop() {}
+
+ override def preRestart(reason: Throwable, lastMessage: Option[Any]) {}
+
+ override def postRestart(reason: Throwable) {}
+
+ override def unhandled(msg: Any) {
+ throw new Exception
+ }
+
+ final protected def receive = {
+ case msg onReceive(msg)
+ }
+}
@@ -0,0 +1,3 @@
+package t1000752_3;
+
+public class J extends ATop {}
@@ -0,0 +1,9 @@
+package t1000752_3
+
+trait Top {
+ protected def foo(t: Array[String])
+}
+
+abstract class ATop extends Top {
+ override def foo(t: Array[String]) {}
+}
@@ -59,9 +59,8 @@ trait ScalaJavaMapper extends ScalaAnnotationHelper with HasLogger { self : Scal
var jdtMods = 0
if(owner.hasFlag(Flags.PRIVATE))
jdtMods = jdtMods | ClassFileConstants.AccPrivate
- else if(owner.hasFlag(Flags.PROTECTED))
- jdtMods = jdtMods | ClassFileConstants.AccProtected
else
+ // protected entities need to be exposed as public to match scala compiler's behavior.
jdtMods = jdtMods | ClassFileConstants.AccPublic
if(owner.hasFlag(Flags.ABSTRACT) || owner.hasFlag(Flags.DEFERRED))
@@ -109,9 +108,8 @@ trait ScalaJavaMapper extends ScalaAnnotationHelper with HasLogger { self : Scal
var jdtMods = 0
if(owner.hasFlag(Flags.PRIVATE))
jdtMods = jdtMods | ClassFileConstants.AccPrivate
- else if(owner.hasFlag(Flags.PROTECTED))
- jdtMods = jdtMods | ClassFileConstants.AccProtected
else
+ // protected entities need to be exposed as public to match scala compiler's behavior.
jdtMods = jdtMods | ClassFileConstants.AccPublic
if(owner.hasFlag(Flags.ABSTRACT) || owner.hasFlag(Flags.DEFERRED))
Oops, something went wrong.

0 comments on commit beb28ca

Please sign in to comment.