dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
628 stars 170 forks source link

proposal: `prefer_abstract_final_over_extension_prohibiting_constructor` #4061

Open srawlins opened 1 year ago

srawlins commented 1 year ago

prefer_abstract_final_over_extension_prohibiting_constructor

Description

Today you can prevent a public class from being extended by adding a private constructor:

class Foo {
  Foo._();
}

Writing this singular private constructor means there is no default (implicit, unnamed, zero arg) constructor, and so the class cannot be extended outside of this library. The new way to achieve this behavior is to mark the class with final or interface.

Kind

Does this enforce style advice? Guard against errors? Other?

Style, I guess. It would let us mark any unused constructor as unused. Today, we never mark a private constructor as unused if it is the only constructor, as it might be there to prevent extension.

Bad Examples

class Foo {
  Foo._();
}

Good Examples

interface class Foo {}

Discussion

I think we can maybe look at how the class is currently used (within the library?) to determine what modifier(s) we would recommend (and fix to).

Discussion checklist

srawlins commented 1 year ago

CC @goderbauer who has real examples from the Flutter framework.

goderbauer commented 1 year ago

Two examples (I think there are more):

https://github.com/flutter/flutter/blob/0f397c08dc99c720bea06ff925106d9858547ee3/packages/flutter/lib/src/material/colors.dart#L194-L196

https://github.com/flutter/flutter/blob/e7b7ebc066c1b2a5aa5c19f8961307427e0142a6/packages/flutter/lib/src/services/system_sound.dart#L30-L32

For these, I think the suggestion should be to mark them "abstract final class". But not sure how to differentiate between what should be final and what should be interface.

bwilkerson commented 1 year ago

The table in this section of the proposal (https://github.com/dart-lang/language/blob/master/accepted/future-releases/class-modifiers/feature-specification.md#syntax) does a good job of describing the semantics of each modifier combination and might be useful.

goderbauer commented 1 year ago

Maybe the lint should be: "If the class only contains static members*, consider marking it abstract final"

*) Ideally, it would also recognize if the only non-static member is an empty private constructor and recommend removing it when marking the class abstract final to account for existing code patterns I linked above.

srawlins commented 1 year ago

I think in the analyzer cases, they we not classes which only declare static members. We direct users to avoid such classes in the first case. I don't see us adding lint rules that protect such classes.