dart-lang / language

Design of the Dart language
Other
2.67k stars 205 forks source link

How do we expect tools to handle import shorthands? #1940

Open leafpetersen opened 3 years ago

leafpetersen commented 3 years ago

@bwilkerson raised some concerns about the tooling experience for import shorthands. If we implement this proposal, there will now be essentially four (or more) different ways to write an import/export.

How do we expect tools to deal with this? Will we be deprecating the old syntax? Presumably not, since it is more general. If not, is this a configuration knob? "Add imports using ...."? Or do the tools try to guess based on existing uses? What if it's invalid to use the predominant syntax?

In general, are there usability issues we need to consider here?

cc @lrhn @munificent @eernstg @natebosch @jakemac53 @bwilkerson

jakemac53 commented 3 years ago

There are also multiple tools which read imports/exports in order to track dependencies. They typically do this by only parsing the file with the analyzer, and then using Uri.parse on the strings for the imports/exports/parts. These tools will all need to be updated, and some live in packages (ie: build_modules etc). This does open up another cliff people will fall off of, where they can't update to the latest sdk because they can't update to the latest build_modules for some reason.

None of that is a deal breaker I don't think, but we do need to budget time to fix up the tools/packages for the rollout.

jakemac53 commented 3 years ago

I also assume we will need lints, and possibly even directly conflicting lints (to enforce one style over the other).

We should discuss if we want to add either form to the recommended or core lint sets as well.

bwilkerson commented 3 years ago

FWIW, last quarter I wrote up a 1-pager describing some of the options we have for allowing users to specify preferences like this to the analysis server. I'm happy to share it if it would be interesting. The four options I came up with are:

Each approach has its pros and cons.

Will we be deprecating the old syntax?

Would this be an opportunity to switch library identity from URI syntax to file location? That's another area that used to confuse users, though it appears to be less of a problem now that we've explained the importance of following the same convention everywhere within a package.

I also assume we will need lints ...

We already have lints for relative vs. absolute, so yes, I expect we'll want at least two more for long-hand vs. short-hand.

Another question: What impact, if any, do we expect this to have on code generators?

jakemac53 commented 3 years ago

Another question: What impact, if any, do we expect this to have on code generators?

I think this would only affect code generators that actually look at imports, which should be very few. Probably the analyzer could paper over some of this as well, at least in the element model?

It may mean code generators trigger additional lints because of the import form they use, but this is already a general problem and generators can add // ignore_for_file: use_import_shorthands or similar which is basically the status quo.

bwilkerson commented 3 years ago

Yes, I would expect the analyzer's element model to normalize the imports to some extent. Clients that ask for the importedLibrary, for example, would get the library without needing to worry about the syntax. On the other hand, if a client asks for the uri they get the actual string and have to deal with the different forms. I expect the same would be true if this proposal is accepted.

It may mean code generators trigger additional lints ...

Or we should encourage them to use the new // ignore_for_file: type=lint syntax so that no lints will be reported.

natebosch commented 3 years ago

I lean towards being opinionated. I would only introduce the opposite-practice lint if someone asks for it.

How do we expect tools to deal with this?

It should be based on the language version. If the package is on the language version that supports them, I think dart fix should rewrite to the new syntax, and analyzer should use the new syntax for all edits.

Will we be deprecating the old syntax? Presumably not, since it is more general.

I can't recall how it's more general. If there are legal values that can't be represented in the new format I think it would be OK to keep the old format around just for those cases.

If not, is this a configuration knob?

IMO only if there is strong user demand for it - can we get an idea of that before we release the feature?

munificent commented 3 years ago

If there are legal values that can't be represented in the new format I think it would be OK to keep the old format around just for those cases.

@natebosch is right. There are theoretically some imports that cannot be represented using the new shorthand: file paths with spaces or other weird cases. Since before Dart 1.0, we have discouraged Dart users from file paths that contain any of those characters, so they should be vanishingly rare.

I think we should be opinionated and migrate users to the shorthand whenever possible.

leafpetersen commented 3 years ago

@jakemac53 raised the issue of stack traces. We need some way of uniquely reporting locations in files. It feels very unfortunate if tools end up surfacing stack traces (and other kinds of errors) using package URIs, since then users end up needing to understand and work with URIs on a regular basis, instead of mostly being able to pretend they don't exist.

@munificent @lrhn thoughts?

lrhn commented 3 years ago

I think we should make an effort to make the user-facing display of package URIs use shorthand syntax where possible (and fall back on URIs where necessary). That could easily include stacktraces. Any package URI package:name:path.dart can be mechanically shorthanded to name:path without knowning anything about the package layout otherwise, so it should be easy to do, and tweak until it becomes readable.

Basically: Any time we generate or show a package URI, we should use the shorthand syntax if possible. However, displaying traditional URIs are not wrong, so we don't necessarily need to migrate all our tools (or stack traces) immediately.