Confusing diagnostics with braceless syntax

[alvae]

Hello fellow Scala enthusiasts!

Imagine you wrote this code:

def make(): List[Int] =
  @tailrec def loop(i: Int, xs: List[Int]): List[Int] =
    if i == 0 then xs else loop(i - 1, (dice.nextInt() % n) :: xs)

The error reported by the compiler is as follows:

-- [E007] Type Mismatch Error: /private/tmp/playground/main.scala:24:70 ----------------------------
24 |        if i <= 0 then xs else loop(i - 1, (dice.nextInt() % n) :: xs)
   |                                                                      ^
   |                                                                      Found:    Unit
   |                                                                      Required: List[Int]

I find this error very confusing and, somewhat embarrassingly, it has caused me to waste a lot of time quite many times. The issue is that the error makes it look like the whole expression has type Unit which is obviously not true. Specifically, because of the location where the problem is reported, I often try to understand why the typer wouldn’t agree with me while the actual error is simply that I forgot to call loop before exiting make.

I find the problem easier to understand with braces:

def make(): List[Int] = {
  @tailrec def loop(i: Int, xs: List[Int]): List[Int] = {
    if i == 0 then xs else loop(i - 1, (dice.nextInt() % n) :: xs)
  }
} // <- error is reported here

Now the error looks like the following:

-- [E007] Type Mismatch Error: /private/tmp/playground/main.scala:26:5 -----------------------------
26 |    }
   |     ^
   |     Found:    Unit
   |     Required: List[Int]

Because the error is reported after the brace that close make, I find it much easier to understand that the problem is not the type of the expression in loop but the lack of a return value in make.

Is there a way we could improve the first diagnostic?

If there’s a better place to write about this kind of issue, please let me know.

[Sporarum]

I don’t really know how to improve this error message, maybe by moving the location, but it’s not clear where

Instead, would it help if there was an additional error “block-scoped function loop is never used” ?
(There might already be a flag that does this, and if so, it should be the default !)

[som-snytt]

Similar to the suggestion, I’ve tried to have the compiler always run checks as far as refchecks, so you can see overriding problems and so on.

That would almost cover -Wunused, except that will run after patternMatcher (after a recent PR makes it in).

It would be nice to have a mechanism to have all the lints run (and only lints), irrespective of when they are configured to run. Then lints, besides promising not to transform trees, would have to be especially tolerant of erroneous trees.

I think it’s the compiletime “does this compile” magic check that tries to have enough phases run to be useful.

Anyway, the fact that the compiler “knows things” about my code and didn’t tell me just makes my blood boil. Probably I am too volatile.

Currently -Vprint:typer shows code with braces and also inserted unit values.

    def make(): List[Int] =
      {
        @tailrec def loop(i: Int, xs: List[Int]): List[Int] =
          if i == 0 then xs else
            loop(i - 1,
              {
                val elem$1: Int = dice.next() % n
                xs.::[Int](elem$1)
              }
            )
        ()
      }

or in color

Sometimes that view will also show errors; I don’t usually run the REPL without showing trees.

I bet it could be made to always show some indication of bad part of the tree.

Edit: worth adding that current doctrine is to show errors before warnings, because they want you fix them first. But “lint” rhymes with “hint”!

[bishabosha]

one thing is that the error message has not much information - i.e. only the location end of the expression, and also only says “expected X got Y”. It could have more information such as “def make …” to know the subject that is wrongly typed

[odersky]

That’s a good idea. We could detect the case where a block ends in a definition and add a note like

Note that Unit was produced for a block ending in the definition of … Maybe you forgot a final return expression?

[som-snytt]

It’s a bit subtle. This inserts an unreachable unit value expression:

def f(): Unit =
  return println(loop(0, Nil))
  @tailrec def loop(i: Int, xs: List[Int]): List[Int] =
    if i == 0 then xs else loop(i - 1, (dice.next() % n) :: xs)

I would like the return to be a “terminal return” (followed only by defs) that would also allow the enclosing method to have an inferred result type.

But either the types align and I’ll get an unused warning if I forgot something, or they don’t and I’ll get the (improved) error.

Edit: I did not lodge a ticket, but I submitted a minimal PR. There is an existing bit of helpful text in case you forgot an else.

Warn about missing expr after defns

[som-snytt]

I expect this will be in the next edition of “The Puzzlers”.

def test: Boolean =
  val x = false
  ! true   // error // error

“The Puzzlers” is that graphic serial novel about a group of Scala devs who may be on the side of good, or possibly they are just itching for ~trouble~ excitement.

This particular example has a “knock-on” error, so-called, I suppose, because one makes an edit and then knocks on wood that it is a fix.

-- [E008] Not Found Error: tests/neg/leading-infix-miss.scala:3:2 ------------------------------------------------------
2 |  val x = false
3 |  ! true   // error // error
  |          ^
  |          value ! is not a member of Boolean.
  |          Note that `!` is treated as an infix operator in Scala 3.
  |          If you do not want that, insert a `;` or empty line in front
  |          or drop any spaces behind the operator.
-- [E007] Type Mismatch Error: tests/neg/leading-infix-miss.scala:3:8 --------------------------------------------------
3 |  ! true   // error // error
  |        ^
  |        Found:    Unit
  |        Required: Boolean
  |        Maybe you are missing an expression after a block with definitions?
  |

What is striking to me is how the caret dangles in space and looks like a “missing given". There are tickets about how best to render that case.

I would prefer it printed out the signature it expects, to make it visible. I also wouldn’t mind, here, to see a pretty-printed tree that makes everything plain.

I’ve sunk to using an IDE, which shows all the implicits all the time, which I find distracting, but I think in a diagnostic, it’s very nice to see what is normally hidden.

[odersky]

@alvae

If there’s a better place to write about this kind of issue, please let me know.

The usual way would be to just file an issue in the Scala 3 repo. But as long as traffic is not too high we can discuss it also here. Anyway, thanks for raising the issue!

See Add error note for type mismatch involving blocks without last expression by odersky · Pull Request #25181 · scala/scala3 · GitHub for a fix.

[som-snytt]

I guess with a ticket, one can assign it!

Yes, that is where I also went a-tweaking.

Thanks, I was too lazy to look for the tree.

[mberndt]

It would be better if the value of a block was just the value of the last expression in the block, even when it is followed by one or more definitions. It’s not clear to me why it’s useful to have a block evaluate to Unit just because it happens to end in a def.
Alas, changing it would be backward incompatible (because it can change method return types), so it’s unlikely to happen.