Skip to content

sbt reporter: Don't crash with malformed positions #10250

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
Nov 16, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented Nov 9, 2020

In https://github.com/Sciss/DottyScalaTestAssertCrash we get a crash
when trying to report a warning in RichNumberSuite.scala: the file
contains 284 characters and yet the pos.point of the error is at
position 8742, this line calls the scalatest assert macro so it seems
that something is wrong with the way we compute positions in macros (or
could the macro have a bug?). It'd be good if we could find the root
cause of the problem but in any case being robust when reporting errors
is better than crashing.

FWIW, after fixing this on top of 3.0.0-M1 and recompiling the
problematic project I see the following the warning:

[warn] -- [E123] Syntax Warning: /home/smarter/opt/DottyScalaTestAssertCrash/src/test/scala/synth/RichNumberSuite.scala:9:6
[warn] 9 | assert(intInt1.isInstanceOf[Int], "found " + intInt1.getClass)
[warn] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warn] |The highlighted type test will always succeed since the scrutinee type (TermRef(NoPrefix,val x).show) is a subtype of Int
[warn] | This location contains code that was inlined from RichNumberSuite.scala:12

Which still looks a bit weird (showing a raw TermRef, mentioning that
the code was inlined from RichNumberSuite.scala:12 even thought that's
just the last line of a file which contains only a }.

In https://github.com/Sciss/DottyScalaTestAssertCrash we get a crash
when trying to report a warning in RichNumberSuite.scala: the file
contains 284 characters and yet the `pos.point` of the error is at
position 8742, this line calls the scalatest `assert` macro so it seems
that something is wrong with the way we compute positions in macros (or
could the macro have a bug?). It'd be good if we could find the root
cause of the problem but in any case being robust when reporting errors
is better than crashing.

FWIW, after fixing this on top of 3.0.0-M1 and recompiling the
problematic project I see the following the warning:

[warn] -- [E123] Syntax Warning: /home/smarter/opt/DottyScalaTestAssertCrash/src/test/scala/synth/RichNumberSuite.scala:9:6
[warn] 9 |      assert(intInt1.isInstanceOf[Int], "found " + intInt1.getClass)
[warn]   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[warn]   |The highlighted type test will always succeed since the scrutinee type (TermRef(NoPrefix,val x).show) is a subtype of Int
[warn]   | This location contains code that was inlined from RichNumberSuite.scala:12

Which still looks a bit weird (showing a raw TermRef, mentioning that
the code was inlined from `RichNumberSuite.scala:12` even thought that's
just the last line of a file which contains only a `}`.
@smarter smarter force-pushed the robust-sbt-reporter branch from 580873f to 341d330 Compare November 16, 2020 14:28
@smarter
Copy link
Member Author

smarter commented Nov 16, 2020

Ping for review (there was a CI failure but it was a CI issue)

@griggt
Copy link
Contributor

griggt commented Nov 16, 2020

I believe I ran into this issue yesterday. Is there a ticket open for this (I didn't find one)?

@smarter
Copy link
Member Author

smarter commented Nov 16, 2020

I don't think there's an issue, it was reported on gitter.

@griggt
Copy link
Contributor

griggt commented Nov 16, 2020

Thanks, I had already started working on writing one up late last night. I will continue in the interest of getting to the root of the problem.

@griggt
Copy link
Contributor

griggt commented Nov 19, 2020

What struck me about this was that I encountered the same issue with different input, but the same erroneous position of 8742.

I opened #10384 to track the root cause.

Oh, and as for the origin of 8742 (actually the span [8742..8746]), it belongs to:

// scalatest/dotty/scalactic/src/main/scala/org/scalactic/BooleanMacro.scala
//   ...
          '{ Bool.isInstanceOfMacroBool(${l.seal}, "isInstanceOf", $name, $res, $prettifier) }.unseal
                                                                          ^^^^^

The file name is somehow mismatched with the span in the source position due to some change(s) introduced by #9900.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants