ceylon / ceylon-spec

DEPRECATED
Apache License 2.0
108 stars 34 forks source link

Give warning for unused destructured variables #1425

Open quintesse opened 8 years ago

quintesse commented 8 years ago

The following code:

void test() {
    value [x, y, z] = [ 1, 2, 3 ];
}

Does not result in warnings for unused variables x, y and z as expected.

lucaswerkmeister commented 8 years ago

But what if I only need x and y? It would be pretty annoying to get a warning about z then…

gavinking commented 8 years ago

That would be like the most annoying feature ever! Can I close this?

jvasileff commented 8 years ago

But what if I only need

Related note: it might signal an overuse of Tuples on my part, but I've wanted an "ignore this slot" a couple times. My code is currently like:

value [x, y, _] = [ 1, 2, 3 ];
value [a, __, c] = [ 1, 2, 3 ]; // can't use '_' again!

but would prefer something like:

value [x, y,] = [ 1, 2, 3 ];
value [a, ,c] = [ 1, 2, 3 ];
gavinking commented 8 years ago

Yeah @jvasileff same thing happens to me. I have toyed with the idea of adding a ... or something.

gdejohn commented 8 years ago

I'd like it if _ meant "don't bind" (like in Haskell) and could therefore be reused.

value [x, y, _] = [ 1, 2, 3 ];
value [a, _, _] = [ 1, 2, 3 ]; // fine to use '_' again!
gavinking commented 8 years ago

@gdejohn you're allowed to write:

value [a, *_] = [ 1, 2, 3 ];

Not sure if that helps ;-)

lucaswerkmeister commented 8 years ago

I don’t like the look of [x,y,] or [a,,c]… I’d prefer the special meaning for _.

quintesse commented 8 years ago

Together with the option to ignore specific items (not just a ...) this becomes a very useful feature indeed.

quintesse commented 8 years ago

So let's rename this issue to "allow tuple items to be ignored when destructuring" and have the warning as a consequence of implementing that.

jvasileff commented 8 years ago

:+1: on the special meaning for _.

lucaswerkmeister commented 8 years ago

Is the special meaning only in destructuring, or in general? (Could also be useful in, e. g., anonymous function params.)

jvasileff commented 8 years ago

I would say in general.

lucaswerkmeister commented 8 years ago

Me too, seems weird to have special variable names only in a few contexts. OTOH that’s quite a big language change…

jvasileff commented 8 years ago

Initially, I imagine it would work to only support _ for destructuring, and otherwise disallow it (certainly it could not also be a valid identifier after taking on a special meaning for destructuring.)

FroMage commented 8 years ago

I personally don't see why

value x = 2;

Would result in an "unused" warning, but

value [x] = [2];

Would not. It sounds so arbitrary and illogical.

I'd much rather have a special character for ignored bits, but I also think that's a feature which can wait for 1.3.

zamfofex commented 8 years ago

I myself don't like how _ looks in this situation. In my opinion it would be better to use a keyword, like ignore, or at least a regular-character-sized character, like #, or $...

gavinking commented 8 years ago

I would also prefer something other than _ FTR

jvasileff commented 8 years ago

I really dislike the underscore character. Sort of wish it were never invented. But in this case, I think it is the most straight forward and intuitive option, immediately obvious to new readers, and void of visual clutter (the reason it's intuitive.)

gdejohn commented 8 years ago

Underscore is used for this by many other languages: Scala, Haskell, F#, Erlang, ML, Prolog. I'm not aware of anyone who does it differently.

gavinking commented 8 years ago

I guess I could just give _ a special interpretation when it occurs as a pattern variable. I.e. Not interpret it as introducing a new variable.

pthariensflame commented 8 years ago

If you really wanted a different character than _, you could go with -. I don't think it would be ambiguous in context (at least, not as Ceylon's capabilities stand now), and it matches the mathematical convention of using dashes to indicate implicit abstraction.

luolong commented 8 years ago

Well, seeing how using _ as I don't care placeholder is already fairly established convention in many languages, I don't really mind formalizing this.

