From d842da2d75552fdcc3878a86b1e4c07031e0935c Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 22 Jul 2020 17:04:46 +0200 Subject: [PATCH] Fix #9392: Allow overriding a Java method when a same-named field is present In #9126, Java fields were marked effectively finals to prevent false overrides. This poses a problem when a Java field and method of the same name are present, because a Scala val or def will match both of these according to Denotation#matches. We could tweak `matches` further to not consider the Java field to match any Scala definition, but that might introduce new problems (we couldn't trust `FullMatch` anymore and would need to check the info of any Java symbol which might cause cycles, we would also probably need to tweak overloading resolution to avoid ambiguity when trying to decide whether to select the Java field or the Scala override of the Java method). This commit takes a different approach: we allow a val or def to match a Java field, but we tweak the logic that checks whether the `override` modifier is correctly used to disallow definitions that _only_ match a Java field (thus preventing tests/neg/i9115 from compiling), while letting through definitions that both match a Java field and something else (thus making tests/pos/i9392 compile). This is all a bit messy but I can't think of a better solution which wouldn't be significantly more complicated. --- .../dotty/tools/dotc/core/SymDenotations.scala | 1 - .../src/dotty/tools/dotc/typer/RefChecks.scala | 6 ++++-- tests/pos/i9392/J.java | 6 ++++++ tests/pos/i9392/S.scala | 17 +++++++++++++++++ 4 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 tests/pos/i9392/J.java create mode 100644 tests/pos/i9392/S.scala diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 70525007707a..1523a07b02fd 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1066,7 +1066,6 @@ object SymDenotations { final def isEffectivelyFinal(using Context): Boolean = isOneOf(EffectivelyFinalFlags) || is(Inline, butNot = Deferred) - || is(JavaDefinedVal, butNot = Method) || !owner.isExtensibleClass /** A class is effectively sealed if has the `final` or `sealed` modifier, or it diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index bc7aef564173..ea70c712dc09 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -778,8 +778,10 @@ object RefChecks { ) def classDecls = inclazz.info.nonPrivateDecl(member.name) - (inclazz != clazz) && - classDecls.hasAltWith(d => isSignatureMatch(d.symbol) && javaAccessCheck(d.symbol)) + (inclazz != clazz) && classDecls.hasAltWith(d => + isSignatureMatch(d.symbol) && javaAccessCheck(d.symbol) && + !d.symbol.is(JavaDefinedVal, butNot = Method) // Java fields cannot be overriden + ) } // 4. Check that every defined member with an `override` modifier overrides some other member. diff --git a/tests/pos/i9392/J.java b/tests/pos/i9392/J.java new file mode 100644 index 000000000000..d2ad42cfc3c7 --- /dev/null +++ b/tests/pos/i9392/J.java @@ -0,0 +1,6 @@ +package pkg; + +public class J { + int i = 0; + public int i() { return 1; } +} diff --git a/tests/pos/i9392/S.scala b/tests/pos/i9392/S.scala new file mode 100644 index 000000000000..c094e773ce08 --- /dev/null +++ b/tests/pos/i9392/S.scala @@ -0,0 +1,17 @@ +class S1 extends pkg.J { + override def i(): Int = 2 +} + +class S2 extends pkg.J { + override def i: Int = 2 +} +object Test { + val s1 = new S1 + + val i1 = s1.i + val i2 = s1.i() + + val s2 = new S2 + + val i3 = s2.i +}