Skip to content

Lint for using a type literal in a switch case #59087

Closed
dart-archive/linter
#4207
@munificent

Description

@munificent

Consider:

switch (obj) {
  case int: print('int');
}

In Dart both before 3.0 and in 3.0 with patterns, this case does not match integers, it matches only the type object int itself. Prior to 3.0, there was no way to do a type test in a switch, so users would rarely ever think to write the name of a type and expect it to type test.

With patterns, we expect type tests to be common in switches. There are two idiomatic ways to write them:

switch (obj) {
  case int(): print('int');

  case int _: print('int');
}

(The first, an object pattern, is useful when you want to do further destructuring on the value after testing its type. It's likely the idiomatic style to use in algebraic datatype style code. The latter, a wildcard variable declaration, allows you to match any possible type annotation. Object patterns only support named types, but not more complex type annotations like records, functions, nullable types, etc.)

Because type tests are common in patterns, we expect that users will sometimes write a type literal when they meant to do a type test. This is valid code (though in some cases it will lead to non-exhaustive switch errors), but it probably doesn't do what the user wants.

We tried hard to tweak the syntax to avoid this footgun, but every other approach we came up with seemed to be worse overall. We also considered making it an error to have a constant in a pattern that resolves to a type literal. This would force users away from the footgun... but it's also a breaking change. There is code in the wild today that deliberately uses type literals in switch cases to test for type objects.

Given that, I think it would help the user experience if we added a lint that reports a type literal being used as a constant in a switch case (in a Dart 3.0 library). Instead, it should suggest that users use either an object pattern or variable pattern if they meant to do a type test, or an explicit == pattern if they really did mean to compare for equality to a type literal.

The heuristic I'd suggest is:

  1. If the matched value type is Type, then suggest they change the case from case SomeType to case == SomeType. That has the same semantics, but makes it clearer to the reader that a type equality test is intended (and silences the lint).

  2. Otherwise, if the matched value type is anything else, suggest that they use case SomeType _ if they indended to do a type test and == SomeType otherwise.

This lint will be useful whenever it arrives, but nothing is blocked in 3.0 around having it. It should just help users avoid what's likely a mistake.

Activity

added
P2A bug or feature request we're likely to work on
on Mar 22, 2023
scheglov

scheglov commented on Mar 22, 2023

@scheglov
Contributor

@pq @bwilkerson Do we want to make it s lint, or a warning inside the analyzer?

bwilkerson

bwilkerson commented on Mar 22, 2023

@bwilkerson
Member

It isn't wrong to use a type literal in a switch case, so I see this as enforcing a coding style, which makes it a lint.

transferred this issue fromdart-lang/sdkon Mar 22, 2023
self-assigned this
on Mar 22, 2023
modified the milestone: Dart 3 beta 3 on Mar 22, 2023
pq

pq commented on Mar 22, 2023

@pq
Member

I'm all for this and agree with Brian that a lint is the right place to implement.

munificent

munificent commented on Mar 22, 2023

@munificent
MemberAuthor

Yes, if it were up to me (and it's not, this is really for the tooling teams to decide), I would make it a lint too. For me, warnings all represent some kind of dead code: Code you can remove or simplify and the result is a program with the same semantics.

bwilkerson

bwilkerson commented on Mar 22, 2023

@bwilkerson
Member

The other major use for warnings is for code that's guaranteed to be wrong, but for reasons that the type system isn't able to express.

munificent

munificent commented on Mar 22, 2023

@munificent
MemberAuthor

Can you give me an example of the latter?

bwilkerson

bwilkerson commented on Mar 22, 2023

@bwilkerson
Member

We have a warning telling you that the callback can't be used as an error handler:

void f(Future<int> future, Future<int> Function({Object a}) callback) {
  future.catchError(callback);
}

The type system can't tell us that because the type of the parameter is Function.

munificent

munificent commented on Mar 22, 2023

@munificent
MemberAuthor

Oh, interesting. So there's some ad hoc overloading support in the analyzer effectively?

40 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P2A bug or feature request we're likely to work onP3A lower priority bug or feature requestdevexp-linterIssues with the analyzer's support for the linter packagelegacy-area-analyzerUse area-devexp instead.linter-lint-proposallinter-new-language-feature

Type

No type

Projects

No projects

Relationships

None yet

    Development

    Participants

    @munificent@pq@fishythefish@scheglov@devoncarew

    Issue actions

      Lint for using a type literal in a switch case · Issue #59087 · dart-lang/sdk