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 should interpret a missing sdk lower constraint as 2.7 #44159

Closed
leafpetersen opened this issue Nov 12, 2020 · 7 comments
Closed

Analyzer should interpret a missing sdk lower constraint as 2.7 #44159

leafpetersen opened this issue Nov 12, 2020 · 7 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@leafpetersen
Copy link
Member

Per language team discussion (summarized briefly here we are changing the interpretation of a pubspec sdk constraint with a missing lower bound (or in general no sdk constraint at all, though that will become a pub error) to imply a language version of 2.7. I believe that the analyzer is one of the few tools that reads pubspec.yaml directly, and hence this will need to be changed there.

This is blocker for beta, and will need to be cherry picked if it doesn't make the beta branch cut.

cc @scheglov @devoncarew

cc @pcsosinski @mit-mit @franklinyow

@leafpetersen leafpetersen added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Nov 12, 2020
@srawlins srawlins added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Nov 12, 2020
@devoncarew
Copy link
Member

devoncarew commented Nov 12, 2020

We offer lints based on the pubspec file, but I believe that we take the .dart_tool/package_config.json info as the system-of-record wrt package language versioning. I suspect there's no work to do here for the analyzer, but @scheglov can confirm.

@pq - do we lint if there's no lower bound constraint (i.e., a constraint of just <=3.0.0)? If no, we should open an issue for a related lint.

@pq
Copy link
Member

pq commented Nov 12, 2020

@pq - do we lint if there's no lower bound constraint (i.e., a constraint of just <=3.0.0)? If no, we should open an issue for a related lint.

tracked: #44054

@scheglov
Copy link
Contributor

My understanding is that in the past, when there is no the lower bound constraint for SDK, pub would generate no languageVersion field for this package in .dart_tool/package_config.json, and we used the language version of the current SDK since 77f2019.

So, if now we decide that in the absence of the lower bound constraint for SDK pub will generate 2.7 in the languageVersion field, then the analyzer has almost nothing to do. The only thing I can think about is maybe that the analyzer should also use 2.7 instead of the current SDK language version, for cases when the analyzer is used as a package, and an old version of pub was used that did not have this 2.7 generating change. @leafpetersen do we need such backward support?

@bwilkerson
Copy link
Member

Do we need to change the default in cases where there is no package_config.json?

@leafpetersen
Copy link
Member Author

The only thing I can think about is maybe that the analyzer should also use 2.7 instead of the current SDK language version, for cases when the analyzer is used as a package, and an old version of pub was used that did not have this 2.7 generating change. @leafpetersen do we need such backward support?

I don't think so.

Do we need to change the default in cases where there is no package_config.json?

No. Files that are not part of a package continue to be opted in by default.

@jakemac53 Do we know of any other ways the analyzer is affected? If not, we can close this out.

@jakemac53
Copy link
Contributor

jakemac53 commented Nov 12, 2020

Nothing I am aware of, build_runner does create a package config manually and pass it to the analyzer but we also generate that from the existing package config and not pubspecs, so as long as pub is writing a number into the package config it should be good to go.

@devoncarew
Copy link
Member

Closing as I don't believe there's anything for the analyzer to do here.

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. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Development

No branches or pull requests

7 participants