Skip to content

document that no_duplicate_case_values is a no-op in Dart 3.0+ #58863

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.

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

Closed
pq opened this issue Sep 8, 2022 · 5 comments · Fixed by dart-archive/linter#4159
Closed

document that no_duplicate_case_values is a no-op in Dart 3.0+ #58863

pq opened this issue Sep 8, 2022 · 5 comments · Fixed by dart-archive/linter#4159
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-new-language-feature linter-set-core P1 A high priority bug; for example, a single project is unusable or has many test failures type-documentation A request to add or improve documentation
Milestone

Comments

@pq
Copy link
Member

pq commented Sep 8, 2022

https://github.com/dart-lang/language/blob/master/working/0546-patterns/patterns-feature-specification.md#switch-expression

@pq pq added linter-new-language-feature P1 A high priority bug; for example, a single project is unusable or has many test failures labels Sep 8, 2022
@pq
Copy link
Member Author

pq commented Sep 9, 2022

If I understand the exhaustiveness proposal right, unreachable cases will be treated as errors so this lint should not fire and there should be nothing to do here.

@pq
Copy link
Member Author

pq commented Sep 9, 2022

@munificent: is this right?

@pq pq added the type-question A question about expected behavior or functionality label Sep 9, 2022
@srawlins
Copy link
Member

srawlins commented Sep 9, 2022

The no_duplicate_case_values example text has this test case:

  const String A = 'a';
  String v = 'aa';

  switch (v) {
    case 'aa':  // OK
    case 'bb':  // OK
    case A + A: // LINT
    case 'bb':  // LINT
    case A + 'b': // OK
    default:
  }

The exhaustiveness spec talks about literal or constant matcher:

We lift other constants to an extract pattern whose type is a synthesized subtype of the constant's type based on the constant's identity.

This means you don't get exhaustiveness checking for constants, but do get reachability checking:

String s = ...
switch (s) {
  case 'a string': ...
  case 'a string': ...
}

Here, there is an unreachable error on the second case since it matches the same string constant. Also the switch has a non-exhaustive error since it doesn't match the entire String type.

no_duplicate_case_values works by evaluating case expressions as constants, and then putting them in a Map and checking whether a case value is already in the Map. This may not be the same as identity... I'm not sure...

Anyhow, even if there is a difference, maybe we can claim its not interesting.

@lrhn
Copy link
Member

lrhn commented Oct 6, 2022

Another thing to worry about with generalized pattern matching: Cases also get when clauses which can change the behavior of the case.
Say:

switch (e) {
  case 1 when random.nextBool():  print("1/1");
  case 1: print("1/2");
}

which is a kind-of silly way to randomly select between two cases, but it's allowed.

Safest aproach is probably to not apply this lint to any case with a when clause. (Such a case would probably also be ignored for exhaustiveness checking.)

@pq pq changed the title ensure no_duplicate_case_values works with switch-expressions ensure no_duplicate_case_values does NOT lint SwitchPatternCases Jan 10, 2023
@pq
Copy link
Member Author

pq commented Jan 10, 2023

As implemented, this lint will not fire on SwitchPatternCases so it won't report anything on 3.0 ASTs. (It will continue to work for libraries that are opted back though.) We should update the docs to make this clear.

@pq pq added type-documentation A request to add or improve documentation and removed type-question A question about expected behavior or functionality labels Jan 10, 2023
@pq pq changed the title ensure no_duplicate_case_values does NOT lint SwitchPatternCases document that no_duplicate_case_values is a no-op in Dart 3.0+ Jan 10, 2023
@pq pq modified the milestones: Dart 3 beta 1, Dart 3 beta 2 Jan 31, 2023
@pq pq modified the milestones: Dart 3 beta 2, Dart 3 beta 3 Mar 13, 2023
copybara-service bot referenced this issue May 26, 2023
The corresponding lint is no-op in 3.0
https://github.com/dart-lang/linter/issues/3674

Bug: #49759
Change-Id: I24fd82240d8e94e763742c546ec2d4cf3c4df8cb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306126
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-new-language-feature linter-set-core P1 A high priority bug; for example, a single project is unusable or has many test failures type-documentation A request to add or improve documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants