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

Remove unused imports #1004

Merged
merged 6 commits into from
Jul 7, 2017
Merged

Conversation

olafurpg
Copy link
Member

@olafurpg olafurpg commented Jul 7, 2017

This diff was automatically created by scalafix + messagehost with no manual edits.

addSbtPlugin("com.thesamet" % "sbt-protoc" % "0.99.6" exclude ("com.trueaccord.scalapb", "protoc-bridge_2.10"))
libraryDependencies += "com.trueaccord.scalapb" %% "compilerplugin-shaded" % "0.6.0-pre5"
addSbtPlugin("com.thesamet" % "sbt-protoc" % "0.99.11" exclude ("com.trueaccord.scalapb", "protoc-bridge_2.10"))
libraryDependencies += "com.trueaccord.scalapb" %% "compilerplugin-shaded" % "0.6.0"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff is actually related to the removed imports. sbt-messagehost was using a conflicting version of scalapb which caused binary compat problems. Now that scalapb has a stable release then we should be safe from further binary compat issues.

@olafurpg
Copy link
Member Author

olafurpg commented Jul 7, 2017

I suspect this is uncontroversially an LGTM

@olafurpg olafurpg merged commit 1af15eb into scalameta:master Jul 7, 2017
@olafurpg olafurpg deleted the remove-unused-imports branch July 7, 2017 08:32
import scala.runtime.ScalaRunTime
import scala.collection.{ mutable, immutable }
import mutable.{ ListBuffer, StringBuilder }
import scala.collection. mutable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks weird.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a known issue scalacenter/scalafix#216 due to be fixed in the next release.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix pending in scalacenter/scalafix#245 the rewrite has limitations for how much it can preserve formatting, it adds a lot of complexity to the rewrite. Ideally we should be using scalafmt to clean up stuff like this.

import scala.collection.{ mutable, immutable }
import mutable.{ ListBuffer, StringBuilder }
import scala.collection. mutable
import mutable. ListBuffer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, too.

@@ -851,7 +851,6 @@ class SuccessSuite extends FunSuite {
}

test("1 t\"$tpe forSome { ..$stats }\"") {
import scala.language.existentials
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is unnecessary here, but I'm wondering whether you have tests for language imports in Scalafix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scalafix piggybacks entirely on "Unused import" warnings from scalac so false negatives/positivies are bugs in scalac.

import scala.collection.{ mutable, immutable }
import scala.language.postfixOps
import mutable.{ ListBuffer, ArrayBuffer }
import scala.collection. mutable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird.

@xeno-by
Copy link
Member

xeno-by commented Jul 11, 2017

@olafurpg Please take a look at the comments above. They may indicate weirdnesses in Scalafix.

xeno-by added a commit to xeno-by/scalameta that referenced this pull request Jul 11, 2017
@olafurpg
Copy link
Member Author

We can enable this check in CI once #991 is fixed. #991 avoids the need to setup the whole protobuf pipeline for sbt-messagehost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants