Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

Add new lint avoid_unused_functions. #1014

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions example/all.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ linter:
- avoid_types_as_parameter_names
- avoid_types_on_closure_parameters
- avoid_unused_constructor_parameters
- avoid_unused_functions
- await_only_futures
- camel_case_types
- cancel_subscriptions
Expand Down
2 changes: 2 additions & 0 deletions lib/src/rules.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import 'package:linter/src/rules/avoid_slow_async_io.dart';
import 'package:linter/src/rules/avoid_types_as_parameter_names.dart';
import 'package:linter/src/rules/avoid_types_on_closure_parameters.dart';
import 'package:linter/src/rules/avoid_unused_constructor_parameters.dart';
import 'package:linter/src/rules/avoid_unused_functions.dart';
import 'package:linter/src/rules/await_only_futures.dart';
import 'package:linter/src/rules/camel_case_types.dart';
import 'package:linter/src/rules/cancel_subscriptions.dart';
Expand Down Expand Up @@ -154,6 +155,7 @@ void registerLintRules() {
..register(new AvoidSlowAsyncIo())
..register(new AvoidTypesAsParameterNames())
..register(new AvoidUnusedConstructorParameters())
..register(new AvoidUnusedFunctions())
..register(new AwaitOnlyFutures())
..registerDefault(new CamelCaseTypes())
..register(new CancelSubscriptions())
Expand Down
176 changes: 176 additions & 0 deletions lib/src/rules/avoid_unused_functions.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
// Copyright (c) 2018, 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/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:linter/src/analyzer.dart';

const _desc = r'Avoid unused functions in expressions.';

const _details = r'''

**AVOID** unused functions in expressions.

Expressions that have no side effects but include a function that is not called
indicate missing parentheses.

For example,

**BAD:**
```
list.clear;
flag ? list.clear() : list.sort;
```

Since an unused tearoff has no effect, this was almost certainly the intent:

**GOOD:**
```
list.clear();
flag ? list.clear() : list.sort();
```

''';

class AvoidUnusedFunctions extends LintRule implements NodeLintRule {
AvoidUnusedFunctions()
: super(
name: 'avoid_unused_functions',
description: _desc,
details: _details,
group: Group.errors);

@override
void registerNodeProcessors(NodeLintRegistry registry) {
final visitor = new _Visitor(new _ReportNoClearEffectVisitor(this));
registry.addExpressionStatement(this, visitor);
registry.addForStatement(this, visitor);
registry.addCascadeExpression(this, visitor);
}
}

class _Visitor extends SimpleAstVisitor<void> {
final _ReportNoClearEffectVisitor reportNoClearEffect;

_Visitor(this.reportNoClearEffect);
@override
void visitCascadeExpression(CascadeExpression node) {
for (var section in node.cascadeSections) {
if (section is PropertyAccess && section.bestType is FunctionType) {
reportNoClearEffect.rule.reportLint(section);
}
}
}

@override
void visitExpressionStatement(ExpressionStatement node) {
if (node.parent is FunctionBody) {
return;
}
node.expression.accept(reportNoClearEffect);
}

@override
void visitForStatement(ForStatement node) {
node.initialization?.accept(reportNoClearEffect);
node.updaters?.forEach((u) {
u.accept(reportNoClearEffect);
});
}
}

class _ReportNoClearEffectVisitor extends UnifyingAstVisitor {
final LintRule rule;

_ReportNoClearEffectVisitor(this.rule);

@override
visitAssignmentExpression(AssignmentExpression node) {
// Has a clear effect
}

@override
visitAwaitExpression(AwaitExpression node) {
// Has a clear effect
}

@override
visitBinaryExpression(BinaryExpression node) {
switch (node.operator.lexeme) {
case '??':
case '||':
case '&&':
// these are OK when used for control flow
node.rightOperand.accept(this);
return;
}

super.visitBinaryExpression(node);
}

@override
visitCascadeExpression(CascadeExpression node) {
// Has a clear effect
}

@override
visitConditionalExpression(ConditionalExpression node) {
node.thenExpression.accept(this);
node.elseExpression.accept(this);
}

@override
visitFunctionExpressionInvocation(FunctionExpressionInvocation node) {
// Has a clear effect
}

@override
visitInstanceCreationExpression(InstanceCreationExpression node) {
// A few APIs use this for side effects, like Timer. Also, for constructors
// that have side effects, they should have tests. Those tests will often
// include an instantiation expression statement with nothing else.
}

@override
visitMethodInvocation(MethodInvocation node) {
// Has a clear effect
}

@override
visitNode(AstNode expression) {
if (expression is Expression && expression.bestType is FunctionType) {
rule.reportLint(expression);
}
}

@override
visitPostfixExpression(PostfixExpression node) {
// Has a clear effect
}

@override
visitPrefixExpression(PrefixExpression node) {
if (node.operator.lexeme == '--' || node.operator.lexeme == '++') {
// Has a clear effect
return;
}
super.visitPrefixExpression(node);
}

@override
visitRethrowExpression(RethrowExpression node) {
// Has a clear effect
}

@override
visitSuperConstructorInvocation(SuperConstructorInvocation node) {
// Has a clear effect
}

@override
visitThrowExpression(ThrowExpression node) {
// Has a clear effect
}
}
161 changes: 161 additions & 0 deletions test/rules/avoid_unused_functions.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
// Copyright (c) 2018, 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.

// test w/ `pub run test -N avoid_unused_tearoffs`

notReturned() {
1; // OK
1 + 1; // OK
foo; // LINT
new MyClass().foo; // LINT
new MyClass()..foo; // LINT
new MyClass()
..getter // OK
..foo() // OK
..foo; // LINT
[]; // OK
<dynamic, dynamic>{}; // OK
"blah"; // OK
~1; // OK

new MyClass(); // OK
foo(); // OK
new MyClass().foo(); // OK
var x = 2; // OK
x++; // OK
x--; // OK
++x; // OK
--x; // OK
try {
throw new Exception(); // OK
} catch (x) {
rethrow; // OK
}
}

asConditionAndReturnOk() {
if (true == someBool) // OK
{
return 1 + 1; // OK
} else if (false == someBool) {
return foo; // OK
}
while (new MyClass() != null) // OK
{
return new MyClass().foo; // OK
}
while (null == someBool) // OK
{
return new MyClass()..foo; // LINT
}
for (; someBool ?? someBool;) // OK
{
return <dynamic, dynamic>{}; // OK
}
do {} while ("blah".isEmpty); // OK
for (var i in []) {} // OK
switch (~1) // OK
{
}

() => new MyClass().foo; // LINT
myfun() => new MyClass().foo; // OK
myfun2() => new MyClass()..foo; // LINT
}

myfun() => new MyClass().foo; // OK
myfun2() => new MyClass()..foo; // LINT

expressionBranching() {
null ?? 1 + 1; // OK
null ?? foo; // LINT
null ?? new MyClass().foo; // LINT
false || 1 + 1 == 2; // OK
false || foo == true; // OK
false || new MyClass() as bool; // OK
false || new MyClass().foo == true; // OK
true && 1 + 1 == 2; // OK
true && foo == true; // OK
true && new MyClass() as bool; // OK
true && new MyClass().foo == true; // OK

// ternaries can detect either/both sides
someBool // OK
? 1 + 1 // OK
: foo(); // OK
someBool // OK
? foo() // OK
: foo; // LINT
someBool // OK
? new MyClass() // OK
: foo(); // OK
someBool // OK
? foo() // OK
: new MyClass().foo; // LINT
someBool // OK
? [] // OK
: {}; // OK

// not unnecessary condition, but unnecessary branching
foo() ?? 1 + 1; // OK
foo() || new MyClass() as bool; // OK
foo() && foo == true; // OK
foo() ? 1 + 1 : foo(); // OK
foo() ? foo() : foo; // LINT
foo() ? foo() : new MyClass().foo; // LINT

null ?? new MyClass(); // OK
null ?? foo(); // OK
null ?? new MyClass().foo(); // OK
false || foo(); // OK
false || new MyClass().foo(); // OK
true && foo(); // OK
true && new MyClass().foo(); // OK
someBool ? foo() : new MyClass().foo(); // OK
foo() ? foo() : new MyClass().foo(); // OK
foo() ? new MyClass() : foo(); // OK
}

inOtherStatements() {
if (foo()) {
1; // OK
}
while (someBool) {
1 + 1; // OK
}
for (foo; foo();) {} // LINT
for (; foo(); 1 + 1) {} // OK
for (;
foo();
foo(), // OK
1 + 1, // OK
new MyClass().foo) {} // LINT
do {
new MyClass().foo; // LINT
} while (foo());

switch (foo()) {
case true:
[]; // OK
break; // OK
case false:
<dynamic, dynamic>{}; // OK
break; // OK
default:
"blah"; // OK
}

for (var i in [1, 2, 3]) {
~1; // OK
}
}

bool someBool = true;
bool foo() => true;

class MyClass {
bool foo() => true;

get getter => true;
}