Skip to content

Commit b946c47

Browse files
srawlinsCommit Queue
authored and
Commit Queue
committed
new lint rule: use_truncating_division
Fixes https://github.com/dart-lang/linter/issues/3930 This allows us to remove (softly) the DIVISION_OPTIMIZATION HintCode, which was not deemed a good candidate for a Warning. Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try Change-Id: Iac48c94687d0b6b32c971e9f366ccb96adf34429 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378543 Reviewed-by: Phil Quitslund <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent 33dd277 commit b946c47

File tree

7 files changed

+183
-0
lines changed

7 files changed

+183
-0
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2485,6 +2485,8 @@ LintCode.use_to_and_as_if_applicable:
24852485
status: noFix
24862486
notes: |-
24872487
This would require renaming the method, which is a refactoring.
2488+
LintCode.use_truncating_division:
2489+
status: hasFix
24882490
LintCode.valid_regexps:
24892491
status: noFix
24902492
LintCode.void_checks:

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,7 @@ import 'package:linter/src/rules/use_raw_strings.dart';
383383
import 'package:linter/src/rules/use_rethrow_when_possible.dart';
384384
import 'package:linter/src/rules/use_string_in_part_of_directives.dart';
385385
import 'package:linter/src/rules/use_super_parameters.dart';
386+
import 'package:linter/src/rules/use_truncating_division.dart';
386387

