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 e2d4d04a2988..90204571b164 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 @@ -2360,6 +2360,8 @@ LintCode.sort_pub_dependencies: status: needsFix LintCode.sort_unnamed_constructors_first: status: hasFix +LintCode.specify_nonobvious_local_variable_types: + status: hasFix LintCode.test_types_in_equals: status: noFix LintCode.throw_in_finally: diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 7017f664b068..1ce517268873 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -612,6 +612,9 @@ final _builtInLintProducers = >{ LinterLintCode.sort_unnamed_constructors_first: [ SortUnnamedConstructorFirst.new, ], + LinterLintCode.specify_nonobvious_local_variable_types: [ + AddTypeAnnotation.bulkFixable, + ], LinterLintCode.type_annotate_public_apis: [ AddTypeAnnotation.bulkFixable, ], diff --git a/pkg/analysis_server/lib/src/services/linter/lint_names.dart b/pkg/analysis_server/lib/src/services/linter/lint_names.dart index d9216b6596d0..295f44197fff 100644 --- a/pkg/analysis_server/lib/src/services/linter/lint_names.dart +++ b/pkg/analysis_server/lib/src/services/linter/lint_names.dart @@ -148,6 +148,8 @@ abstract final class LintNames { static const String sort_constructors_first = 'sort_constructors_first'; static const String sort_unnamed_constructors_first = 'sort_unnamed_constructors_first'; + static const String specify_nonobvious_local_variable_types = + 'specify_nonobvious_local_variable_types'; static const String type_annotate_public_apis = 'type_annotate_public_apis'; static const String type_init_formals = 'type_init_formals'; static const String type_literal_in_constant_pattern = diff --git a/pkg/analysis_server/test/src/services/correction/fix/add_type_annotation_test.dart b/pkg/analysis_server/test/src/services/correction/fix/add_type_annotation_test.dart index 0ddc3454b3c5..91666dfa507a 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/add_type_annotation_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/add_type_annotation_test.dart @@ -19,6 +19,9 @@ void main() { defineReflectiveTests(PreferTypingUninitializedVariablesBulkTest); defineReflectiveTests(PreferTypingUninitializedVariablesInFileTest); defineReflectiveTests(PreferTypingUninitializedVariablesLintTest); + defineReflectiveTests(SpecifyNonObviousLocalVariableTypesBulkTest); + defineReflectiveTests(SpecifyNonObviousLocalVariableTypesInFileTest); + defineReflectiveTests(SpecifyNonObviousLocalVariableTypesLintTest); defineReflectiveTests(TypeAnnotatePublicAPIsBulkTest); defineReflectiveTests(TypeAnnotatePublicAPIsInFileTest); defineReflectiveTests(TypeAnnotatePublicAPIsLintTest); @@ -404,6 +407,171 @@ void f() { } } +@reflectiveTest +class SpecifyNonObviousLocalVariableTypesBulkTest extends BulkFixProcessorTest { + @override + String get lintCode => LintNames.specify_nonobvious_local_variable_types; + + Future test_bulk() async { + await resolveTestCode(''' +void main() { + final a = x; + var b = x; +} +int x = 1; +'''); + await assertHasFix(''' +void main() { + final int a = x; + int b = x; +} +int x = 1; +'''); + } +} + +@reflectiveTest +class SpecifyNonObviousLocalVariableTypesInFileTest + extends FixInFileProcessorTest { + Future test_File() async { + createAnalysisOptionsFile( + lints: [LintNames.specify_nonobvious_local_variable_types]); + await resolveTestCode(r''' +f() { + var x = g(0), y = g(''); + return (x, y); +} + +T g(T d) => d; +'''); + var fixes = await getFixesForFirstError(); + expect(fixes, hasLength(0)); + } +} + +@reflectiveTest +class SpecifyNonObviousLocalVariableTypesLintTest extends FixProcessorLintTest { + @override + FixKind get kind => DartFixKind.ADD_TYPE_ANNOTATION; + + @override + String get lintCode => LintNames.specify_nonobvious_local_variable_types; + + Future test_listPattern_destructured() async { + await resolveTestCode(''' +f() { + var [a] = [1, 1.5]; + print(a); +} +'''); + await assertHasFix(''' +f() { + var [num a] = [1, 1.5]; + print(a); +} +'''); + } + + Future test_local_variable() async { + await resolveTestCode(''' +int f() { + final f = x; + return f; +} +int x = 1; +'''); + await assertHasFix(''' +int f() { + final int f = x; + return f; +} +int x = 1; +'''); + } + + Future test_mapPattern_destructured() async { + await resolveTestCode(''' +f() { + var {'a': a} = {'a': x}; + print(a); +} +int x = 1; +'''); + await assertHasFix(''' +f() { + var {'a': int a} = {'a': x}; + print(a); +} +int x = 1; +'''); + } + + Future test_objectPattern_switch_final() async { + await resolveTestCode(''' +class A { + int a; + A(this.a); +} +var a = A(1); +f() { + switch (a) { + case A(a: >0 && final b): print(b); + } + } +'''); + await assertHasFix(''' +class A { + int a; + A(this.a); +} +var a = A(1); +f() { + switch (a) { + case A(a: >0 && final int b): print(b); + } + } +'''); + } + + Future test_recordPattern_switch() async { + await resolveTestCode(''' +f() { + switch ((1, x)) { + case (final a, int b): print(a); print(b); + } +} +int x = 2; +'''); + await assertHasFix(''' +f() { + switch ((1, x)) { + case (final int a, int b): print(a); print(b); + } +} +int x = 2; +'''); + } + + Future test_recordPattern_switch_var() async { + await resolveTestCode(''' +f() { + switch ((1, x)) { + case (int a, var b): print(a); print(b); + } +} +int x = 2; +'''); + await assertHasFix(''' +f() { + switch ((1, x)) { + case (int a, int b): print(a); print(b); + } +} +int x = 2; +'''); + } +} + @reflectiveTest class TypeAnnotatePublicAPIsBulkTest extends BulkFixProcessorTest { @override diff --git a/pkg/linter/example/all.yaml b/pkg/linter/example/all.yaml index aaa5e4d375f0..2cc46425714a 100644 --- a/pkg/linter/example/all.yaml +++ b/pkg/linter/example/all.yaml @@ -168,6 +168,7 @@ linter: - sort_constructors_first - sort_pub_dependencies - sort_unnamed_constructors_first + - specify_nonobvious_local_variable_types - test_types_in_equals - throw_in_finally - tighten_type_of_initializing_formals diff --git a/pkg/linter/lib/src/linter_lint_codes.dart b/pkg/linter/lib/src/linter_lint_codes.dart index 8e028a1bb73a..48e8c8f68c4a 100644 --- a/pkg/linter/lib/src/linter_lint_codes.dart +++ b/pkg/linter/lib/src/linter_lint_codes.dart @@ -975,7 +975,7 @@ class LinterLintCode extends LintCode { static const LintCode omit_obvious_local_variable_types = LinterLintCode( 'omit_obvious_local_variable_types', - "Unnecessary and obvious type annotation on a local variable.", + "Omit the type annotation on a local variable when the type is obvious.", correctionMessage: "Try removing the type annotation.", ); @@ -1462,6 +1462,13 @@ class LinterLintCode extends LintCode { "Try moving the unnamed constructor before all other constructors.", ); + static const LintCode specify_nonobvious_local_variable_types = + LinterLintCode( + 'specify_nonobvious_local_variable_types', + "Specify the type of a local variable when the type is non-obvious.", + correctionMessage: "Try adding a type annotation.", + ); + static const LintCode test_types_in_equals = LinterLintCode( 'test_types_in_equals', "Missing type test for '{0}' in '=='.", diff --git a/pkg/linter/lib/src/rules.dart b/pkg/linter/lib/src/rules.dart index 9a0f5f9db0eb..797fcdf4c857 100644 --- a/pkg/linter/lib/src/rules.dart +++ b/pkg/linter/lib/src/rules.dart @@ -181,6 +181,7 @@ import 'rules/slash_for_doc_comments.dart'; import 'rules/sort_child_properties_last.dart'; import 'rules/sort_constructors_first.dart'; import 'rules/sort_unnamed_constructors_first.dart'; +import 'rules/specify_nonobvious_local_variable_types.dart'; import 'rules/super_goes_last.dart'; import 'rules/test_types_in_equals.dart'; import 'rules/throw_in_finally.dart'; @@ -422,6 +423,7 @@ void registerLintRules() { ..register(SortPubDependencies()) ..register(SortUnnamedConstructorsFirst()) ..register(SuperGoesLast()) + ..register(SpecifyNonObviousLocalVariableTypes()) ..register(TestTypesInEquals()) ..register(ThrowInFinally()) ..register(TightenTypeOfInitializingFormals()) diff --git a/pkg/linter/lib/src/rules/omit_local_variable_types.dart b/pkg/linter/lib/src/rules/omit_local_variable_types.dart index e2ee2e6a5be4..927b65acc4b6 100644 --- a/pkg/linter/lib/src/rules/omit_local_variable_types.dart +++ b/pkg/linter/lib/src/rules/omit_local_variable_types.dart @@ -73,7 +73,10 @@ class OmitLocalVariableTypes extends LintRule { categories: {LintRuleCategory.style}); @override - List get incompatibleRules => const ['always_specify_types']; + List get incompatibleRules => const [ + 'always_specify_types', + 'specify_nonobvious_local_variable_types', + ]; @override LintCode get lintCode => LinterLintCode.omit_local_variable_types; diff --git a/pkg/linter/lib/src/rules/omit_obvious_local_variable_types.dart b/pkg/linter/lib/src/rules/omit_obvious_local_variable_types.dart index 738ba45db92f..b28533db1d0a 100644 --- a/pkg/linter/lib/src/rules/omit_obvious_local_variable_types.dart +++ b/pkg/linter/lib/src/rules/omit_obvious_local_variable_types.dart @@ -7,8 +7,8 @@ import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/type.dart'; import '../analyzer.dart'; -import '../extensions.dart'; import '../linter_lint_codes.dart'; +import '../util/obvious_types.dart'; const _desc = r'Omit obvious type annotations for local variables.'; @@ -24,7 +24,7 @@ variable type annotations that are obvious should be omitted. ```dart List> possibleDesserts(Set pantry) { List> desserts = >[]; - for (List recipe in cookbook) { + for (final List recipe in cookbook) { if (pantry.containsAll(recipe)) { desserts.add(recipe); } @@ -32,13 +32,15 @@ List> possibleDesserts(Set pantry) { return desserts; } + +const cookbook = >[....]; ``` **GOOD:** ```dart List> possibleDesserts(Set pantry) { var desserts = >[]; - for (List recipe in cookbook) { + for (final List recipe in cookbook) { if (pantry.containsAll(recipe)) { desserts.add(recipe); } @@ -46,6 +48,8 @@ List> possibleDesserts(Set pantry) { return desserts; } + +const cookbook = >[....]; ``` Sometimes the inferred type is not the type you want the variable to have. For @@ -74,9 +78,6 @@ or removed. Feedback on its behavior is welcome! The main issue is here: https://github.com/dart-lang/linter/issues/3480. '''; -bool _sameOrNull(DartType? t1, DartType? t2) => - t1 == null || t2 == null || t1 == t2; - class OmitObviousLocalVariableTypes extends LintRule { OmitObviousLocalVariableTypes() : super( @@ -152,148 +153,3 @@ class _Visitor extends SimpleAstVisitor { rule.reportLint(node.type); } } - -extension on CollectionElement { - DartType? get elementType { - var self = this; // Enable promotion. - switch (self) { - case MapLiteralEntry(): - return null; - case ForElement(): - // No need to compute the type of a non-obvious element. - return null; - case IfElement(): - // We just need a candidate type, ignore `else`. - return self.thenElement.elementType; - case Expression(): - return self.staticType; - case SpreadElement(): - return self.expression.staticType.elementTypeOfIterable; - case NullAwareElement(): - // This should be the non-nullable version of `self.value.staticType`, - // but since it requires computation, we return null. - return null; - } - } - - bool get hasObviousType { - var self = this; // Enable promotion. - switch (self) { - case MapLiteralEntry(): - return self.key.hasObviousType && self.value.hasObviousType; - case ForElement(): - return false; - case IfElement(): - return self.thenElement.hasObviousType && - (self.elseElement?.hasObviousType ?? true); - case Expression(): - return self.hasObviousType; - case SpreadElement(): - return self.expression.hasObviousType; - case NullAwareElement(): - return self.value.hasObviousType; - } - } - - DartType? get keyType { - var self = this; // Enable promotion. - switch (self) { - case MapLiteralEntry(): - return self.key.elementType; - default: - return null; - } - } - - DartType? get valueType { - var self = this; // Enable promotion. - switch (self) { - case MapLiteralEntry(): - return self.value.elementType; - default: - return null; - } - } -} - -extension on DartType? { - DartType? get elementTypeOfIterable { - var self = this; // Enable promotion. - if (self == null) return null; - if (self is InterfaceType) { - var iterableInterfaces = - self.implementedInterfaces.where((type) => type.isDartCoreIterable); - if (iterableInterfaces.length == 1) { - return iterableInterfaces.first.typeArguments.first; - } - } - return null; - } -} - -extension on Expression { - bool get hasObviousType { - var self = this; // Enable promotion. - switch (self) { - case TypedLiteral(): - if (self.typeArguments != null) { - // A collection literal with explicit type arguments is trivial. - return true; - } - // A collection literal with no explicit type arguments. - DartType? theObviousType, theObviousKeyType, theObviousValueType; - NodeList elements = switch (self) { - ListLiteral() => self.elements, - SetOrMapLiteral() => self.elements - }; - for (var element in elements) { - if (element.hasObviousType) { - theObviousType ??= element.elementType; - theObviousKeyType ??= element.keyType; - theObviousValueType ??= element.valueType; - if (!_sameOrNull(theObviousType, element.elementType) || - !_sameOrNull(theObviousKeyType, element.keyType) || - !_sameOrNull(theObviousValueType, element.valueType)) { - return false; - } - } else { - return false; - } - } - var theSelfElementType = self.staticType.elementTypeOfIterable; - return theSelfElementType == theObviousType; - case Literal(): - // An atomic literal: `Literal` and not `TypedLiteral`. - if (self is IntegerLiteral && - (self.staticType?.isDartCoreDouble ?? false)) { - return false; - } - return true; - case InstanceCreationExpression(): - var createdType = self.constructorName.type; - if (createdType.typeArguments != null) { - // Explicit type arguments provided. - return true; - } else { - DartType? dartType = createdType.type; - if (dartType != null) { - if (dartType is InterfaceType && - dartType.element.typeParameters.isNotEmpty) { - // A raw type is not trivial. - return false; - } - // A non-generic class or extension type. - return true; - } else { - // An unknown type is not trivial. - return false; - } - } - case CascadeExpression(): - return self.target.hasObviousType; - case AsExpression(): - return true; - } - return false; - } -} diff --git a/pkg/linter/lib/src/rules/specify_nonobvious_local_variable_types.dart b/pkg/linter/lib/src/rules/specify_nonobvious_local_variable_types.dart new file mode 100644 index 000000000000..3a80e77f4bb5 --- /dev/null +++ b/pkg/linter/lib/src/rules/specify_nonobvious_local_variable_types.dart @@ -0,0 +1,214 @@ +// Copyright (c) 2024, 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/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type.dart'; + +import '../analyzer.dart'; +import '../linter_lint_codes.dart'; +import '../util/obvious_types.dart'; + +const _desc = r'Specify non-obvious type annotations for local variables.'; + +const _details = r''' +Do type annotate initialized local variables when the type is non-obvious. + +Type annotations on local variables can serve as a request for type inference, +documenting the expected outcome of the type inference step, and declaratively +allowing the compiler and analyzer to solve the possibly complex task of +finding type arguments and annotations in the initializing expression that +yield the desired result. + +Type annotations on local variables can also inform readers about the type +of the initializing expression, which will allow them to proceed reading the +subsequent lines of code with known good information about the type of the +given variable (which may not be immediately evident by looking at the +initializing expression). + +An expression is considered to have a non-obvious type when it does not +have an obvious type. + +An expression e has an obvious type in the following cases: + +- e is a non-collection literal. For instance, 1, true, 'Hello, $name!'. +- e is a collection literal with actual type arguments. For instance, + {}. +- e is a list literal or a set literal where at least one element has an + obvious type, and all elements have the same type. For instance, [1, 2] and + { [true, false], [] }, but not [1, 1.5]. +- e is a map literal where all key-value pair have a key with an obvious type + and a value with an obvious type, and all keys have the same type, and all + values have the same type. For instance, { #a: [] }, but not + {1: 1, 2: true}. +- e is an instance creation expression whose class part is not raw. For + instance C(14) if C is a non-generic class, or C(14) if C accepts one + type argument, but not C(14) if C accepts one or more type arguments. +- e is a cascade whose target has an obvious type. For instance, + 1..isEven..isEven has an obvious type because 1 has an obvious type. +- e is a type cast. For instance, myComplexpression as int. + +**BAD:** +```dart +List> possibleDesserts(Set pantry) { + var desserts = genericFunctionDeclaredFarAway([42], 'Something'); + for (final recipe in cookbook) { + if (pantry.containsAll(recipe)) { + desserts.add(recipe); + } + } + + return desserts; +} + +const List> cookbook = ...; +``` + +**GOOD:** +```dart +List> possibleDesserts(Set pantry) { + List> desserts = genericFunctionDeclaredFarAway( + [42], + 'Something', + ); + for (final List recipe in cookbook) { + if (pantry.containsAll(recipe)) { + desserts.add(recipe); + } + } + + return desserts; +} + +const List> cookbook = ...; +``` + +**This rule is experimental.** It is being evaluated, and it may be changed +or removed. Feedback on its behavior is welcome! The main issue is here: +https://github.com/dart-lang/linter/issues/3480. +'''; + +class SpecifyNonObviousLocalVariableTypes extends LintRule { + SpecifyNonObviousLocalVariableTypes() + : super( + name: 'specify_nonobvious_local_variable_types', + description: _desc, + details: _details, + state: State.experimental(), + categories: {LintRuleCategory.style}); + + @override + List get incompatibleRules => const ['omit_local_variable_types']; + + @override + LintCode get lintCode => + LinterLintCode.specify_nonobvious_local_variable_types; + + @override + void registerNodeProcessors( + NodeLintRegistry registry, LinterContext context) { + var visitor = _Visitor(this); + registry.addForStatement(this, visitor); + registry.addPatternVariableDeclarationStatement(this, visitor); + registry.addSwitchStatement(this, visitor); + registry.addVariableDeclarationStatement(this, visitor); + } +} + +class _PatternVisitor extends GeneralizingAstVisitor { + LintRule rule; + _PatternVisitor(this.rule); + + @override + void visitDeclaredVariablePattern(DeclaredVariablePattern node) { + var staticType = node.type?.type; + if (staticType != null && + staticType is! DynamicType && + !staticType.isDartCoreNull) { + return; + } + rule.reportLint(node); + } +} + +class _Visitor extends SimpleAstVisitor { + final LintRule rule; + + _Visitor(this.rule); + + @override + void visitForStatement(ForStatement node) { + var loopParts = node.forLoopParts; + if (loopParts is ForPartsWithDeclarations) { + _visitVariableDeclarationList(loopParts.variables); + } else if (loopParts is ForEachPartsWithDeclaration) { + var loopVariableType = loopParts.loopVariable.type; + var staticType = loopVariableType?.type; + if (staticType != null && staticType is! DynamicType) { + return; + } + var iterable = loopParts.iterable; + if (iterable.hasObviousType) { + return; + } + rule.reportLint(loopParts.loopVariable, ignoreSyntheticNodes: false); + } + } + + @override + void visitPatternVariableDeclarationStatement( + PatternVariableDeclarationStatement node) { + if (node.declaration.expression.hasObviousType) return; + _PatternVisitor(rule).visitDartPattern(node.declaration.pattern); + } + + @override + void visitSwitchExpression(SwitchExpression node) {} + + @override + void visitSwitchStatement(SwitchStatement node) { + if (node.expression.hasObviousType) return; + for (SwitchMember member in node.members) { + if (member is SwitchPatternCase) { + _PatternVisitor(rule).visitSwitchPatternCase(member); + } + } + } + + @override + void visitVariableDeclarationStatement(VariableDeclarationStatement node) { + _visitVariableDeclarationList(node.variables); + } + + void _visitVariableDeclarationList(VariableDeclarationList node) { + var staticType = node.type?.type; + if (staticType != null && + staticType is! DynamicType && + !staticType.isDartCoreNull) { + return; + } + bool aDeclaredTypeIsNeeded = false; + var variablesThatNeedAType = []; + for (var child in node.variables) { + var initializer = child.initializer; + if (initializer == null) { + aDeclaredTypeIsNeeded = true; + variablesThatNeedAType.add(child); + } else { + if (!initializer.hasObviousType) { + aDeclaredTypeIsNeeded = true; + variablesThatNeedAType.add(child); + } + } + } + if (aDeclaredTypeIsNeeded) { + if (node.variables.length == 1) { + rule.reportLint(node); + } else { + // Multiple variables, report each of them separately. No fix. + variablesThatNeedAType.forEach(rule.reportLint); + } + } + } +} diff --git a/pkg/linter/lib/src/util/obvious_types.dart b/pkg/linter/lib/src/util/obvious_types.dart new file mode 100644 index 000000000000..5e7605075e77 --- /dev/null +++ b/pkg/linter/lib/src/util/obvious_types.dart @@ -0,0 +1,184 @@ +// Copyright (c) 2024, 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/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; + +import '../extensions.dart'; + +bool _sameOrNull(DartType? t1, DartType? t2) => + t1 == null || t2 == null || t1 == t2; + +extension CollectionElementExtensions on CollectionElement { + DartType? get elementType { + var self = this; // Enable promotion. + switch (self) { + case MapLiteralEntry(): + return null; + case ForElement(): + // No need to compute the type of a non-obvious element. + return null; + case IfElement(): + // We just need a candidate type, ignore `else`. + return self.thenElement.elementType; + case Expression(): + return self.staticType; + case SpreadElement(): + return self.expression.staticType.elementTypeOfIterable; + case NullAwareElement(): + // This should be the non-nullable version of `self.value.staticType`, + // but since it requires computation, we return null. + return null; + } + } + + bool get hasObviousType { + var self = this; // Enable promotion. + switch (self) { + case Expression(): + return self.hasObviousType; + case MapLiteralEntry(): + return self.key.hasObviousType && self.value.hasObviousType; + case IfElement(): + return self.thenElement.hasObviousType && + (self.elseElement?.hasObviousType ?? true); + case SpreadElement(): + return self.expression.hasObviousType; + case NullAwareElement(): + return self.value.hasObviousType; + case ForElement(): + return false; + } + } + + DartType? get keyType { + var self = this; // Enable promotion. + switch (self) { + case MapLiteralEntry(): + return self.key.elementType; + default: + return null; + } + } + + DartType? get valueType { + var self = this; // Enable promotion. + switch (self) { + case MapLiteralEntry(): + return self.value.elementType; + default: + return null; + } + } +} + +extension DartTypeExtensions on DartType? { + DartType? get elementTypeOfIterable { + var self = this; // Enable promotion. + if (self == null) return null; + if (self is InterfaceType) { + var iterableInterfaces = + self.implementedInterfaces.where((type) => type.isDartCoreIterable); + if (iterableInterfaces.length == 1) { + return iterableInterfaces.first.typeArguments.first; + } + } + return null; + } +} + +extension ExpressionExtensions on Expression { + bool get hasObviousType { + var self = this; // Enable promotion. + switch (self) { + case TypedLiteral(): + if (self.typeArguments != null) { + // A collection literal with explicit type arguments is trivial. + return true; + } + // A collection literal with no explicit type arguments. + DartType? theObviousType, theObviousKeyType, theObviousValueType; + NodeList elements = switch (self) { + ListLiteral() => self.elements, + SetOrMapLiteral() => self.elements + }; + for (var element in elements) { + if (element.hasObviousType) { + theObviousType ??= element.elementType; + theObviousKeyType ??= element.keyType; + theObviousValueType ??= element.valueType; + if (!_sameOrNull(theObviousType, element.elementType) || + !_sameOrNull(theObviousKeyType, element.keyType) || + !_sameOrNull(theObviousValueType, element.valueType)) { + return false; + } + } else { + return false; + } + } + var theSelfElementType = self.staticType.elementTypeOfIterable; + return theSelfElementType == theObviousType; + case RecordLiteral(): + for (var expression in self.fields) { + if (!expression.hasObviousType) return false; + } + return true; + case Literal(): + // An atomic literal: `Literal` and not `TypedLiteral`. + if (self is IntegerLiteral && + (self.staticType?.isDartCoreDouble ?? false)) { + return false; + } + return true; + case SimpleIdentifier(): + if (self.isQualified) return false; + var declaration = self.staticElement?.declaration; + if (declaration is! LocalVariableElement) return false; + return self.staticType == declaration.type; + case InstanceCreationExpression(): + var createdType = self.constructorName.type; + if (createdType.typeArguments != null) { + // Explicit type arguments provided. + return true; + } else { + DartType? dartType = createdType.type; + if (dartType != null) { + if (dartType is InterfaceType && + dartType.element.typeParameters.isNotEmpty) { + // A raw type is not trivial. + return false; + } + // A non-generic class or extension type. + return true; + } else { + // An unknown type is not trivial. + return false; + } + } + case CascadeExpression(): + return self.target.hasObviousType; + case AsExpression(): + return true; + case ConditionalExpression(): + if (self.thenExpression.hasObviousType && + self.elseExpression.hasObviousType) { + var staticTypeThen = self.thenExpression.elementType; + var staticTypeElse = self.elseExpression.elementType; + return staticTypeThen == staticTypeElse; + } + case IsExpression(): + return true; + case ThrowExpression(): + return true; + case ParenthesizedExpression(): + return self.expression.hasObviousType; + case ThisExpression(): + return true; + case TypeLiteral(): + return true; + } + return false; + } +} diff --git a/pkg/linter/test/rules/all.dart b/pkg/linter/test/rules/all.dart index 1c64946e03a6..8e46357f9e29 100644 --- a/pkg/linter/test/rules/all.dart +++ b/pkg/linter/test/rules/all.dart @@ -222,6 +222,8 @@ import 'sort_constructors_first_test.dart' as sort_constructors_first; import 'sort_pub_dependencies_test.dart' as sort_pub_dependencies; import 'sort_unnamed_constructors_first_test.dart' as sort_unnamed_constructors_first; +import 'specify_nonobvious_local_variable_types_test.dart' + as specify_nonobvious_local_variable_types; import 'test_types_in_equals_test.dart' as test_types_in_equals; import 'throw_in_finally_test.dart' as throw_in_finally; import 'tighten_type_of_initializing_formals_test.dart' @@ -461,6 +463,7 @@ void main() { sort_constructors_first.main(); sort_pub_dependencies.main(); sort_unnamed_constructors_first.main(); + specify_nonobvious_local_variable_types.main(); test_types_in_equals.main(); throw_in_finally.main(); tighten_type_of_initializing_formals.main(); diff --git a/pkg/linter/test/rules/omit_obvious_local_variable_types_test.dart b/pkg/linter/test/rules/omit_obvious_local_variable_types_test.dart index c82dfa306812..e5b402147349 100644 --- a/pkg/linter/test/rules/omit_obvious_local_variable_types_test.dart +++ b/pkg/linter/test/rules/omit_obvious_local_variable_types_test.dart @@ -73,9 +73,9 @@ f() { test_forEach_nonObviousIterable() async { await assertNoDiagnostics(r''' f() { - var list = [1, 2, 3]; for (int i in list) { } } +var list = [1, 2, 3]; '''); } diff --git a/pkg/linter/test/rules/specify_nonobvious_local_variable_types_test.dart b/pkg/linter/test/rules/specify_nonobvious_local_variable_types_test.dart new file mode 100644 index 000000000000..4d8a77960584 --- /dev/null +++ b/pkg/linter/test/rules/specify_nonobvious_local_variable_types_test.dart @@ -0,0 +1,358 @@ +// Copyright (c) 2024, 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:test_reflective_loader/test_reflective_loader.dart'; + +import '../rule_test_support.dart'; + +main() { + defineReflectiveSuite(() { + defineReflectiveTests(SpecifyNonObviousLocalVariableTypesTest); + }); +} + +@reflectiveTest +class SpecifyNonObviousLocalVariableTypesTest extends LintRuleTest { + @override + String get lintRule => 'specify_nonobvious_local_variable_types'; + + test_cascade() async { + await assertNoDiagnostics(r''' +f() { + var a = A()..x..x..x; +} + +class A { + final x = 0; +} +'''); + } + + test_forEach_inferredList() async { + await assertDiagnostics(r''' +f() { + for (var i in [1, 2, 'Hello'.length]) { + print(i); + } +} +''', [ + lint(13, 5), + ]); + } + + test_forEach_listWithNonObviousElement() async { + await assertDiagnostics(r''' +f() { + int j = "Hello".length; + for (var i in [j, 1, j + 1]) { } +} +''', [ + lint(39, 5), + ]); + } + + test_forEach_noDeclaredType() async { + await assertNoDiagnostics(r''' +f() { + for (var i in [1, 2, 3]) { } +} +'''); + } + + test_forEach_nonObviousIterable() async { + await assertNoDiagnostics(r''' +f() { + var list = [1, 2, 3]; + for (int i in list) { } +} +'''); + } + + test_forEach_typedList() async { + await assertNoDiagnostics(r''' +f() { + for (int i in [1, 2, 3]) { } +} +'''); + } + + test_genericInvocation_paramIsType() async { + await assertDiagnostics(r''' +String f() { + final h = bar(''); + return h; +} + +T bar(T d) => d; +''', [ + lint(15, 17), + ]); + } + + test_genericInvocation_paramIsType_ok() async { + await assertNoDiagnostics(r''' +String f() { + String h = bar(''); + return h; +} + +T bar(T d) => d; +'''); + } + + test_genericInvocation_typeNeededForInference() async { + await assertDiagnostics(r''' +f() { + var h = bar(''); + return h; +} + +T bar(dynamic d) => d; +''', [ + lint(8, 15), + ]); + } + + test_genericInvocation_typeNeededForInference_ok() async { + await assertNoDiagnostics(r''' +String f() { + String h = bar(''); + return h; +} + +T bar(dynamic d) => d; +'''); + } + + test_genericInvocation_typeParamProvided() async { + await assertDiagnostics(r''' +String f() { + var h = bar(''); + return h; +} + +T bar(dynamic d) => d; +''', [ + lint(15, 23), + ]); + } + + test_genericInvocation_typeParamProvided_ok() async { + await assertNoDiagnostics(r''' +String f() { + String h = bar(''); + return h; +} + +T bar(dynamic d) => d; +'''); + } + + test_instanceCreation_generic() async { + await assertDiagnostics(r''' +f() { + var a = A(1); +} + +class A { + A(X x); +} +''', [ + lint(8, 12), + ]); + } + + test_instanceCreation_generic_ok1() async { + await assertNoDiagnostics(r''' +f() { + A a = A(); +} + +class A {} +'''); + } + + test_instanceCreation_generic_ok2() async { + await assertNoDiagnostics(r''' +f() { + A a = A(); +} + +class A {} +'''); + } + + test_instanceCreation_nonGeneric() async { + await assertNoDiagnostics(r''' +f() { + A a = A(); +} + +class A {} +'''); + } + + test_literal_bool() async { + await assertNoDiagnostics(r''' +f() { + bool b = true; +} +'''); + } + + test_literal_double() async { + await assertNoDiagnostics(r''' +f() { + double d = 1.5; +} +'''); + } + + // The type is not obvious. + test_literal_doubleTypedInt() async { + await assertNoDiagnostics(r''' +f() { + double d = 1; +} +'''); + } + + test_literal_int() async { + await assertNoDiagnostics(r''' +f() { + int i = 1; +} +'''); + } + + // `Null` is not obvious, the inferred type is `dynamic`. + test_literal_null() async { + await assertNoDiagnostics(r''' +f() { + Null nil = null; +} +'''); + } + + test_literal_string() async { + await assertNoDiagnostics(r''' +f() { + String s = "A string"; +} +'''); + } + + test_literal_symbol() async { + await assertNoDiagnostics(r''' +f() { + Symbol s = #print; +} +'''); + } + + test_local_multiple() async { + await assertDiagnostics(r''' +f() { + var a = 'a' + 'a', b = 'b'.toString(); +} +''', [ + lint(12, 13), + lint(27, 18), + ]); + } + + test_local_multiple_ok() async { + await assertNoDiagnostics(r''' +f() { + String a = 'a' + 'a', b = 'b'.toString(); +} +'''); + } + + test_local_no_promotion() async { + await assertNoDiagnostics(r''' +f() { + num local = 2; + var x = local; + return x; +} +'''); + } + + test_local_promotion() async { + await assertDiagnostics(r''' +f() { + num local = 2; + if (local is! int) return; + var x = local; + return x; +} +''', [ + lint(54, 13), + ]); + } + + test_pattern_list_destructured() async { + await assertNoDiagnostics(r''' +f() { + var [int a] = [1]; +} +'''); + } + + test_pattern_map_destructured() async { + await assertNoDiagnostics(r''' +f() { + var {'a': int a} = {'a': 1}; +} +'''); + } + + test_pattern_object_destructured() async { + await assertNoDiagnostics(r''' +class A { + int a; + A(this.a); +} +f() { + final A(a: int _b) = A(1); +} +'''); + } + + test_pattern_record_destructured() async { + await assertNoDiagnostics(r''' +f(Object o) { + switch (o) { + case (int x, String s): + } +} +'''); + } + + test_switch_pattern_object() async { + await assertNoDiagnostics(r''' +class A { + int a; + A(this.a); +} + +f() { + switch (A(1)) { + case A(a: >0 && int b): + } +} +'''); + } + + test_switch_pattern_record() async { + await assertNoDiagnostics(r''' +f() { + switch ((1, 2)) { + case (int a, final int b): + } +} +'''); + } +} diff --git a/pkg/linter/tool/since/sdk.yaml b/pkg/linter/tool/since/sdk.yaml index 48141082dbce..ecba7e408c1a 100644 --- a/pkg/linter/tool/since/sdk.yaml +++ b/pkg/linter/tool/since/sdk.yaml @@ -175,6 +175,7 @@ sort_child_properties_last: 2.4.0 sort_constructors_first: 2.0.0 sort_pub_dependencies: 2.1.0 sort_unnamed_constructors_first: 2.0.0 +specify_nonobvious_local_variable_types: 3.6.0-wip super_goes_last: 2.0.0 test_types_in_equals: 2.0.0 throw_in_finally: 2.0.0