Skip to content

Use Java rules for member lookup in .java sources #12884

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

Merged
merged 5 commits into from
Jun 24, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 40 additions & 3 deletions compiler/src/dotty/tools/dotc/core/ContextOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package dotty.tools.dotc
package core

import Contexts._, Symbols._, Types._, Flags._, Scopes._, Decorators._, NameOps._
import Denotations._
import SymDenotations.LazyType, Names.Name, StdNames.nme
import Denotations._, SymDenotations._
import Names.Name, StdNames.nme
import ast.untpd

/** Extension methods for contexts where we want to keep the ctx.<methodName> syntax */
Expand Down Expand Up @@ -34,14 +34,51 @@ object ContextOps:
if (elem.name == name) return elem.sym.denot // return self
}
val pre = ctx.owner.thisType
pre.findMember(name, pre, required, excluded)
if ctx.isJava then javaFindMember(name, pre, required, excluded)
else pre.findMember(name, pre, required, excluded)
}
else // we are in the outermost context belonging to a class; self is invisible here. See inClassContext.
ctx.owner.findMember(name, ctx.owner.thisType, required, excluded)
else
ctx.scope.denotsNamed(name).filterWithFlags(required, excluded).toDenot(NoPrefix)
}

final def javaFindMember(name: Name, pre: Type, required: FlagSet = EmptyFlags, excluded: FlagSet = EmptyFlags): Denotation =
assert(ctx.isJava)
inContext(ctx) {

val preSym = pre.typeSymbol

// 1. Try to search in current type and parents.
val directSearch = pre.findMember(name, pre, required, excluded)

// 2. Try to search in companion class if current is an object.
def searchCompanionClass = if preSym.is(Flags.Module) then
preSym.companionClass.thisType.findMember(name, pre, required, excluded)
else NoDenotation

// 3. Try to search in companion objects of super classes.
// In Java code, static inner classes, which we model as members of the companion object,
// can be referenced from an ident in a subclass or by a selection prefixed by the subclass.
def searchSuperCompanionObjects =
val toSearch = if preSym.is(Flags.Module) then
if preSym.companionClass.exists then
preSym.companionClass.asClass.baseClasses
else Nil
else
preSym.asClass.baseClasses

toSearch.iterator.map { bc =>
val pre1 = bc.companionModule.namedType
pre1.findMember(name, pre1, required, excluded)
}.find(_.exists).getOrElse(NoDenotation)

if preSym.isClass then
directSearch orElse searchCompanionClass orElse searchSuperCompanionObjects
else
directSearch
}

