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

Analyzer doesn't warn on invalid constant expression with a generic #33612

Closed
DanTup opened this Issue Jun 25, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@DanTup
Copy link
Member

commented Jun 25, 2018

I don't know if there's another issue that covers this, but I couldn't see anything obvious. I'm working on some tests in flutter_tools but since it's a non-Flutter project it's using the VM directly (latest dev build - v2.0.0-64.1). I'm passing --preview-dart-2 as an analyzer arg and a VM arg (since I don't think it's on my default when invoked directly):

Spawning /Users/dantup/Dev/dart-sdk/v2/bin/dart
    with args [
        "/Users/dantup/Dev/dart-sdk/v2/bin/snapshots/analysis_server.dart.snapshot",
        "--client-id=Dart-Code.dart-code",
        "--client-version=2.15.0-beta.1",
        "--preview-dart-2"
    ]

I wrote some code like this:

const isInstanceOf<T>()

Originally it had a type name in it, then I made it generic. I get no warnings on it, but when I run the code I get:

test/integration/flutter_tester_hot_reload_test.dart:91:34: Error: Not a constant expression.
  expect(ref, const isInstanceOf<T>());
                                 ^

Seems like the analyzer should pick this up?

screen shot 2018-06-25 at 3 15 17 pm

@DanTup

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2018

Related: if I delete the word const and add new the Analyzer tells me to prefer const with constant constructors (which also fails my Flutter PR branch). Unsure if I'm doing something wrong or this is a bug.

@lrhn

This comment has been minimized.

Copy link
Member

commented Jun 26, 2018

It's a bug, you're doing the right thing.

The expression is not constant if it contains a type variable, so you can't use const.
If the analyzer tells you to use const, then it's misguided.

@scheglov

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2018

I see that we do report the corresponding error.

class A<T> {
  const A();
}

main<T>() {
  const A<T>();
}

But the way we report it (in ErrorVerifier) is not useful for linter (which uses ConstantVerifier).

https://dart-review.googlesource.com/c/sdk/+/73283 will fix this.
Linter also should be updated to check for this error in HasConstantErrorListener.
I think I will close this issue once the Analyzer changes lands.

@scheglov scheglov closed this Sep 5, 2018

dart-bot pushed a commit that referenced this issue Sep 6, 2018

Move CONST_WITH_TYPE_PARAMETERS reporting to ConstantVerifier.
Linter also should be updated to check for this error in HasConstantErrorListener.

R=brianwilkerson@google.com

Bug: #33612
Change-Id: If4a2535ffe04d965bbd46b5c92679e6132bf500e
Reviewed-on: https://dart-review.googlesource.com/73283
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.