Also, I've found myself using exactly this convention in many cases where I want to ignore the variable.

Usually I've want to ignore just one argument though, so I never really stumbled upon a situation where I would feel that _ variables should not be bound to a variable, but that might just be a lucky incident.

I think easiest fix would probably be to treat identifiers consisting only of underscore characters specially in that they would not produce a warning of unused variables. We could revisit giving single underscore a special status some time later?

tombentley commented 8 years ago

For a long time I didn't know what _ meant in languages like scala, and it's not especially easy to google for. So I don't think it is terribly intuitive, even if it is somewhat conventional.

If I was writing this sort of code, I'd just use the name ignored (which generalises to ignored2 etc).

FroMage commented 8 years ago

Or:

value [first,,third,] = [1,2,3,4,5,6];

That is, skip the second and last elements.

luolong commented 8 years ago

Well, to be honest, ignored keyword would be more Ceylonic albeit a bit verbose...

Still I would vote for TC treating _ identifier specially as "do not check for unused variable".

quintesse commented 8 years ago

If I was writing this sort of code, I'd just use the name ignored

Sure, but that's just ignoring the whole discussion we're having here ;)

We have warnings for unused variables probably because people find them useful as warnings that you've forgotten something.

For the case of single values it's easy if you want to call a method and not do anything with its return value so you won't get any warnings: you just don't assign it to a variable (duh).

But you can't do that with tuples. So we've decided to just don't give warnings, which is the easy way out, but now we've lost our warning that tells as something might be wrong.

To me personally I don't see anything wrong with just leaving a gap: value [x, , z] = coords(), there are no symbols to learn and I think it's pretty intuitive for most people that it just means that nothing gets done with that value.

Edit: What @FroMage says in one line ;)

luolong commented 8 years ago

@quintesse, this is example looks somewhat nice, but it also opens up a chance for unintended errors. What if the extra comma is a typo?

luolong commented 8 years ago

What I want to say is, that we need some way for a developer to show intention to ignore the item between commas...

tombentley commented 8 years ago
value [first,,third,] = [1,2,3,4,5,6];

Seriously? It seems to me that that is just as likely to be a typo as it is intentional (especially the trailing comma one). I think that would be worse than the current situation.

lucaswerkmeister commented 8 years ago

I really don’t like the [first,,third,] idea. Especially if you write it like @FroMage does, without spaces, it’s very easy for a reader to miss out on the fact that there’s a name missing, and as @luolong says, it could also be a typo.

quintesse commented 8 years ago

@luolong well this is where I'd change @FroMage 's proposal a tiny bit, I wouldn't let a single empty space at the end match all the rest of the elements, so the number of names + gaps must still be the same as the right-hand value.

I'd suggest this:

value [first,,third,*] = [1,2,3,4,5,6];
luolong commented 8 years ago

As for discoverability, when we do not overload underscore with too many meanings, it is really very intuitive.

As the spec says, underscore is a perfectly valid identifier. There is nothing new to learn. Type Checker could detect that this variable is never used and silently ignore it altogether when generating bytecode.

quintesse commented 8 years ago

@lucaswerkmeister you might not "like" it, estethically speaking, but I think it's the only option that won't confuse people (not without taking away the ability to use _ as a variable in all cases) and doesn't make people try to google for the meaning of _ in Ceylon tuples.

quintesse commented 8 years ago

I mean when people start speaking about special meanings for _ or removing warnings for variables that start with underscores all kinds of warning bells start to go off.

luolong commented 8 years ago

The trouble with underscores in Scala for example is that it has too many meanings.

In Ceylon, an identifier consisting of just underscores is just another variable. Same as everything else.

As a convenient convention though, we can by default suppress any "unused" warnings for identifiers that consist of only underscores (or even more strictly, only a single underscore)

quintesse commented 8 years ago

we can by default suppress any "unused" warnings for identifiers that consist of only underscores (or even more strictly, only a single underscore)

And do things like:

value [one, two, _, __, ___, six] = [1, 2, 3, 4, 5, 6];

? yew.

FroMage commented 8 years ago

