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

[proposal] quick-fix for depend_on_referenced_packages #48998

Open
pq opened this issue May 11, 2022 · 4 comments
Open

[proposal] quick-fix for depend_on_referenced_packages #48998

pq opened this issue May 11, 2022 · 4 comments
Labels
analyzer-quick-fix 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-enhancement A request for a change that isn't a bug

Comments

@pq
Copy link
Member

pq commented May 11, 2022

Getting the best version constraint for an added package dependency is hard (see discussions in dart-lang/lints#42 and dart-lang/pub#3333). Without ruling out the possibility that we can do better, I'd like to explore a fix that does the simplest thing, adding a dependency on package_name: any with a #todo comment to improve the constraint. (We might consider a separate tool to help, likely in pub [1], but I think that's out of scope here.)

Besides enabling a fix for depend_on_referenced_packages, such a fix will help complete the flow for data-driven fixes that require package additions (for example, #48997).

/fyi @bwilkerson @keertip

/fyi @jonasfj

[1] Like "build cleaner"?

@pq pq added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-quick-fix P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels May 11, 2022
@pq
Copy link
Member Author

pq commented May 11, 2022

/fyi @jakemac53

@jakemac53
Copy link
Contributor

Does pub lish warn about any constraints? If it does, I would be ok with this solution.

@jonasfj
Copy link
Member

jonasfj commented May 12, 2022

hmm, I thought that there was something that forbid publishing with any constraints, but I can't see anything on the server-side. But on the client side dart pub publish will warn about any constraints.

If this is insufficient, we could choose to disallow implicit-any constraints.

example

dependencies:
  # We can write a dependency on retry: any as follows:
  retry: any
  # Or we can write `null` which is the same "any" (the default value):
  retry: null  # this is the same as "any"
  # Or we can simply omit the value, which is the same as `null`, and thus implicitly "any":
  retry:        # this is the same as writing any

We could make the quick-fix write: retry:

Which is the same as writing retry: any, but we could then:

  • (A) make dart pub publish refuse publishing if you have retry: or (retry: null),
  • (B) but allow publishing if you have retry: any.

(Making this distinction might require a few hacks in pub, but I think it can be confined to just affecting the publishing validation).

That way, the default value inserted by the quick-fix won't be allowed when publishing, but if someone wants to publish with an any constraint that is possible, they just have to explicitly write retry: any.


For application development, having a constraint in pubspec.yaml is less important, because you should keep your pubspec.lock under revision control. It will make distinction between dart pub upgrade and dart pub upgrade --major-versions obsolete -- but many people will probably fix this by inserting a version constraint.

Indeed for application development, it's not an unreasonable opinion to argue that using any-constraints in pubspec.yaml is fine. I personally prefer and would recommend an explicit dependency constraint, but omitting it is not crazy -- when doing application development.

@sigurdm
Copy link
Contributor

sigurdm commented May 12, 2022

There once was a suggestion for a special value as a version constraint that pub get will recognise and replace by the ^resolvable constraint dart-lang/pub#2895 maybe we should reopen that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-quick-fix 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-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants