8000 [Wildcard Variables] `UNUSED_LOCAL_VARIABLE` support · Issue #55719 · dart-lang/sdk · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Wildcard Variables] UNUSED_LOCAL_VARIABLE support #55719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Tracked by #55680
pq opened this issue May 14, 2024 · 4 comments
Open
Tracked by #55680

[Wildcard Variables] UNUSED_LOCAL_VARI 8000 ABLE support #55719

pq opened this issue May 14, 2024 · 4 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-warning Issues with the analyzer's Warning codes feature-wildcard-variables Implementation of the wildcard variables feature P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@pq
Copy link
Member
pq commented May 14, 2024 •
edited
Loading

As per the discussion in #59470, we want to start reporting UNUSED_LOCAL_VARIABLE for non wildcard underscore cases (e.g., __, ___, etc).

UPDATE: non-wildcard underscores will produce diagnostics but whether we want to start reporting on other identifiers (e.g., e or exception) is still an open question. (See proposal below.)
@pq pq added legacy-area-analyzer Use area-devexp instead. devexp-warning Issues with the analyzer's Warning codes P2 A bug or feature request we're likely to work on feature-wildcard-variables Implementation of the wildcard variables feature labels May 14, 2024
@pq pq self-assigned this May 14, 2024
@pq pq mentioned this issue May 14, 2024
Closed
42 tasks
@pq pq changed the title [Wildcard Variable] Update UNUSED_LOCAL_VARIABLE reporting to flag multiple _s [Wildcard Variables] Update UNUSED_LOCAL_VARIABLE reporting to flag multiple _s May 14, 2024
@kallentu kallentu mentioned this issue May 14, 2024
Closed
5 tasks
@pq pq mentioned this issue May 14, 2024
Open
@pq pq changed the title [Wildcard Variables] Update UNUSED_LOCAL_VARIABLE reporting to flag multiple _s [Wildcard Variables] UNUSED_LOCAL_VARIABLE support May 17, 2024
@pq pq reopened this May 17, 2024
@pq pq mentioned this issue May 17, 2024
Closed
@pq
Copy link
Member Author
pq commented May 17, 2024

Follow-up from #57132,

Proposal: report UNUSED_LOCAL_VARIABLE on all unused bound catch clause parameters

Specifically, this means we start flagging the following (which we don't today) with an UNUSED_LOCAL_VARIABLE diagnostic:

try {
} catch(e) { }

providing a quick-fix conversion to:

try {
} catch(_) { }

Since the idiom of catching (and ignoring) an exception named e or similar is well established, this will produce a bunch of diagnostics in some projects but with dart fix code can effortlessly be migrated.

/cc @dart-lang/language-team @dart-lang/analyzer-team

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label May 31, 2024
@kallentu
Copy link
Member
kallentu commented Aug 13, 2024 •
edited by pq
Loading

This resurfaced while testing const evaluation in the analyzer, in a comment here.

We should probably remove the UNUSED_LOCAL_VARIABLE diagnostic that's being reported here and other places.

test() {
  const _ = true; // Should not have `UNUSED_LOCAL_VARIABLE`, but currently does.
  const c = _;
}

EDIT: WAI; see below.

8000
@pq
Copy link
Member Author
pq commented Aug 27, 2024

Revisiting this case, I think it's actually working as intended and we're getting tripped up by the test framework.

Here's the test:

  test_visitSimpleIdentifier_wildcard_local() async {
    await assertErrorsInCode(r'''
test() {
  const _ = true;
  const c = _;
}
''', [
      error(WarningCode.UNUSED_LOCAL_VARIABLE, 35, 1),
      error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 39, 1),
      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 39,
          1),
    ]);
  }

The unused variable being reported is actually c (the _ is at offset 17).

Offset-based tests test expectations are easy to get tripped up by!

@kallentu: do you recall the intention of the test?

If it's valuable as-is, we could make it a little more explicit by updating the expectation like so:

error(WarningCode.UNUSED_LOCAL_VARIABLE, 35, 1, messageContains: ["'c'"]),

@kallentu
Copy link
Member

Ah, that makes a lot more sense. Woops, sorry for the noise.

The test was mainly trying to make sure we would produce an error if we tried to access/use the const _. I think it's useful to have still.

copybara-service bot pushed a commit that referenced this issue Aug 27, 2024
@pq
[wildcards] clarify evaluation test expectation
77f9cbf 
See: #55719

Change-Id: I4e7bea8c571f3c52313a6f69af3837982a829207
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382441
Reviewed-by: Kallen Tu <kallentu@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 27, 2025
@pq pq removed their assignment Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-warning Issues with the analyzer's Warning codes feature-wildcard-variables Implementation of the wildcard variables feature P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@pq @srawlins @bwilkerson @kallentu
0