Skip to content
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

Lint for cast to extension type (formerly known as inline class) #4515

Open
Cat-sushi opened this issue Jun 30, 2023 · 7 comments
Open

Lint for cast to extension type (formerly known as inline class) #4515

Cat-sushi opened this issue Jun 30, 2023 · 7 comments
Labels
linter-lint-request linter-new-language-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

@Cat-sushi
Copy link

Cat-sushi commented Jun 30, 2023

I understand that as is a responsibility delegation from the compiler to users.
And, the compiler doesn't check down casts with as, but the runtime does.

On the other hand, the runtime skips user-defined check logic on constructor of inline class.

So, I propose a new lint to warn a cast from the representation type to the inline class with check logic or body on its constructor, without implicit constructor.

@eernstg
Copy link
Member

eernstg commented Jun 30, 2023

Right, we've discussed having this kind of lint as well. It should apply in the case where any expression is subject to a cast to an inline type, e is InlineType, but probably also for ary cast where this kind of change is a consequence of the structure: myListOfRepresentationType as List<InlineType>, or casting a void Function(InlineType) to void Function(RepresentationType) (because this cast will allow us to silently turn an expression of RepresentationType into an InlineType every time we call that function).

On the other hand, if the inline class can be declared to be a subtype of a non-inline class then the authors/maintainers of the InlineClass have already decided that it should be very easy to go from one to the other and back. In that case we would presumably not emit any lints.

@eernstg eernstg transferred this issue from dart-lang/language Jun 30, 2023
@eernstg
Copy link
Member

eernstg commented Jun 30, 2023

(I transferred this issue to the linter repo because it's a request for a lint.)

@srawlins
Copy link
Member

CC @pq I don't think this has been implemented yet...

@srawlins srawlins added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Sep 11, 2023
@pq
Copy link
Member

pq commented Sep 11, 2023

Correct. No yet implemented. (But a really interesting idea!)

@eernstg eernstg changed the title Lint for cast to inline class Lint for cast to extension type (formerly known as inline class) Sep 13, 2023
@eernstg
Copy link
Member

eernstg commented Sep 13, 2023

@Cat-sushi, the targeted feature has been renamed from 'inline class' to 'extension type'. I adjusted the title accordingly. Please re-edit as needed if you disagree.

@srawlins
Copy link
Member

@matanlurey and I were discussing: it would be good if this caught some non-explicit as-casts that stem from SDK libraries as well:

void f(List<int> employeeIds) {
  employeeIds.cast<EmployeeId>();
}

should be reported. And other cast methods from the SDK.

@matanlurey
Copy link
Contributor

Yes please!

The current implementation reminds me a lot of new type. I can imagine casting should be rare but possible. For example:

Future<int> getUserInput(String prompt) { ... }

void main() async {
  final maybeId = await getUserInput('What is your employee ID?');

  // Throws ArgumentError if <= 0.
  final checkedId = EmployeeId(maybeId);
}

You'd only want to cast when it would be performance prohibitive to perform validation on pre-validation numbers:

// I'd argue this is a bad example, because checking a number to see if it's <= 0 is so cheap...

Future<List<int>> readAllUserIds() { ... }

void main() {
  final allIds = (await readAllUserIds()).cast<EmployeeId>();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter-lint-request linter-new-language-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
Development

No branches or pull requests

6 participants