[linter, DAS] Makes use_null_aware_elements to report on cascade elements
Fixes: https://github.com/dart-lang/sdk/issues/62660 Change-Id: I8daee991353cae128ea84cf8d71ee430a884f550 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480560 Reviewed-by: Nate Biggs <natebiggs@google.com> Auto-Submit: Felipe Morschel <git@fmorschel.dev> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Nate Biggs <natebiggs@google.com>
This commit is contained in:
+12
-8
@@ -36,7 +36,9 @@ class ConvertNullCheckToNullAwareElementOrEntry
|
||||
)) {
|
||||
if (node.caseClause == null) {
|
||||
// An element or entry of the form `if (x != null) ...`.
|
||||
if (thenElement is SimpleIdentifier) {
|
||||
if (thenElement
|
||||
case SpreadElement(expression: SimpleIdentifier element) ||
|
||||
SimpleIdentifier element) {
|
||||
// In case of a list or set element with a promotable target, we
|
||||
// simply replace the entire element with the then-element prefixed by
|
||||
// '?'.
|
||||
@@ -44,11 +46,13 @@ class ConvertNullCheckToNullAwareElementOrEntry
|
||||
// `if (x != null) x` is rewritten as `?x`
|
||||
await builder.addDartFileEdit(file, (builder) {
|
||||
builder.addSimpleReplacement(
|
||||
range.startStart(node, thenElement),
|
||||
'?',
|
||||
range.startStart(node, element),
|
||||
thenElement is SpreadElement ? '...?' : '?',
|
||||
);
|
||||
});
|
||||
} else if (thenElement is PostfixExpression) {
|
||||
} else if (thenElement
|
||||
case SpreadElement(expression: PostfixExpression element) ||
|
||||
PostfixExpression element) {
|
||||
// In case of a list or set element with a getter target, we replace
|
||||
// the entire element with the then-element target identifier prefixed
|
||||
// by '?'. Note that in the case of a getter target, the null-check
|
||||
@@ -57,10 +61,10 @@ class ConvertNullCheckToNullAwareElementOrEntry
|
||||
// `if (x != null) x!` is rewritten as `?x`
|
||||
await builder.addDartFileEdit(file, (builder) {
|
||||
builder.addSimpleReplacement(
|
||||
range.startStart(node, thenElement),
|
||||
'?',
|
||||
range.startStart(node, element),
|
||||
thenElement is SpreadElement ? '...?' : '?',
|
||||
);
|
||||
builder.addDeletion(range.endEnd(thenElement.operand, thenElement));
|
||||
builder.addDeletion(range.endEnd(element.operand, element));
|
||||
});
|
||||
} else if (thenElement is MapLiteralEntry) {
|
||||
// In case of a map entry we need to check if it's the key that's
|
||||
@@ -146,7 +150,7 @@ class ConvertNullCheckToNullAwareElementOrEntry
|
||||
await builder.addDartFileEdit(file, (builder) {
|
||||
builder.addSimpleReplacement(
|
||||
range.startStart(node, condition),
|
||||
'?',
|
||||
thenElement is SpreadElement ? '...?' : '?',
|
||||
);
|
||||
builder.addDeletion(range.endEnd(condition, node));
|
||||
});
|
||||
|
||||
@@ -74,10 +74,7 @@ class ImportLibrary extends MultiCorrectionProducer {
|
||||
if (names.isEmpty) {
|
||||
return const [];
|
||||
}
|
||||
return [
|
||||
for (var name in names)
|
||||
if (await name.producers case var producers?) ...producers,
|
||||
];
|
||||
return [for (var name in names) ...?(await name.producers)];
|
||||
}
|
||||
|
||||
/// A map of all the diagnostic codes that this fix can be applied to and the
|
||||
|
||||
+125
@@ -390,6 +390,131 @@ Set<int> f(int? x) {
|
||||
?x,
|
||||
};
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_case_getter_list() async {
|
||||
await resolveTestCode('''
|
||||
List<int>? get x => null;
|
||||
List<int> f() => [
|
||||
if (x case var y?) ...y,
|
||||
];
|
||||
''');
|
||||
await assertHasFix('''
|
||||
List<int>? get x => null;
|
||||
List<int> f() => [
|
||||
...?x,
|
||||
];
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_case_getter_map() async {
|
||||
await resolveTestCode('''
|
||||
Map<int, int>? get x => null;
|
||||
Map<int, int> f() => {
|
||||
if (x case var y?) ...y,
|
||||
};
|
||||
''');
|
||||
await assertHasFix('''
|
||||
Map<int, int>? get x => null;
|
||||
Map<int, int> f() => {
|
||||
...?x,
|
||||
};
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_case_list() async {
|
||||
await resolveTestCode('''
|
||||
List<int> f(List<int>? x) => [
|
||||
if (x case var y?) ...y,
|
||||
];
|
||||
''');
|
||||
await assertHasFix('''
|
||||
List<int> f(List<int>? x) => [
|
||||
...?x,
|
||||
];
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_case_map() async {
|
||||
await resolveTestCode('''
|
||||
Map<int, int> f(Map<int, int>? x) => {
|
||||
if (x case var y?) ...y,
|
||||
};
|
||||
''');
|
||||
await assertHasFix('''
|
||||
Map<int, int> f(Map<int, int>? x) => {
|
||||
...?x,
|
||||
};
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_getter_list() async {
|
||||
await resolveTestCode('''
|
||||
List<int>? get x => null;
|
||||
List<int> f() => [
|
||||
if (x != null) ...x!,
|
||||
];
|
||||
''');
|
||||
await assertHasFix('''
|
||||
List<int>? get x => null;
|
||||
List<int> f() => [
|
||||
...?x,
|
||||
];
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_getter_map() async {
|
||||
await resolveTestCode('''
|
||||
Map<int, int>? get x => null;
|
||||
Map<int, int> f() => {
|
||||
if (x != null) ...x!,
|
||||
};
|
||||
''');
|
||||
await assertHasFix('''
|
||||
Map<int, int>? get x => null;
|
||||
Map<int, int> f() => {
|
||||
...?x,
|
||||
};
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_list() async {
|
||||
await resolveTestCode('''
|
||||
List<int> f(List<int>? x) => [
|
||||
if (x != null) ...x,
|
||||
];
|
||||
''');
|
||||
await assertHasFix('''
|
||||
List<int> f(List<int>? x) => [
|
||||
...?x,
|
||||
];
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_map() async {
|
||||
await resolveTestCode('''
|
||||
Map<int, int> f(Map<int, int>? x) => {
|
||||
if (x != null) ...x,
|
||||
};
|
||||
''');
|
||||
await assertHasFix('''
|
||||
Map<int, int> f(Map<int, int>? x) => {
|
||||
...?x,
|
||||
};
|
||||
''');
|
||||
}
|
||||
|
||||
Future<void> test_spread_set() async {
|
||||
await resolveTestCode('''
|
||||
Set<int> f(List<int>? x) => {
|
||||
if (x != null) ...x,
|
||||
};
|
||||
''');
|
||||
await assertHasFix('''
|
||||
Set<int> f(List<int>? x) => {
|
||||
...?x,
|
||||
};
|
||||
''');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -100,7 +100,7 @@ abstract class Generator implements Comparable<Generator> {
|
||||
'description': description,
|
||||
'year': DateTime.now().year.toString(),
|
||||
'author': '<your name>',
|
||||
if (additionalVars != null) ...additionalVars,
|
||||
...?additionalVars,
|
||||
};
|
||||
|
||||
for (TemplateFile file in files) {
|
||||
|
||||
@@ -7139,7 +7139,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
|
||||
);
|
||||
return [
|
||||
if (types && typeArguments != null) ...typeArguments,
|
||||
if (positionalArguments != null) ...positionalArguments,
|
||||
...?positionalArguments,
|
||||
if (namedArguments != null) js_ast.ObjectInitializer([...namedArguments]),
|
||||
];
|
||||
}
|
||||
|
||||
@@ -8010,7 +8010,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
|
||||
);
|
||||
return [
|
||||
if (types && typeArguments != null) ...typeArguments,
|
||||
if (positionalArguments != null) ...positionalArguments,
|
||||
...?positionalArguments,
|
||||
if (namedArguments != null) js_ast.ObjectInitializer([...namedArguments]),
|
||||
];
|
||||
}
|
||||
|
||||
@@ -249,7 +249,7 @@ class ExpressionCompilerWorker {
|
||||
..fileSystem = fileSystem
|
||||
..omitPlatform = true
|
||||
..environmentDefines = addGeneratedVariables({
|
||||
if (environmentDefines != null) ...environmentDefines,
|
||||
...?environmentDefines,
|
||||
}, enableAsserts: enableAsserts)
|
||||
..explicitExperimentalFlags = explicitExperimentalFlags
|
||||
..onDiagnostic = _onDiagnosticHandler(errors, warnings, infos)
|
||||
|
||||
@@ -75,7 +75,10 @@ class _Visitor extends SimpleAstVisitor<void> {
|
||||
// {..., if (x != null) x, ...}
|
||||
case (
|
||||
PromotableElementImpl(),
|
||||
SimpleIdentifier(canonicalElement: var reference),
|
||||
SimpleIdentifier(canonicalElement: var reference) ||
|
||||
SpreadElement(
|
||||
expression: SimpleIdentifier(canonicalElement: var reference),
|
||||
),
|
||||
):
|
||||
// List and set elements with getters:
|
||||
//
|
||||
@@ -84,9 +87,15 @@ class _Visitor extends SimpleAstVisitor<void> {
|
||||
case (
|
||||
GetterElement(),
|
||||
PostfixExpression(
|
||||
operand: SimpleIdentifier(canonicalElement: var reference),
|
||||
operator: Token(lexeme: '!'),
|
||||
),
|
||||
operand: SimpleIdentifier(canonicalElement: var reference),
|
||||
operator: Token(lexeme: '!'),
|
||||
) ||
|
||||
SpreadElement(
|
||||
expression: PostfixExpression(
|
||||
operand: SimpleIdentifier(canonicalElement: var reference),
|
||||
operator: Token(lexeme: '!'),
|
||||
),
|
||||
),
|
||||
):
|
||||
if (nullCheckTarget == reference) {
|
||||
rule.reportAtToken(node.ifKeyword);
|
||||
|
||||
@@ -16,7 +16,7 @@ void main() {
|
||||
@reflectiveTest
|
||||
class UseNullAwareElementsTest extends LintRuleTest {
|
||||
@override
|
||||
String get lintRule => 'use_null_aware_elements';
|
||||
String get lintRule => LintNames.use_null_aware_elements;
|
||||
|
||||
test_nonPromotable_nullCheck_list() async {
|
||||
await assertDiagnostics(
|
||||
@@ -168,4 +168,66 @@ List<int> f(int? x) {
|
||||
}
|
||||
''');
|
||||
}
|
||||
|
||||
test_spread_getter_list() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
List<int>? get x => null;
|
||||
List<int> f() => [
|
||||
if (x != null) ...x!,
|
||||
if (x case var y?) ...y,
|
||||
];
|
||||
''',
|
||||
[lint(47, 2), lint(71, 2)],
|
||||
);
|
||||
}
|
||||
|
||||
test_spread_getter_map() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
Map<int, int>? get x => null;
|
||||
Map<int, int> f() => {
|
||||
if (x != null) ...x!,
|
||||
if (x case var y?) ...y,
|
||||
};
|
||||
''',
|
||||
[lint(55, 2), lint(79, 2)],
|
||||
);
|
||||
}
|
||||
|
||||
test_spread_list() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
List<int> f(List<int>? x) => [
|
||||
if (x != null) ...x,
|
||||
if (x case var y?) ...y,
|
||||
];
|
||||
''',
|
||||
[lint(33, 2), lint(56, 2)],
|
||||
);
|
||||
}
|
||||
|
||||
test_spread_map() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
Map<int, int> f(Map<int, int>? x) => {
|
||||
if (x != null) ...x,
|
||||
if (x case var y?) ...y,
|
||||
};
|
||||
''',
|
||||
[lint(41, 2), lint(64, 2)],
|
||||
);
|
||||
}
|
||||
|
||||
test_spread_set() async {
|
||||
await assertDiagnostics(
|
||||
'''
|
||||
Set<int> f(List<int>? x) => {
|
||||
if (x != null) ...x,
|
||||
if (x case var y?) ...y,
|
||||
};
|
||||
''',
|
||||
[lint(32, 2), lint(55, 2)],
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user