Skip to content

analyzer should warn when using Set literals with SDK < 2.2.0 #36024

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

Closed
kevmoo opened this issue Feb 25, 2019 · 11 comments
Closed

analyzer should warn when using Set literals with SDK < 2.2.0 #36024

kevmoo opened this issue Feb 25, 2019 · 11 comments
Assignees
Labels
legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Milestone

Comments

@kevmoo
Copy link
Member

kevmoo commented Feb 25, 2019

Just like we did for Future without dart:async imports, we should warn package authors if they are using a feature outside of their defined SDK constraint.

VERY helpful to avoid publishing packages that silently break folks.

CC @mit-mit @devoncarew @stereotype441

@kevmoo kevmoo added P1 A high priority bug; for example, a single project is unusable or has many test failures legacy-area-analyzer Use area-devexp instead. type-enhancement A request for a change that isn't a bug labels Feb 25, 2019
@kevmoo kevmoo added this to the Dart2.2 milestone Feb 25, 2019
@devoncarew
Copy link
Member

Are checks like these things we should roll into pub publish itself?

@kevmoo
Copy link
Member Author

kevmoo commented Feb 25, 2019

Are checks like these things we should roll into pub publish itself?

Likely, although it should be done server-side. The problem: it'll make publish slow. CC @mit-mit

Either way, having it analyzer itself is a good first step.

@kevmoo
Copy link
Member Author

kevmoo commented Feb 25, 2019

FYI: marked this P1 because I think it's critical to help our users "do the right thing"

@bwilkerson
Copy link
Member

Is this something we're going to want for every experiment that gets enabled? (I think the answer it "yes", but wanted to confirm.)

If so, we should make sure that it's part of the analyzer's implementation plan from now on.

Is this going to hold up shipping 2.2.0? (I think that's the implication of the P1 and milestone, but again wanted to confirm.)

@kevmoo
Copy link
Member Author

kevmoo commented Feb 25, 2019

Is this something we're going to want for every experiment that gets enabled? (I think the answer it "yes", but wanted to confirm.)

If so, we should make sure that it's part of the analyzer's implementation plan from now on.

I think so! As language features are introduced, it'd be amazing if we could warn people about this.

(I'd love to do it for APIs, too.)

@Since("2.2")
class Since {
final String version;
const Since(this.version);
}

Is this going to hold up shipping 2.2.0? (I think that's the implication of the P1 and milestone, but again wanted to confirm.)

I'd like it to block 2.2.0 – later and the ship has sailed. We should talk to @dgrove and @mit-mit about that

@devoncarew
Copy link
Member

devoncarew commented Feb 26, 2019

Given the amount of moving parts between landing a change in the sdk repo, and it making it's way into even a dev sdk, I suspect we've already missed 2.2.0. We should likely plan for a change landing in a .x patch release, and design the feature we'd ultimately like to see land.

@kevmoo
Copy link
Member Author

kevmoo commented Feb 26, 2019

okay...would be good to track.

@bwilkerson
Copy link
Member

@kevmoo
Copy link
Member Author

kevmoo commented Mar 14, 2019

Closed in 5fdb3c5

@kevmoo
Copy link
Member Author

kevmoo commented Mar 14, 2019

Reverted in 0f4a208

@kevmoo kevmoo reopened this Mar 14, 2019
@stereotype441
Copy link
Member

Fix was re-landed in 495c319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants