Skip to content
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

Documentation on how to narrow types when using super parameters #50647

Open
kaboc opened this issue Dec 7, 2022 · 11 comments
Open

Documentation on how to narrow types when using super parameters #50647

kaboc opened this issue Dec 7, 2022 · 11 comments
Labels
area-documentation Prefer using 'type-documentation' and a specific area label.

Comments

@kaboc
Copy link
Contributor

kaboc commented Dec 7, 2022

class Foo {
  const Foo(this.value);

  final int? value;
}

class Bar extends Foo {
  const Bar(int value)
      : _value = value,
        super(value);

  final int _value;

  @override
  int get value => _value;
}

Bar accepts only a value of type int, while Foo accepts int?. The use_super_parameter rule warns there although it is intentional that I've avoided super.value so that the value of Bar never becomes null.

It is possible to suppress the warning with the // ignore: use_super_parameter comment, but it'll be better if the intention is considered. Is it too much for the analyzer to handle?

@asashour
Copy link
Contributor

asashour commented Dec 7, 2022

The lints are parts of the linter project, while the analyzer offers the fixes.

There is a fix to convert the code to

  const Bar(int super.value)
       : _value = value;

which would invoke super.value and ensure Bar is constructed with a not-nullable as well, so there is a way to use a super parameter while preserving the original semantic, so the lint is ok I guess.

@kaboc
Copy link
Contributor Author

kaboc commented Dec 7, 2022

Wow, I didn't know that. Thank you @asashour! I noticed just now that dart fix --apply handles it correctly. I should've tried it first.

It should definitely be documented in the language tour or somewhere.
(Can someone please transfer this issue to the linter project if the document is really missing?)

It is a pity that it only prevents the parameter from being nullable and I still have to add a getter to make the value available as a non-null type.

class Bar extends Foo {
  const Bar(int super.value) : _value = value;

  final int _value;

  @override
  int get value => _value; // This is still necessary.
}

@asashour
Copy link
Contributor

asashour commented Dec 7, 2022

From Visual Studio Code or IntelliJ, you can open the quick fixes for the diagnostics (if applicable), without using dart fix (as not all are offered as bulk), please have a look at this.

Regarding

int get value => _value;

value is nullable because it defined so in Foo, while _value is not because it is defined so in Bar, I don't think something can be done in this regard, as they are different fields with different types. However the language allows overriding a nullable field with a non-nullable, but not vice versa.

@kaboc
Copy link
Contributor Author

kaboc commented Dec 7, 2022

Yep, I actually used the quick fix feature on IntelliJ instead of using the dart fix --apply command directly.

I don't think something can be done in this regard,

I agree. It's just that I'm trying to do something unusual, so the linter is not the one to blame. I think the actual issue is only about documentation.

@kaboc kaboc changed the title use_super_parameters warns where a super parameter is intentionally avoided Documentation on how to narrow types when using super parameters Dec 7, 2022
@bwilkerson
Copy link
Member

@atsansone @MaryaBelanger For feedback on documentation.

@kaboc
Copy link
Contributor Author

kaboc commented Dec 7, 2022

It's just that I'm trying to do something unusual

On second thought, when you want the parameter to be non-null, I think you probably want the value to be accessed as non-null too. So the use case may not be so unusual.

@kevmoo kevmoo added the area-documentation Prefer using 'type-documentation' and a specific area label. label Dec 7, 2022
@MaryaBelanger
Copy link
Contributor

MaryaBelanger commented Dec 7, 2022

Since it's just docs, should we move the issue? I'm thinking either:

  • The linter repo
  • Or, the site-www repo
    • IF there needs to be some more long form concepts/instructions to support this, maybe with a combination of lint updates
    • I don't think the language tour is necessarily the spot, since this seems pretty specific (?), but we can come up with somewhere else for this info (the problem is really where does this type of info go, but we can discuss that when the issue moves) actually we don't have a better system at the moment, so the language tour is probably the best place

I don't understand the topic enough at this point to make the /linter or /site-www call, so if @bwilkerson or @kevmoo could make that call, I'll pick it up from there

@MaryaBelanger
Copy link
Contributor

@kaboc If you have a minute, it'd be really interesting and valuable to hear how you navigated the problem when it first arose (if you remember, no worries if not)

It seems like you noticed the quick fix after creating the issue, so before that and before creating this issue, did you get to look around the docs sites? For example,

  • Did you go right to the language tour?
    • Did you ctrl+F a certain word or phrase?
    • Did you skim the table of contents?
  • Did you follow links to other pages from there?
  • Did you search around api.dart.dev?
  • Did you use the search feature on dart.dev? How'd that go?

This is just to help make sure the info ends up where users will naturally go looking for it, so any insight helps! Thanks

@kaboc
Copy link
Contributor Author

kaboc commented Dec 8, 2022

@MaryaBelanger Thank you for the quick action. I appreciate the way you kindly listen to my experience to make improvements.

It was actually several months ago that I first needed to solve the issue. I had already learned by then how to use super parameters, so I skipped the language tour, thinking there wouldn't be newly added descriptions. I tried the quick fix then. Although I don't remember clearly, I think it didn't work the same way it does now or I misunderstood it didn't work.

So this was the second time I had needed a solution. I visited the language tour this time to see the latest info. It is the page I usually open first when I'm not sure about the grammar. I jump to more detailed pages from there.

Did you ctrl+F a certain word or phrase?

I searched in the page with "super." with a dot at the end because it is always used in the super.xxxxx form.

Did you skim the table of contents?

I only had a quick look at it to make sure the search navigated me to the relevant part of the page.

Did you follow links to other pages from there?

No, because I mainly looked at code pieces to find only the specific usage I needed. I would've read more closely and followed links if I had never read it before.

Did you search around api.dart.dev?

I didn't, because the issue was more about the grammar rather than the API. But I usually see the API doc when I want to learn a particular API.

Did you use the search feature on dart.dev? How'd that go?

Yes. I typed "super parameter" in the box in the top right corner of the language tour page. I believe the search box is not only for the tour page but for the entire dart.dev site.

@MaryaBelanger
Copy link
Contributor

@bwilkerson I opened a doc issue, if you want to close this. I'll keep track over there if I think changes to the lint need to be made.

@asashour qq: Where is the source of the quick fix associated with a lint? Is it written into the rule itself?

I looked here: use_super_parameters for the fix mentioned in your comment, but am not quite sure how to parse that...

@bwilkerson
Copy link
Member

The mapping between lints (or any other kind of diagnostic) and their fixes is in the class FixProcessor. Open the file and search for the lint name to find the entry in the map. That will give you the name of the class (in the analysis_server package) that implements the fix. If you're in an IDE you can navigate to the class directly. If you're just using GitHub, then append the name of the class, converted to snake case, and .dart to https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/lib/src/services/correction/dart/, in this case yielding https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/lib/src/services/correction/dart/convert_to_super_parameters.dart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-documentation Prefer using 'type-documentation' and a specific area label.
Projects
None yet
Development

No branches or pull requests

5 participants