-
Notifications
You must be signed in to change notification settings - Fork 205
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
Allow shadowing local final variables #3322
Comments
Extensions are an alternative We can write: final value = SomeThing()
.transform()
.otherTransform(); |
Or just don't make the variable final? What is the value of shadowing versus a mutable variable? Imo allowing this would just make it easy to accidentally shadow final variables that weren't meant to change (and they would effectively now be changed in the rest of the body). Note also that for most (but not all) uses of shadowing you can still access the shadowed variable (ie: |
Shadowing is a completely different thing thing than rebindable declaration. Var will only allow rebinding with object of same type. With shadowing every transformation can result in object of different type. And each declaration is final and deliberate so you won't rebind it by accident.
They wouldn't change. They just wouldn't be accessible. I've seen this argument many times, but having used languages that permit this, such as Elixir and Rust, I can't remember a single instance where that would be an actual problem in practice.
Indeed. That's part of the pattern and often a desirable thing. In the example above there is no reason to access the original value or the one after first transformation steps. More often than not that would happen by accident and be a bug. |
The variable could be typed
But shadowing is almost the same effect (I get that it is different, but it can lead to the same problems as accidental rebinding). To me this removes much of the value of In general I think shadowing is bad though, except in very narrow cases (such as shadowing an instance variable with a local one just to get field promotion, but even that can lead to issues). It is generally very error prone. So expanding it from my perspective is not a good thing.
Effectively, they have changed. All references later on now refer to a different value (sure it's an entirely different variable, but that is splitting hairs imo, the effect when reading the code is the same). If I see I have fixed bugs in the past that were a direct result of shadowing (in particular, local function parameters can shadow local variables which gets extremely confusing).
I am not sure how you would accidentally use a variable with a totally different type? If they have the same type just make it mutable. Or just use a transformation style like what @rrousselGit suggested via extensions or top level methods if you want to avoid polluting the namespace with variables that are only used once. |
Does this come from a practical experience? I'm asking because that's exactly what my assumption was until I've actually used languages that permit this and I can easily say that for me the ability to write functional style code with multiple shadowing transformations is much more useful than knowing that a final variable can never be shadowed.
That is confusing, but arguably more confusing and error prone than shadowing local variable.
Using extensions is much more limiting than shadowing variables, especially after adding destructuring and switch expressions. |
To a degree. This does not really play nicely with destructuring or switch expressions. |
An alternative would be #1246. Although not the same thing, I think they achieve the same? final value = SomeThing()
▷ transform
▷ otherTransform; |
FWIW, rust clippy has a let data = get_data();
let data = base64::decode(data); is fine while let data = get_data();
let data = c; prints a warning. |
That's very nice and would basically solve all potentials issues regarding shadowing. |
Shadowing makes a lot of sense in Rust because the language also has ownership semantics, basically a form of linear typing. In Rust, you can have functions that consume a value (which means the variable it's bound to can no longer be used). To play nice with that, it's idiomatic for functions to consume values and return new ones, instead of mutating values in place. When you have a lot of functions that consume values and produce new ones that represent the "same thing" after some transformation, it gets hard to keep coming up with new names for the thing. And, also, you can't use the untransformed one anymore either, since the borrow checker has told the compiler the old version is now dead. Given that, it makes a lot of sense for the language to allow you to shadow the previous variable with another of the same name. But none of that applies to Dart. There is no ownership system or linear types. Mutation is common and idiomatic. It's relatively rare for you to need multiple names for the same thing because most APIs let you just update the actual thing itself. Shadowing can lead to confusing code where a reader might not realize that a closure is accessing a different variable than one later in the block: test() {
var x = 'before';
closure() {
print(x);
}
var x = 'shadow';
x = 'assigned';
closure(); // Prints 'before'.
} You restrict shadowing to only final variables to avoid this, like the proposal suggests. But that would mean that changing a variable from final to non-final might require you to also rename it to prevent shadowing. Overall, I just don't think it's a net improvement in usability to allow shadowing within the same scope. |
Quite a bit of my code consists of immutable classes. Quite a bit of Flutter API is immutable. Immutable objects don't seems to be an uncommon paradigm.
If I wasn't experiencing this with Dart i wouldn't have opened the issue.
It may be dead in Rust, but it's not dead in elixir. I don't see borrow checker or linear types being preconditions to be able to shadow local variables. Consider a hypothetical example: final value = getBytes();
final value = sanitize(value);
final value = decode(value); Currently I need to write it like this: final value = getBytes();
final valueSanitized = sanitize(value);
final valueDecoded = decode(valueSanitized); It's very easy to call |
Shadowing help if I can't think of better names. This is the only thing that it solves. It has well known problems and all of them exists in Elixir. They're OK with them because Erlang's variables are invariable and immutable. They stripped of the invariable part because that was easy to strip and that's what their shadowing is. There's nothing functional about it. It tries to justify issues related with shadowing as "Well we've at least less problems than Erlang" which by the way is subjective because Erlang's problems are different and no-shadowing isn't one of them, and this statement cannot be used to justify why it's OK to give problems to users who don't even want shadowing. Enabling shadowing on final variables will defeat the purpose of making a variable final because users can easily introduce a rebinding even in the presence of the lint suggested in this thread. Enabling shadowing on var variables is a good idea. These are variables that can exhibit this behavior without an issue. But I think there are many users using var to mean final because final is too damn difficult to type so keep them in mind. Or Make shadowing explicit using a sigil or a modifier like In general, shadowing is helpful when I want to get things done fast but it also trips me up regularly because there's no way to disable it. Slapping problematic features on every single user is what most languages like to do. Erlang is one of the few languages in which you get both immutability and no-shadowing(it has function scope vars but there are warnings for them too). Erlang was never a popular one and it never will be one but it knows that shadowing gets in your way as much as it helps so it should be either explicit or completely disallowed. |
Solving naming is not small thing. But it's also not the only problem. With every transformation state being in scope it is easy to reference wrong state later in code by accident.
Even with shadowing you can't rebind final variable, only declare a new one. With the lint above you could only redeclare final variable when the initializer references previous variable with same name. That should be a pretty robust solution for redeclaring variable by accident.
|
I don't underestimate the problem so I should rephrase it: Shadowing doesn't solve anything. All it does is 'You can't think of names? use the same ones but at your own risk'.
Having 'Alicek03ros2c' and 'Alice' in the same room is better than having two 'Alice' then. You'll never remember the one you don't want to talk to. (I know shadowing help write your example clearly but having it everywhere implicitily is problematic)
Redeclaring a final variable isn't much different from rebinding it. final v = 1;
// ... some code
useV1(v); And later: final v = 1;
// ...
// someone else added code(they didn't realise that there's a 'v' below)
final v = plus(v); // int
final v = plusPlus(v); // int
useV2(v); // int
// ...
useV1(v); // now v is messed up
That's what you're asking for and some more. |
I like to use shadowing to keep my scope of accessible variables small. IE make variables I don't need anymore effectively inaccessible. This in combination with simpler names makes code easier to understand, not harder.
As already mentioned it's entirely different, because you can have different types while maintaining type safety + your IDE can understand that this is actually a different variable.
"There are only two hard problems in Computer Science: …" |
A child might find balancing on a bicycle as a "hard problem". There's nothing wrong with the child; rather, it's a design limitation in bicycles that everyone has to overcome with practice. You can add extra training wheels to mitigate the problem but making them permanent on every bicycle is problematic. It make bicycles inflexible and slow, especially for people who don't need them. Explicit shadowing(as roughly described in this thread) doesn't solve the problem because it won't help you come up with better names but at least it isn't taken from someone's book. It's mine and it's better because it not only mitigate the problem but also ensure that training wheels aren't permanent. I can extend it to functions, classes and every single thing where it's difficult to name but I lack the motivation to do that.
I don't read, enough. I just look for motives behind the first force before Newton see a force, action and reaction. If you've a motive, then you'll see a "prime mover" aka "unmoved mover" that'll either eliminate the problem or die trying. And if it turns out that the problem is inherent in your design, then you must be willing to accept radical changes to your design or they'll label you as the only problem in this universe. Saying 'X is hard' is easy and shows that "My only dream is to become hopeless". |
No doubt, training wheels is exactly what I need every now and then and they help in cases where one actually need them(like the example in this issue). In dynamic languages, one can go without them easily but that's a different topic. I've no issue if language decides to support them or not(I'm going to vote +1 because it's a valid request whether I like it or not), and all the different arguments appears to be valid from the person's prespective who is making them. I need to focus at work so going to unsub, apologies if I don't reply back. I'd like to briefly mention that if language is going to enable this in future(today or after ten years), then something worth considering is to enable shadowing on var variables and then allow those variables to be redeclared as final.// #1 ---------------------------
final value = getBytes();
final value = sanitize(value); // ERROR, final isn't allowed
// #2 ---------------------------
var value = getBytes();
var value = sanitize(value); // ALLOWED, 'value' is shadowed
// #3 ---------------------------
var value = getBytes();
var value = sanitize(value);
final value = decode(value); // 'value' is shadowed as final
value = 1; // ERROR
var value = ddecode(value); // ERROR
final value = ddecode(value); // ERROR It's slightly less-problematic than shadowing final variables. |
We didn't get a new pipe operator (as proposed in #1246), but something like the following might suffice, especially in a codebase where the given construct (here: // ----- Pipelines.
extension<X1, X2> on (X1, X2 Function(X1)) {
X2 operator ~() => $2($1);
}
extension<X1, X2, X3> on (X1, X2 Function(X1), X3 Function(X2)) {
X3 operator ~() => $3($2($1));
}
// And so on up to tuples with k components for some k.
// ----- Example.
String transform(int i) => '$i';
bool otherTransform(String s) => s.length < 3;
void main() {
final value = ~(
10,
transform,
otherTransform,
);
print(value); // 'true'.
} This doesn't allow for interleaving with other things: // Assuming that the proposal of this issue is accepted.
void main() {
final value = 10;
// ... lots of stuff.
final value = transform(value);
// ... lots of stuff.
final value = otherTransform(value);
} However, too much interleaved stuff would also make it hard to read, which would presumably exacerbate the dangers associated with shadowing. |
Typically after each transformation my value is of a different type, so I'm prevented at compile time from confusing my local variable with one of the shadowed ones of the same name. |
With Dart 3.0 it is now more viable than ever to write functional-style code. However the lack of support for shadowing local variable makes it problematic to write code like
Traditionally shadowing local variable in same scope may seem a bit contentious, but it is used in functional languages or even Rust. Right now after every transform it is necessary to come up with a separate name, with all previous values being in scope, which adds some friction and can be error-prone:
I think it would be acceptable if this was only possible for final bindings. Shadowing rebindable variables could still be an error.
EDIT: Possibly a lint or an error could be introduced where the shadowing variable would need to reference the original variable in the initializer:
This would would make it possible to use shadowing during transformations, but would prevent redefining the variable by accident.
The text was updated successfully, but these errors were encountered: