Skip to content

overridden_fields should not lint if more specific type is used #57548

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

Closed
a14n opened this issue Apr 20, 2017 · 7 comments
Closed

overridden_fields should not lint if more specific type is used #57548

a14n opened this issue Apr 20, 2017 · 7 comments
Assignees
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead.

Comments

@a14n
Copy link
Contributor

a14n commented Apr 20, 2017

The following code snippet from Flutter has a lint on FlutterError#message but shouldn't IMHO.

class AssertionError extends Error {
  final Object message;
  AssertionError([this.message]);
  String toString() => "Assertion failed";
}

class FlutterError extends AssertionError {
  FlutterError(this.message);
  @override
  final String message;
  @override
  String toString() => message;
}
@JPaulsen
Copy link
Contributor

Why not @a14n? The rule says:

DON'T override fields.

That means that should lint all overriden fields even if they are useful or not?

@a14n
Copy link
Contributor Author

a14n commented Apr 20, 2017

If I remove the field in the child class I loose the type information that message is a String.

BTW I can't find this rule in Effective Dart.

@zoechi
Copy link
Contributor

zoechi commented Apr 20, 2017

That was a DDC and strong-mode restriction that overriding fields was not supported, which was fixed recently.
I'm pretty sure there never was a Dart style guide rule.
It's still discouraged because of weird issues it can cause with memory usage and some code might see only the field in the superclass while other code might see the subclass' field.

@a14n
Copy link
Contributor Author

a14n commented Apr 20, 2017

Thanks for the explanation. I will try to refactor the code and perhaps close this issue.

@alexeieleusis
Copy link
Contributor

alexeieleusis commented Apr 20, 2017

This lint does not come from effective dart but rather from #25567 I don't think it is intended that rules come from effective dart.

I myself have had cases where I want to tighten the type on the extending class, can it be annotated in the base class? like with @checked of covariant, I honestly don't know, that's why I ask. Do we want any other signal than tightening the type? How will the constructors be affected in the derived class? Should that particular case be implemented as an abstract getter where the proposed annotations work?

@Hixie
Copy link
Contributor

Hixie commented Apr 20, 2017

The FlutterError code above is bogus. It's introducing a new field which means the object has two slots but should only have one.

It should just say:

   @override
   String get message => super.message;

...and the constructor should pass message up to the superclass.

When we wrote FlutterError, AssertionError didn't have a message field.

@a14n
Copy link
Contributor Author

a14n commented Apr 21, 2017

@Hixie thanks

@a14n a14n closed this as completed Apr 21, 2017
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead.
Projects
None yet
Development

No branches or pull requests

6 participants