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 #57140

Open
nate-thegrate opened this issue May 28, 2024 · 2 comments
Open

proposal: abstract_private_class_member #57140

nate-thegrate opened this issue May 28, 2024 · 2 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request 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 field public or adding the sealed class modifier.

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 #57139)
  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!

@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 18, 2024
@dart-github-bot
Copy link
Collaborator

Summary: The proposal suggests a new lint rule to flag abstract private class members. These are considered useless as they can't be implemented by subclasses.

@dart-github-bot dart-github-bot added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Nov 18, 2024
@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package and removed triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Nov 18, 2024
@pq pq added the P3 A lower priority bug or feature request label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants