Skip to content

Commit

Permalink
When reporting unused elements, do not count a CommentReference as usage
Browse files Browse the repository at this point in the history
Fixes #37116

Change-Id: I9ac563036a367a5e5da4fbba8ab521f174062570
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127223
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
srawlins authored and commit-bot@chromium.org committed Dec 6, 2019
1 parent 7075032 commit 0939101
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 19 deletions.
24 changes: 19 additions & 5 deletions pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ class GatherUsedLocalElementsVisitor extends RecursiveAstVisitor {
if (node.inDeclarationContext()) {
return;
}
if (_inCommentReference(node)) {
return;
}
Element element = node.staticElement;
// Store un-parameterized members.
if (element is ExecutableMember) {
Expand Down Expand Up @@ -131,18 +134,18 @@ class GatherUsedLocalElementsVisitor extends RecursiveAstVisitor {
if (element == null) {
return;
}
// check if a local element
// Check if [element] is a local element.
if (!identical(element.library, _enclosingLibrary)) {
return;
}
// ignore references to an element from itself
// Ignore references to an element from itself.
if (identical(element, _enclosingClass)) {
return;
}
if (identical(element, _enclosingExec)) {
return;
}
// ignore places where the element is not actually used
// Ignore places where the element is not actually used.
if (node.parent is TypeName) {
if (element is ClassElement) {
AstNode parent2 = node.parent.parent;
Expand All @@ -161,13 +164,24 @@ class GatherUsedLocalElementsVisitor extends RecursiveAstVisitor {
usedElements.addElement(element);
}

/// Returns whether [identifier] is found in a [CommentReference].
static bool _inCommentReference(SimpleIdentifier identifier) {
var parent = identifier.parent;
return parent is CommentReference || parent?.parent is CommentReference;
}

/// Returns whether the value of [node] is _only_ being read at this position.
///
/// Returns `false` if [node] is not a read access, or if [node] is a combined
/// read/write access.
static bool _isReadIdentifier(SimpleIdentifier node) {
// not reading at all
// Not reading at all.
if (!node.inGetterContext()) {
return false;
}
// check if useless reading
// Check if useless reading.
AstNode parent = node.parent;

if (parent.parent is ExpressionStatement) {
if (parent is PrefixExpression || parent is PostfixExpression) {
// v++;
Expand Down
69 changes: 55 additions & 14 deletions pkg/analyzer/test/src/diagnostics/unused_element_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ main() {
]);
}

test_functionTop_notUsed_referenceInComment() async {
await assertErrorsInCode(r'''
/// [_f] is a great function.
_f(int p) => 7;
''', [
error(HintCode.UNUSED_ELEMENT, 30, 2),
]);
}

test_functionTypeAlias_isUsed_isExpression() async {
await assertNoErrorsInCode(r'''
typedef _F(a, b);
Expand Down Expand Up @@ -461,20 +470,6 @@ class A {
]);
}

test_method_isNotUsed_hasSameNameAsUsed() async {
await assertErrorsInCode(r'''
class A {
void _m1() {}
}
class B {
void public() => _m1();
void _m1() {}
}
''', [
error(HintCode.UNUSED_ELEMENT, 17, 3),
]);
}

test_method_isUsed_hasReference_implicitThis() async {
await assertNoErrorsInCode(r'''
class A {
Expand Down Expand Up @@ -645,6 +640,20 @@ main() {
''');
}

test_method_notUsed_hasSameNameAsUsed() async {
await assertErrorsInCode(r'''
class A {
void _m1() {}
}
class B {
void public() => _m1();
void _m1() {}
}
''', [
error(HintCode.UNUSED_ELEMENT, 17, 3),
]);
}

test_method_notUsed_noReference() async {
await assertErrorsInCode(r'''
class A {
Expand All @@ -667,6 +676,29 @@ class A {
]);
}

test_method_notUsed_referenceInComment() async {
await assertErrorsInCode(r'''
/// [A] has a method, [_f].
class A {
int _f(int p) => 7;
}
''', [
error(HintCode.UNUSED_ELEMENT, 44, 2),
]);
}

test_method_notUsed_referenceInComment_outsideEnclosingClass() async {
await assertErrorsInCode(r'''
class A {
int _f(int p) => 7;
}
/// This is similar to [A._f].
int g() => 7;
''', [
error(HintCode.UNUSED_ELEMENT, 16, 2),
]);
}

test_setter_isUsed_invocation_implicitThis() async {
await assertNoErrorsInCode(r'''
class A {
Expand Down Expand Up @@ -753,4 +785,13 @@ main() {
error(HintCode.UNUSED_ELEMENT, 4, 2),
]);
}

test_topLevelVariable_notUsed_referenceInComment() async {
await assertErrorsInCode(r'''
/// [_a] is a great variable.
int _a = 7;
''', [
error(HintCode.UNUSED_ELEMENT, 34, 2),
]);
}
}
11 changes: 11 additions & 0 deletions pkg/analyzer/test/src/diagnostics/unused_field_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,17 @@ class A {
]);
}

test_notUsed_referenceInComment() async {
await assertErrorsInCode(r'''
/// [A._f] is great.
class A {
int _f;
}
''', [
error(HintCode.UNUSED_FIELD, 37, 2),
]);
}

test_notUsed_simpleAssignment() async {
await assertErrorsInCode(r'''
class A {
Expand Down

0 comments on commit 0939101

Please sign in to comment.