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

Confusion in type error when two types have same simple name #18678

Closed
bishabosha opened this issue Oct 12, 2023 · 11 comments · Fixed by #19204
Closed

Confusion in type error when two types have same simple name #18678

bishabosha opened this issue Oct 12, 2023 · 11 comments · Fixed by #19204
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc better-errors Issues concerned with improving confusing/unhelpful diagnostic messages itype:enhancement Spree Suitable for a future Spree

Comments

@bishabosha
Copy link
Member

bishabosha commented Oct 12, 2023

originally reported by @hnrklssn in https://stackoverflow.com/questions/68146374/why-cant-a-string-literal-be-assigned-to-variable-of-type-string/68146434, #17842, and https://contributors.scala-lang.org/t/use-fqcn-in-type-error-message-if-two-or-more-types-have-the-same-name/5166

Compiler version

3.3.1

Minimized example

class String // could be wildcard imported

val s: String = "hello"

Output Error/Warning message

-- [E007] Type Mismatch Error: -------------------------------------------------
3 |val s: String = "hello"
  |                ^^^^^^^
  |                Found:    ("hello" : String)
  |                Required: String
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |
  | Tree: "hello"
  | I tried to show that
  |   ("hello" : String)
  | conforms to
  |   String
  | but the comparison trace ended with `false`:
  |
  |   ==> ("hello" : String)  <:  String
  |     ==> String  <:  String (left is approximated)
  |     <== String  <:  String (left is approximated) = false
  |   <== ("hello" : String)  <:  String = false
  |
  | The tests were made under the empty constraint
   -----------------------------------------------------------------------------
1 error found

Why this Error/Warning was not helpful

The message was unhelpful because it claims that String is not String, If I was unaware of a shadowed import then I get zero help here to suggest that could be the reason.

Suggested improvement

It could be made more helpful by:

  • printing fully qualified name of the type
  • otherwise signalling the origin of each type
@bishabosha bishabosha added itype:enhancement area:reporting Error reporting including formatting, implicit suggestions, etc stat:needs triage Every issue needs to have an "area" and "itype" label better-errors Issues concerned with improving confusing/unhelpful diagnostic messages and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Oct 12, 2023
@odersky
Copy link
Contributor

odersky commented Oct 23, 2023

It would be good to see more cases where this fails. In particular, with wildcard imports, and with other types.

@nicolasstucki
Copy link
Contributor

It seems that as soon as the user's String is defined in a package, this is not an issue

package foo
class String
val s: String = "hello"
Details
-- [E007] Type Mismatch Error: t/Test.scala:3:16 -------------------------------
3 |val s: String = "hello"
  |                ^^^^^^^
  |                Found:    ("hello" : String)
  |                Required: foo.String
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |
  | Tree: "hello"
  | I tried to show that
  |   ("hello" : String)
  | conforms to
  |   foo.String
  | but the comparison trace ended with `false`:
  |
  |   ==> ("hello" : String)  <:  foo.String
  |     ==> String  <:  foo.String (left is approximated)
  |     <== String  <:  foo.String (left is approximated) = false
  |   <== ("hello" : String)  <:  foo.String = false
  |
  | The tests were made under the empty constraint
   -----------------------------------------------------------------------------
package foo {
  class String
}
import foo.String
val s: String = "hello"
Details
-- [E007] Type Mismatch Error: t/Test.scala:5:16 -------------------------------
5 |val s: String = "hello"
  |                ^^^^^^^
  |                Found:    ("hello" : String)
  |                Required: foo.String
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |
  | Tree: "hello"
  | I tried to show that
  |   ("hello" : String)
  | conforms to
  |   foo.String
  | but the comparison trace ended with `false`:
  |
  |   ==> ("hello" : String)  <:  foo.String
  |     ==> String  <:  foo.String (left is approximated)
  |     <== String  <:  foo.String (left is approximated) = false
  |   <== ("hello" : String)  <:  foo.String = false
  |
  | The tests were made under the empty constraint
   -----------------------------------------------------------------------------

@nicolasstucki
Copy link
Contributor

With Int we get a slightly different output

