Kallen Tu (Gerrit)

unread,

Feb 20, 2025, 1:21:06 PM2/20/25

to Brian Wilkerson, Phil Quitslund, Commit Queue, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Brian Wilkerson and Phil Quitslund

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:

Brian Wilkerson
Phil Quitslund

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Phil Quitslund (Gerrit)

unread,

Feb 20, 2025, 1:36:36 PM2/20/25

to Kallen Tu, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Brian Wilkerson and Kallen Tu

Phil Quitslund added 2 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Phil Quitslund . resolved

Nice!

File pkg/linter/test/rules/strict_top_level_inference_test.dart

Line 1018, Patchset 1 (Latest): test_wildcardVariable_constructorParameter() async {

Phil Quitslund . unresolved

It would be great to add at least one test that ensures the expected behavior if wildcards are *not* enabled.

In the `DuplicateDefinitionTest`, for example we do this:

```
  test_block_localFunction_wildcard_preWildcards() async {
    await assertErrorsInCode(r'''
// @dart = 3.4
// (pre wildcard-variables)

void f() {
  void _() {}
  int _(int _) => 42;
  String _(int _) => "42";
}
''', [
      error(WarningCode.UNUSED_ELEMENT, 62, 1),
      error(CompileTimeErrorCode.DUPLICATE_DEFINITION, 75, 1,
          contextMessages: [message(testFile, 62, 1)]),
      error(WarningCode.UNUSED_ELEMENT, 75, 1),
      error(CompileTimeErrorCode.DUPLICATE_DEFINITION, 100, 1,
          contextMessages: [message(testFile, 62, 1)]),
      error(WarningCode.UNUSED_ELEMENT, 100, 1),
    ]);
  }
```

(3.4 is kind of arbitrary but safely *pre*-wildcards.)

Open in Gerrit

Related details

Attention is currently required from:

Brian Wilkerson
Kallen Tu

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Brian Wilkerson (Gerrit)

unread,

Feb 20, 2025, 1:50:09 PM2/20/25

to Kallen Tu, Brian Wilkerson, Phil Quitslund, Commit Queue, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Kallen Tu

Brian Wilkerson voted and added 1 comment

Votes added by Brian Wilkerson

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Brian Wilkerson . resolved

LGTM once the test is added.

Open in Gerrit

Related details

Attention is currently required from:

Kallen Tu

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Kallen Tu (Gerrit)

unread,

Feb 20, 2025, 1:58:23 PM2/20/25

to Brian Wilkerson, Phil Quitslund, Commit Queue, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Phil Quitslund

Kallen Tu voted and added 1 comment

Votes added by Kallen Tu

Commit-Queue

+1

1 comment

File pkg/linter/test/rules/strict_top_level_inference_test.dart

Line 1018, Patchset 1: test_wildcardVariable_constructorParameter() async {

Phil Quitslund . resolved

It would be great to add at least one test that ensures the expected behavior if wildcards are *not* enabled.

In the `DuplicateDefinitionTest`, for example we do this:

```
  test_block_localFunction_wildcard_preWildcards() async {
    await assertErrorsInCode(r'''
// @dart = 3.4
// (pre wildcard-variables)

void f() {
  void _() {}
  int _(int _) => 42;
  String _(int _) => "42";
}
''', [
      error(WarningCode.UNUSED_ELEMENT, 62, 1),
      error(CompileTimeErrorCode.DUPLICATE_DEFINITION, 75, 1,
          contextMessages: [message(testFile, 62, 1)]),
      error(WarningCode.UNUSED_ELEMENT, 75, 1),
      error(CompileTimeErrorCode.DUPLICATE_DEFINITION, 100, 1,
          contextMessages: [message(testFile, 62, 1)]),
      error(WarningCode.UNUSED_ELEMENT, 100, 1),
    ]);
  }
```

(3.4 is kind of arbitrary but safely *pre*-wildcards.)

Kallen Tu

Good call, thanks! Added 3 new tests that mirrors the existing ones, but with the version set at 3.4.

Open in Gerrit

Related details

Attention is currently required from:

Phil Quitslund

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Phil Quitslund (Gerrit)

unread,

Feb 20, 2025, 2:16:14 PM2/20/25

to Kallen Tu, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Kallen Tu

Phil Quitslund voted and added 1 comment

Votes added by Phil Quitslund

Code-Review	+1
Commit-Queue	+2

1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Phil Quitslund . resolved

Sweet. Thanks!

Open in Gerrit

Related details

Attention is currently required from:

Kallen Tu

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Kallen Tu (Gerrit)

unread,

Feb 20, 2025, 4:24:32 PM2/20/25

to Phil Quitslund, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org

Attention needed from Phil Quitslund

Kallen Tu voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Phil Quitslund

Submit Requirements:

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Commit Queue (Gerrit)

unread,

Feb 20, 2025, 4:24:42 PM2/20/25

to Kallen Tu, Phil Quitslund, Brian Wilkerson, dart-analys...@google.com, rev...@dartlang.org

Commit Queue submitted the change

Change information

Commit message:

[linter] Issue 60136: Update strict_top_level_inference to ignore wildcard variables.

Avoid linting on wildcard variables. Wildcard variables won't be used, so it's okay that the type will be `dynamic`.

Fixes: https://github.com/dart-lang/sdk/issues/60136

Bug: https://github.com/dart-lang/sdk/issues/60136

Change-Id: I6bc0543bfbd3b8eac09ca934fc5098b6c825e1d6

Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410940

Reviewed-by: Phil Quitslund <pquit...@google.com>

Reviewed-by: Brian Wilkerson <brianwi...@google.com>

Commit-Queue: Kallen Tu <kall...@google.com>

Files:

M pkg/linter/lib/src/rules/strict_top_level_inference.dart
M pkg/linter/test/rules/strict_top_level_inference_test.dart

Change size: M

Delta: 2 files changed, 75 insertions(+), 4 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Phil Quitslund, +1 by Brian Wilkerson

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

[S] Change in dart/sdk[main]: [linter] Issue 60136: Update strict_top_level_inference to ignore wil...

Kallen Tu (Gerrit)

New activity on the change

Related details

Phil Quitslund (Gerrit)

Phil Quitslund added 2 comments

Related details

Brian Wilkerson (Gerrit)

Brian Wilkerson voted and added 1 comment

Votes added by Brian Wilkerson

1 comment

Related details

Kallen Tu (Gerrit)

Kallen Tu voted and added 1 comment

Votes added by Kallen Tu

1 comment

Related details

Phil Quitslund (Gerrit)

Phil Quitslund voted and added 1 comment

Votes added by Phil Quitslund

1 comment

Related details

Kallen Tu (Gerrit)

Kallen Tu voted Commit-Queue+2

Related details

Commit Queue (Gerrit)

Commit Queue submitted the change

Change information