Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Give warning for unused destructured variables #4531

Open
CeylonMigrationBot opened this issue Sep 17, 2015 · 51 comments
Open

Give warning for unused destructured variables #4531

CeylonMigrationBot opened this issue Sep 17, 2015 · 51 comments

Comments

@CeylonMigrationBot
Copy link

[@quintesse] 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.

[Migrated from ceylon/ceylon-spec#1425]

@CeylonMigrationBot
Copy link
Author

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

@CeylonMigrationBot
Copy link
Author

[@gavinking] That would be like the most annoying feature ever! Can I close this?

@CeylonMigrationBot
Copy link
Author

[@jvasileff]

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 ];

@CeylonMigrationBot
Copy link
Author

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

@CeylonMigrationBot
Copy link
Author

[@gdejohn] 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!

@CeylonMigrationBot
Copy link
Author

[@gavinking]
@gdejohn you're allowed to write:

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

Not sure if that helps ;-)

@CeylonMigrationBot
Copy link
Author

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

@CeylonMigrationBot
Copy link
Author

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

@CeylonMigrationBot
Copy link
Author

[@quintesse] 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.

@CeylonMigrationBot
Copy link
Author

[@jvasileff] 👍 on the special meaning for _.

@CeylonMigrationBot
Copy link
Author

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

@CeylonMigrationBot
Copy link
Author

[@jvasileff] I would say in general.

@CeylonMigrationBot
Copy link
Author

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

@CeylonMigrationBot
Copy link
Author

[@jvasileff] 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.)

@CeylonMigrationBot
Copy link
Author

[@FroMage] 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.

@CeylonMigrationBot
Copy link
Author

[@Zambonifofex] 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 $...

@CeylonMigrationBot
Copy link
Author

[@gavinking] I would also prefer something other than _ FTR

@CeylonMigrationBot
Copy link
Author

[@jvasileff] 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.)

@CeylonMigrationBot
Copy link
Author

[@gdejohn] 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.

@CeylonMigrationBot
Copy link
Author

[@gavinking] 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.

@CeylonMigrationBot
Copy link
Author

[@pthariensflame] 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.

@CeylonMigrationBot
Copy link
Author

[@luolong] 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?

@CeylonMigrationBot
Copy link
Author

[@tombentley] 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).

@CeylonMigrationBot
Copy link
Author

[@FroMage] Or:

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

That is, skip the second and last elements.

@CeylonMigrationBot
Copy link
Author

[@luolong] 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".

@CeylonMigrationBot
Copy link
Author

[@quintesse]

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 ;)

@CeylonMigrationBot
Copy link
Author

[@luolong] @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?

@CeylonMigrationBot
Copy link
Author

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

@CeylonMigrationBot
Copy link
Author

[@tombentley]

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.

@CeylonMigrationBot
Copy link
Author

[@lucaswerkmeister] 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.

@CeylonMigrationBot
Copy link
Author

[@quintesse]
@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];

@CeylonMigrationBot
Copy link
Author

[@luolong] 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.

@CeylonMigrationBot
Copy link
Author

[@quintesse] @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.

@CeylonMigrationBot
Copy link
Author

[@quintesse] 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.

@CeylonMigrationBot
Copy link
Author

[@luolong] 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)

@CeylonMigrationBot
Copy link
Author

[@quintesse]

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.

@CeylonMigrationBot
Copy link
Author

[@FroMage] 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.

@CeylonMigrationBot
Copy link
Author

[@gavinking] 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.

@CeylonMigrationBot
Copy link
Author

[@FroMage] 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().

@CeylonMigrationBot
Copy link
Author

[@FroMage]

OTOH that approach doesn't work well for entries.

What's the special case there?

@CeylonMigrationBot
Copy link
Author

[@gavinking]

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.

@CeylonMigrationBot
Copy link
Author

[@FroMage] 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.

@CeylonMigrationBot
Copy link
Author

[@quintesse] @FroMage you really want to use Perl as a source of inspiration??? 😉

@CeylonMigrationBot
Copy link
Author

[@FroMage] 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.

@CeylonMigrationBot
Copy link
Author

[@luolong]

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];

@CeylonMigrationBot
Copy link
Author

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

@CeylonMigrationBot
Copy link
Author

[@luolong] True. I do prefer underscore for the same reason.

But this discussion is getting kind of religious ...

@CeylonMigrationBot
Copy link
Author

[@lucaswerkmeister] 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 ;)

@CeylonMigrationBot
Copy link
Author

[@ePaul] Maybe as a compromise, warn when all of the variables are unused?

@ncorai
Copy link

ncorai commented Sep 27, 2016

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];

@luolong
Copy link

luolong commented Sep 27, 2016

a question. Is currently this possible:

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

I think this would solve most of the "ugliness" of most patterns of ignoring some of the tuple elements.

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

4 participants