/** A fresh local context with given tree and owner.
* Owner might not exist (can happen for self valdefs), in which case
* no owner is set in result context
Expand Down
42 changes: 3 additions & 39 deletions compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -751,35 +751,8 @@ object JavaParsers {
makeTemplate(List(), statics, List(), false)).withMods((cdef.mods & Flags.RetainedModuleClassFlags).toTermFlags)
}

def importCompanionObject(cdef: TypeDef): Tree =
Import(
Ident(cdef.name.toTermName).withSpan(NoSpan),
ImportSelector(Ident(nme.WILDCARD)) :: Nil)

// Importing the companion object members cannot be done uncritically: see
// ticket #2377 wherein a class contains two static inner classes, each of which
// has a static inner class called "Builder" - this results in an ambiguity error
// when each performs the import in the enclosing class's scope.
//
// To address this I moved the import Companion._ inside the class, as the first
// statement. This should work without compromising the enclosing scope, but may (?)
// end up suffering from the same issues it does in scala - specifically that this
// leaves auxiliary constructors unable to access members of the companion object
// as unqualified identifiers.
def addCompanionObject(statics: List[Tree], cdef: TypeDef): List[Tree] = {
// if there are no statics we can use the original cdef, but we always
// create the companion so import A._ is not an error (see ticket #1700)
val cdefNew =
if (statics.isEmpty) cdef
else {
val template = cdef.rhs.asInstanceOf[Template]
cpy.TypeDef(cdef)(cdef.name,
cpy.Template(template)(body = importCompanionObject(cdef) :: template.body))
.withMods(cdef.mods)
}

List(makeCompanionObject(cdefNew, statics), cdefNew)
}
Copy link
Contributor Author

@changvvb changvvb Jun 24, 2021

Choose a reason for hiding this comment

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

I also deleted the import companion object in the java class.
I think this will make both JavaParsers and generated java AST more simplified.

def addCompanionObject(statics: List[Tree], cdef: TypeDef): List[Tree] =
List(makeCompanionObject(cdef, statics), cdef)

def importDecl(): List[Tree] = {
val start = in.offset
Expand Down Expand Up @@ -901,16 +874,7 @@ object JavaParsers {
members) ++= decls
}
}
def forwarders(sdef: Tree): List[Tree] = sdef match {
case TypeDef(name, _) if (parentToken == INTERFACE) =>
var rhs: Tree = Select(Ident(parentName.toTermName), name)
List(TypeDef(name, rhs).withMods(Modifiers(Flags.Protected)))
case _ =>
List()
}
val sdefs = statics.toList
val idefs = members.toList ::: (sdefs flatMap forwarders)
(sdefs, idefs)
(statics.toList, members.toList)
}
def annotationParents: List[Select] = List(
scalaAnnotationDot(tpnme.Annotation),
Expand Down
9 changes: 7 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package typer

import core._
import ast._
import Contexts._, Constants._, Types._, Symbols._, Names._, Flags._, Decorators._
import Contexts._, ContextOps._, Constants._, Types._, Symbols._, Names._, Flags._, Decorators._
import ErrorReporting._, Annotations._, Denotations._, SymDenotations._, StdNames._
import util.Spans._
import util.SrcPos
Expand Down Expand Up @@ -145,7 +145,12 @@ trait TypeAssigner {
// this is exactly what Erasure will do.
case _ =>
val pre = maybeSkolemizePrefix(qualType, name)
val mbr = qualType.findMember(name, pre)
val mbr =
if ctx.isJava then
ctx.javaFindMember(name, pre)
else
qualType.findMember(name, pre)

if reallyExists(mbr) then qualType.select(name, mbr)
else if qualType.isErroneous || name.toTermName == nme.ERROR then UnspecifiedErrorType
else NoType
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,9 @@ class Typer extends Namer
if (qualifies(defDenot)) {
val found =
if (isSelfDenot(defDenot)) curOwner.enclosingClass.thisType
else {
else if (ctx.isJava && defDenot.symbol.isStatic) {
defDenot.symbol.namedType
} else {
val effectiveOwner =
if (curOwner.isTerm && defDenot.symbol.maybeOwner.isType)
// Don't mix NoPrefix and thisType prefixes, since type comparer
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,4 @@ i2797a
# GADT cast applied to singleton type difference
i4176-gadt.scala

java-inherited-type1
2 changes: 2 additions & 0 deletions compiler/test/dotc/run-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,5 @@ varargs-abstract
zero-arity-case-class.scala
i12194.scala
i12753
t6138
t6138-2
19 changes: 19 additions & 0 deletions tests/pos/java-inherited-type/Client.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
object Client {
def test= {
Test.Outer.Nested.sig
Test.Outer.Nested.sig1
Test.Outer.Nested.sig2
val o = new Test.Outer
new o.Nested1().sig
new o.Nested1().sig1
new o.Nested1().sig2
}

def test1 = {
val t = new Test
val o = new t.Outer1
new o.Nested1().sig
new o.Nested1().sig1
new o.Nested1().sig2
}
}
30 changes: 30 additions & 0 deletions tests/pos/java-inherited-type/Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
public class Test {
static class OuterBase implements OuterBaseInterface {
static class StaticInner {}
class Inner {}
}
interface OuterBaseInterface {
interface InnerFromInterface {}
}
public static class Outer extends OuterBase {
public static class Nested {
public static P<StaticInner, Inner, InnerFromInterface> sig; // was: "type StaticInner", "not found: type Inner", "not found: type InnerFromInterface"
public static P<Outer.StaticInner, Outer.Inner, Outer.InnerFromInterface> sig1; // was: "type StaticInner is not a member of Test.Outer"
public static P<OuterBase.StaticInner, OuterBase.Inner, OuterBaseInterface.InnerFromInterface> sig2;

}
public class Nested1 {
public P<StaticInner, Inner, InnerFromInterface> sig; // was: "not found: type StaticInner"
public P<Outer.StaticInner, Outer.Inner, Outer.InnerFromInterface> sig1; // was: "type StaticInner is not a member of Test.Outer"
public P<OuterBase.StaticInner, OuterBase.Inner, OuterBaseInterface.InnerFromInterface> sig2;
}
}
public class Outer1 extends OuterBase {
public class Nested1 {
public P<StaticInner, Inner, InnerFromInterface> sig; // was: "not found: type StaticInner"
public P<Outer.StaticInner, Outer.Inner, Outer.InnerFromInterface> sig1; // was: "type StaticInner is not a member of Test.Outer"
public P<OuterBase.StaticInner, OuterBase.Inner, OuterBaseInterface.InnerFromInterface> sig2;
}
}
public static class P<A, B, C>{}
}
9 changes: 9 additions & 0 deletions tests/pos/java-inherited-type1/J.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class J extends S {
// These references all work in Javac because `object O { class I }` erases to `O$I`

void select1(S1.Inner1 i) { new S1.Inner1(); }
void ident(Inner i) {}

void ident1(Inner1 i) {}
void select(S.Inner i) { new S.Inner(); }
}
9 changes: 9 additions & 0 deletions tests/pos/java-inherited-type1/S.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class S extends S1
object S {
class Inner
}

class S1
object S1 {
class Inner1
}
13 changes: 13 additions & 0 deletions tests/pos/java-inherited-type1/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
object Test {
val j = new J
// force completion of these signatures
j.ident(null);
j.ident1(null);
j.select(null);
j.select1(null);

val message:TestMessage = null
val builder:TestMessage.Builder = message.toBuilder
builder.setName("name")

}
17 changes: 17 additions & 0 deletions tests/pos/java-inherited-type1/TestMessage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
abstract class AbstractMessage {
public static abstract class Builder<BuilderType extends Builder<BuilderType>> {
}
}

class TestMessage extends AbstractMessage {

public Builder toBuilder() {
return null;
}

public static class Builder extends AbstractMessage.Builder<Builder> {
public Builder setName(String name) {
return this;
}
}
}
1 change: 1 addition & 0 deletions tests/run/t6138-2.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Foo$Bar was instantiated!
4 changes: 4 additions & 0 deletions tests/run/t6138-2/JavaClass.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public class JavaClass {
// This is defined in ScalaClass
public static final Foo.Bar bar = new Foo.Bar();
}
18 changes: 18 additions & 0 deletions tests/run/t6138-2/ScalaClass.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* Similar to t10490 -- but defines `Foo` in the object.
* Placing this test within t10490 makes it work without a fix, that's why it's independent.
* Note that this was already working, we add it to make sure we don't regress
*/

class Foo
object Foo {
class Bar {
override def toString: String = "Foo$Bar was instantiated!"
}
}

object Test {
def main(args: Array[String]): Unit = {
// JavaClass is the user of the Scala defined classes
println(JavaClass.bar)
}
}
4 changes: 4 additions & 0 deletions tests/run/t6138/JavaClass.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public class JavaClass {
// This is defined in ScalaClass
public static final Foo.Bar bar = (new Foo()).new Bar();
}
13 changes: 13 additions & 0 deletions tests/run/t6138/ScalaClass.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class Foo {
class Bar {
override def toString: String = "Foo$Bar was instantiated!"
}
}

object Test {
def main(args: Array[String]): Unit = {
// JavaClass is the user of the Scala defined classes
println(JavaClass.bar)
//println(JavaClass.baz)
}
}
1 change: 1 addition & 0 deletions tests/run/t6238.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Foo$Bar was instantiated!