package foo
class Int
val s: Int = 1
Details
-- [E007] Type Mismatch Error: t/Test.scala:5:13 -------------------------------
5 |val s: Int = 1
  |             ^
  |             Found:    (1 : Int)
  |             Required: foo.Int²
  |
  |             where:    Int  is a class in package scala
  |                       Int² is a class in package foo
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |
  | Tree: 1
  | I tried to show that
  |   (1 : Int)
  | conforms to
  |   foo.Int
  | but the comparison trace ended with `false`:
  |
  |   ==> (1 : Int)  <:  foo.Int
  |     ==> Int  <:  foo.Int (left is approximated)
  |     <== Int  <:  foo.Int (left is approximated) = false
  |   <== (1 : Int)  <:  foo.Int = false
  |
  | The tests were made under the empty constraint
   -----------------------------------------------------------------------------
</details> 

@nicolasstucki
Copy link
Contributor

nicolasstucki commented Oct 23, 2023

With Int it is only the explain that is not clear (see #18748)

class Int
val s: Int = 1
Details
-- [E007] Type Mismatch Error: t/Test.scala:2:13 -------------------------------
2 |val s: Int = 1
  |             ^
  |             Found:    (1 : Int)
  |             Required: Int²
  |
  |             where:    Int  is a class in package scala
  |                       Int² is a class
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |
  | Tree: 1
  | I tried to show that
  |   (1 : Int)
  | conforms to
  |   Int
  | but the comparison trace ended with `false`:
  |
  |   ==> (1 : Int)  <:  Int
  |     ==> Int  <:  Int (left is approximated)
  |     <== Int  <:  Int (left is approximated) = false
  |   <== (1 : Int)  <:  Int = false
  |
  | The tests were made under the empty constraint
   -----------------------------------------------------------------------------

@nicolasstucki
Copy link
Contributor

It is more problematic for java.lang.String

class String
val s: String = "hello" : java.lang.String
Details
-- [E007] Type Mismatch Error: t/Test.scala:2:16 -------------------------------
2 |val s: String = ??? : java.lang.String
  |                ^^^^^^^^^^^^^^^^^^^^^^
  |                Found:    String
  |                Required: String
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |
  | Tree: ??? :java.lang.String
  | I tried to show that
  |   String
  | conforms to
  |   String
  | but the comparison trace ended with `false`:
  |
  |   ==> String  <:  String
  |   <== String  <:  String = false
  |
  | The tests were made under the empty constraint
   -----------------------------------------------------------------------------

@nicolasstucki
Copy link
Contributor

Tuples can also lead to confusion

class Tuple2[A, B]
val s: Tuple2[Int, Int] = (1, 3)
Details
-- [E007] Type Mismatch Error: t/Test.scala:2:26 -------------------------------
2 |val s: Tuple2[Int, Int] = (1, 3)
  |                          ^^^^^^
  |                          Found:    (Int, Int)
  |                          Required: Tuple2[Int, Int]
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |
  | Tree: Tuple2.apply[Int, Int](1, 3)
  | I tried to show that
  |   (Int, Int)
  | conforms to
  |   Tuple2[Int, Int]
  | but the comparison trace ended with `false`:
  |
  |   ==> (Int, Int)  <:  Tuple2[Int, Int]
  |   <== (Int, Int)  <:  Tuple2[Int, Int] = false
  |
  | The tests were made under the empty constraint
   -----------------------------------------------------------------------------

@nicolasstucki
Copy link
Contributor

The issue is in general between types defined in the root package, scala package, or the java lang package. None of these types show their prefix which can cause confusion.

@nicolasstucki
Copy link
Contributor

In #18748 there is an improvement in the main message when dealing with primitive types.

@odersky
Copy link
Contributor

odersky commented Oct 29, 2023

Yes, but I have the impression this is only for String and only if string is defined in the empty package. Which makes it rather more marginal as an issue, maybe not worth a general fix. That's why I asked for more examples that wnet wrong.

@nicolasstucki
Copy link
Contributor

Yes, but I have the impression this is only for String and only if string is defined in the empty package.

Types in the scala package also have this issue but it manifests in a slightly different way.

Which makes it rather more marginal as an issue, maybe not worth a general fix.

Unfortunately, this is the case in the REPL which makes it less marginal.

@odersky
Copy link
Contributor

odersky commented Oct 30, 2023

If it happens in the REPL it's important to fix, I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc better-errors Issues concerned with improving confusing/unhelpful diagnostic messages itype:enhancement Spree Suitable for a future Spree
Projects
None yet
4 participants