Skip to content

Commit

Permalink
[wildcards] update UNUSED_LOCAL_VARIABLE for wildcards
Browse files Browse the repository at this point in the history
Updates `UNUSED_LOCAL_VARIABLE` to flag `__`s when the wildcard variables feature is enabled.

Fixes: #55719

The implementation is quick so I'm very open to feedback.

Also, I'm not sure how exhaustively we want/need to test. Coverage is incomplete and I'm not sure it needs to be *but* for sure if there are interesting cases missed, please do let me know!



Change-Id: I4d95d5b94141fa033b3c426ac4b5a721a12fc107
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366403
Reviewed-by: Kallen Tu <kallentu@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
  • Loading branch information
pq authored and Commit Queue committed May 14, 2024
1 parent 88f5c43 commit 3f53542
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 8 deletions.
23 changes: 16 additions & 7 deletions pkg/analyzer/lib/src/error/unused_local_elements_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'dart:collection';
import 'dart:math' as math;

import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
Expand Down Expand Up @@ -485,14 +486,19 @@ class UnusedLocalElementsVerifier extends RecursiveAstVisitor<void> {
/// The URI of the library being verified.
final Uri _libraryUri;

/// Whether the `wildcard_variables` feature is enabled.
final bool _wildCardVariablesEnabled;

/// The current set of pattern variable elements, used to track whether _all_
/// within a [PatternVariableDeclaration] are used.
List<BindPatternVariableElement>? _patternVariableElements;

/// Create a new instance of the [UnusedLocalElementsVerifier].
UnusedLocalElementsVerifier(this._errorListener, this._usedElements,
this._inheritanceManager, LibraryElement library)
: _libraryUri = library.source.uri;
: _libraryUri = library.source.uri,
_wildCardVariablesEnabled =
library.featureSet.isEnabled(Feature.wildcard_variables);

@override
void visitCatchClauseParameter(CatchClauseParameter node) {
Expand Down Expand Up @@ -662,7 +668,7 @@ class UnusedLocalElementsVerifier extends RecursiveAstVisitor<void> {
if (isUsed) {
return;
}
if (!_isNamedUnderscore(element)) {
if (!_isNamedWildcard(element)) {
elementsToReport.add(element);
}
}
Expand Down Expand Up @@ -749,16 +755,19 @@ class UnusedLocalElementsVerifier extends RecursiveAstVisitor<void> {
return correspondingParameter;
}

/// Returns whether the name of [element] consists only of underscore
/// characters.
bool _isNamedUnderscore(LocalVariableElement element) {
/// Returns whether the name of [element] should be treated as a wildcard.
bool _isNamedWildcard(LocalVariableElement element) {
String name = element.name;
for (int index = name.length - 1; index >= 0; --index) {
var length = name.length;
if (length > 1 && _wildCardVariablesEnabled) return false;

for (int index = length - 1; index >= 0; --index) {
if (name.codeUnitAt(index) != 0x5F) {
// 0x5F => '_'
return false;
}
}

return true;
}

Expand Down Expand Up @@ -1010,7 +1019,7 @@ class UnusedLocalElementsVerifier extends RecursiveAstVisitor<void> {
}

void _visitLocalVariableElement(LocalVariableElement element) {
if (!_isUsedElement(element) && !_isNamedUnderscore(element)) {
if (!_isUsedElement(element) && !_isNamedWildcard(element)) {
ErrorCode errorCode;
if (_usedElements.isCatchException(element)) {
errorCode = WarningCode.UNUSED_CATCH_CLAUSE;
Expand Down
118 changes: 117 additions & 1 deletion pkg/analyzer/test/src/diagnostics/unused_local_variable_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// 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/analysis/features.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

Expand All @@ -10,6 +11,7 @@ import '../dart/resolution/context_collection_resolution.dart';
main() {
defineReflectiveSuite(() {
defineReflectiveTests(UnusedLocalVariableTest);
defineReflectiveTests(UnusedLocalVariableWildCardVariablesTest);
});
}

Expand Down Expand Up @@ -128,7 +130,7 @@ void f(Object? x) {
''');
}

test_inFor_underscore_ignored() async {
test_inFor_underscores() async {
await assertNoErrorsInCode(r'''
main() {
for (var _ in [1,2,3]) {
Expand Down Expand Up @@ -502,3 +504,117 @@ main() {
''');
}
}

@reflectiveTest
class UnusedLocalVariableWildCardVariablesTest extends UnusedLocalVariableTest {
@override
List<String> get experiments => [
...super.experiments,
Feature.wildcard_variables.enableString,
];

@override
test_inFor_underscores() async {
await assertErrorsInCode(r'''
main() {
for (var _ in [1,2,3]) {
for (var __ in [4,5,6]) {
// do something
}
}
}
''', [
error(WarningCode.UNUSED_LOCAL_VARIABLE, 49, 2),
]);
}

test_localVariable_forElement_underscores() async {
await assertErrorsInCode(r'''
main() {
[
for (var __ in [1, 2, 3]) 1
];
}
''', [
error(WarningCode.UNUSED_LOCAL_VARIABLE, 30, 2),
]);
}

test_localVariable_underscores() async {
await assertErrorsInCode(r'''
main() {
var __ = 0;
var ___ = 0;
}
''', [
error(WarningCode.UNUSED_LOCAL_VARIABLE, 15, 2),
error(WarningCode.UNUSED_LOCAL_VARIABLE, 29, 3),
]);
}

test_localVariable_wildcard() async {
await assertNoErrorsInCode(r'''
main() {
var _ = 0;
}
''');
}

test_localVariableListPattern_underscores() async {
await assertErrorsInCode(r'''
main() {
var [__] = [1];
}
''', [
error(WarningCode.UNUSED_LOCAL_VARIABLE, 16, 2),
]);
}

test_localVariableListPattern_wildcard() async {
await assertNoErrorsInCode(r'''
main() {
var [_] = [1];
}
''');
}

test_localVariablePattern_underscores() async {
await assertErrorsInCode(r'''
main() {
var (__) = (1);
}
''', [
error(WarningCode.UNUSED_LOCAL_VARIABLE, 16, 2),
]);
}

test_localVariablePattern_wildcard() async {
await assertNoErrorsInCode(r'''
main() {
var (_) = (1);
}
''');
}

test_localVariableSwitchListPattern_underscores() async {
await assertErrorsInCode(r'''
void f(Object o) {
switch(o) {
case [var __] : {}
}
}
''', [
error(WarningCode.UNUSED_LOCAL_VARIABLE, 47, 2),
]);
}

test_localVariableSwitchListPattern_wildcard() async {
await assertNoErrorsInCode(r'''
void f(Object o) {
switch(o) {
case [var _] : {}
}
}
''');
}
}

0 comments on commit 3f53542

Please sign in to comment.