Skip to content

Helper for unused parameters #10790

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

Closed
fommil opened this issue Mar 20, 2018 · 25 comments
Closed

Helper for unused parameters #10790

fommil opened this issue Mar 20, 2018 · 25 comments

Comments

@fommil
Copy link

fommil commented Mar 20, 2018

This makes @som-snytt's wonderful work on unused warnings sooo much easier to enable

class unused extends deprecated("unused", "")

There are many valid usecases for ignoring a parameter. For example, in scalaz's .xmap.

@pjrt
Copy link

pjrt commented Mar 21, 2018

Why not something similar to what we do in Haskell: parameters which start with _ are not considered "not used". This allows you to turn off the warning for specific parameters, instead of turn it off for the whole scope (and doesn't require ugly annotations).

@som-snytt
Copy link

@pjrt The difficulty with a naming convention is named args.

On the ticket about requiring args to @deprecated, I asked them not to. That's a very slight mitigation of ugliness.

In Java, a deprecated parameter will never warn if used, so it seems like a safe convention to use the same marker for never warn if unused.

Someone recently tweeted that unused parameters are evil. I'll attach a link if I can find it on twitter, which I won't.

@som-snytt
Copy link

Not warning on meh is an easy win.

https://twitter.com/MorseCode8086/status/970879623083982850

@SethTisue
Copy link
Member

@som-snytt but seriously, what do you think the right way forward here is?

@SethTisue SethTisue added this to the Backlog milestone Apr 17, 2018
@som-snytt
Copy link

@SethTisue it's like adding Regex.matches, it's a convenience, and every convenience requires approval of the scala center board. It would be easy to sneak in as an inconvenience.

Also, shouldn't it be called @used? It's like asInstanceOf, you're telling the compiler to assume that it's used even if it thinks otherwise. If you call it @unused, you're saying it's unused, so of course the compiler should warn about that. If we introduce both @used and @unused, the ergonomics would be inconvenient enough that we wouldn't have to wait for the next board meeting.

But seriously, I'm fine with this proposal.

@SethTisue
Copy link
Member

okay so suppose we go with @unused, we don't literally want unused extends deprecated, right? you'd change the unused-code-a-lyzer to recognize @unused directly?

@hrhino
Copy link

hrhino commented Apr 17, 2018

Have we considered litotes? @notUnused isn't as strong as @used but also isn't the exact opposite of what I mean.

(serious part of the comment: I think this is a subset of "let's make silencer part of stdlib", though, if we want to take on that scope creep. Is there a reason it hasn't been done, other than no one wanting to?)

@som-snytt
Copy link

I gave a moment's thought to implementation while writing my previous comment, and hoped to try something out before the issue was marked good first issue. My momentary insight was that extends deprecation is not hokey, because if something is unused then you should not be using it, hence it is implicitly deprecated. I will check whether deprecation always silences unused warnings. This reasoning also settles the naming question.

Then this annotation also expresses deprecation-bc-unused more generally, so it is independent of message-suppression as such.

@som-snytt
Copy link

Sorry my excessive wordiness finally got on fommil's nerves. I'll take this up again, probably on Seth's line, with extra code to respect unused annotation.

@fommil
Copy link
Author

fommil commented Jul 6, 2018

@som-snytt unrelated 😄 I'm just closing all my scala related tickets due to a loss of interest in contributing to the language. I added scalaz.unused in any case, which is good enough for me.

@SethTisue
Copy link
Member

SethTisue commented Jul 9, 2018

@fommil if we reopen a particular ticket of yours — for example, this one — that someone expresses an interest in, is that okay with you? (or will you just close it again?)

@fommil
Copy link
Author

fommil commented Jul 9, 2018

sure, that's fine, I can always unsubscribe if it's noisy.

@som-snytt
Copy link

I can be pretty noisy. Sorry, I just did it again.

@dwijnand
Copy link
Member

This is going to be less easy to do with Scala 2.13's @deprecated becoming final: scala/scala#7449

@fommil

This comment has been minimized.

@He-Pin
Copy link
Contributor

He-Pin commented Dec 14, 2018

refs: scala/scala#7527

@fommil The problem is we are extremely overloading the usage of deprecated not the motivation itself a problem ok?.

suiryc added a commit to suiryc/suiryc-scala that referenced this issue Dec 19, 2018
Shorter - and more understandable - than @deprecated("unused", "").
See: scala/bug#10790
@som-snytt som-snytt self-assigned this Jan 9, 2019
@nilskp
Copy link

nilskp commented Jan 10, 2019

It would be nice if we could just use _ for unused parameters, like in pattern matching, but that's obviously way outside the scope of this ticket.

@fommil

This comment has been minimized.

@dwijnand
Copy link
Member

In Rust bindings that begin with an underscore are exempted, which is kind of nice: for example (_key, value) for when you only use the value of a key-value tuple.

@nilskp

This comment has been minimized.

@nilskp
Copy link

nilskp commented Jan 10, 2019

@dwijnand strikes me as unnecessary to have to name a parameter that you're not using. An underscore should be sufficient, no?

@dwijnand
Copy link
Member

Underscore is also available. But underscore-prefixed is convenient for cases like when you're only temporarily not using a variable (e.g commented out a code block) or for documentation/labelling purposes.

@fommil

This comment has been minimized.

@som-snytt
Copy link

@nilskp one issue (earlier in discussion) is that param name is part of API (due to named args). The ergonomics might also depend on why the param is unused (no longer used, or should be used in override, etc).

@fommil

This comment has been minimized.

@scala scala locked as off-topic and limited conversation to collaborators Jan 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants