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

Have a plan for replacements for strong-mode options that will be deprecated #32661

Closed
kevmoo opened this issue Mar 23, 2018 · 18 comments
Closed
Assignees
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).

Comments

@kevmoo
Copy link
Member

kevmoo commented Mar 23, 2018

This is a "discussion" issue for now...but we should figure this out.

Guessing we want to keep the implicit- flags.
Guessing we want to deprecate (And eventually remove) the strong-mode parent.

analyzer:
  strong-mode:
    implicit-casts: false
    implicit-dynamic: false

CC @kwalrath @dgrove

@kevmoo kevmoo added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Mar 23, 2018
@kevmoo kevmoo added this to the Dart2 Beta 3 milestone Mar 23, 2018
@bwilkerson
Copy link
Member

The strong-mode flag/group does not make sense in Dart 2. We are not going to allow users to disable strong-mode semantics. So, yes, we will remove it.

We should allow users to request restrictions that are not required in Dart 2 if there is sufficient demand for the functionality. The mechanism we have defined for doing this is lints. So, for each of the options under discussion, if we decide to support them, we should create a lint rule that flags the style of code that some users might want to disallow, and we should remove the flags from the analysis options file.

As an aside, there are three options that are currently allowed under the strong-mode group: declaration-casts, implicit-casts, and implicit-dynamic. I'm not sure which of those actually make sense in the context of Dart 2. @leafpetersen @lrhn Could you address this question?

@kevmoo Can you determine, for each existing flag that makes sense, whether there is sufficient demand for us to create lint rules to cover those semantics? (I think you know more about how to ask our customers about this issue than I do.)

@lrhn
Copy link
Member

lrhn commented Mar 26, 2018

As an aside, there are three options that are currently allowed under the strong-mode group: declaration-casts, implicit-casts, and implicit-dynamic.

I'm not absolutely sure I know what they mean, so let's see if I get it right:

  • "implicit dynamic": Disallows programs where inference would infer dynamic in some (all?) position.
  • "declaration casts": Disallows programs with implicit down-cast of a variable declaration initializer expression.
  • "implicit casts": Disallows programs with implicit down-cast anywhere (except in declarations?)

In any case, these are all valid lints for Dart 2 - they disallow some programs, but they don't change the behavior of any accepted program.

As lints, it's important to limit the damage to your own code. It would be sad if someone enables such a lint, and then can't use a package that didn't use the lint. I guess this is no different from any other lint.

(And I think it would be more consistent if the new flags were named so that they default to false).

@kevmoo kevmoo added the P2 A bug or feature request we're likely to work on label Mar 26, 2018
@bwilkerson
Copy link
Member

Ok. I believe we have a plan. I'm going to rename this to reflect implementing the plan and move it to a later milestone.

@bwilkerson bwilkerson changed the title Make a plan for implicit-[casts|dynamic] in analysis_options (still under strong-mode?) Implement replacements for strong-mode options that will be deprecated Mar 29, 2018
@bwilkerson bwilkerson modified the milestones: Dart2 Beta 3, Dart2 Stable Mar 29, 2018
@kevmoo
Copy link
Member Author

kevmoo commented Mar 29, 2018

Ok. I believe we have a plan. I'm going to rename this to reflect implementing the plan and move it to a later milestone.

What is the plan?

Can we split up the work? Ship new options soon and give the user deprecation warnings

...then remove support in the last milestone?

@leafpetersen
Copy link
Member

This sounds reasonable to me.

@leafpetersen
Copy link
Member

How does this affect the command line analyzer? I'm kind of confused about how lints interact with the general analyzer ecosystem - they tend to be off to the side, but these are much more integrated.

We also need support for this in the new front end, so there needs to be some common story there as well.

@bwilkerson
Copy link
Member

How does this affect the command line analyzer?

The command-line analyzer runs any lints that are either (a) specified in the analysis options file or (b) specified on the command-line. Server also runs lints specified in the analysis options file, but doesn't have command-line support.

... these are much more integrated.

In what way?

We also need support for this in the new front end ...

We do? Why? (I'm not trying to challenge the statement, just understand the implications.)

@leafpetersen
Copy link
Member

... these are much more integrated.

In what way?

Even with the existing no-implicit-dynamic, it seems difficult but perhaps not impossible to implement this separately from the code that does inference.

With proposed changes to no-implicit-dynamic (or a similar new flag, see #30397), I think it may be hard to do this: you need to know something about how dynamic was arrived at (e.g. via downwards inference, upwards inference, LUB, etc).

We also need support for this in the new front end ...

We do? Why? (I'm not trying to challenge the statement, just understand the implications.)

If this isn't implemented in the NFE, then you certainly need at least enough support to implement it in the analyzer behind the NFE. But more broadly, these should be usable via the DDC and other compiler interfaces as well, which I think won't be connected to the analyzer.

@leafpetersen
Copy link
Member

Not sure if this #32235 should be de-duped into this issue?

@bwilkerson
Copy link
Member

With proposed changes to no-implicit-dynamic (or a similar new flag, see #30397) ...

I'd forgotten about that discussion. Thanks for reminding me.

But more broadly, these should be usable via the DDC and other compiler interfaces as well, which I think won't be connected to the analyzer.

I agree that they won't be connected to the analyzer. I have no opinion (at this point) whether it makes sense for them to be usable by the compilers.

But assuming that it does make sense, what's the right way for users to think of these options? Do they change the semantics of the compiled code, or just the set of errors that are reported? Are they "language" options (choosing a variant of the language)?

Do we have a complete (or even partial) list of the options that fall into this category?

@robbecker-wf
Copy link

robbecker-wf commented Mar 31, 2018

I can live without the implicit-* options especially since currently they apply to all dependent code and not just the current project code. I would prefer moving these to lint rules, if possible, that only apply to the current codebase.
Data point of one, I find implicit-casts: false more valuable that implicit-dynamic: false.
I mostly just want some help with sneaky issues mentioned in #31410

@bwilkerson
Copy link
Member

@leafpetersen What is the path forward here? This issue was presumably assigned to me because it originally looked like an analyzer issue. But if the compilers need to know about it (still not sure why; I'd love to understand, though) then it's no longer an analyzer issue. I'll mark it as a language issue and assign it to you for now, but please update it as you see fit. (I'm also moving the milestone back, given that the original request was to have an answer by the end of this milestone.)

@bwilkerson bwilkerson modified the milestones: Dart2 Stable, Dart2 Beta 3 Apr 1, 2018
@bwilkerson bwilkerson added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). and removed area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Apr 1, 2018
@bwilkerson bwilkerson assigned leafpetersen and unassigned bwilkerson Apr 1, 2018
@bwilkerson bwilkerson changed the title Implement replacements for strong-mode options that will be deprecated Have a plan for replacements for strong-mode options that will be deprecated Apr 1, 2018
@dgrove
Copy link
Contributor

dgrove commented Apr 6, 2018

@leafpetersen what are your thoughts?

@leafpetersen leafpetersen modified the milestones: Dart2 Beta 3, Dart2 Beta 4 Apr 11, 2018
@dgrove
Copy link
Contributor

dgrove commented Apr 28, 2018

@leafpetersen can you give an update? We're getting close on Beta4.

@leafpetersen
Copy link
Member

We need to keep supporting these in the analyzer. For the CFE, we can worry about adding support for them post Dart 2 if needed.

@dgrove dgrove removed this from the Dart2Beta4 milestone May 9, 2018
@dgrove
Copy link
Contributor

dgrove commented May 9, 2018

Removed from the Dart2 set of milestones.

@srawlins
Copy link
Member

This is on my plate for this Q and next.

@srawlins
Copy link
Member

Good news! strict-casts (new in 2.16), strict-inference, and strict-raw-types are three new analysis modes that replace the "strong-mode" options. We still need some general documentation at dart.dev, see dart-lang/site-www#2338.

We also have not deprecated the strong-mode options yet; nothing is reported when you use them. This is #47902.

I'm closing this as we have come up with a plan and mostly executed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).
Projects
None yet
Development

No branches or pull requests

7 participants