-
Notifications
You must be signed in to change notification settings - Fork 220
Extend private named parameters to apply to positional parameters too. #4486
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
base: main
Are you sure you want to change the base?
Conversation
To avoid breakage (which doesn't seem worth it to me), it's more conservative with how collisions in private positional parameters are handled in order to not be a breaking change. (I don't think the benefits to doc comments are enough to warrant a breaking change.) Fix #4479.
cc @srawlins and @kallentu if you have thoughts about this PR in relation to my comment here: #4479 (comment). |
If we take this change, is it worth renaming the feature and experiment flag to just "private parameters" or "private parameter names"? |
If the parameter initializes or declares an instance variable, the declared | ||
name determines the name of the corresponding instance variable. | ||
|
||
* A parameter's **access name** is the name that allows users of surrounding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure "access name" works for me. It feels like the name you use to read it, not to set (pass) it.
I'd just use "parameter name", the name of the parameter in the function type of the function/constructor.
A parameter declaration has a declared name, which is the syntactic identifier used to introduce the formal parameter. (Or it might not, if it's declared identifier is _
.)
Examples of parameters where name
is the declared name: name
, int name
, this.name
, final int name
, super.name
, and (for completeness, not encouragement) int name()
.
The declaration introduces a local parameter variable into the parameter scope (normal parameters only) and the initializer list scope (all kinds of parameter), bound to the argument value passed to that parameter.
The parameter variable's name is the same as the declared name.
Example where x
is in the initializer list scope and deltaY
is in the parameter
scope:
class Point {
final int x, y;
Point(this.x, this.y);
Point._offSetFromDiagonal(this.x, int deltaY) : y = x + deltaY {
if (deltaY == 0) log("Another diagonal point!: ($x, $x)");
}
}
The parameter declaration also introduces a function parameter to the constructors function signature. This name occurs in the function signature, but not in any scope, and it is the name used to refer to the parameter from outside of the constructor declaration. For named parameters, it is the name used to pass an argument at call sites. For positional parameters, the name is never used in code to denote the parameter outside of the function. Both can be referenced indirectly by tools that refer to source names, like DartDoc, in error messages, or while debugging.
Example with references to the names of value
and negated
:
extension type const N._(int value) {
/// Creates [N] with [value] as [N.value], negated if [negated] is true.
const N(int value, {bool negated = false})
: assert(!(value == 0 && negated),
"The 'value' must not be zero when 'negated' is true"),
value = negated ? -value : value;
}
For non-constructor parameters, there is also a lint to ensure that an overriding method has the same positional parameter names as the
function it overrides.
... and the parameter name of field parameters and initializing formals
is now not always the declared name ...
(Now I waxed didacticly again. I really need to learn to stop writing what I'm thinking. Or while I'm thinking. And I'm doing it again!)
Point is: "access name" feels like the opposite of "name used to pass an argument.
I like "parameter name" because that's what we already call the name of the parameter of a function from the outside, which is what this name is.
Use what you want of the above, or ignore it. I tend towards over-explaining things.
Today we use "parameter" and occasionally "parameter name" for internal local variables today, because we haven't made a distinction. Now we should make that distinction, so I suggest "parameter" and "parameter variable", and "parameter name" and "parameter variable name". And for field parameters also a "field name" of the instance variable declared by the field parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure "access name" works for me. It feels like the name you use to read it, not to set (pass) it.
Me either, but it was the best I could think of. I thought about it some more and came up with "visible name". I think that works well. It's not exactly the same as "public" (so we can distinguish a public identifier from a visible parameter), but it conveys pretty much the same thing.
(Now I waxed didacticly again. I really need to learn to stop writing what I'm thinking. Or while I'm thinking. And I'm doing it again!)
<3 :D
|
||
Given a positional parameter with private name *p* in formal parameter list L: | ||
|
||
* If *p* has corresponding public name *n* and no other parameter in L has |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And maybe also make it explicit that it's a compile-time error if any other parameter has declared name p.
(But it is true that every named parameter introduces a binding into the initializer list scope, for its private name, so we will get a "same name twice in one scope" error.)
|
||
Since a positional parameter can't be passed using a named argument, this | ||
doesn't affect the language semantics. It does mean that generated docs, error | ||
messages, inferred super parameter names, etc. should all use the public name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "inferred super parameter names" are the ones of anonymous mixin applications?
Maybe say that: "of mixin application classes". They're probably not specified as super-parameters, just as "forwarding constructors", so pedantically: "Names of parameters of forwarding constructors of mixin application classes".
Or maybe drop it, it's not an exhaustive list, and it's hard to explain.
parameters, regardless of whether they are initializing formals, declaring | ||
parameters, or even constructor parameters at all. This is because the language | ||
already allows using private names in all positional parameters, and we want a | ||
consistent user experience for all of them.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's a good idea. Maybe it's inconsistent that it doesn't work for other kinds of named parameters too. I could see myself wanting to write bar({int _foo = 0}) { ... }
to not shadow a surrounding foo
, while still getting foo
as parameter name.
I think it's an easier story to tell if it only works for initializing formals and field parameters, parameters whose name also refers to something else, and that other thing might want to be private. For everything else, if you choose a private name, it's because you wanted one, and you could just use a public name.
Given a positional parameter with private name *p* in formal parameter list L: | ||
|
||
* If *p* has corresponding public name *n* and no other parameter in L has | ||
declared name *n*, then the access name of the parameter is *n*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... , otherwise it's p.
And for all other parameter declarations, the parameter name is the declared name.
Can't fall back on defaults without an "otherwise", because we don't want to hit the default definition at all, which would says that the access name is the declared name. If we can hit that by simple fallthrough, then we'll also hit it from the clause that says the access name is n, and then it has two access names. Exhaust all the cases!
So, in total:
- If a named parameter is an initializing formal or field parameter with a private declared name p which has a corresponding public name n, then:
- It's a compile time error if any other parameter of the same constructor has declared name p.
- The access name of the parameter is n.
- If a positional parameter has a private declared name p with corresponding public name n, and no other parameter of the same function or constructor has declared name p or n,
- then the access name of the parameter is n.
- Otherwise the access name of the parameter is p.
- It's a compile-time error if two parameters of the same function or constructor have the same access name.
Is that about right?
list. *For example, an error related to calling a function with an incorrect | ||
parameter type.* | ||
|
||
Errors that refer to accessing the parameter from within the function or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... accessing the parameter variable ...
(to make a distinction between the parameter and its introduced variable.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left my main comment on the issue here, but for the record here, I am opposed to this change.
To avoid breakage (which doesn't seem worth it to me), it's more conservative with how collisions in private positional parameters are handled in order to not be a breaking change. (I don't think the benefits to doc comments are enough to warrant a breaking change.)
Fix #4479.