From 4b21d2aa4e3ec83e6097bbcac339307c144b4043 Mon Sep 17 00:00:00 2001 From: antonioaversa Date: Thu, 3 Oct 2024 08:43:16 +0000 Subject: [PATCH] Create rule S7113: @immutable classes should only have const constructors (prefer_const_constructors_in_immutables) --- rules/S7113/dart/metadata.json | 24 ++++++ rules/S7113/dart/rule.adoc | 138 +++++++++++++++++++++++++++++++++ rules/S7113/metadata.json | 2 + 3 files changed, 164 insertions(+) create mode 100644 rules/S7113/dart/metadata.json create mode 100644 rules/S7113/dart/rule.adoc create mode 100644 rules/S7113/metadata.json diff --git a/rules/S7113/dart/metadata.json b/rules/S7113/dart/metadata.json new file mode 100644 index 00000000000..d0eed2713f8 --- /dev/null +++ b/rules/S7113/dart/metadata.json @@ -0,0 +1,24 @@ +{ + "title": "@immutable classes should only have const constructors", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "2min" + }, + "tags": [ + "performance" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-7113", + "sqKey": "S7113", + "scope": "All", + "defaultQualityProfiles": ["Sonar way"], + "quickfix": "unknown", + "code": { + "impacts": { + "RELIABILITY": "HIGH" + }, + "attribute": "EFFICIENT" + } +} diff --git a/rules/S7113/dart/rule.adoc b/rules/S7113/dart/rule.adoc new file mode 100644 index 00000000000..2488caec44d --- /dev/null +++ b/rules/S7113/dart/rule.adoc @@ -0,0 +1,138 @@ +Classes marked with the https://api.flutter.dev/flutter/meta/immutable-constant.html[`@immutable` annotation] should only have https://dart.dev/language/constructors#constant-constructors[`const` constructors]. + +== Why is this an issue? + +The `meta.dart` package defines an `@immutable` annotation that can be used to mark classes that are https://en.wikipedia.org/wiki/Immutable_object[immutable], meaning that all their instance variables, directly defined or inherited, have to be https://dart.dev/language/variables#final-and-const[`final`], that is, once the instance is created, it cannot be modified. + +However, like any other annotation, the `@immutable` annotation only provides information to the developer using the class and to developer tools such as the analyzer package, and it does not enforce any constraint. + +Therefore, there is nothing that prevents a developer from creating a mutable class and marking it as `@immutable`, as in the following example: + +[source,dart] +---- +import 'package:meta/meta.dart'; + +@immutable +class Point { + int x; + int y; + Point(this.x, this.y); +} + +void main() { + var p = Point(2, 3); + p.x = 4; + print(p.x); // Will print 4 +} +---- + +This is a problem because it can lead to confusion and bugs, as developers might expect the class to be immutable and not realize that it is actually not. + +The best way to prevent this from happening is to make sure that classes marked with the `@immutable` annotation only have https://dart.dev/language/constructors#constant-constructors[`const` constructors]. This will in turn ensure that all instance variables are `final`, hence the instance is truly immutable. + +[source,dart] +---- +import 'package:meta/meta.dart'; + +@immutable +class Point { + final int x; // Final required by the const constructor + final int y; // Final required by the const constructor + const Point(this.x, this.y); +} + +void main() { + const p = Point(2, 3); + // p.x = 4; // This will now be a compile-time error + print(p.x); +} +---- + +=== What is the potential impact? + +Even if the `@immutable` class is truly immutable, missing the `const` constraint on its constructors will lead to subpar performance, as the Dart compiler will not create compile-time constants in a non-constant context, and will raise a compile-time error in a const context. + +[source,dart] +---- +import 'package:meta/meta.dart'; + +@immutable +class Point { + final int x; + final int y; + Point(this.x, this.y); // Missing const +} + +void main() { + const p = Point(2, 3); // Compile-time error + const list = [Point(2, 3)]; // Compile-time error + var list = const [Point(2, 3)]; // Compile-time error +} +---- + +== How to fix it + +Add the constant constraint to each of the class's constructors, by adding the `const` keyword before the constructor's name. + +=== Code examples + +==== Noncompliant code example + +[source,dart,diff-id=1,diff-type=noncompliant] +---- +import 'package:meta/meta.dart'; + +@immutable +class Point { + final int x; + final int y; + Point(this.x, this.y); // Non compliant +} +---- + +==== Compliant solution + +[source,dart,diff-id=1,diff-type=compliant] +---- +import 'package:meta/meta.dart'; + +@immutable +class Point { + final int x; + final int y; + const Point(this.x, this.y); +} +---- + +== Resources + +=== Documentation + +* Dart Docs - https://dart.dev/tools/linter-rules/prefer_const_constructors_in_immutables[Dart Linter rule - prefer_const_constructors_in_immutables] +* Dart Docs - https://dart.dev/language/variables#final-and-const[Language - Final and const] +* Dart Docs - https://dart.dev/language/constructors#constant-constructors[Language - Constant constructors] +* Flutter Docs - https://api.flutter.dev/flutter/meta/immutable-constant.html[meta.dart - immutable top-level constant] +* Wikipedia - https://en.wikipedia.org/wiki/Immutable_object[Immutable object] + +=== Related rules + +* S7113 - Const constructors should be invoked with const + + +ifdef::env-github,rspecator-view[] + +''' +== Implementation Specification +(visible only on this page) + +=== Message + +Constructors in '@immutable' classes should be declared as 'const'. + +=== Highlighting + +The name identifier of the constructor: e.g. `Point` in `Point(this.x, this.y);`. + +In the case of an `extension type`, it's the name identifier of the `extension type` itself: e.g. `ExtensionTypeName` in `extension type ExtensionTypeName(int i)`. + +endif::env-github,rspecator-view[] diff --git a/rules/S7113/metadata.json b/rules/S7113/metadata.json new file mode 100644 index 00000000000..2c63c085104 --- /dev/null +++ b/rules/S7113/metadata.json @@ -0,0 +1,2 @@ +{ +}