Yeah, I find _ more confusing than the [a,,b] syntax with holes. To me _ screams as a weird symbol that must have magical meaning but I don't quite know what it is. If it is a special wildcard character, how many items does it match? Can I have more than one? If yes, are they supposed to match the exact same value? As in: [a,b,a]?

The hole syntax does not have these questions. BUT I agree with @tombentley it may be written by mistake by a programmer who did not intend to write that double comma. Except that if we do strict number of items matching, for it to be a mistake you'd need to have two mistakes, because that would just not pass if you had the right number of variables and a comma mistake:

value [a,,b] = [1,2];

So you'd need to have a comma typo and a missing variable name or value for it to be an unnoticed mistake.

As for @quintesse's suggestion of requiring a * at the end rather than just a dangling comma, I think it is nicer as it is more explicit and differentiates ignoring a single last value or every remaining value.

gavinking commented 8 years ago

The typechecker would pick up any typo related to too many commas. It knows how many "slots" there are in a tuple.

OTOH that approach doesn't work well for entries.

FroMage commented 8 years ago

Relevant Perl manual:

Lists may be assigned to only when each element of the list is itself legal to assign to:

($a, $b, $c) = (1, 2, 3);
($map{'red'}, $map{'blue'}, $map{'green'}) = (0x00f, 0x0f0, 0xf00);

An exception to this is that you may assign to undef in a list. This is useful for throwing away some of the return values of a function:

($dev, $ino, undef, undef, $uid, $gid) = stat($file);

As of Perl 5.22, you can also use (undef)x2 instead of undef, undef . (You can also do ($x) x 2 , which is less useful, because it assigns to the same variable twice, clobbering the first value assigned.)

The final element of a list assignment may be an array or a hash:

    ($a, $b, @rest) = split;
    my($a, $b, %rest) = @_;

You can actually put an array or hash anywhere in the list, but the first one in the list will soak up all the values, and anything after it will become undefined. This may be useful in a my() or local().

FroMage commented 8 years ago

OTOH that approach doesn't work well for entries.

What's the special case there?

gavinking commented 8 years ago

What's the special case there?

Huh? There's no special case. We support for destructuring for entries wherever you can destructure a tuple.

I mean, I suppose this is sort of OK:

for (->item in map) { ... }

But I definitely don't love it.

FroMage commented 8 years ago

I don't think we have the same problem with entries, since they can only have two parts (so ignoring one is less common), and you can always use map.keys or map.items.

quintesse commented 8 years ago

@FroMage you really want to use Perl as a source of inspiration??? :wink:

FroMage commented 8 years ago

Well, the language as a whole has many good features, like C++ or Scala. It's the sum of the features that doesn't make them great languages, not their parts.

luolong commented 8 years ago

And do things like:

value [one, two, _, __, ___, six] = [1, 2, 3, 4, 5, 6];

? yew.

Well, if you throw away half of the items in a tuple, why are you using destructuring then?

Actually, the following would not look too ugly either:

value [one, two, ignore, ignore, ignore, six] = [1, 2, 3, 4, 5, 6];
lucaswerkmeister commented 8 years ago

I don’t like how ignore is longer and thus visually more prominent than the elements we actually care about.

luolong commented 8 years ago

True. I do prefer underscore for the same reason.

But this discussion is getting kind of religious ...

lucaswerkmeister commented 8 years ago

True. I think if we’re not going to make any radical changes before 1.2 (like [x,,y] syntax or changing the meaning of _ identifiers), then it’s best to keep the current behavior, and not warn about any unused destructured variables. KISS ;)

ePaul commented 8 years ago

Maybe as a compromise, warn when all of the variables are unused?

ncorai commented 7 years ago

Faced this in my code today and couldn't remember if the question had been settled. Apparently not.

So how about

value [one, two, void, void, void, six] = [1, 2, 3, 4, 5, 6];

I like the fact that void is both short and explicit, but I have no idea if such a use would trip the typechecker. And would the following be acceptable?

value [one, two, void] = [1, 2, 3, 4, 5, 6];