387388
final _builtInLintMultiProducers = {
388389
CommentReferences.code: [
@@ -859,6 +860,9 @@ final _builtInLintProducers = <LintCode, List<ProducerGenerator>>{
859860
UseSuperParameters.multipleParams: [
860861
ConvertToSuperParameters.new,
861862
],
863+
UseTruncatingDivision.code: [
864+
UseEffectiveIntegerDivision.new,
865+
],
862866
};
863867

864868
final _builtInNonLintMultiProducers = {

pkg/analyzer/lib/src/error/best_practices_verifier.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,8 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
852852
/// Checks the passed binary expression for [HintCode.DIVISION_OPTIMIZATION].
853853
///
854854
/// Returns whether a hint code is generated.
855+
// TODO(srawlins): Remove this ASAP, as it is being replaced by the
856+
// 'use_truncating_division' lint rule, to avoid double reporting.
855857
bool _checkForDivisionOptimizationHint(BinaryExpression node) {
856858
if (node.operator.type != TokenType.SLASH) return false;
857859

pkg/linter/example/all.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,5 +223,6 @@ linter:
223223
- use_super_parameters
224224
- use_test_throws_matchers
225225
- use_to_and_as_if_applicable
226+
- use_truncating_division
226227
- valid_regexps
227228
- void_checks

pkg/linter/lib/src/rules.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ import 'rules/use_string_in_part_of_directives.dart';
237237
import 'rules/use_super_parameters.dart';
238238
import 'rules/use_test_throws_matchers.dart';
239239
import 'rules/use_to_and_as_if_applicable.dart';
240+
import 'rules/use_truncating_division.dart';
240241
import 'rules/valid_regexps.dart';
241242
import 'rules/void_checks.dart';
242243

@@ -476,6 +477,7 @@ void registerLintRules() {
476477
..register(UseSuperParameters())
477478
..register(UseTestThrowsMatchers())
478479
..register(UseToAndAsIfApplicable())
480+
..register(UseTruncatingDivision())
479481
..register(ValidRegexps())
480482
..register(VoidChecks());
481483
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Copyright (c) 2024, 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:analyzer/dart/ast/ast.dart';
6+
import 'package:analyzer/dart/ast/token.dart';
7+
import 'package:analyzer/dart/ast/visitor.dart';
8+
9+
import '../analyzer.dart';
10+
11+
const _desc = r'Use truncating division.';
12+
13+
const _details = r'''
14+
**DO** use truncating division, '~/', instead of regular division ('/') followed
15+
by 'toInt()'.
16+
17+
Dart features a "truncating division" operator which is the same operation as
18+
division followed by truncation, but which is more concise and expressive, and
19+
may be more performant on some platforms, for certain inputs.
20+
21+
**BAD:**
22+
```dart
23+
var x = (2 / 3).toInt();
24+
```
25+
26+
**GOOD:**
27+
```dart
28+
var x = 2 ~/ 3;
29+
```
30+
31+
''';
32+
33+
class UseTruncatingDivision extends LintRule {
34+
static const LintCode code = LintCode(
35+
'use_truncating_division',
36+
'Use truncating division.',
37+
correctionMessage:
38+
"Try using truncating division, '~/', instead of regular division "
39+
"('/') followed by 'toInt()'.",
40+
);
41+
42+
UseTruncatingDivision()
43+
: super(
44+
name: 'use_truncating_division',
45+
description: _desc,
46+
details: _details,
47+
categories: {LintRuleCategory.languageFeatureUsage});
48+
49+
@override
50+
LintCode get lintCode => code;
51+
52+
@override
53+
void registerNodeProcessors(
54+
NodeLintRegistry registry, LinterContext context) {
55+
var visitor = _Visitor(this);
56+
registry.addBinaryExpression(this, visitor);
57+
}
58+
}
59+
60+
class _Visitor extends SimpleAstVisitor<void> {
61+
final LintRule rule;
62+
63+
_Visitor(this.rule);
64+
65+
@override
66+
void visitBinaryExpression(BinaryExpression node) {
67+
if (node.operator.type != TokenType.SLASH) return;
68+
69+
// Return if the two operands are not each `int`.
70+
var leftType = node.leftOperand.staticType;
71+
if (leftType == null || !leftType.isDartCoreInt) return;
72+
73+
var rightType = node.rightOperand.staticType;
74+
if (rightType == null || !rightType.isDartCoreInt) return;
75+
76+
// Return if the '/' operator is not defined in core, or if we don't know
77+
// its static type.
78+
var methodElement = node.staticElement;
79+
if (methodElement == null) return;
80+
81+
var libraryElement = methodElement.library;
82+
if (!libraryElement.isDartCore) return;
83+
84+
var parent = node.parent;
85+
if (parent is! ParenthesizedExpression) return;
86+
87+
var outermostParentheses = parent.thisOrAncestorMatching(
88+
(e) => e.parent is! ParenthesizedExpression)!
89+
as ParenthesizedExpression;
90+
var grandParent = outermostParentheses.parent;
91+
if (grandParent is MethodInvocation &&
92+
grandParent.methodName.name == 'toInt' &&
93+
grandParent.argumentList.arguments.isEmpty) {
94+
rule.reportLint(grandParent);
95+
}
96+
}
97+
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Copyright (c) 2024, 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:test_reflective_loader/test_reflective_loader.dart';
6+
7+
import '../rule_test_support.dart';
8+
9+
main() {
10+
defineReflectiveSuite(() {
11+
defineReflectiveTests(UseTruncatingDivisionTest);
12+
});
13+
}
14+
15+
@reflectiveTest
16+
class UseTruncatingDivisionTest extends LintRuleTest {
17+
@override
18+
String get lintRule => 'use_truncating_division';
19+
20+
test_double_divide_truncate() async {
21+
await assertNoDiagnostics(r'''
22+
void f(double x, double y) {
23+
(x / y).toInt();
24+
}
25+
''');
26+
}
27+
28+
test_int_divide_truncate() async {
29+
await assertDiagnostics(r'''
30+
void f(int x, int y) {
31+
(x / y).toInt();
32+
}
33+
''', [
34+
// TODO(srawlins): ASAP, remove this Hint.
35+
error(HintCode.DIVISION_OPTIMIZATION, 25, 15),
36+
lint(25, 15),
37+
]);
38+
}
39+
40+
test_int_divide_truncate_moreParensAroundDivision() async {
41+
await assertDiagnostics(r'''
42+
void f(int x, int y) {
43+
(((x / y))).toInt();
44+
}
45+
''', [
46+
// TODO(srawlins): ASAP, remove this Hint.
47+
error(HintCode.DIVISION_OPTIMIZATION, 25, 19),
48+
lint(25, 19),
49+
]);
50+
}
51+
52+
test_int_divide_truncate_moreParensAroundOperands() async {
53+
await assertDiagnostics(r'''
54+
void f(int x, int y) {
55+
((x + 1) / (y - 1)).toInt();
56+
}
57+
''', [
58+
// TODO(srawlins): ASAP, remove this Hint.
59+
error(HintCode.DIVISION_OPTIMIZATION, 25, 27),
60+
lint(25, 27),
61+
]);
62+
}
63+
64+
test_intExtensionType_divide_truncate() async {
65+
await assertNoDiagnostics(r'''
66+
void f(ET x, int y) {
67+
(x / y).toInt();
68+
}
69+
70+
extension type ET(int it) {
71+
int operator /(int other) => 7;
72+
}
73+
''');
74+
}
75+
}

0 commit comments

Comments
 (0)