Handle private named parameters in generated docs.#4202
Conversation
|
Sorry for the redundancy with @lrhn's PR. I forgot that he did that a while back. :) |
lib/src/model/parameter.dart
Outdated
| // constructors here too. | ||
| if (element is FieldFormalParameterElement && element.hasPrivateName) { | ||
| // Use the public name. | ||
| return element.displayName.substring(1); |
There was a problem hiding this comment.
If possible, I would like us to get rid of the string manipulation and just get the public name directly from the analyzer. Given the docs on FieldFormalParameterElement the name property should give us the public name. (If it doesn't I think it's a bug in the analyzer)
There was a problem hiding this comment.
If possible, I would like us to get rid of the string manipulation and just get the public name directly from the analyzer.
Good point.
Given the docs on
FieldFormalParameterElementthenameproperty should give us the public name.
Oh, you're right it should. The fact that this is working is actually wrong. The name for a FieldFormalParameterElement declared with a private should be the public name only if the field formal is a valid private named parameter. For that to be true, the parameter needs to be in a library on the latest language version and when the "private-named-parameters" experiment is enabled. Neither of those is currently true for the dartdoc tests.
So the reason this test passed is because the this._x parameter was not a valid private named parameter, and thus name used the original private name, and thus hasPrivateName returns true. If the library was correctly using private named parameters, then the name would be x, and we wouldn't get into here at all. So this code does the wrong thing when private parameters are disabled and when they are enabled. Oops!
To fix this, I need to update the code here, but also:
- Change the dartdoc tests to enable the experiment and use the latest language version for the code being analyzed.
- Get a new version of analyzer published that exposes the
privateNamegetter we need to tell whether a field formal is a valid private named parameter or not. - Update dartdoc to use that new analyzer version.
I'll start a thread with analyzer folks about 2 and then update this PR.
b7ba288 to
7387f38
Compare
|
I'd want to land this anyway, because it works on positional parameters too. I'll bet the analyzer name-fiddling only provides the public name for named parameters, and then I'll have to do: Foo(int banana) : _banana = banana;for all eternity anyway. |
NP, this looks cleaner. (BTW, should we consider moving the |
I've certainly thought about this for dart_style. But I do like that:
|
The current state of this PR might work for positional parameters, but it's also broken. The correct fix is to use the new But that only works for private named parameters. For private positional parameters, the If we want dartdoc to remove the |
Private named parameters affect Dartdoc in two places:
* When generating docs with a comment reference that refers to a private
named parameter, use the corresponding public name.
* When generating a signature for a constructor with a private named
parameter, use the corresponding public name for the parameter in the
signature.
So given a class like:
```dart
class C {
int _x;
/// Make a new [C] with [_x].
C({required this._x});
}
```
Then the Dartdoc looks like:
> Make a new C with `x`.
>
> `C({required int x});`
Fix #4114.
7387f38 to
c04879d
Compare
c04879d to
3d1b1a3
Compare
|
Looks great, thank you! |
Private named parameters affect Dartdoc in two places:
When generating docs with a comment reference that refers to a private named parameter, use the corresponding public name.
When generating a signature for a constructor with a private named parameter, use the corresponding public name for the parameter in the signature.
So given a class like:
Then the Dartdoc looks like:
Fix #4114.