Skip to content

Commit

Permalink
[analyzer] Report warnings on expressions with non-nullable types
Browse files Browse the repository at this point in the history
Part of #56836

Change-Id: I3fcdceff667f371fcf4b66c12bd16e2f34e6446c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391621
Reviewed-by: Keerti Parthasarathy <[email protected]>
Reviewed-by: Erik Ernst <[email protected]>
Commit-Queue: Chloe Stefantsova <[email protected]>
  • Loading branch information
chloestefantsova authored and Commit Queue committed Oct 25, 2024
1 parent b7aa0fa commit 253e715
Show file tree
Hide file tree
Showing 14 changed files with 343 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3375,6 +3375,12 @@ StaticWarningCode.INVALID_NULL_AWARE_OPERATOR:
status: hasFix
StaticWarningCode.INVALID_NULL_AWARE_OPERATOR_AFTER_SHORT_CIRCUIT:
status: hasFix
StaticWarningCode.INVALID_NULL_AWARE_ELEMENT:
status: needsEvaluation
StaticWarningCode.INVALID_NULL_AWARE_MAP_ENTRY_KEY:
status: needsEvaluation
StaticWarningCode.INVALID_NULL_AWARE_MAP_ENTRY_VALUE:
status: needsEvaluation
StaticWarningCode.MISSING_ENUM_CONSTANT_IN_SWITCH:
status: hasFix
StaticWarningCode.UNNECESSARY_NON_NULL_ASSERTION:
Expand Down
31 changes: 31 additions & 0 deletions pkg/analyzer/lib/src/error/codes.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6070,6 +6070,37 @@ class StaticWarningCode extends AnalyzerErrorCode {
hasPublishedDocs: true,
);

/// No parameters.
static const StaticWarningCode INVALID_NULL_AWARE_ELEMENT = StaticWarningCode(
'INVALID_NULL_AWARE_OPERATOR',
"The element can't be null, so the null-aware operator '?' is unnecessary.",
correctionMessage: "Try removing the operator '?'.",
hasPublishedDocs: true,
uniqueName: 'INVALID_NULL_AWARE_ELEMENT',
);

/// No parameters.
static const StaticWarningCode INVALID_NULL_AWARE_MAP_ENTRY_KEY =
StaticWarningCode(
'INVALID_NULL_AWARE_OPERATOR',
"The map entry key can't be null, so the null-aware operator '?' is "
"unnecessary.",
correctionMessage: "Try removing the operator '?'.",
hasPublishedDocs: true,
uniqueName: 'INVALID_NULL_AWARE_MAP_ENTRY_KEY',
);

/// No parameters.
static const StaticWarningCode INVALID_NULL_AWARE_MAP_ENTRY_VALUE =
StaticWarningCode(
'INVALID_NULL_AWARE_OPERATOR',
"The map entry value can't be null, so the null-aware operator '?' is "
"unnecessary.",
correctionMessage: "Try removing the operator '?'.",
hasPublishedDocs: true,
uniqueName: 'INVALID_NULL_AWARE_MAP_ENTRY_VALUE',
);

/// Parameters:
/// 0: the null-aware operator that is invalid
/// 1: the non-null-aware operator that can replace the invalid operator
Expand Down
3 changes: 3 additions & 0 deletions pkg/analyzer/lib/src/error/error_code_values.g.dart
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,9 @@ const List<ErrorCode> errorCodeValues = [
ScannerErrorCode.UNTERMINATED_MULTI_LINE_COMMENT,
ScannerErrorCode.UNTERMINATED_STRING_LITERAL,
StaticWarningCode.DEAD_NULL_AWARE_EXPRESSION,
StaticWarningCode.INVALID_NULL_AWARE_ELEMENT,
StaticWarningCode.INVALID_NULL_AWARE_MAP_ENTRY_KEY,
StaticWarningCode.INVALID_NULL_AWARE_MAP_ENTRY_VALUE,
StaticWarningCode.INVALID_NULL_AWARE_OPERATOR,
StaticWarningCode.INVALID_NULL_AWARE_OPERATOR_AFTER_SHORT_CIRCUIT,
StaticWarningCode.MISSING_ENUM_CONSTANT_IN_SWITCH,
Expand Down
47 changes: 43 additions & 4 deletions pkg/analyzer/lib/src/generated/error_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1157,6 +1157,21 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
super.visitListLiteral(node);
}

@override
void visitMapLiteralEntry(MapLiteralEntry node) {
if (node.keyQuestion != null) {
_checkForUnnecessaryNullAware(node.key, node.keyQuestion!,
nullAwareElementOrMapEntryKind:
_NullAwareElementOrMapEntryKind.mapEntryKey);
}
if (node.valueQuestion != null) {
_checkForUnnecessaryNullAware(node.value, node.valueQuestion!,
nullAwareElementOrMapEntryKind:
_NullAwareElementOrMapEntryKind.mapEntryValue);
}
super.visitMapLiteralEntry(node);
}

@override
void visitMethodDeclaration(covariant MethodDeclarationImpl node) {
var element = node.declaredElement!;
Expand Down Expand Up @@ -1296,6 +1311,14 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
super.visitNativeFunctionBody(node);
}

@override
void visitNullAwareElement(NullAwareElement node) {
_checkForUnnecessaryNullAware(node.value, node.question,
nullAwareElementOrMapEntryKind:
_NullAwareElementOrMapEntryKind.element);
super.visitNullAwareElement(node);
}

