From 4d93f4d5863b37726339a3ba42d7f252defb11db Mon Sep 17 00:00:00 2001 From: Brian Wilkerson Date: Thu, 7 May 2026 13:46:54 -0700 Subject: [PATCH] Lint all cases in which a primary contructor could be used The purpose of this lint is to help automate testing of the primary constructors feature. It is unlikely to be shipped in its current form. The goal is to flag all cases where a secondary constructor could be converted to a primary constructor. This includes classes with a default constructor. There is an assist that should convert most of these cases, but it has not yet been enhanced to work as a fix. That will be done in a future CL. I want to implement the lint first so that I know all of the conditions that the fix needs to handle. The most important part of this review is to ensure that the tests are reasonably complete. If there are any missing cases where the lint should flag a constructor for conversion, or any missing cases where a conversion should not be requested, please let me know. Change-Id: Ib8256677a0659479ab341974b87bd03c953ff644 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500583 Reviewed-by: Paul Berry Commit-Queue: Brian Wilkerson --- .../services/correction/error_fix_status.yaml | 2 + pkg/linter/example/all.yaml | 1 + pkg/linter/lib/src/diagnostic.g.dart | 10 ++ pkg/linter/lib/src/lint_names.g.dart | 2 + pkg/linter/lib/src/rules.dart | 2 + .../src/rules/use_primary_constructors.dart | 133 ++++++++++++++ pkg/linter/messages.yaml | 11 ++ pkg/linter/test/rules/all.dart | 2 + .../rules/use_primary_constructors_test.dart | 162 ++++++++++++++++++ 9 files changed, 325 insertions(+) create mode 100644 pkg/linter/lib/src/rules/use_primary_constructors.dart create mode 100644 pkg/linter/test/rules/use_primary_constructors_test.dart diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml index 33f57c803fa..a3daa2ce078 100644 --- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml +++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml @@ -1690,6 +1690,8 @@ use_of_private_parameter_name: status: hasFix use_of_void_result: status: noFix +use_primary_constructors: + status: needsFix values_declaration_in_enum: status: noFix since: 2.17 diff --git a/pkg/linter/example/all.yaml b/pkg/linter/example/all.yaml index cac53e21002..e51fb7eab1c 100644 --- a/pkg/linter/example/all.yaml +++ b/pkg/linter/example/all.yaml @@ -227,6 +227,7 @@ linter: - use_late_for_private_fields_and_variables - use_named_constants - use_null_aware_elements + - use_primary_constructors - use_raw_strings - use_rethrow_when_possible - use_setters_to_change_properties diff --git a/pkg/linter/lib/src/diagnostic.g.dart b/pkg/linter/lib/src/diagnostic.g.dart index 5bbdd6ad257..34cf009e6d2 100644 --- a/pkg/linter/lib/src/diagnostic.g.dart +++ b/pkg/linter/lib/src/diagnostic.g.dart @@ -3733,6 +3733,16 @@ const LinterLintWithoutArguments useNullAwareElements = expectedTypes: [], ); +/// No parameters. +const LinterLintWithoutArguments usePrimaryConstructors = + LinterLintWithoutArguments( + name: 'use_primary_constructors', + problemMessage: "Use a primary constructor.", + correctionMessage: "Try using a primary constructor.", + uniqueName: 'use_primary_constructors', + expectedTypes: [], + ); + /// No parameters. const LinterLintWithoutArguments useRawStrings = LinterLintWithoutArguments( name: 'use_raw_strings', diff --git a/pkg/linter/lib/src/lint_names.g.dart b/pkg/linter/lib/src/lint_names.g.dart index fed6ae56a81..4a44fc78981 100644 --- a/pkg/linter/lib/src/lint_names.g.dart +++ b/pkg/linter/lib/src/lint_names.g.dart @@ -632,6 +632,8 @@ abstract final class LintNames { static const String use_null_aware_elements = 'use_null_aware_elements'; + static const String use_primary_constructors = 'use_primary_constructors'; + static const String use_raw_strings = 'use_raw_strings'; static const String use_rethrow_when_possible = 'use_rethrow_when_possible'; diff --git a/pkg/linter/lib/src/rules.dart b/pkg/linter/lib/src/rules.dart index c27f62d163a..6ee37786bb6 100644 --- a/pkg/linter/lib/src/rules.dart +++ b/pkg/linter/lib/src/rules.dart @@ -250,6 +250,7 @@ import 'rules/use_key_in_widget_constructors.dart'; import 'rules/use_late_for_private_fields_and_variables.dart'; import 'rules/use_named_constants.dart'; import 'rules/use_null_aware_elements.dart'; +import 'rules/use_primary_constructors.dart'; import 'rules/use_raw_strings.dart'; import 'rules/use_rethrow_when_possible.dart'; import 'rules/use_setters_to_change_properties.dart'; @@ -510,6 +511,7 @@ void registerLintRules() { ..registerLintRule(UseLateForPrivateFieldsAndVariables()) ..registerLintRule(UseNamedConstants()) ..registerLintRule(UseNullAwareElements()) + ..registerLintRule(UsePrimaryConstructors()) ..registerLintRule(UseRawStrings()) ..registerLintRule(UseRethrowWhenPossible()) ..registerLintRule(UseSettersToChangeProperties()) diff --git a/pkg/linter/lib/src/rules/use_primary_constructors.dart b/pkg/linter/lib/src/rules/use_primary_constructors.dart new file mode 100644 index 00000000000..8c7772109fe --- /dev/null +++ b/pkg/linter/lib/src/rules/use_primary_constructors.dart @@ -0,0 +1,133 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/analysis/features.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/error/error.dart'; + +import '../analyzer.dart'; +import '../diagnostic.dart' as diag; + +const _desc = r'Use a primary constructor.'; + +class UsePrimaryConstructors extends AnalysisRule { + UsePrimaryConstructors() + : super(name: LintNames.use_primary_constructors, description: _desc); + + @override + DiagnosticCode get diagnosticCode => diag.usePrimaryConstructors; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + if (!context.isFeatureEnabled(Feature.primary_constructors)) return; + var visitor = _Visitor(this); + registry.addClassDeclaration(this, visitor); + registry.addEnumDeclaration(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + final AnalysisRule rule; + + _Visitor(this.rule); + + @override + void visitClassDeclaration(ClassDeclaration node) { + // There can only be one primary constructor. + if (node.namePart is! PrimaryConstructorDeclaration) { + _checkMembers( + members: node.body.members, + containerName: node.namePart.typeName, + ); + } + } + + @override + void visitEnumDeclaration(EnumDeclaration node) { + // There can only be one primary constructor. + if (node.namePart is! PrimaryConstructorDeclaration) { + _checkMembers( + members: node.body.members, + containerName: node.namePart.typeName, + ); + } + } + + void _checkMembers({ + required Token containerName, + required List members, + }) { + var hasConstructor = false; + ConstructorDeclaration? root; + for (var member in members) { + if (member is ConstructorDeclaration) { + if (member.externalKeyword != null) { + // Classes with an external constructor can't be converted to use a + // primary constructor. + return; + } + hasConstructor = true; + if (member.factoryKeyword == null) { + if (member.redirect == null) { + if (root != null) { + // If there's more than one non-redirecting generative + // constructor, then none of them can be a primary constructor. + return; + } + root = member; + } + } + } + } + if (!hasConstructor) { + // Use an explicit primary constructor rather than a default constructor. + rule.reportAtToken(containerName); + return; + } + if (root == null) { + // If there aren't any non-redirecting constructors, then there's nothing + // to convert. + return; + } + // Otherwise, there's a single non-redirecting generative constructor, so it + // can be converted. + _reportConstructor(root); + } + + void _reportConstructor(ConstructorDeclaration constructor) { + var name = constructor.name; + if (name != null) { + rule.reportAtToken(name); + return; + } + var typeName = constructor.typeName; + if (typeName != null) { + rule.reportAtNode(typeName); + return; + } + var keyword = constructor.newKeyword; + if (keyword != null) { + rule.reportAtToken(keyword); + } + } +} + +extension on ConstructorDeclaration { + ConstructorElement? get redirect { + var initializer = initializers.lastOrNull; + if (initializer is RedirectingConstructorInvocation) { + return initializer.element; + } + return null; + } +} diff --git a/pkg/linter/messages.yaml b/pkg/linter/messages.yaml index 01cf15341de..43755608bd6 100644 --- a/pkg/linter/messages.yaml +++ b/pkg/linter/messages.yaml @@ -16917,6 +16917,17 @@ LinterLintCode: ```dart f(String? key) => {?key: "value"}; ``` + usePrimaryConstructors: + type: lint + parameters: none + problemMessage: "Use a primary constructor." + correctionMessage: "Try using a primary constructor." + state: + experimental: "3.13" + categories: [style] + hasPublishedDocs: false + deprecatedDetails: |- + Use a primary constructor everywhere it's valid to do so. useRawStrings: type: lint parameters: none diff --git a/pkg/linter/test/rules/all.dart b/pkg/linter/test/rules/all.dart index 5d2155a1ddb..32c6f2f99a5 100644 --- a/pkg/linter/test/rules/all.dart +++ b/pkg/linter/test/rules/all.dart @@ -324,6 +324,7 @@ import 'use_late_for_private_fields_and_variables_test.dart' as use_late_for_private_fields_and_variables; import 'use_named_constants_test.dart' as use_named_constants; import 'use_null_aware_elements_test.dart' as use_null_aware_elements; +import 'use_primary_constructors_test.dart' as use_primary_constructors; import 'use_raw_strings_test.dart' as use_raw_strings; import 'use_rethrow_when_possible_test.dart' as use_rethrow_when_possible; import 'use_setters_to_change_properties_test.dart' @@ -573,6 +574,7 @@ void main() { use_late_for_private_fields_and_variables.main(); use_named_constants.main(); use_null_aware_elements.main(); + use_primary_constructors.main(); use_raw_strings.main(); use_rethrow_when_possible.main(); use_setters_to_change_properties.main(); diff --git a/pkg/linter/test/rules/use_primary_constructors_test.dart b/pkg/linter/test/rules/use_primary_constructors_test.dart new file mode 100644 index 00000000000..bb16df8eb7b --- /dev/null +++ b/pkg/linter/test/rules/use_primary_constructors_test.dart @@ -0,0 +1,162 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +//import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../rule_test_support.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(UsePrimaryConstructorsInClassTest); + defineReflectiveTests(UsePrimaryConstructorsInEnumTest); + }); +} + +@reflectiveTest +class UsePrimaryConstructorsInClassTest extends LintRuleTest { + @override + String get lintRule => LintNames.use_primary_constructors; + + test_class_withDefaultConstructor() async { + await assertDiagnosticsFromMarkdown(r''' +class [!C!]; +'''); + } + + test_class_withExternalConstructor() async { + await assertNoDiagnostics(r''' +class C { + external C(); +} +'''); + } + + test_class_withFactory() async { + await assertDiagnosticsFromMarkdown(r''' +class C { + C.[!a!](); + factory C.b() => C.a(); +} +'''); + } + + test_class_withMultipleLevelsOfRedirect() async { + await assertDiagnosticsFromMarkdown(r''' +class C { + C.[!a!](); + C.b() : this.a(); + C.c() : this.b(); +} +'''); + } + + test_class_withMultipleRoots() async { + await assertNoDiagnostics(r''' +class C { + C.a(); + C.b(); +} +'''); + } + + test_class_withPrimaryConstructor() async { + await assertNoDiagnostics(r''' +class C(); +'''); + } + + test_class_withSingleGenerativeConstructor() async { + await assertDiagnosticsFromMarkdown(r''' +class C { + [!C!](); +} +'''); + } + + test_class_withSingleLevelOfRedirect() async { + await assertDiagnosticsFromMarkdown(r''' +class C { + C.[!a!](); + C.b() : this.a(); +} +'''); + } +} + +@reflectiveTest +class UsePrimaryConstructorsInEnumTest extends LintRuleTest { + @override + String get lintRule => LintNames.use_primary_constructors; + + test_enum_withDefaultConstructor() async { + await assertDiagnosticsFromMarkdown(r''' +enum [!E!] { + a, b, c +} +'''); + } + + test_enum_withFactory() async { + await assertDiagnosticsFromMarkdown(r''' +enum E { + a, b, c; + + [!E!](); + factory E.f() => b; +} +'''); + } + + test_enum_withMultipleLevelsOfRedirect() async { + await assertDiagnosticsFromMarkdown(r''' +enum E { + a.e(), b.f(), c.g(); + + E.[!e!](); + E.f() : this.e(); + E.g() : this.f(); +} +'''); + } + + test_enum_withPrimaryConstructor() async { + await assertNoDiagnostics(r''' +enum E() { + a, b, c; +} +'''); + } + + test_enum_withSingleGenerativeConstructor() async { + await assertDiagnosticsFromMarkdown(r''' +enum E { + a, b, c; + + [!E!](); +} +'''); + } + + test_enum_withSingleLevelOfRedirect() async { + await assertDiagnosticsFromMarkdown(r''' +enum E { + a.e(), b.f(); + + E.[!e!](); + E.f() : this.e(); +} +'''); + } + + test_enums_withExternalConstructor() async { + await assertNoDiagnostics(r''' +enum E { + a, b, c; + + external E(); +} +'''); + } +}