This repository has been archived by the owner on Jul 16, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 265
Add avoid non null assertion rule #285
Merged
dkrutskikh
merged 5 commits into
dart-code-checker:master
from
incendial:avoid-non-null-assertion
Apr 26, 2021
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
0131b87
feat: add avoid non null assertion rule
incendial 733e50b
Merge remote-tracking branch 'upstream/master' into avoid-non-null-as…
incendial cddb25c
test: migrate rule tests to the new approach
incendial 82cd2a8
Review fixes
incendial 9dbbe53
Merge remote-tracking branch 'upstream/master' into avoid-non-null-as…
incendial File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
# Avoid non null assertion | ||
|
||
## Rule id | ||
|
||
avoid-non-null-assertion | ||
|
||
## Description | ||
|
||
Warns when non null assertion operator (**!** or “bang” operator) is used for a property access or method invocation. The operator check works at runtime and it may fail and throw a runtime exception. | ||
|
||
The rule ignores the index `[]` operator on the Map class because it's considered the idiomatic way to access a known-present element in a map with `[]!` according to [the docs](https://dart.dev/null-safety/understanding-null-safety#the-map-index-operator-is-nullable). | ||
|
||
Use this rule if you want to avoid possible unexpected runtime exceptions. | ||
|
||
### Example | ||
|
||
Bad: | ||
|
||
```dart | ||
class Test { | ||
String? field; | ||
|
||
Test? object; | ||
|
||
void method() { | ||
field!.contains('other'); // LINT | ||
|
||
object!.field!.contains('other'); // LINT | ||
|
||
final map = {'key': 'value'}; | ||
map['key']!.contains('other'); | ||
|
||
object!.method(); // LINT | ||
} | ||
} | ||
``` | ||
|
||
Good: | ||
|
||
```dart | ||
class Test { | ||
String? field; | ||
|
||
Test? object; | ||
|
||
void method() { | ||
field?.contains('other'); | ||
|
||
object?.field?.contains('other'); | ||
|
||
final map = {'key': 'value'}; | ||
map['key']!.contains('other'); | ||
|
||
object?.method(); | ||
} | ||
} | ||
``` |
70 changes: 70 additions & 0 deletions
70
lib/src/obsoleted/rules/avoid_non_null_assertion_rule.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
import 'package:analyzer/dart/analysis/results.dart'; | ||
import 'package:analyzer/dart/ast/ast.dart'; | ||
import 'package:analyzer/dart/ast/token.dart'; | ||
import 'package:analyzer/dart/ast/visitor.dart'; | ||
|
||
import '../../models/issue.dart'; | ||
import '../../models/severity.dart'; | ||
import '../../utils/node_utils.dart'; | ||
import '../../utils/rule_utils.dart'; | ||
import 'obsolete_rule.dart'; | ||
|
||
class AvoidNonNullAssertionRule extends ObsoleteRule { | ||
static const String ruleId = 'avoid-non-null-assertion'; | ||
static const _documentationUrl = 'https://git.io/JO5Ju'; | ||
|
||
static const _failure = 'Avoid using non null assertion.'; | ||
|
||
AvoidNonNullAssertionRule({Map<String, Object> config = const {}}) | ||
: super( | ||
id: ruleId, | ||
documentationUrl: Uri.parse(_documentationUrl), | ||
severity: readSeverity(config, Severity.warning), | ||
excludes: readExcludes(config), | ||
); | ||
|
||
@override | ||
Iterable<Issue> check(ResolvedUnitResult source) { | ||
final visitor = _Visitor(); | ||
|
||
source.unit?.visitChildren(visitor); | ||
|
||
return visitor.expressions | ||
.map((expression) => createIssue( | ||
rule: this, | ||
location: nodeLocation( | ||
node: expression, | ||
source: source, | ||
withCommentOrMetadata: true, | ||
), | ||
message: _failure, | ||
)) | ||
.toList(growable: false); | ||
} | ||
} | ||
|
||
class _Visitor extends RecursiveAstVisitor<void> { | ||
final _expressions = <Expression>[]; | ||
|
||
Iterable<Expression> get expressions => _expressions; | ||
|
||
@override | ||
void visitPostfixExpression(PostfixExpression node) { | ||
super.visitPostfixExpression(node); | ||
|
||
if (node.operator.type == TokenType.BANG && | ||
!_isMapIndexOperator(node.operand)) { | ||
_expressions.add(node); | ||
} | ||
} | ||
|
||
bool _isMapIndexOperator(Expression operand) { | ||
if (operand is IndexExpression) { | ||
final type = operand.target?.staticType; | ||
|
||
return type != null && type.isDartCoreMap; | ||
} | ||
|
||
return false; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
49 changes: 49 additions & 0 deletions
49
test/obsoleted/rules/avoid_non_null_assertion/avoid_non_null_assertion_test.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
@TestOn('vm') | ||
import 'package:dart_code_metrics/src/models/severity.dart'; | ||
import 'package:dart_code_metrics/src/obsoleted/rules/avoid_non_null_assertion_rule.dart'; | ||
import 'package:test/test.dart'; | ||
|
||
import '../../../helpers/rule_test_helper.dart'; | ||
|
||
const _examplePath = | ||
'test/obsoleted/rules/avoid_non_null_assertion/examples/example.dart'; | ||
|
||
void main() { | ||
group('AvoidNonNullAssertion', () { | ||
test('initialization', () async { | ||
final unit = await RuleTestHelper.resolveFromFile(_examplePath); | ||
final issues = AvoidNonNullAssertionRule().check(unit); | ||
|
||
RuleTestHelper.verifyInitialization( | ||
issues: issues, | ||
ruleId: 'avoid-non-null-assertion', | ||
severity: Severity.warning, | ||
); | ||
}); | ||
|
||
test('reports about found issues', () async { | ||
final unit = await RuleTestHelper.resolveFromFile(_examplePath); | ||
final issues = AvoidNonNullAssertionRule().check(unit); | ||
|
||
RuleTestHelper.verifyIssues( | ||
issues: issues, | ||
startOffsets: [70, 208, 208, 460], | ||
startLines: [7, 15, 15, 27], | ||
startColumns: [5, 5, 5, 5], | ||
endOffsets: [76, 215, 222, 467], | ||
locationTexts: [ | ||
'field!', | ||
'object!', | ||
'object!.field!', | ||
'object!', | ||
], | ||
messages: [ | ||
'Avoid using non null assertion.', | ||
'Avoid using non null assertion.', | ||
'Avoid using non null assertion.', | ||
'Avoid using non null assertion.', | ||
], | ||
); | ||
}); | ||
}); | ||
} |
31 changes: 31 additions & 0 deletions
31
test/obsoleted/rules/avoid_non_null_assertion/examples/example.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
class Test { | ||
String? field; | ||
|
||
Test? object; | ||
|
||
void method() { | ||
field!.contains('other'); // LINT | ||
|
||
field?.replaceAll('from', 'replace'); | ||
|
||
if (filed != null) { | ||
field.split(' '); | ||
} | ||
|
||
object!.field!.contains('other'); // LINT | ||
|
||
object?.field?.contains('other'); | ||
|
||
final field = object?.field; | ||
if (field != null) { | ||
field.contains('other'); | ||
} | ||
|
||
final map = {'key': 'value'}; | ||
map['key']!.contains('other'); | ||
|
||
object!.method(); // LINT | ||
|
||
object?.method(); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@incendial please remove
withCommentOrMetadata: true,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.