@override
void visitPatternVariableDeclarationStatement(
covariant PatternVariableDeclarationStatementImpl node,
Expand Down Expand Up @@ -5551,7 +5574,8 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
}
}

void _checkForUnnecessaryNullAware(Expression target, Token operator) {
void _checkForUnnecessaryNullAware(Expression target, Token operator,
{_NullAwareElementOrMapEntryKind? nullAwareElementOrMapEntryKind}) {
if (target is SuperExpression) {
return;
}
Expand All @@ -5560,9 +5584,20 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
Token endToken = operator;
List<Object> arguments = const [];
if (operator.type == TokenType.QUESTION) {
errorCode = StaticWarningCode.INVALID_NULL_AWARE_OPERATOR;
endToken = operator.next!;
arguments = ['?[', '['];
if (nullAwareElementOrMapEntryKind == null) {
errorCode = StaticWarningCode.INVALID_NULL_AWARE_OPERATOR;
endToken = operator.next!;
arguments = ['?[', '['];
} else {
switch (nullAwareElementOrMapEntryKind) {
case _NullAwareElementOrMapEntryKind.element:
errorCode = StaticWarningCode.INVALID_NULL_AWARE_ELEMENT;
case _NullAwareElementOrMapEntryKind.mapEntryKey:
errorCode = StaticWarningCode.INVALID_NULL_AWARE_MAP_ENTRY_KEY;
case _NullAwareElementOrMapEntryKind.mapEntryValue:
errorCode = StaticWarningCode.INVALID_NULL_AWARE_MAP_ENTRY_VALUE;
}
}
} else if (operator.type == TokenType.QUESTION_PERIOD) {
errorCode = StaticWarningCode.INVALID_NULL_AWARE_OPERATOR;
arguments = [operator.lexeme, '.'];
Expand Down Expand Up @@ -7164,6 +7199,10 @@ class _MacroTypeAnnotationLocationConverter {
}
}

/// Signals the kind of the null-aware element or entry observed in list, set,
/// or map literals.
enum _NullAwareElementOrMapEntryKind { element, mapEntryKey, mapEntryValue }

/// Recursively visits a type annotation, looking uninstantiated bounds.
class _UninstantiatedBoundChecker extends RecursiveAstVisitor<void> {
final ErrorReporter _errorReporter;
Expand Down
26 changes: 26 additions & 0 deletions pkg/analyzer/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22673,6 +22673,14 @@ StaticWarningCode:
has the value `null`, the cast will fail and the invocation of `length`
will not happen.

The following code produces this diagnostic because `s` can't be `null`:

```dart
List<String> makeSingletonList(String s) {
return <String>[[!?!]s];
}
```

#### Common fixes

Replace the null-aware operator with a non-null-aware equivalent; for
Expand All @@ -22695,6 +22703,24 @@ StaticWarningCode:
Parameters:
0: the null-aware operator that is invalid
1: the non-null-aware operator that can replace the invalid operator
INVALID_NULL_AWARE_ELEMENT:
sharedName: INVALID_NULL_AWARE_OPERATOR
problemMessage: "The element can't be null, so the null-aware operator '?' is unnecessary."
correctionMessage: "Try removing the operator '?'."
hasPublishedDocs: true
comment: No parameters.
INVALID_NULL_AWARE_MAP_ENTRY_KEY:
sharedName: INVALID_NULL_AWARE_OPERATOR
problemMessage: "The map entry key can't be null, so the null-aware operator '?' is unnecessary."
correctionMessage: "Try removing the operator '?'."
hasPublishedDocs: true
comment: No parameters.
INVALID_NULL_AWARE_MAP_ENTRY_VALUE:
sharedName: INVALID_NULL_AWARE_OPERATOR
problemMessage: "The map entry value can't be null, so the null-aware operator '?' is unnecessary."
correctionMessage: "Try removing the operator '?'."
hasPublishedDocs: true
comment: No parameters.
MISSING_ENUM_CONSTANT_IN_SWITCH:
problemMessage: "Missing case clause for '{0}'."
correctionMessage: Try adding a case clause for the missing constant, or adding a default clause.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// 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/src/error/codes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

import '../dart/resolution/context_collection_resolution.dart';

main() {
defineReflectiveSuite(() {
defineReflectiveTests(InvalidNullAwareElementsErrorTest);
});
}

@reflectiveTest
class InvalidNullAwareElementsErrorTest extends PubPackageResolutionTest {
test_invalid_null_aware_element_in_list() async {
await assertErrorsInCode('''
const stringConst = "";
const list = [0, ?stringConst];
''', [
error(StaticWarningCode.INVALID_NULL_AWARE_ELEMENT, 41, 1),
]);
}

test_invalid_null_aware_element_in_set() async {
await assertErrorsInCode('''
const stringConst = "";
const set = {0, ?stringConst};
''', [
error(StaticWarningCode.INVALID_NULL_AWARE_ELEMENT, 40, 1),
]);
}

test_invalid_null_aware_key_in_map() async {
await assertErrorsInCode('''
const intConst = 0;
const map = {?0: intConst};
''', [
error(StaticWarningCode.INVALID_NULL_AWARE_MAP_ENTRY_KEY, 33, 1),
]);
}

test_invalid_null_aware_value_in_map() async {
await assertErrorsInCode('''
const intConst = 0;
const map = {0: ?intConst};
''', [
error(StaticWarningCode.INVALID_NULL_AWARE_MAP_ENTRY_VALUE, 36, 1),
]);
}
}
Loading

0 comments on commit 253e715

Please sign in to comment.