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

Strong mode differs based on how it's enabled #26129

Closed
nex3 opened this issue Mar 29, 2016 · 9 comments
Closed

Strong mode differs based on how it's enabled #26129

nex3 opened this issue Mar 29, 2016 · 9 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@nex3
Copy link
Member

nex3 commented Mar 29, 2016

I have the following file called test.dart:

import 'dart:async';

abstract class Result<T> {
  static Future/*<T>*/ release/*<T>*/(Future<Result/*<T>*/> future) =>
      future.then/*<Future<T>>*/((result) => result.asFuture);

  Future<T> get asFuture;
}

If I run dartanalyzer test.dart with no .analysis_options file, or if I run dartanalyzer --strong test.dart with or without an .analysis_options file, it prints no errors. However, if I create the following .analysis_options file:

analyzer:
  strong-mode: true

and I run dartanalyzer test.dart (without --strong), it prints the following error:

[error] The return type 'Future<Future<T>>' is not a 'Future<T>', as defined by the method 'release'.
@nex3 nex3 added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-strong-mode type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Mar 29, 2016
@bwilkerson bwilkerson added the P2 A bug or feature request we're likely to work on label Mar 29, 2016
@jmesserly
Copy link

Crazy! Probably picking up different SDKs?

(aside: not sure if it was intended, but in that example the generic method "T" shadows the class "T" ... we should probably be issuing an error for that, but might not be implemented)

@nex3
Copy link
Member Author

nex3 commented Mar 30, 2016

(aside: not sure if it was intended, but in that example the generic method "T" shadows the class "T" ... we should probably be issuing an error for that, but might not be implemented)

That is intended; I'd expect it not to be a problem, since it's a static method.

@jmesserly
Copy link

That is intended; I'd expect it not to be a problem, since it's a static method.

Ah, right! Yeah, should be fine in that case. (There used to be an analyzer warning disallowing any type parameters in static methods, I think I fixed it to only disallow the class's type parameters...)

@lrhn
Copy link
Member

lrhn commented Mar 30, 2016

I don't think it should be a problem to shadow a type variable in a non-static method either. Shadowing is allowed for all other names.

@leafpetersen
Copy link
Member

@bwilkerson @pq Any chance either of you could spare a few cycles to try to track this down? It's small but insidious. It looks like something with the way that analyzer options are getting propagated from command line vs .analysis_options. Maybe not getting copied in somewhere when set from the .analysis_options file?

@pq
Copy link
Member

pq commented May 3, 2016

@leafpetersen : I can repro this and have a breakpoint set in the debugger but don't quite know what to make of what I'm seeing... Maybe we can take a look tomorrow?

@pq
Copy link
Member

pq commented May 4, 2016

Running this again today I'm seeing different results in the CLI.

[error] The type '((Result<T>) → dynamic, {onError: Function}) → Future<dynamic>' is declared with 0 type parameters, but 1 type arguments were given (/src/sandbox/console-full/lib/test.dart, line 5, col 14)
[warning] Unsound implicit cast from Future<dynamic> to Future<T> (/src/sandbox/console-full/lib/test.dart, line 5, col 7)
1 error and 1 warning found.

The basic issue is that we're not properly propagating .analysis_options configurations to the SDK context. I'll work on fixing that. In the meantime, if there's anything interesting about the new errors, chime in!

@leafpetersen
Copy link
Member

@pq That new error is what you get when you analyze strong mode code against a non-strong mode SDK. Basically the declaration (I think Future.then here?) has been analyzed ignoring generic methods, and now in your application at the call site a generic parameter is being passed.

@pq pq closed this as completed in 7913968 May 10, 2016
@nex3
Copy link
Member Author

nex3 commented May 10, 2016

Yayyyy

pq added a commit to pq/flutter that referenced this issue May 10, 2016
The latest dev build has stable summaries so we should start using them.

(Also ensures that analysis options are propogating to the SDK analysis context --- see:  dart-lang/sdk#26129.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants