Skip to content

Commit c9391d7

Browse files
eernstgCommit Queue
authored andcommitted
Set obvious/property lints to hasFix
The lints `omit_obvious_property_types` and `specify_nonobvious_property_types` previously had error fix status 'needsEvaluation'. This CL changes it to 'hasFix'. This is needed in order to get access to the fix in IntelliJ. This CL also corrects a bug whereby a map literal was considered to have an obvious type in cases where there were no actual type arguments and no elements with an obvious type. Change-Id: Id9305152d086604b78636b696bd97e318ed6bdb5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393280 Commit-Queue: Erik Ernst <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent 92663b7 commit c9391d7

File tree

10 files changed

+240
-27
lines changed

10 files changed

+240
-27
lines changed

pkg/analysis_server/lib/src/services/correction/dart/replace_with_var.dart

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ class ReplaceWithVar extends ResolvedCorrectionProducer {
4444
var grandparent = parent?.parent;
4545
if (parent is VariableDeclarationList &&
4646
(grandparent is VariableDeclarationStatement ||
47-
grandparent is ForPartsWithDeclarations)) {
47+
grandparent is ForPartsWithDeclarations ||
48+
grandparent is TopLevelVariableDeclaration ||
49+
grandparent is FieldDeclaration)) {
4850
var variables = parent.variables;
4951
if (variables.length != 1) {
5052
return;
@@ -159,6 +161,10 @@ class ReplaceWithVar extends ResolvedCorrectionProducer {
159161
return true;
160162
}
161163
return false;
164+
} else if (parent is TopLevelVariableDeclaration) {
165+
return _canConvertVariableDeclarationList(parent.variables);
166+
} else if (parent is FieldDeclaration) {
167+
return _canConvertVariableDeclarationList(parent.fields);
162168
}
163169
parent = parent.parent;
164170
}

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2199,7 +2199,7 @@ LintCode.omit_local_variable_types:
21992199
LintCode.omit_obvious_local_variable_types:
22002200
status: hasFix
22012201
LintCode.omit_obvious_property_types:
2202-
status: needsEvaluation
2202+
status: hasFix
22032203
LintCode.one_member_abstracts:
22042204
status: noFix
22052205
notes: |-
@@ -2362,7 +2362,7 @@ LintCode.sort_unnamed_constructors_first:
23622362
LintCode.specify_nonobvious_local_variable_types:
23632363
status: hasFix
23642364
LintCode.specify_nonobvious_property_types:
2365-
status: needsEvaluation
2365+
status: hasFix
23662366
LintCode.test_types_in_equals:
23672367
status: noFix
23682368
LintCode.throw_in_finally:

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,7 @@ final _builtInLintProducers = <LintCode, List<ProducerGenerator>>{
378378
LinterLintCode.null_closures: [ReplaceNullWithClosure.new],
379379
LinterLintCode.omit_local_variable_types: [ReplaceWithVar.new],
380380
LinterLintCode.omit_obvious_local_variable_types: [ReplaceWithVar.new],
381+
LinterLintCode.omit_obvious_property_types: [ReplaceWithVar.new],
381382
LinterLintCode.prefer_adjacent_string_concatenation: [RemoveOperator.new],
382383
LinterLintCode.prefer_collection_literals: [
383384
ConvertToMapLiteral.new,
@@ -460,6 +461,9 @@ final _builtInLintProducers = <LintCode, List<ProducerGenerator>>{
460461
LinterLintCode.specify_nonobvious_local_variable_types: [
461462
AddTypeAnnotation.bulkFixable,
462463
],
464+
LinterLintCode.specify_nonobvious_property_types: [
465+
AddTypeAnnotation.bulkFixable,
466+
],
463467
LinterLintCode.type_annotate_public_apis: [AddTypeAnnotation.bulkFixable],
464468
LinterLintCode.type_init_formals: [RemoveTypeAnnotation.other],
465469
LinterLintCode.type_literal_in_constant_pattern: [
@@ -1085,8 +1089,8 @@ final _builtInNonLintProducers = <ErrorCode, List<ProducerGenerator>>{
10851089
// updated so that only the appropriate subset is generated.
10861090
QualifyReference.new,
10871091
],
1088-
CompileTimeErrorCode
1089-
.UNQUALIFIED_REFERENCE_TO_STATIC_MEMBER_OF_EXTENDED_TYPE: [
1092+
CompileTimeErrorCode.UNQUALIFIED_REFERENCE_TO_STATIC_MEMBER_OF_EXTENDED_TYPE:
1093+
[
10901094
// TODO(brianwilkerson): Consider adding fixes to create a field, getter,
10911095
// method or setter. The existing producers would need to be updated so
10921096
// that only the appropriate subset is generated.

pkg/analysis_server/test/src/services/correction/fix/replace_with_var_test.dart

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ void main() {
1515
defineReflectiveTests(OmitLocalVariableTypesLintTest);
1616
defineReflectiveTests(OmitObviousLocalVariableTypesLintBulkTest);
1717
defineReflectiveTests(OmitObviousLocalVariableTypesLintTest);
18+
defineReflectiveTests(OmitObviousPropertyTypesLintBulkTest);
19+
defineReflectiveTests(OmitObviousPropertyTypesLintTest);
1820
});
1921
}
2022

@@ -654,3 +656,187 @@ String f() {
654656
''');
655657
}
656658
}
659+
660+
@reflectiveTest
661+
class OmitObviousPropertyTypesLintBulkTest extends BulkFixProcessorTest {
662+
@override
663+
String get lintCode => LintNames.omit_obvious_property_types;
664+
665+
Future<void> test_singleFile() async {
666+
await resolveTestCode('''
667+
List<String> l = ['a'];
668+
669+
class A {
670+
static List<String> l = ['a'];
671+
}
672+
''');
673+
await assertHasFix('''
674+
var l = <String>['a'];
675+
676+
class A {
677+
static var l = <String>['a'];
678+
}
679+
''');
680+
}
681+
}
682+
683+
@reflectiveTest
684+
class OmitObviousPropertyTypesLintTest extends FixProcessorLintTest {
685+
@override
686+
FixKind get kind => DartFixKind.REPLACE_WITH_VAR;
687+
688+
@override
689+
String get lintCode => LintNames.omit_obvious_property_types;
690+
691+
Future<void> test_generic_instanceCreation_cascade() async {
692+
await resolveTestCode('''
693+
Set<String> s = Set<String>()..addAll([]);
694+
''');
695+
await assertHasFix('''
696+
var s = Set<String>()..addAll([]);
697+
''');
698+
}
699+
700+
Future<void> test_generic_instanceCreation_withArguments() async {
701+
await resolveTestCode('''
702+
final C<int> c = C<int>();
703+
704+
class C<T> {}
705+
''');
706+
await assertHasFix('''
707+
final c = C<int>();
708+
709+
class C<T> {}
710+
''');
711+
}
712+
713+
Future<void> test_generic_listLiteral() async {
714+
await resolveTestCode('''
715+
List<num> l = <num>[x];
716+
717+
int x = 2 + 'Not obvious'.length;
718+
''');
719+
await assertHasFix('''
720+
var l = <num>[x];
721+
722+
int x = 2 + 'Not obvious'.length;
723+
''');
724+
}
725+
726+
Future<void> test_generic_listLiteral_const() async {
727+
await resolveTestCode('''
728+
const List<String> values = const ['a'];
729+
''');
730+
await assertHasFix('''
731+
const values = const <String>['a'];
732+
''');
733+
}
734+
735+
Future<void> test_generic_mapLiteral() async {
736+
await resolveTestCode('''
737+
Map<String, double> m = {'a': 1.5};
738+
''');
739+
await assertHasFix('''
740+
var m = <String, double>{'a': 1.5};
741+
''');
742+
}
743+
744+
Future<void> test_generic_mapLiteral_const() async {
745+
await resolveTestCode('''
746+
const Map<String, double> m = {'a': 1.5};
747+
''');
748+
await assertHasFix('''
749+
const m = <String, double>{'a': 1.5};
750+
''');
751+
}
752+
753+
Future<void> test_generic_setLiteral() async {
754+
await resolveTestCode('''
755+
Set<double> s = {1.5};
756+
''');
757+
await assertHasFix('''
758+
var s = <double>{1.5};
759+
''');
760+
}
761+
762+
Future<void> test_generic_setLiteral_cascade() async {
763+
await resolveTestCode('''
764+
Set<String> s = {'a'}..addAll([]);
765+
''');
766+
await assertHasFix('''
767+
var s = <String>{'a'}..addAll([]);
768+
''');
769+
}
770+
771+
Future<void> test_generic_setLiteral_const() async {
772+
await resolveTestCode('''
773+
const Set<String> s = const {'a'};
774+
''');
775+
await assertHasFix('''
776+
const s = const <String>{'a'};
777+
''');
778+
}
779+
780+
Future<void> test_generic_setLiteral_recordType() async {
781+
await resolveTestCode('''
782+
Set<(double, double)> s = {(1.5, 2.5)};
783+
''');
784+
await assertHasFix('''
785+
var s = <(double, double)>{(1.5, 2.5)};
786+
''');
787+
}
788+
789+
Future<void> test_simple() async {
790+
await resolveTestCode('''
791+
String s = '';
792+
''');
793+
await assertHasFix('''
794+
var s = '';
795+
''');
796+
}
797+
798+
Future<void> test_simple_const() async {
799+
await resolveTestCode('''
800+
const String s = '';
801+
''');
802+
await assertHasFix('''
803+
const s = '';
804+
''');
805+
}
806+
807+
Future<void> test_simple_final() async {
808+
await resolveTestCode('''
809+
final String s = '';
810+
''');
811+
await assertHasFix('''
812+
final s = '';
813+
''');
814+
}
815+
816+
Future<void> test_simple_recordType() async {
817+
await resolveTestCode(r'''
818+
(double, String) r = (3.5, '');
819+
''');
820+
await assertHasFix(r'''
821+
var r = (3.5, '');
822+
''');
823+
}
824+
825+
Future<void> test_top_level() async {
826+
await resolveTestCode('''
827+
List<String> list = ['a'];
828+
''');
829+
await assertHasFix('''
830+
var list = <String>['a'];
831+
''');
832+
}
833+
834+
Future<void> test_top_level_final() async {
835+
await resolveTestCode('''
836+
final List<String> list = ['a'];
837+
''');
838+
await assertHasFix('''
839+
final list = <String>['a'];
840+
''');
841+
}
842+
}

pkg/analyzer_utilities/lib/tools.dart

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,8 @@ class DartFormat {
226226
static String get _dartPath => Platform.resolvedExecutable;
227227

228228
static void formatFile(File file) {
229-
var result = Process.runSync(_dartPath, [
230-
'format',
231-
'--language-version=latest',
232-
file.path
233-
]);
229+
var result = Process.runSync(
230+
_dartPath, ['format', '--language-version=latest', file.path]);
234231
_throwIfExitCode(result);
235232
}
236233

pkg/linter/lib/src/rules/omit_local_variable_types.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ class OmitLocalVariableTypes extends LintRule {
2323
List<String> get incompatibleRules => const [
2424
LintNames.always_specify_types,
2525
LintNames.specify_nonobvious_local_variable_types,
26-
LintNames.specify_nonobvious_property_types,
2726
];
2827

2928
@override

pkg/linter/lib/src/rules/specify_nonobvious_property_types.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,19 @@ import '../analyzer.dart';
1010
import '../extensions.dart';
1111
import '../util/obvious_types.dart';
1212

13-
const _desc = r'Specify non-obvious type annotations for local variables.';
13+
const _desc =
14+
r'Specify non-obvious type annotations for top-level and static variables.';
1415

1516
class SpecifyNonObviousPropertyTypes extends LintRule {
1617
SpecifyNonObviousPropertyTypes()
1718
: super(
18-
name: 'specify_nonobvious_property_types',
19+
name: LintNames.specify_nonobvious_property_types,
1920
description: _desc,
2021
state: State.experimental(),
2122
);
2223

2324
@override
24-
List<String> get incompatibleRules => const ['omit_local_variable_types'];
25+
List<String> get incompatibleRules => const [];
2526

2627
@override
2728
LintCode get lintCode => LinterLintCode.specify_nonobvious_property_types;

pkg/linter/lib/src/util/obvious_types.dart

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,20 @@ extension DartTypeExtensions on DartType? {
8787
}
8888
return null;
8989
}
90+
91+
({DartType? key, DartType? value})? get keyValueTypeOfMap {
92+
var self = this; // Enable promotion.
93+
if (self == null) return null;
94+
if (self is InterfaceType) {
95+
var mapInterfaces =
96+
self.implementedInterfaces.where((type) => type.isDartCoreMap);
97+
if (mapInterfaces.length == 1) {
98+
var [key, value] = mapInterfaces.first.typeArguments;
99+
return (key: key, value: value);
100+
}
101+
}
102+
return null;
103+
}
90104
}
91105

92106
extension ExpressionExtensions on Expression {
@@ -118,8 +132,17 @@ extension ExpressionExtensions on Expression {
118132
return false;
119133
}
120134
}
121-
var theSelfElementType = self.staticType.elementTypeOfIterable;
122-
return theSelfElementType == theObviousType;
135+
if (theObviousType != null) {
136+
var theSelfElementType = self.staticType.elementTypeOfIterable;
137+
return theSelfElementType == theObviousType;
138+
} else if (theObviousKeyType != null && theObviousValueType != null) {
139+
var keyValueTypes = self.staticType.keyValueTypeOfMap;
140+
if (keyValueTypes == null) return false;
141+
return theObviousKeyType == keyValueTypes.key &&
142+
theObviousValueType == keyValueTypes.value;
143+
} else {
144+
return false;
145+
}
123146
case RecordLiteral():
124147
for (var expression in self.fields) {
125148
if (!expression.hasObviousType) return false;

pkg/linter/test/rules/specify_nonobvious_property_types_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ main() {
1515
@reflectiveTest
1616
class SpecifyNonObviousPropertyTypesTest extends LintRuleTest {
1717
@override
18-
String get lintRule => 'specify_nonobvious_property_types';
18+
String get lintRule => LintNames.specify_nonobvious_property_types;
1919

2020
test_as_dynamic_instance() async {
2121
await assertNoDiagnostics(r'''

0 commit comments

Comments
 (0)