Skip to content

Properly report clashes between classes and objects #10928

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 1 commit into from
Jan 4, 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
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
CannotExtendJavaEnumID,
InvalidReferenceInImplicitNotFoundAnnotationID,
TraitMayNotDefineNativeMethodID,
JavaEnumParentArgsID
JavaEnumParentArgsID,
AlreadyDefinedID

def errorNumber = ordinal - 2
}
13 changes: 13 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1847,6 +1847,19 @@ import transform.SymUtils._
def explain = ""
}

class AlreadyDefined(name: Name, owner: Symbol, conflicting: Symbol)(using Context) extends NamingMsg(AlreadyDefinedID):
private def where: String =
if conflicting.effectiveOwner.is(Package) && conflicting.associatedFile != null then
i" in ${conflicting.associatedFile}"
else if conflicting.owner == owner then ""
else i" in ${conflicting.owner}"
def msg =
if conflicting.isTerm != name.isTermName then
em"$name clashes with $conflicting$where; the two must be defined together"
else
em"$name is already defined as $conflicting$where"
def explain = ""

class PackageNameAlreadyDefined(pkg: Symbol)(using Context) extends NamingMsg(PackageNameAlreadyDefinedID) {
lazy val (where, or) =
if pkg.associatedFile == null then ("", "")
Expand Down
12 changes: 12 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,17 @@ trait Checking {
em"Implementation restriction: ${path.tpe.classSymbol} is not a valid prefix " +
"for a wildcard export, as it is a package.", path.srcPos)

/** Check that module `sym` does not clash with a class of the same name
* that is concurrently compiled in another source file.
*/
def checkNoModuleClash(sym: Symbol)(using Context): Unit =
if sym.effectiveOwner.is(Package)
&& sym.owner.info.member(sym.name.moduleClassName).symbol.isAbsent()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the logic here. It seems that we always call checkNoModuleClash with a module symbol. If that's the case, then shouldn't this symbol always be non-absent? Or is it that this symbol was marked absent when we added the clashing class's symbol as a member?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the latter.

then
val conflicting = sym.owner.info.member(sym.name.toTypeName).symbol
if conflicting.exists then
report.error(AlreadyDefined(sym.name, sym.owner, conflicting), sym.srcPos)

/** Check that `tp` is a class type.
* Also, if `traitReq` is true, check that `tp` is a trait.
* Also, if `stablePrefixReq` is true and phase is not after RefChecks,
Expand Down Expand Up @@ -1266,6 +1277,7 @@ trait ReChecking extends Checking {
override def checkEnumCaseRefsLegal(cdef: TypeDef, enumCtx: Context)(using Context): Unit = ()
override def checkAnnotApplicable(annot: Tree, sym: Symbol)(using Context): Boolean = true
override def checkMatchable(tp: Type, pos: SrcPos, pattern: Boolean)(using Context): Unit = ()
override def checkNoModuleClash(sym: Symbol)(using Context) = ()
}

trait NoChecking extends ReChecking {
Expand Down
9 changes: 4 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,10 @@ class Namer { typer: Typer =>
var conflictsDetected = false

def conflict(conflicting: Symbol) =
val where: String =
if conflicting.owner == owner then ""
else if conflicting.owner.isPackageObject then i" in ${conflicting.associatedFile}"
else i" in ${conflicting.owner}"
report.error(i"$name is already defined as $conflicting$where", ctx.source.atSpan(span))
val other =
if conflicting.is(ConstructorProxy) then conflicting.companionClass
else conflicting
report.error(AlreadyDefined(name, owner, other), ctx.source.atSpan(span))
conflictsDetected = true

def checkNoConflictIn(owner: Symbol) =
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1979,6 +1979,7 @@ class Typer extends Namer
val ValDef(name, tpt, _) = vdef
completeAnnotations(vdef, sym)
if (sym.isOneOf(GivenOrImplicit)) checkImplicitConversionDefOK(sym)
if sym.is(Module) then checkNoModuleClash(sym)
val tpt1 = checkSimpleKinded(typedType(tpt))
val rhs1 = vdef.rhs match {
case rhs @ Ident(nme.WILDCARD) => rhs withType tpt1.tpe
Expand Down
4 changes: 4 additions & 0 deletions tests/neg/i9472a.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- [E161] Naming Error: tests/neg/i9472a/B.scala:3:0 -------------------------------------------------------------------
3 |object Reproduction // error
|^
|Reproduction clashes with class Reproduction in tests/neg/i9472a/A.scala; the two must be defined together
4 changes: 4 additions & 0 deletions tests/neg/i9472a/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package example.reproduction

class Reproduction

5 changes: 5 additions & 0 deletions tests/neg/i9472a/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package example.reproduction

object Reproduction // error


4 changes: 4 additions & 0 deletions tests/neg/i9472b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- [E161] Naming Error: tests/neg/i9472b/A.scala:3:0 -------------------------------------------------------------------
3 |object Reproduction // error
|^
|Reproduction clashes with class Reproduction in tests/neg/i9472b/B.scala; the two must be defined together
4 changes: 4 additions & 0 deletions tests/neg/i9472b/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package example.reproduction

object Reproduction // error

4 changes: 4 additions & 0 deletions tests/neg/i9472b/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package example.reproduction

class Reproduction

4 changes: 2 additions & 2 deletions tests/neg/trailingCommas.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ trait ArgumentExprs2 { validMethod(23, "bar")(Ev0, Ev1, ) } // error // error
trait ArgumentExprs3 { new ValidClass(23, "bar", )(Ev0, Ev1) } // error // error
trait ArgumentExprs4 { new ValidClass(23, "bar")(Ev0, Ev1, ) } // error // error

trait Params1 { def f(foo: Int, bar: String, )(implicit ev0: Ev0, ev1: Ev1, ) = 1 } // error // error // error
trait Params1 { def f(foo: Int, bar: String, )(implicit ev0: Ev0, ev1: Ev1, ) = 1 } // error // error

trait Params2 { def f(foo: Int, bar: String, )(implicit ev0: Ev0, ev1: Ev1, ) = 1 } // error // error // error
trait Params2 { def f(foo: Int, bar: String, )(implicit ev0: Ev0, ev1: Ev1, ) = 1 } // error // error

trait ClassParams1 { final class C(foo: Int, bar: String, )(implicit ev0: Ev0, ev1: Ev1) } // error
trait ClassParams2 { final class C(foo: Int, bar: String)(implicit ev0: Ev0, ev1: Ev1, ) } // error
Expand Down