Skip to content

Commit 2cce183

Browse files
pqCommit Queue
authored and
Commit Queue
committed
quick fix for flutter_style_todos
Handles: * missing space before `TODO` * missing colon * lower case `todo` Change-Id: Ib0e00e992d42c4d29fee87679c45dae10448653d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/335024 Commit-Queue: Phil Quitslund <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent abb299e commit 2cce183

File tree

8 files changed

+226
-8
lines changed

8 files changed

+226
-8
lines changed
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:_fe_analyzer_shared/src/scanner/token.dart';
6+
import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
7+
import 'package:analysis_server/src/services/correction/fix.dart';
8+
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
9+
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
10+
import 'package:analyzer_plugin/utilities/range_factory.dart';
11+
import 'package:linter/src/rules/flutter_style_todos.dart';
12+
13+
class ConvertToFlutterStyleTodo extends ResolvedCorrectionProducer {
14+
@override
15+
bool get canBeAppliedInBulk => true;
16+
17+
@override
18+
bool get canBeAppliedToFile => true;
19+
20+
@override
21+
FixKind get fixKind => DartFixKind.CONVERT_TO_FLUTTER_STYLE_TODO;
22+
23+
@override
24+
FixKind get multiFixKind => DartFixKind.CONVERT_TO_FLUTTER_STYLE_TODO_MULTI;
25+
26+
@override
27+
Future<void> compute(ChangeBuilder builder) async {
28+
var diagnosticOffset = diagnostic?.problemMessage.offset;
29+
if (diagnosticOffset == null) return;
30+
31+
// Find the token that follows the reported diagnostic.
32+
var token = node.beginToken;
33+
do {
34+
if (token.offset > diagnosticOffset) break;
35+
36+
token = token.next!;
37+
} while (token != node.endToken);
38+
39+
// Identify the right comment.
40+
Token? comment = token.precedingComments;
41+
while (comment != null) {
42+
if (comment.offset >= diagnosticOffset) break;
43+
comment = comment.next;
44+
}
45+
if (comment == null) return;
46+
47+
var content = comment.lexeme;
48+
49+
// Try adding a missing leading space before `TODO`.
50+
if (!content.startsWith('// ')) {
51+
content = content.replaceFirst('//', '// ');
52+
}
53+
54+
// Try removing an unwanted space after `TODO`.
55+
if (content.length > 7 && content[7] == ' ') {
56+
content = content.replaceRange(7, 8, '');
57+
}
58+
59+
// Try adding a colon.
60+
var index = content.indexOf(')') + 1;
61+
if (content.length > index && !content.startsWith(':', index)) {
62+
content = content.replaceFirst(')', '):');
63+
}
64+
65+
// Try fixing the TODO case.
66+
if (content.startsWith('// todo')) {
67+
content = content.replaceRange(3, 7, 'TODO');
68+
}
69+
70+
// TODO(pq): consider adding missing user info.
71+
// Possibly inserting '(${Platform.environment['USER'] ?? Platform.environment['USERNAME']}')
72+
// (assuming the environment variable is set).
73+
74+
// If the generated content doesn't match flutter style, don't apply it.
75+
if (!content.startsWith(FlutterStyleTodos.todoExpectedRegExp)) return;
76+
77+
await builder.addDartFileEdit(file, (builder) {
78+
builder.addReplacement(range.token(comment as Token), (builder) {
79+
builder.write(content);
80+
});
81+
});
82+
}
83+
}

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2083,10 +2083,7 @@ LintCode.file_names:
20832083
notes: |-
20842084
The fix is to rename the file, but that's a refactoring.
20852085
LintCode.flutter_style_todos:
2086-
status: noFix
2087-
notes: |-
2088-
The fix would be to convert the existing TODO to the appropriate style, but
2089-
that can't reliably be done.
2086+
status: hasFix
20902087
LintCode.hash_and_equals:
20912088
status: hasFix
20922089
LintCode.implementation_imports:

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,16 @@ class DartFixKind {
483483
DartFixKindPriority.IN_FILE,
484484
'Convert to double quoted strings everywhere in file',
485485
);
486+
static const CONVERT_TO_FLUTTER_STYLE_TODO = FixKind(
487+
'dart.fix.convert.toFlutterStyleTodo',
488+
DartFixKindPriority.DEFAULT,
489+
'Convert to flutter style todo',
490+
);
491+
static const CONVERT_TO_FLUTTER_STYLE_TODO_MULTI = FixKind(
492+
'dart.fix.convert.toFlutterStyleTodo.multi',
493+
DartFixKindPriority.IN_FILE,
494+
'Convert to flutter style todos everywhere in file',
495+
);
486496
static const CONVERT_TO_FOR_ELEMENT = FixKind(
487497
'dart.fix.convert.toForElement',
488498
DartFixKindPriority.DEFAULT,

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import 'package:analysis_server/src/services/correction/dart/convert_to_cascade.
6363
import 'package:analysis_server/src/services/correction/dart/convert_to_constant_pattern.dart';
6464
import 'package:analysis_server/src/services/correction/dart/convert_to_contains.dart';
6565
import 'package:analysis_server/src/services/correction/dart/convert_to_expression_function_body.dart';
66+
import 'package:analysis_server/src/services/correction/dart/convert_to_flutter_style_todo.dart';
6667
import 'package:analysis_server/src/services/correction/dart/convert_to_function_declaration.dart';
6768
import 'package:analysis_server/src/services/correction/dart/convert_to_generic_function_syntax.dart';
6869
import 'package:analysis_server/src/services/correction/dart/convert_to_if_null.dart';
@@ -566,6 +567,9 @@ class FixProcessor extends BaseProcessor {
566567
LintNames.exhaustive_cases: [
567568
AddMissingEnumLikeCaseClauses.new,
568569
],
570+
LintNames.flutter_style_todos: [
571+
ConvertToFlutterStyleTodo.new,
572+
],
569573
LintNames.hash_and_equals: [
570574
CreateMethod.equalsOrHashCode,
571575
],

pkg/analysis_server/lib/src/services/linter/lint_names.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ class LintNames {
7777
static const String empty_statements = 'empty_statements';
7878
static const String eol_at_end_of_file = 'eol_at_end_of_file';
7979
static const String exhaustive_cases = 'exhaustive_cases';
80+
static const String flutter_style_todos = 'flutter_style_todos';
8081
static const String hash_and_equals = 'hash_and_equals';
8182
static const String implicit_call_tearoffs = 'implicit_call_tearoffs';
8283
static const String implicit_reopen = 'implicit_reopen';
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analysis_server/src/services/correction/fix.dart';
6+
import 'package:analysis_server/src/services/linter/lint_names.dart';
7+
import 'package:analyzer/src/dart/error/todo_codes.dart';
8+
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
9+
import 'package:test_reflective_loader/test_reflective_loader.dart';
10+
11+
import 'fix_processor.dart';
12+
13+
void main() {
14+
defineReflectiveSuite(() {
15+
defineReflectiveTests(ConvertToFlutterStyleTodoBulkTest);
16+
defineReflectiveTests(ConvertToFlutterStyleTodoTest);
17+
});
18+
}
19+
20+
@reflectiveTest
21+
class ConvertToFlutterStyleTodoBulkTest extends BulkFixProcessorTest {
22+
@override
23+
String get lintCode => LintNames.flutter_style_todos;
24+
25+
Future<void> test_singleFile() async {
26+
await resolveTestCode('''
27+
// TODO(user) msg.
28+
void f() {
29+
// todo(user) msg.
30+
}
31+
//TODO(user): msg.
32+
void g() { }
33+
''');
34+
await assertHasFix('''
35+
// TODO(user): msg.
36+
void f() {
37+
// TODO(user): msg.
38+
}
39+
// TODO(user): msg.
40+
void g() { }
41+
''');
42+
}
43+
}
44+
45+
@reflectiveTest
46+
class ConvertToFlutterStyleTodoTest extends FixProcessorLintTest {
47+
@override
48+
FixKind get kind => DartFixKind.CONVERT_TO_FLUTTER_STYLE_TODO;
49+
50+
@override
51+
String get lintCode => LintNames.flutter_style_todos;
52+
53+
Future<void> test_lowerCase() async {
54+
await resolveTestCode('''
55+
// todo(user): msg.
56+
void f() { }
57+
''');
58+
await assertHasFix('''
59+
// TODO(user): msg.
60+
void f() { }
61+
''');
62+
}
63+
64+
Future<void> test_missingColon() async {
65+
await resolveTestCode('''
66+
// TODO(user) msg.
67+
void f() { }
68+
''');
69+
await assertHasFix('''
70+
// TODO(user): msg.
71+
void f() { }
72+
''', errorFilter: (e) => e.errorCode != TodoCode.TODO);
73+
}
74+
75+
Future<void> test_missingColon_surroundingComments() async {
76+
await resolveTestCode('''
77+
// Leading comment.
78+
// TODO(user) msg.
79+
// Trailing comment.
80+
void f() { }
81+
''');
82+
await assertHasFix('''
83+
// Leading comment.
84+
// TODO(user): msg.
85+
// Trailing comment.
86+
void f() { }
87+
''', errorFilter: (e) => e.errorCode != TodoCode.TODO);
88+
}
89+
90+
Future<void> test_missingColonAndMessage() async {
91+
await resolveTestCode('''
92+
// TODO(user)
93+
void f() {}
94+
''');
95+
await assertNoFix(errorFilter: (e) => e.errorCode != TodoCode.TODO);
96+
}
97+
98+
Future<void> test_missingLeadingSpace() async {
99+
await resolveTestCode('''
100+
//TODO(user): msg.
101+
void f() {}
102+
''');
103+
await assertHasFix('''
104+
// TODO(user): msg.
105+
void f() {}
106+
''', errorFilter: (e) => e.errorCode != TodoCode.TODO);
107+
}
108+
109+
Future<void> test_unwantedSpaceBeforeUser() async {
110+
await resolveTestCode('''
111+
// TODO (user): msg.
112+
void f() {}
113+
''');
114+
await assertHasFix('''
115+
// TODO(user): msg.
116+
void f() {}
117+
''', errorFilter: (e) => e.errorCode != TodoCode.TODO);
118+
}
119+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ import 'convert_to_constant_pattern_test.dart' as convert_to_constant_pattern;
7373
import 'convert_to_contains_test.dart' as convert_to_contains;
7474
import 'convert_to_double_quoted_string_test.dart'
7575
as convert_to_double_quoted_string;
76+
import 'convert_to_flutter_style_todo_test.dart'
77+
as convert_to_flutter_style_todo;
7678
import 'convert_to_for_element_test.dart' as convert_to_for_element;
7779
import 'convert_to_function_declaration_test.dart'
7880
as convert_to_function_declaration;
@@ -340,6 +342,7 @@ void main() {
340342
convert_to_constant_pattern.main();
341343
convert_to_contains.main();
342344
convert_to_double_quoted_string.main();
345+
convert_to_flutter_style_todo.main();
343346
convert_to_for_element.main();
344347
convert_to_function_declaration.main();
345348
convert_to_generic_function_syntax.main();

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ class FlutterStyleTodos extends LintRule {
3131
'flutter_style_todos', "To-do comment doesn't follow the Flutter style.",
3232
correctionMessage: 'Try following the Flutter style for to-do comments.');
3333

34+
/// A regular expression that matches a correctly formatted TODO.
35+
static final RegExp todoExpectedRegExp =
36+
RegExp(r'// TODO\([a-zA-Z0-9][-a-zA-Z0-9]*\): ');
37+
3438
FlutterStyleTodos()
3539
: super(
3640
name: 'flutter_style_todos',
@@ -52,9 +56,6 @@ class FlutterStyleTodos extends LintRule {
5256
class _Visitor extends SimpleAstVisitor<void> {
5357
static final _todoRegExp = RegExp(r'//+(.* )?TODO\b', caseSensitive: false);
5458

55-
static final _todoExpectedRegExp =
56-
RegExp(r'// TODO\([a-zA-Z0-9][-a-zA-Z0-9]*\): ');
57-
5859
final LintRule rule;
5960

6061
_Visitor(this.rule);
@@ -80,7 +81,7 @@ class _Visitor extends SimpleAstVisitor<void> {
8081
void _checkComment(Token node) {
8182
var content = node.lexeme;
8283
if (content.startsWith(_todoRegExp) &&
83-
!content.startsWith(_todoExpectedRegExp)) {
84+
!content.startsWith(FlutterStyleTodos.todoExpectedRegExp)) {
8485
rule.reportLintForToken(node);
8586
}
8687
}

0 commit comments

Comments
 (0)