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

OrganizeImports.targetDialect: wildcard/rename syntax & curly braces stripping #1896

Merged
merged 4 commits into from
Feb 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions docs/rules/OrganizeImports.md
Original file line number Diff line number Diff line change
Expand Up @@ -1328,3 +1328,149 @@ object RemoveUnused {
val long: JLong = JLong.parseLong("0")
}
```

`targetDialect`
--------------

Specify the [wildcards and renames](https://docs.scala-lang.org/scala3/reference/changed-features/imports.html)
syntax to use.

### Value type

Enum: `Auto | Scala2 | Scala3 | StandardLayout`

#### `Auto`

Infer the dialect from compilation settings (Scala version or `-Xsource` when
provided) and behave like `Scala2` or `Scala3`. This is safe only for sources
that are not cross-compiled and therefore it is NOT the default value (see
rationale below).

#### `Scala2`

For all files,
* use `_` as wildcard and `=>` for renames
* curly braces are stripped for importers with a single regular importee

#### `Scala3`

For all files,
* use `*` as wildcard and `as` for renames
* curly braces are stripped for importers with a single importee

#### `StandardLayout`

For files containing `scala-3` in their path,
* use `*` as wildcard and `as` for renames
* curly braces are stripped for importers with a single importee

For others,
* use `_` as wildcard and `=>` for renames
* curly braces are stripped for importers with a single regular importee

### Default value

`StandardLayout`

Rationale: `Auto` is not a safe default for projects cross-compiling with
Scala 3.x and Scala 2.x without `-Xsource:3`, as the Scala 3.x Scalafix run
would introduce Scala 2.x compilation errors.
Comment on lines +1375 to +1377
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's really a shame we can't default to Auto 😞

@tgodzik maybe metals has more information and could force Auto whenever we know for sure that sources won't be cross-compiled with Scala2? I am not convinced as we would get a lot of false negatives with the checks I have in mind

  • source files rooted in a scala-3 directory
  • source files using other Scala 3 features (I am not sure we can pull this information for the tree - as far as I know the only way would be to try and fail to parse with a Scala 2 dialect...)

We could do something similar in sbt-scalafix (although this would be the first time we mingle with rule configuration) with fairly low risk when detecting that we both have

  • no usage of sbt-projectmatrix
  • no crossScalaVersions set

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we have that information even in Metals especially that with crossScalaVersion only one version would be imported.

The alternative to Auto would be to have StandardLayout which would do Scala2 target under scala and scala-2, Scala3 for scala-3 directories.

Anyway, I think having Scala2 as default makes sense

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It turned out StandardLayout wasn't so hard to implement so I added it and made it the default. Thanks 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome! Thanks!


### Examples

#### `Scala2`

```conf
OrganizeImports {
targetDialect = Scala2
}
```

Before:

```scala
import scala.collection.immutable.{List => L}
import scala.collection.mutable.{Map}
import scala.collection.mutable.{Buffer => _, Seq => S, _}
```

After:

```scala
import scala.collection.immutable.{List => L}
import scala.collection.mutable.Map
import scala.collection.mutable.{Buffer => _, Seq => S, _}
```

#### `Scala3`

```conf
OrganizeImports {
targetDialect = Scala3
}
```

Before:

```scala
import scala.collection.immutable.{List => L}
import scala.collection.mutable.{Map}
import scala.collection.mutable.{Buffer => _, Seq => S, _}
import scala.concurrent.ExecutionContext.Implicits.{given scala.concurrent.ExecutionContext}
```

After:

```scala
import scala.collection.immutable.List as L
import scala.collection.mutable.Map
import scala.collection.mutable.{Buffer as _, Seq as S, *}
import scala.concurrent.ExecutionContext.Implicits.given scala.concurrent.ExecutionContext
```

#### `StandardLayout`

```conf
OrganizeImports {
targetDialect = StandardLayout
}
```

##### `**/scala-3/**` files

Before:

```scala
import scala.collection.immutable.{List => L}
import scala.collection.mutable.{Map}
import scala.collection.mutable.{Buffer => _, Seq => S, _}
import scala.concurrent.ExecutionContext.Implicits.{given scala.concurrent.ExecutionContext}
```

After:

```scala
import scala.collection.immutable.List as L
import scala.collection.mutable.Map
import scala.collection.mutable.{Buffer as _, Seq as S, *}
import scala.concurrent.ExecutionContext.Implicits.given scala.concurrent.ExecutionContext
```

##### Other files

Before:

```scala
import scala.collection.immutable.{List => L}
import scala.collection.mutable.{Map}
import scala.collection.mutable.{Buffer => _, Seq => S, _}
```

After:

```scala
import scala.collection.immutable.{List => L}
import scala.collection.mutable.Map
import scala.collection.mutable.{Buffer => _, Seq => S, _}
```

Loading