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

proposal: abstract_private_class_member #4984

Open
nate-thegrate opened this issue May 28, 2024 · 1 comment
Open

proposal: abstract_private_class_member #4984

nate-thegrate opened this issue May 28, 2024 · 1 comment
Labels
lint-proposal status-pending type-enhancement A request for a change that isn't a bug

Comments

@nate-thegrate
Copy link

nate-thegrate commented May 28, 2024

Description

Don't define an abstract private member within a public class.

Can be fixed by making the value public or adding a sealed or final modifier to the class.

Details

Kind: AVOID

Currently, the following produces zero static analysis warnings:

abstract base class A {
  void _foo();

  A() {
    _foo(); // always throws an error
  }
}

Marking a field as abstract signifies that "subclasses should implement this", so making it private entirely defeats the purpose.

Examples

abstract class A {
  // BAD
  int get _value;

  // BAD
  void _foo();

  // BAD
  abstract final bool _value;
}

// GOOD: class is private instead
abstract class _A {
  void foo();
}

// GOOD: class is marked as `sealed`
sealed class A {
  void _foo();
}

Discussion

If an abstract private field is added to a non-sealed public class, there are 3 possibilities:

  1. The unused_element rule is triggered
  2. It's referenced in a public scope (see proposal: private_field_outside_class #4983)
  3. It's referenced only within other private scope(s) in the same library

Even though possibility number 3 would never result in an error, I still think it'd be good for this rule to apply: a lack of sealed or final indicates that the class can be built upon outside the declaring library, so if the private field provides utility within the library, IMO it should either be refactored or exposed as a public field.

Probably low-priority

If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

I've never seen a class with an abstract private field.

@nate-thegrate
Copy link
Author

It turns out that there are a couple of abstract classes in the Flutter framework that benefit from this change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lint-proposal status-pending type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

2 participants