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

REPL summarizes warnings #7756

Merged
merged 2 commits into from
Feb 3, 2020
Merged

REPL summarizes warnings #7756

merged 2 commits into from
Feb 3, 2020

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Feb 15, 2019

Emit the summary "there were deprecations" after
successful parse.

Tweaks #7482

Previously, fatal warning trumped incomplete input, so switch that around. Previously:

Welcome to Scala 2.13.0-20190213-233114-f1c1d62 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_144).
Type in expressions for evaluation. Or try :help.

scala> def f() = { 'abc
                   ^
       warning: symbol literal is deprecated; use sym"abc" instead
error: No warnings can be incurred under -Xfatal-warnings.

Wait for complete input to issue summary.

Summary, -deprecation, -Xfatal-warnings:

$ skala -feature

     ________ ___   / /  ___
    / __/ __// _ | / /  / _ |
  __\ \/ /__/ __ |/ /__/ __ |
 /____/\___/_/ |_/____/_/ | |
                          |/  version 2.13.0-20190213-233114-f1c1d62

scala> 'abc
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res0: Symbol = 'abc

scala> trait T { def f() }
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
defined trait T

scala> :replay -deprecation
replay> 'abc
        ^
        warning: symbol literal is deprecated; use sym"abc" instead
res0: Symbol = 'abc

replay> trait T { def f() }
                         ^
        warning: procedure syntax is deprecated: instead, add `: Unit` to explicitly declare `f`'s return type
defined trait T


scala> :replay -Xsource:2.14
replay> 'abc
        ^
        error: symbol literal is unsupported; use sym"abc" instead

replay> trait T { def f() }
                         ^
        error: procedure syntax is unsupported: instead, add `: Unit` to explicitly declare `f`'s return type


scala> :quit

Fixes scala/bug#11402

@scala-jenkins scala-jenkins added this to the 2.13.1 milestone Feb 15, 2019
@SethTisue SethTisue modified the milestones: 2.13.1, 2.13.0-RC1 Feb 15, 2019
@som-snytt som-snytt added the WIP label Feb 16, 2019
@SethTisue SethTisue modified the milestones: 2.13.0-RC1, 2.13.1 Feb 26, 2019
@SethTisue
Copy link
Member

I've moved this to the 2.13.1 milestone since it isn't really a release blocker, but it would certainly be very good to have this fixed.

@szeiger szeiger modified the milestones: 2.13.1, 2.13.2 Aug 26, 2019
@lrytz lrytz self-assigned this Jan 31, 2020
@som-snytt
Copy link
Contributor Author

and have 2 and 2002 different commits each, respectively.

@som-snytt som-snytt force-pushed the issue/11402 branch 2 times, most recently from af2347c to 22e8876 Compare February 1, 2020 22:58
@som-snytt
Copy link
Contributor Author

Very tough to reason about a stew of side effects. I had even forgotten who tells global what is the current run. (Answer is the current run.)

This fix is to remove reporter.reset mole-whacking. It's reset after getting a parsable unit, so you only see parse warnings once; and it's reset after compiling the line, and in addition the conditional warnings are cleared, because otherwise they hang around while parsing the next line, since parsing doesn't create a new run. Note that reporter is also reset by global before compiling as well; there are currently two compilations per line for $read and $eval containers.

Emit the summary after successful parse in case
"there were deprecations".

To reduce repetitive parser warnings on multiline
input, only reset the reporter after a final parse
and also after final compilation result (a result
that is not incomplete, but may represent either
a compile error or success).

The underlying global compiler will also reset
the reporter before compilation of repl artifacts.
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

The change sounds good and the diff looks reasonable, and all the tests are passing. I think it would be good to capture this as a test no? I don't exactly see the behaviour you describe in the transcript in the PR description in any of the changed tests.

@som-snytt
Copy link
Contributor Author

som-snytt commented Feb 2, 2020

The updated check files demonstrate parser-based deprecations warning is shown now.

I'll see if a repl test will show the multiline case not warning on every line, see the comment from last Valentine's.

@som-snytt
Copy link
Contributor Author

With deprecation enabled:

     ________ ___   / /  ___
    / __/ __// _ | / /  / _ |
  __\ \/ /__/ __ |/ /__/ __ |
 /____/\___/_/ |_/____/_/ | |
                          |/  version 2.13.2-20200202-014822-0e6426d

scala> def f = {
     | val x = 'abc
       val x = 'abc
               ^
On line 2: warning: symbol literal is deprecated; use Symbol("abc") instead
     | val y = x.toString
     | y
     | }
def f: String

scala> :quit

versus

Welcome to Scala 2.13.1 (OpenJDK 64-Bit Server VM, Java 11.0.5).
Type in expressions for evaluation. Or try :help.

scala> def f = {
     | val x = 'abc
       val x = 'abc
               ^
On line 2: warning: symbol literal is deprecated; use Symbol("abc") instead
     | val y = x.toString
       val x = 'abc
               ^
On line 2: warning: symbol literal is deprecated; use Symbol("abc") instead
     | y
       val x = 'abc
               ^
On line 2: warning: symbol literal is deprecated; use Symbol("abc") instead
     | }
       val x = 'abc
               ^
On line 2: warning: symbol literal is deprecated; use Symbol("abc") instead
f: String

scala> :quit

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Thanks!

@lrytz lrytz merged commit bc05029 into scala:2.13.x Feb 3, 2020
@som-snytt
Copy link
Contributor Author

Note that SessionTest doesn't handle mid-input feedback from parser.

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