Explicit imports now shadow locally defined identifiers #6589

som-snytt · 2018-05-02T18:38:23Z

Original title: "Import shadows def in other unit, per spec"

Forward port of #5339 .

Fixes scala/bug#2458
Fixes scala/bug#9552

som-snytt · 2018-05-03T20:27:09Z

NB: two import fixes were still required, two others in tests pass after "Forgive import ambiguity". Those changes squashed in lead commit.

SethTisue · 2018-05-04T07:42:38Z

/synch

som-snytt · 2018-05-04T15:21:31Z

Even Seth doesn't know the password.

Hey, how can settings be broken already? I'm only creating a -Y option that forks language semantics.

Edit: rebased

SethTisue · 2018-05-04T15:33:35Z

/synch goddamn it Scabot or I'll come over and put my foot right through your circuits

hrhino · 2018-05-04T16:06:45Z

"important" is clever but the previous PR had the benefit of ~~iambic~~ trochaic pentameter.

som-snytt · 2018-05-31T20:54:58Z

Sample erroneous shadowing. The import java.util isn't used, of course.

[warn] /home/amarki/projects/scala/test/junit/scala/tools/nsc/DeterminismTest.scala:5: imported `util' is permanently hidden by definition of package util in package nsc

lrytz

I think we should split out the -Yimports change into a separate PR.

The changes should come with tests.

Could you also summarize the changes in the commit messages?

What's the reason for changing the imports (the second commit)?

lrytz · 2018-06-08T09:08:50Z

src/reflect/scala/reflect/internal/TreeInfo.scala

+        val pkg = sym.enclosingPackage.name
+        !unitDefinesClassOrObject(pkg, sym.name) && !hasExplicitImport(body, pkg, sym.name)
+      }
+      if (!ok) report(sym)


https://xkcd.com/838/

som-snytt · 2018-06-08T14:21:50Z

I was hoping for feedback on simple -Yimports versus the other proposal, but I think the folks who want the mechanism are just using Haskell.

The imports in the few changed files are ambiguous.

Very well, I'll split out -Yimports; I don't think the code requires working imports, but using them correctly does. I think that these two-year-old PRs are more likely to die on the vine again.

The linked PR on 2.12 has full commit messages; I didn't expect it to get shut down.

som-snytt · 2018-06-08T14:27:41Z

Also, the other PR includes the spec change for the precedence of "root" imports. As the message here says, this was a minimal forward port.

lrytz · 2018-06-08T14:32:24Z

I definitely like what I saw from -Yimports, but I didn't test it locally to see what it does in detail. Reverse-engineering all corner cases from looking at the code was a bit too hard, so either test cases or more description would help discussing the proposal.

The imports in the few changed files are ambiguous.

It confused me that the commit fixing imports comes after the change to imports handling. But now I see that's a case of https://help.github.com/articles/why-are-my-commits-in-the-wrong-order

som-snytt · 2018-06-08T14:40:18Z

That just happened to me on another PR. I'll rebase and recreate commits here, adding what's missing from the 2.12 PR. Thanks for taking a look. I have high hopes for -Yimports, especially as a way for newbies to auto-import implicits, such as ExecutionContext.Implicits etc.

For example, ``` import p._ ; package p { S } ``` where S is both made available in p and also explicitly imported in enclosing scope. The usage S is ambiguous because it must not shadow the import.

Fixes the implementation of point 4. from SLS chapter 2: > Definitions made available by a package clause not in the > compilation unit where the reference occurs have lowest precedence. The implementation walks the context chain, but tries to be smart about how far it must walk, checking whether it has risen to an adequate depth. This check was previously broken for this case.

"Root" imports, implicitly supplied by the compiler, are lowest precedence.

If a definition and an import introduce an ambiguity, because the definition is at level 4 binding precedence, but imported symbol is just the definition, then ignore the import at that location. The pos test for ambiguous import was failing before.

Add words and counterexamples.

SethTisue · 2018-06-29T02:27:04Z

specs2 diffs — this one was easier

maybe due to scala/scala#6589 https://scala-ci.typesafe.com/job/scala-2.13.x-integrate-community-build/1302/consoleFull ``` [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:62:31: too many arguments (2) for method apply: (key: String)(implicit evidence$1: sys.Prop.Creator[T])scala.sys.Prop[T] in object Prop [specs2-more] [error] val nameProp = Prop("name", "eric") [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:63:18: trait Prop is abstract; cannot be instantiated [specs2-more] [error] val noValues = new Prop("name") [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:64:25: type mismatch; [specs2-more] [error] found : Int(18) [specs2-more] [error] required: String [specs2-more] [error] val actualOnly = Prop(18) [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:65:22: trait Prop is abstract; cannot be instantiated [specs2-more] [error] val expectedOnly = new Prop("", Property(), Property(18)) [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:66:20: wrong number of type arguments for scala.sys.Prop, should be 1 [specs2-more] [error] val constrained: Prop[String, String] = Prop("name", "eric", (s1: String, s2: String) => s1 must contain(s2)) [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:66:56: too many arguments (3) for method apply: (key: String)(implicit evidence$1: sys.Prop.Creator[T])scala.sys.Prop[T] in object Prop [specs2-more] [error] val constrained: Prop[String, String] = Prop("name", "eric", (s1: String, s2: String) => s1 must contain(s2)) [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:67:34: too many arguments (3) for method apply: (key: String)(implicit evidence$1: sys.Prop.Creator[T])scala.sys.Prop[T] in object Prop [specs2-more] [error] val withMatcher = Prop("name", "eric", contain(_:String)) [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:75:20: too many arguments (3) for method apply: (key: String)(implicit evidence$1: sys.Prop.Creator[T])scala.sys.Prop[T] in object Prop [specs2-more] [error] e6 := Prop("", 1, be_>(0).mute).execute === Success("") [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:76:20: 3 more arguments than can be applied to method apply: (key: String)(implicit evidence$1: sys.Prop.Creator[T])scala.sys.Prop[T] in object Prop [specs2-more] [error] e7 := Prop("", 1, 2, be_>(0).mute).execute === Success("") [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:80:11: trait Prop is abstract; cannot be instantiated [specs2-more] [error] e1 := new Prop("name", expected = Property("fanny")).toString === "name: _ (expected: fanny)" [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:80:28: not found: value expected [specs2-more] [error] e1 := new Prop("name", expected = Property("fanny")).toString === "name: _ (expected: fanny)" [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:81:11: trait Prop is abstract; cannot be instantiated [specs2-more] [error] e2 := new Prop("name", actual = Property("eric")).toString === "name: eric" [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:81:28: not found: value actual [specs2-more] [error] e2 := new Prop("name", actual = Property("eric")).toString === "name: eric" [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:82:11: trait Prop is abstract; cannot be instantiated [specs2-more] [error] e3 := new Prop("name", Property("eric"), Property("eric")).toString === "name: eric" [specs2-more] [error] ^ [specs2-more] [error] /home/jenkins/workspace/scala-2.13.x-integrate-community-build/target-0.9.13/project-builds/specs2-more-db45127d55bd79c54d280645a81534fbc13e7a8b/form/src/test/scala/org/specs2/form/PropSpec.scala:86:24: too many arguments (2) for method apply: (key: String)(implicit evidence$1: sys.Prop.Creator[T])scala.sys.Prop[T] in object Prop [specs2-more] [error] e1 := Prop("name", "eric")("paolo").expected.toOption must_== Some("paolo") [specs2-more] [error] ^ [specs2-more] [error] 15 errors found [specs2-more] [error] (form / Test / compileIncremental) Compilation failed ```

Scala 2.13 support was added to build via #5454. This PR adjusts the code so that it compiles with 2.11, 2.12 and 2.13. Changes: * Add `scala-collection-compat` dependency. * Import `scala.collection.Seq` in a number of places for consistent behavior between Scala 2.11, 2.12 and 2.13. * Remove wildcard imports that were causing the Java classes to have priority over the Scala ones, related Scala issue: scala/scala#6589. * Replace parallel collection usage with `Future`. The former is no longer included by default in the standard library. * Replace val _: Unit workaround with one that is more concise and works with Scala 2.13 * Replace `filterKeys` with `filter` when we expect a `Map`. `filterKeys` returns a view that doesn't implement the `Map` trait in Scala 2.13. * Replace `mapValues` with `map` or add a `toMap` as an additional transformation when we expect a `Map`. `mapValues` returns a view that doesn't implement the `Map` trait in Scala 2.13. * Replace `breakOut` with `iterator` and `to`, `breakOut` was removed in Scala 2.13. * Replace to() with toMap, toIndexedSeq and toSet * Replace `mutable.Buffer.--` with `filterNot`. * ControlException is an abstract class in Scala 2.13. * Variable arguments can only receive arrays or immutable.Seq in Scala 2.13. * Use `Factory` instead of `CanBuildFrom` in DecodeJson. `CanBuildFrom` behaves a bit differently in Scala 2.13 and it's been deprecated. `Factory` has the behavior we need and it's available via the compat library. * Fix failing tests due to behavior change in Scala 2.13, "Map.values.map is not strict in Scala 2.13" (scala/bug#11589). * Use Java collections instead of Scala ones in StreamResetter (a Java class). * Adjust CheckpointFile.write to take an `Iterable` instead of `Seq` to avoid unnecessary collection copies. * Fix DelayedElectLeader to use a Map instead of Set and avoid `to` call that doesn't work in Scala 2.13. * Use unordered map for mapping in SimpleAclAuthorizer, mapping of ordered maps require an `Ordering` in Scala 2.13 for safety reasons. * Adapt `ConsumerGroupCommand` to compile with Scala 2.13. * CoreUtils.min takes an `Iterable` instead of `TraversableOnce`, the latter does not exist in Scala 2.13. * Replace `Unit` with `()` in a couple places. Scala 2.13 is stricter when it expects a value instead of a type. * Fix bug in CustomQuotaCallbackTest where we did not necessarily set `partitionRatio` correctly, `forall` can terminate early. * Add a couple of spotbugs exclusions that are needed by code generated by Scala 2.13 * Remove unused variables, simplify some code and remove procedure syntax in a few places. * Remove unused `CoreUtils.JSONEscapeString`. Reviewers: Manikumar Reddy <manikumar.reddy@gmail.com>, José Armando García Sancio <jsancio@users.noreply.github.com>

SethTisue · 2019-07-03T14:31:20Z

There is now some further discussion and justification at scala/bug#11593

Scala -> 2.13 (previously 2.12) Play -> 2.8 (2.6) SBT -> 1.5.6 (0.13) This was a bit of a gigantic upgrade due to dependency binary incompatibilities (dependency coupling) and general breaking changes. Hopefully in future we can do smaller and more frequent updates. To break down the changes: 1. Removal of some niche dependencies The list is: diffson-play-json, scala-ssh, henkan-convert. These were either unused or used only a little. 2. .mapValues -> .view.mapValues(...).toMap https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes 3. other Scala 2.12 and 2.13 changes * .par now requires a special import * Seq is now core.immutable.Seq * import precendence and shadowing seems to have changed (see: scala/scala#6589), which caused issues as task and deployment_types packages share many names * requirement to drop parens when a method (typically controller method) was defined without them * etc. 4. library breaking changes For example, in the Guardian's own Play Secret and GoogleAuth libraries, or in the Boostrap ('b3') library, especially in how forms are defined. 5. SBT changes The deprecation of 'in' for '/' instead.

scala-jenkins added this to the 2.13.0-M5 milestone May 2, 2018

som-snytt force-pushed the topic/important branch 4 times, most recently from 348fe23 to 8784687 Compare May 3, 2018 20:23

This comment has been minimized.

Sign in to view

som-snytt force-pushed the topic/important branch from 8784687 to 9a4bcc0 Compare May 7, 2018 12:45

som-snytt mentioned this pull request May 30, 2018

SI-2458 Import shadows def in other unit #5339

Closed

lrytz requested changes Jun 8, 2018

View reviewed changes

som-snytt added 9 commits June 8, 2018 08:56

Fix ambiguous imports by the spec

efedda3

For example, ``` import p._ ; package p { S } ``` where S is both made available in p and also explicitly imported in enclosing scope. The usage S is ambiguous because it must not shadow the import.

Tests for SI-2458

92b62c3

Tests for issue 9552

188ce44

Update spec for name binding

e439688

"Root" imports, implicitly supplied by the compiler, are lowest precedence.

Tests for issue 2458

a9c2b57

Clarify neg test for ambiguous import

75d3c4e

Add words and counterexamples.

Upgrade code comment describing name binding

48fb060

som-snytt force-pushed the topic/important branch from 9a4bcc0 to 48fb060 Compare June 8, 2018 20:20

som-snytt changed the title ~~important~~ Import shadows def in other unit, per spec Jun 8, 2018

SethTisue mentioned this pull request Jun 29, 2018

changes for 2.13 community build etorreborre/specs2#680

Closed

SethTisue mentioned this pull request Aug 2, 2018

publish for Scala 2.13.0-M5? monix/minitest#23

Closed

xuwei-k mentioned this pull request Aug 5, 2018

fix compile error with latest Scala 2.13.x etorreborre/specs2#698

Merged

som-snytt deleted the topic/important branch August 5, 2018 21:01

This was referenced Oct 5, 2018

current package not preferred over imported package scala/scala3#5206

Closed

publish for Scala 2.13.0-M5 scalatest/scalatest#1409

Closed

Type parameter bound check regression in Scala 2.13.0-M5 scala/bug#11202

Closed

SethTisue mentioned this pull request Oct 17, 2018

2.13.0-M5 regression: Skinny ORM library fails to compile only with 2.13.0-M5 scala/bug#11206

Closed

raboof mentioned this pull request Dec 5, 2018

scala 2.13.0-M5 support akka/akka#26043

Merged

adriaanm mentioned this pull request Jan 17, 2019

Drop package objects with inheritance scala/scala-dev#441

Closed

dwijnand mentioned this pull request Jun 21, 2019

Build and test docs in scala 2.13 playframework/playframework#9413

Merged

dwijnand changed the title ~~Import shadows def in other unit, per spec~~ Explicit imports now shadow locally defined identifiers Jun 21, 2019

ennru mentioned this pull request Jun 27, 2019

MINOR: Make the build compile with Scala 2.13 apache/kafka#6989

Merged

3 tasks

SethTisue mentioned this pull request Jul 3, 2019

Name clash between wildcard import and class in same package regression 2.12 -> 2.13 scala/bug#11593

Closed

jtjeferreira mentioned this pull request Jul 17, 2019

[WIP] Scala 2.13 sangria-graphql/sangria#451

Closed

poslegm mentioned this pull request Oct 9, 2019

Update Scala to 2.13 scalameta/scalafmt#1522

Merged

SethTisue mentioned this pull request Nov 27, 2019

Illegal cyclic reference regression in 2.13 scala/bug#11734

Open

chia7712 mentioned this pull request Sep 2, 2020

MINOR: Fix build scala 2.12 build after KAFKA-10020 apache/kafka#9245

Merged

philvarner mentioned this pull request Mar 11, 2021

resolve amgiguity in spark and raster Implicits classes with explicit imports shadowing local identifiers locationtech/geotrellis#3354

Merged

5 tasks

nicl mentioned this pull request Jan 5, 2022

Scala 2.13, SBT 1.5.7, Play 2.8 (and related upgrades) guardian/riff-raff#666

Merged

unkarjedy mentioned this pull request Jul 18, 2022

given definition from wildcard import doesn't shadow given definition in current package scala/scala3#15698

Closed

mbarton mentioned this pull request Oct 17, 2022

Scala 2.13 guardian/giant#75

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explicit imports now shadow locally defined identifiers #6589

Explicit imports now shadow locally defined identifiers #6589

som-snytt commented May 2, 2018 •

edited by dwijnand

Loading

som-snytt commented May 3, 2018

This comment has been minimized.

SethTisue commented May 4, 2018

som-snytt commented May 4, 2018 •

edited

Loading

SethTisue commented May 4, 2018 •

edited

Loading

hrhino commented May 4, 2018 •

edited

Loading

som-snytt commented May 31, 2018

lrytz left a comment

lrytz Jun 8, 2018

som-snytt commented Jun 8, 2018

som-snytt commented Jun 8, 2018

lrytz commented Jun 8, 2018

som-snytt commented Jun 8, 2018

SethTisue commented Jun 29, 2018

SethTisue commented Jul 3, 2019

Explicit imports now shadow locally defined identifiers #6589

Explicit imports now shadow locally defined identifiers #6589

Conversation

som-snytt commented May 2, 2018 • edited by dwijnand Loading

som-snytt commented May 3, 2018

This comment has been minimized.

SethTisue commented May 4, 2018

som-snytt commented May 4, 2018 • edited Loading

SethTisue commented May 4, 2018 • edited Loading

hrhino commented May 4, 2018 • edited Loading

som-snytt commented May 31, 2018

lrytz left a comment

Choose a reason for hiding this comment

lrytz Jun 8, 2018

Choose a reason for hiding this comment

som-snytt commented Jun 8, 2018

som-snytt commented Jun 8, 2018

lrytz commented Jun 8, 2018

som-snytt commented Jun 8, 2018

SethTisue commented Jun 29, 2018

SethTisue commented Jul 3, 2019

som-snytt commented May 2, 2018 •

edited by dwijnand

Loading

som-snytt commented May 4, 2018 •

edited

Loading

SethTisue commented May 4, 2018 •

edited

Loading

hrhino commented May 4, 2018 •

edited

Loading