dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.58k forks source link

proposal: `consider_const_constructor` #59001

Open KyleFin opened 1 year ago

KyleFin commented 1 year ago

consider_const_constructor

Description

Classes with no non-final fields may benefit from defining a const constructor.

Details

https://dart.dev/guides/language/effective-dart/design#consider-making-your-constructor-const-if-the-class-supports-it

Kind

Suggest potential optimization. Make authors deliberately choose to use const OR ignore lint. Point to useful Effective Dart reference to help them decide.

Bad Examples

class MyException extends Exception {}
class Square {
  Square(this.sideLength);

  Square.fromRadius(double radius) : sideLength = 2 * radius;

  final double sideLength;
}
class Square {
  Square(this.sideLength, this.color);

  final double sideLength;
  final Color color;
}

Good Examples

// Add a const constructor
class MyException extends Exception {
  const MyException();
}
// Make an existing constructor const
class Square {
  Square(this.sideLength);

  const Square.fromRadius(double radius) : sideLength = 2 * radius;

  final double sideLength;
}
// Explicitly ignore lint
class Square {
  // ignore: consider_const_constructor
  // Reason: we may someday change color to a getter.
  Square(this.sideLength, this.color);

  final double sideLength;
  final Color color;
}

Discussion

Background

I came upon this when reviewing some code using bloc. I assumed the default constructor was const and almost suggested removing the const constructor from an event class:

class TimerEnded extends EndingEvent {
  const TimerEnded();
}

// What I almost suggested:
class TimerEnded extends EndingEvent {}

I investigated more and learned that the default constructor is not const. Some resources I came across that helped me better understand why that's the case and when to use or not use const include:

Considerations

Discussion checklist

a14n commented 1 year ago

I'd rather go in this direction. I'd prefer an add_immutable_annotation lint that trigger on classes with only final fields. Thus there would be no overlapping with other lints and it could easily be used together with prefer_const_constructors_in_immutables.

incendial commented 1 year ago

Available in DCM https://dcm.dev/docs/rules/common/prefer-declaring-const-constructor/