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

Allow customizing root imports #474

Closed
lrytz opened this Issue Feb 26, 2018 · 7 comments

Comments

Projects
None yet
6 participants
@lrytz
Copy link
Member

lrytz commented Feb 26, 2018

Summary of the discussion in scala/scala#5350:

  • Remove (not deprecate) -Ysysdef and -Ypredef and -Yno-predef and -Yno-imports
  • -Yimports:scala._,scala.Predef._, requires fully qualified imports (like if they all started with _root_.). -Yimports:_ for none. Colon-syntax using MultiStringSetting.
  • An explicit import of Predef.x in a source file removes the implicit scala.Predef._ import from that file. This is current behavior, and it will stay.
    • This special treatment also applies if -Yimports is present and includes scala.Predef._ -- this can still be debated (comment)
    • TODO: find a way to declare which root imports get predef-style treatment -- this is why the PR initially had the split between -Ysysdef and -Ypredef
    • Imports listed in -Yimports cannot be un-imported in individual files (things like import Predef.{_ => _} or import scala.{util => _, _}) -- maybe this can still be considered. For predefy-style imports it can be done by importing a single member that's not actually used.

Aftermath:

  • Notify IDE developers about the change (comment)
  • Update warn-unused (comment)

It also sounds like an export feature as outlined here is universally welcomed. There is (probably?) still a chance to get this in 2.13 if someone takes the lead. It should be coordinated with #442 (ideas to simplify imports).

@dwijnand

This comment has been minimized.

Copy link
Member

dwijnand commented Feb 26, 2018

I don't have a comprehensive view of scalac's options, but the fact that -Xlint:_ means "all" but -Yimports:_ means "none" seems too confusing.

@hrhino

This comment has been minimized.

Copy link
Member

hrhino commented Feb 26, 2018

For -Xprint: and other PhasesSettings, it seems that -Xprint: means "none" and -Xprint:_ means "phase named _".

@jvican

This comment has been minimized.

Copy link
Member

jvican commented Mar 5, 2018

I'd like to stir up some discussion about the export feature proposed by @adriaanm. It seems a great idea, but it also has its fair amount of danger in the language; in particular, it makes it difficult to evolve and maintain code over time because what's in scope is determined by a dependent library and not by the imports in a given source file. In particular, I don't think this plays nicely with implicit search, which is context dependent.

As the feature is envisioned now, I can imagine scenarios in which updating to a binary-compatible library could break lots of source code. Even changing the order of the export statements would be a source-breaking change. People live with this happily now because they are in control of these imports, but this is not the case at the moment a template of default imports comes from the classpath. Note that this problem already happens now (for instance, when you write code in a package that is provided by a dependency), but adding export would extend significantly extend this behaviour to virtually every source file.

What is the feature really trying to solve? Does it aim to provide a nice way to import things at use-site without the user knowing much about the organisation of the library?

If this is the case, it's worth researching how we could address this problem with external tooling. For example, what if library authors could write typechecked templates (via a macro or a compiler plugin) and then have IDEs automatically import them? What if the recommended imports (that would be represented via export) are instead present in a Scaladoc of the top-level package object of the library?

@ShaneDelmore

This comment has been minimized.

Copy link

ShaneDelmore commented Mar 5, 2018

Anytime you use import some.lib._ what is in scope is already determined by some.lib and can change when upgrading versions. This doesn’t feel like a new risk to me for most use cases. It would allow a library to restructure it’s internals without causing a breakage or needing a user side migration which seems like as much a benefit as a risk. Hopefully others will chime in but to my mind the two important usages of an export type functionality are:

  1. Remove the need for “bundled imports”, the pattern where every time you use a feature from lib you need to import the same two items together (and only remembering to import one of them will of course lead to a compilation error, without a library specific error message).
  2. Solve the orphaned implicits problem. Library authors already have the ability to add implicits in scope for their own types by adding them to companions, but do not for types they don’t own.

Personally this also changes how I organize code. I end up putting logic in my companions that I would not otherwise just to get the “free import” from the companion object.

@jvican

This comment has been minimized.

Copy link
Member

jvican commented Mar 5, 2018

Anytime you use import some.lib._ what is in scope is already determined by some.lib and can change when upgrading versions.

Sure, but the point I'm trying to make is that in the export case the control of those imports is not at the use site, but in the library, in code that you cannot potentially change or even look at without access to source jars.

@som-snytt

This comment has been minimized.

Copy link

som-snytt commented May 2, 2018

My simple implementation (simplementation) is over on scala/scala#5339.

All it supports is -Yimports:java.lang,scala,scala.Predef, so you specify modules to import from. Anything that isn't a package gets the Predef escape hatch of importing at top-level or top-level of package. The import can actually go at the bottom of the file. (REPL needs it to be leading, IIRC.)

That PR implements the level 4 name binding rule; to make that work usefully, it also introduces a "rule" that preamble imports are also level 4 precedence. That means they can't introduce ambiguities in user code, which is how it works now.

On another ticket or PR, @fommil mentions that the typelevel feature can introduce ambiguities.

The intuition is that if I can't see it in my code, make it a lower precedence so it only pops up when I need it. Otherwise, the rules for shadowing are unchanged. One case where it matters is for symbols in my package but not defined in my file: ordinary imports (at level 2 or 3) would create an ambiguity.

Level 2 and 3 correspond to specific and wildcard imports; but preamble imports are always wildcard and always level 4.

@som-snytt

This comment has been minimized.

Copy link

som-snytt commented Jan 17, 2019

The implementing PR for -Yimports is scala/scala#6764

Closing this in favor of the linked ticket for simplifying import syntax and maybe adding export.

@som-snytt som-snytt closed this Jan 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment