Skip to content

REPL summarizes warnings#7756

Merged
lrytz merged 2 commits intoscala:2.13.xfrom
som-snytt:issue/11402
Feb 3, 2020
Merged

REPL summarizes warnings#7756
lrytz merged 2 commits intoscala:2.13.xfrom
som-snytt:issue/11402

Conversation

@som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Feb 15, 2019 •
edited
Loading

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

SethTisue commented Feb 26, 2019

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

som-snytt commented Feb 1, 2020

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

som-snytt commented Feb 1, 2020

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.

@som-snytt som-snytt removed the WIP label Feb 1, 2020
@som-snytt
REPL summarizes warnings
0e6426d 
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.
dwijnand
dwijnand reviewed Feb 2, 2020
View reviewed changes
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 •
edited
Loading

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

som-snytt commented Feb 2, 2020

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

dwijnand
dwijnand approved these changes Feb 2, 2020
View reviewed changes
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

som-snytt commented Feb 3, 2020

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

@som-snytt som-snytt deleted the issue/11402 branch February 3, 2020 14:20
@dwijnand dwijnand mentioned this pull request Oct 16, 2020
Merged
12 tasks
@dwijnand dwijnand mentioned this pull request Nov 2, 2020
Merged
@som-snytt som-snytt mentioned this pull request Mar 5, 2021
Merged
@som-snytt som-snytt mentioned this pull request Dec 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dwijnand dwijnand dwijnand approved these changes

Assignees

@lrytz lrytz

Labels

None yet

Projects

None yet

Milestone

2.13.2

Development

Successfully merging this pull request may close these issues.

REPL doesn't summarize parser deprecations

6 participants

@som-snytt @SethTisue @dwijnand @szeiger @lrytz @scala-jenkins