languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-16 04:27:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Report HintCode.UNUSED_LOCAL_VARIABLE for local variables whose value is never used.

Browse source 
R=brianwilkerson@google.com
BUG=

Review URL: https://codereview.chromium.org//686113007

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@41568 260f80e4-7a28-3924-810f-c04153c831b5
This commit is contained in:
scheglov@google.com 2014-11-06 17:00:28 +00:00
parent eb3c3e9381
commit ca91f9f770
5 changed files with 237 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
pkg/analysis_server/test/services/correction/fix_test.dart
View file

@ -2304,6 +2304,9 @@ main() {
AnalysisError _findErrorToFix() {
List<AnalysisError> errors = context.computeErrors(testSource);
errors.removeWhere((error) {
return error.errorCode == HintCode.UNUSED_LOCAL_VARIABLE;
});
if (checkHasSingleError) {
expect(errors, hasLength(1));
}

40
pkg/analyzer/lib/src/generated/element.dart
View file

@ -8275,6 +8275,9 @@ class LocalVariableElementImpl extends VariableElementImpl implements LocalVaria
@override
bool get isPotentiallyMutatedInScope => hasModifier(Modifier.POTENTIALLY_MUTATED_IN_SCOPE);
@override
bool get isUsed => hasModifier(Modifier.IS_USED_VARIABLE);
/**
* Specifies that this variable is potentially mutated somewhere in closure.
*/
@ -8289,6 +8292,13 @@ class LocalVariableElementImpl extends VariableElementImpl implements LocalVaria
setModifier(Modifier.POTENTIALLY_MUTATED_IN_SCOPE, true);
}
/**
* Specifies that the value of this variable is used.
*/
void markUsed() {
setModifier(Modifier.IS_USED_VARIABLE, true);
}
/**
* Set the toolkit specific information objects attached to this variable.
*
@ -8749,48 +8759,53 @@ class Modifier extends Enum<Modifier> {
*/
static const Modifier HAS_EXT_URI = const Modifier('HAS_EXT_URI', 9);
/**
* Indicates that the value of a variable is used - read or invoked.
*/
static const Modifier IS_USED_VARIABLE = const Modifier('IS_USED_VARIABLE', 10);
/**
* Indicates that a class can validly be used as a mixin.
*/
static const Modifier MIXIN = const Modifier('MIXIN', 10);
static const Modifier MIXIN = const Modifier('MIXIN', 11);
/**
* Indicates that the value of a parameter or local variable might be mutated within the context.
*/
static const Modifier POTENTIALLY_MUTATED_IN_CONTEXT = const Modifier('POTENTIALLY_MUTATED_IN_CONTEXT', 11);
static const Modifier POTENTIALLY_MUTATED_IN_CONTEXT = const Modifier('POTENTIALLY_MUTATED_IN_CONTEXT', 12);
/**
* Indicates that the value of a parameter or local variable might be mutated within the scope.
*/
static const Modifier POTENTIALLY_MUTATED_IN_SCOPE = const Modifier('POTENTIALLY_MUTATED_IN_SCOPE', 12);
static const Modifier POTENTIALLY_MUTATED_IN_SCOPE = const Modifier('POTENTIALLY_MUTATED_IN_SCOPE', 13);
/**
* Indicates that a class contains an explicit reference to 'super'.
*/
static const Modifier REFERENCES_SUPER = const Modifier('REFERENCES_SUPER', 13);
static const Modifier REFERENCES_SUPER = const Modifier('REFERENCES_SUPER', 14);
/**
* Indicates that the pseudo-modifier 'set' was applied to the element.
*/
static const Modifier SETTER = const Modifier('SETTER', 14);
static const Modifier SETTER = const Modifier('SETTER', 15);
/**
* Indicates that the modifier 'static' was applied to the element.
*/
static const Modifier STATIC = const Modifier('STATIC', 15);
static const Modifier STATIC = const Modifier('STATIC', 16);
/**
* Indicates that the element does not appear in the source code but was implicitly created. For
* example, if a class does not define any constructors, an implicit zero-argument constructor
* will be created and it will be marked as being synthetic.
*/
static const Modifier SYNTHETIC = const Modifier('SYNTHETIC', 16);
static const Modifier SYNTHETIC = const Modifier('SYNTHETIC', 17);
/**
* Indicates that a class was defined using an alias. TODO(brianwilkerson) This should be renamed
* to 'ALIAS'.
*/
static const Modifier TYPEDEF = const Modifier('TYPEDEF', 17);
static const Modifier TYPEDEF = const Modifier('TYPEDEF', 18);
static const List<Modifier> values = const [
ABSTRACT,
@ -8803,6 +8818,7 @@ class Modifier extends Enum<Modifier> {
GENERATOR,
GETTER,
HAS_EXT_URI,
IS_USED_VARIABLE,
MIXIN,
POTENTIALLY_MUTATED_IN_CONTEXT,
POTENTIALLY_MUTATED_IN_SCOPE,
@ -11584,6 +11600,14 @@ abstract class VariableElementImpl extends ElementImpl implements VariableElemen
@override
bool get isFinal => hasModifier(Modifier.FINAL);
/**
* Return `true` if this variable is used (i.e. purely read or invoked)
* somewhere in its scope. This information is only available for local
* variables (including parameters) and only after the compilation unit
* containing the variable has been resolved.
*/
bool get isUsed => false;
/**
* Return `true` if this variable is potentially mutated somewhere in a closure. This
* information is only available for local variables (including parameters) and only after the

16
pkg/analyzer/lib/src/generated/error.dart
View file

@ -2667,17 +2667,22 @@ class HintCode extends Enum<HintCode> implements ErrorCode {
static const HintCode UNNECESSARY_TYPE_CHECK_TRUE = const HintCode.con1('UNNECESSARY_TYPE_CHECK_TRUE', 26, "Unnecessary type check, the result is always true");
/**
* Unused imports are imports which are never not used.
* Unused imports are imports which are never used.
*/
static const HintCode UNUSED_IMPORT = const HintCode.con1('UNUSED_IMPORT', 27, "Unused import");
/**
* Unused local variables are local varaibles which are never read.
*/
static const HintCode UNUSED_LOCAL_VARIABLE = const HintCode.con1('UNUSED_LOCAL_VARIABLE', 28, "The value of the local variable '{0}' is not used");
/**
* Hint for cases where the source expects a method or function to return a non-void result, but
* the method or function signature returns void.
*
* @param name the name of the method or function that returns void
*/
static const HintCode USE_OF_VOID_RESULT = const HintCode.con1('USE_OF_VOID_RESULT', 28, "The result of '{0}' is being used, even though it is declared to be 'void'");
static const HintCode USE_OF_VOID_RESULT = const HintCode.con1('USE_OF_VOID_RESULT', 29, "The result of '{0}' is being used, even though it is declared to be 'void'");
/**
* It is a bad practice for a source file in a package "lib" directory hierarchy to traverse
@ -2685,7 +2690,7 @@ class HintCode extends Enum<HintCode> implements ErrorCode {
* contain a directive such as `import '../web/some.dart'` which references a file outside
* the lib directory.
*/
static const HintCode FILE_IMPORT_INSIDE_LIB_REFERENCES_FILE_OUTSIDE = const HintCode.con1('FILE_IMPORT_INSIDE_LIB_REFERENCES_FILE_OUTSIDE', 29, "A file in the 'lib' directory hierarchy should not reference a file outside that hierarchy");
static const HintCode FILE_IMPORT_INSIDE_LIB_REFERENCES_FILE_OUTSIDE = const HintCode.con1('FILE_IMPORT_INSIDE_LIB_REFERENCES_FILE_OUTSIDE', 30, "A file in the 'lib' directory hierarchy should not reference a file outside that hierarchy");
/**
* It is a bad practice for a source file ouside a package "lib" directory hierarchy to traverse
@ -2693,14 +2698,14 @@ class HintCode extends Enum<HintCode> implements ErrorCode {
* contain a directive such as `import '../lib/some.dart'` which references a file inside
* the lib directory.
*/
static const HintCode FILE_IMPORT_OUTSIDE_LIB_REFERENCES_FILE_INSIDE = const HintCode.con1('FILE_IMPORT_OUTSIDE_LIB_REFERENCES_FILE_INSIDE', 30, "A file outside the 'lib' directory hierarchy should not reference a file inside that hierarchy. Use a package: reference instead.");
static const HintCode FILE_IMPORT_OUTSIDE_LIB_REFERENCES_FILE_INSIDE = const HintCode.con1('FILE_IMPORT_OUTSIDE_LIB_REFERENCES_FILE_INSIDE', 31, "A file outside the 'lib' directory hierarchy should not reference a file inside that hierarchy. Use a package: reference instead.");
/**
* It is a bad practice for a package import to reference anything outside the given package, or
* more generally, it is bad practice for a package import to contain a "..". For example, a
* source file should not contain a directive such as `import 'package:foo/../some.dart'`.
*/
static const HintCode PACKAGE_IMPORT_CONTAINS_DOT_DOT = const HintCode.con1('PACKAGE_IMPORT_CONTAINS_DOT_DOT', 31, "A package import should not contain '..'");
static const HintCode PACKAGE_IMPORT_CONTAINS_DOT_DOT = const HintCode.con1('PACKAGE_IMPORT_CONTAINS_DOT_DOT', 32, "A package import should not contain '..'");
static const List<HintCode> values = const [
ARGUMENT_TYPE_NOT_ASSIGNABLE,
@ -2731,6 +2736,7 @@ class HintCode extends Enum<HintCode> implements ErrorCode {
UNNECESSARY_TYPE_CHECK_FALSE,
UNNECESSARY_TYPE_CHECK_TRUE,
UNUSED_IMPORT,
UNUSED_LOCAL_VARIABLE,
USE_OF_VOID_RESULT,
FILE_IMPORT_INSIDE_LIB_REFERENCES_FILE_OUTSIDE,
FILE_IMPORT_OUTSIDE_LIB_REFERENCES_FILE_INSIDE,

53
pkg/analyzer/lib/src/generated/resolver.dart
View file

@ -1909,6 +1909,32 @@ class Dart2JSVerifier extends RecursiveAstVisitor<Object> {
}
}
/**
* Instances of the class [UnusedLocalVariableVerifier] traverse an element
* structure looking for cases of [HintCode.UNUSED_LOCAL_VARIABLE].
*/
class UnusedLocalVariableVerifier extends RecursiveElementVisitor {
/**
* The error reporter by which errors will be reported.
*/
final ErrorReporter _errorReporter;
/**
* Create a new instance of the [UnusedLocalVariableVerifier].
*/
UnusedLocalVariableVerifier(this._errorReporter);
@override
visitLocalVariableElement(LocalVariableElement element) {
if (element is LocalVariableElementImpl && !element.isUsed) {
_errorReporter.reportErrorForElement(
HintCode.UNUSED_LOCAL_VARIABLE,
element,
[element.displayName]);
}
}
}
/**
* Instances of the class `DeadCodeVerifier` traverse an AST structure looking for cases of
* [HintCode#DEAD_CODE].
@ -3474,9 +3500,14 @@ class ElementBuilder extends RecursiveAstVisitor<Object> {
Object visitCatchClause(CatchClause node) {
SimpleIdentifier exceptionParameter = node.exceptionParameter;
if (exceptionParameter != null) {
// exception
LocalVariableElementImpl exception = new LocalVariableElementImpl.forNode(exceptionParameter);
_currentHolder.addLocalVariable(exception);
exceptionParameter.staticElement = exception;
// we cannot catch an exception without declaring a variable,
// so the exception variable is always used
exception.markUsed();
// stack trace
SimpleIdentifier stackTraceParameter = node.stackTraceParameter;
if (stackTraceParameter != null) {
LocalVariableElementImpl stackTrace = new LocalVariableElementImpl.forNode(stackTraceParameter);
@ -5394,6 +5425,7 @@ class HintGenerator {
unit.accept(_importsVerifier);
// dead code analysis
unit.accept(new DeadCodeVerifier(errorReporter));
unit.element.accept(new UnusedLocalVariableVerifier(errorReporter));
// dart2js analysis
if (_enableDart2JSHints) {
unit.accept(new Dart2JSVerifier(errorReporter));
@ -15280,7 +15312,9 @@ class VariableResolverVisitor extends ScopedVisitor {
if (parent is PropertyAccess && identical(parent.propertyName, node)) {
return null;
}
if (parent is MethodInvocation && identical(parent.methodName, node)) {
if (parent is MethodInvocation &&
identical(parent.methodName, node) &&
parent.target != null) {
return null;
}
if (parent is ConstructorName) {
@ -15298,13 +15332,28 @@ class VariableResolverVisitor extends ScopedVisitor {
ElementKind kind = element.kind;
if (kind == ElementKind.LOCAL_VARIABLE) {
node.staticElement = element;
LocalVariableElementImpl variableImpl = element as LocalVariableElementImpl;
if (node.inSetterContext()) {
LocalVariableElementImpl variableImpl = element as LocalVariableElementImpl;
variableImpl.markPotentiallyMutatedInScope();
if (element.enclosingElement != _enclosingFunction) {
variableImpl.markPotentiallyMutatedInClosure();
}
}
if (node.inGetterContext()) {
if (parent.parent is ExpressionStatement &&
(parent is PrefixExpression ||
parent is PostfixExpression ||
parent is AssignmentExpression && parent.leftHandSide == node)) {
// v++;
// ++v;
// v += 2;
} else {
variableImpl.markUsed();
}
}
if (parent is MethodInvocation && parent.methodName == node) {
variableImpl.markUsed();
}
} else if (kind == ElementKind.PARAMETER) {
node.staticElement = element;
if (node.inSetterContext()) {

140
pkg/analyzer/test/generated/resolver_test.dart
View file

@ -3206,6 +3206,137 @@ class B {}''');
verify([source, source2]);
}
void test_unusedLocalVariable() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
main() {
var v = 1;
v = 2;
}''');
resolve(source);
assertErrors(source, [HintCode.UNUSED_LOCAL_VARIABLE]);
verify([source]);
}
void test_unusedLocalVariable_isRead_notUsed_compoundAssign() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
main() {
var v = 1;
v += 2;
}''');
resolve(source);
assertErrors(source, [HintCode.UNUSED_LOCAL_VARIABLE]);
verify([source]);
}
void test_unusedLocalVariable_isRead_notUsed_postfixExpr() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
main() {
var v = 1;
v++;
}''');
resolve(source);
assertErrors(source, [HintCode.UNUSED_LOCAL_VARIABLE]);
verify([source]);
}
void test_unusedLocalVariable_isRead_notUsed_prefixExpr() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
main() {
var v = 1;
++v;
}''');
resolve(source);
assertErrors(source, [HintCode.UNUSED_LOCAL_VARIABLE]);
verify([source]);
}
void test_unusedLocalVariable_isRead_usedArgument() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
main() {
var v = 1;
print(++v);
}
print(x) {}''');
resolve(source);
assertErrors(source, []);
verify([source]);
}
void test_unusedLocalVariable_isRead_usedInvocationTarget() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
class A {
foo() {}
}
main() {
var a = new A();
a.foo();
}
''');
resolve(source);
assertErrors(source, []);
verify([source]);
}
void test_unusedLocalVariable_isInvoked() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
typedef Foo();
main() {
Foo foo;
foo();
}''');
resolve(source);
assertErrors(source, []);
verify([source]);
}
void test_unusedLocalVariable_inCatch_exception() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
main() {
try {
} catch (exception) {
}
}''');
resolve(source);
assertErrors(source, []);
verify([source]);
}
void test_unusedLocalVariable_inCatch_stackTrace() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
main() {
try {
} catch (exception, stackTrace) {
}
}''');
resolve(source);
assertErrors(source, [HintCode.UNUSED_LOCAL_VARIABLE]);
verify([source]);
}
void test_unusedLocalVariable_inCatch_stackTrace_used() {
enableUnusedLocalVariable = true;
Source source = addSource(r'''
main() {
try {
} catch (exception, stackTrace) {
print('exception at $stackTrace');
}
}
print(x) {}''');
resolve(source);
assertErrors(source, []);
verify([source]);
}
void test_useOfVoidResult_assignmentExpression_function() {
Source source = addSource(r'''
void f() {}
@ -6200,6 +6331,11 @@ class ResolverTestCase extends EngineTestCase {
*/
AnalysisContextImpl analysisContext2;
/**
* Specifies if [assertErrors] should check for [HintCode.UNUSED_LOCAL_VARIABLE].
*/
bool enableUnusedLocalVariable = false;
@override
void setUp() {
reset();
@ -6242,6 +6378,10 @@ class ResolverTestCase extends EngineTestCase {
void assertErrors(Source source, List<ErrorCode> expectedErrorCodes) {
GatheringErrorListener errorListener = new GatheringErrorListener();
for (AnalysisError error in analysisContext2.computeErrors(source)) {
if (error.errorCode == HintCode.UNUSED_LOCAL_VARIABLE &&
!enableUnusedLocalVariable) {
continue;
}
errorListener.onError(error);
}
errorListener.assertErrorsWithCodes(expectedErrorCodes);