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

Warning when assigning null to Unit #18722

Closed
kavedaa opened this issue Oct 18, 2023 · 5 comments · Fixed by #18723
Closed

Warning when assigning null to Unit #18722

kavedaa opened this issue Oct 18, 2023 · 5 comments · Fixed by #18723
Assignees
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc better-errors Issues concerned with improving confusing/unhelpful diagnostic messages itype:enhancement

Comments

@kavedaa
Copy link

kavedaa commented Oct 18, 2023

Compiler version

3.3.0

Minimized example

scala> val a: Unit = null

Output Error/Warning message

1 warning found
-- [E129] Potential Issue Warning: ---------------------------------------------
1 |val a: Unit = null
  |              ^^^^
  |A pure expression does nothing in statement position; you may be omitting necessary parentheses
  |
  | longer explanation available when compiling with `-explain`
val a: Unit = ()

Why this Error/Warning was not helpful

The message was unhelpful because...

...it's hard to understand why it is relevant in this scenario. I initially thought that it's perhaps not allowed to assign null to a value of Unit type, but since it's just a warning and not an error, then I guess it is. So I don't really understand at all why this warning appears here.

Suggested improvement

It could be made more helpful by...

...not giving a warning at all, or at least changing the explanation if it's actually a legitimate warning.

@kavedaa kavedaa added area:reporting Error reporting including formatting, implicit suggestions, etc better-errors Issues concerned with improving confusing/unhelpful diagnostic messages itype:enhancement stat:needs triage Every issue needs to have an "area" and "itype" label labels Oct 18, 2023
@kavedaa
Copy link
Author

kavedaa commented Oct 18, 2023

Duh...I was clearly not thinking clearly when wrote this. In the sense that I probably don't really understand how Unit works. Closing.

@kavedaa kavedaa closed this as completed Oct 18, 2023
@som-snytt
Copy link
Contributor

Warnings are designed for when we are not thinking clearly!

The error here is that it does not warn with -Wvalue-discard.

Scala 2 emits both warnings:

scala> val x: Unit = null
                     ^
       warning: discarded non-Unit value of type Null(null)
                     ^
       warning: a pure expression does nothing in statement position
val x: Unit = ()

I'll leave the ticket closed, but I think it is a bug.

@som-snytt som-snytt closed this as not planned Won't fix, can't repro, duplicate, stale Oct 18, 2023
@som-snytt
Copy link
Contributor

Welcome to Scala 3.4.0-RC1-bin-SNAPSHOT-git-8c1cdc2 (21, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> val x: Unit = null
1 warning found
-- [E129] Potential Issue Warning: -------------------------------------------------------------------------------------
1 |val x: Unit = null
  |              ^^^^
  |              A pure expression does nothing in statement position; you may be omitting necessary parentheses
  |
  | longer explanation available when compiling with `-explain`
val x: Unit = ()

scala> def i = 42
def i: Int

scala> val x: Unit = i
1 warning found
-- [E175] Potential Issue Warning: -------------------------------------------------------------------------------------
1 |val x: Unit = i
  |              ^
  |              discarded non-Unit value of type (i : => Int)
val x: Unit = ()

Also it drives me nuts that I expand the edit window, then github UI re-minimizes it. Thanks for listening.

@kavedaa
Copy link
Author

kavedaa commented Oct 18, 2023

Thanks for taking a look. I agree, it would be clearer with the "discarded" warning.

@nicolasstucki nicolasstucki removed the stat:needs triage Every issue needs to have an "area" and "itype" label label Oct 19, 2023
@nicolasstucki nicolasstucki self-assigned this Oct 19, 2023
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Oct 19, 2023
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Oct 19, 2023
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Oct 19, 2023
@bishabosha
Copy link
Member

Thanks for opening the issue, I agree that it can be confusing, as assigning a value to Unit type acts in a special way, looks like #18723 will make the situation clearer

@bishabosha bishabosha reopened this Oct 19, 2023
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Oct 20, 2023
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Oct 20, 2023
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Oct 20, 2023
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Oct 20, 2023
WojciechMazur added a commit that referenced this issue Jun 21, 2024
WojciechMazur added a commit that referenced this issue Jun 23, 2024
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants