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 <paulberry@google.com> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
committed by
dart-scoped@luci-project-accounts.iam.gserviceaccount.com
parent
8a965f86c1
commit
4d93f4d586
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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<void> {
|
||||
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<ClassMember> 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;
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
''');
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user