Skip to content

Commit 4dfd024

Browse files
authored
Fail OrganizeImports when RemoveUnused.imports is enabled (#2347)
1 parent eb00acd commit 4dfd024

2 files changed

Lines changed: 98 additions & 0 deletions

File tree

scalafix-rules/src/main/scala/scalafix/internal/rule/OrganizeImports.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ class OrganizeImports(
7474
config.conf
7575
.getOrElse("OrganizeImports")(OrganizeImportsConfig())
7676
.andThen(patchPreset(_, config.conf))
77+
.andThen(checkRemoveUnusedConflict(_, config.conf))
7778
.andThen(checkScalacOptions(_, config.scalacOptions, config.scalaVersion))
7879

7980
override def fix(implicit doc: SemanticDocument): Patch = {
@@ -864,6 +865,20 @@ object OrganizeImports {
864865
ConfDecoder[OrganizeImportsConfig].read(mergedConf)
865866
}
866867

868+
private def checkRemoveUnusedConflict(
869+
ruleConf: OrganizeImportsConfig,
870+
conf: Conf
871+
): Configured[OrganizeImportsConfig] =
872+
conf.get[RemoveUnusedConfig]("RemoveUnused") match {
873+
case Configured.Ok(config) if config.imports =>
874+
Configured.error(
875+
"\"RemoveUnused.imports\" and \"OrganizeImports\" should not be used together as they can produce broken code. " +
876+
"Please disable \"RemoveUnused.imports\" by setting it to false, " +
877+
"and use \"OrganizeImports.removeUnused\" instead to safely remove unused imports."
878+
)
879+
case _ => Configured.ok(ruleConf)
880+
}
881+
867882
private def checkScalacOptions(
868883
conf: OrganizeImportsConfig,
869884
scalacOptions: List[String],
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package scalafix.tests.config
2+
3+
import metaconfig.Conf
4+
import metaconfig.Configured
5+
import metaconfig.typesafeconfig._
6+
import org.scalatest.funsuite.AnyFunSuite
7+
import scalafix.internal.rule.OrganizeImports
8+
import scalafix.v1.Configuration
9+
10+
class OrganizeImportsConfigSuite extends AnyFunSuite {
11+
12+
test("OrganizeImports should fail when RemoveUnused.imports is enabled") {
13+
val rawConfig =
14+
"""|rules = [OrganizeImports]
15+
|OrganizeImports.removeUnused = true
16+
|RemoveUnused.imports = true
17+
|""".stripMargin
18+
19+
val conf = Conf.parseString("test", rawConfig).get
20+
val config =
21+
Configuration().withConf(conf).withScalacOptions(List("-Wunused"))
22+
val rule = new OrganizeImports()
23+
24+
val result = rule.withConfiguration(config)
25+
26+
assert(
27+
result.isNotOk,
28+
"Expected OrganizeImports to fail with RemoveUnused.imports=true"
29+
)
30+
31+
result match {
32+
case Configured.NotOk(error) =>
33+
val errorMsg = error.msg
34+
assert(
35+
errorMsg.contains("RemoveUnused.imports") &&
36+
errorMsg.contains("OrganizeImports") &&
37+
errorMsg.contains("should not be used together"),
38+
s"Error message should mention the conflict. Got: $errorMsg"
39+
)
40+
case _ =>
41+
fail("Expected Configured.NotOk")
42+
}
43+
}
44+
45+
test("OrganizeImports should succeed when RemoveUnused.imports is disabled") {
46+
val rawConfig =
47+
"""|rules = [OrganizeImports]
48+
|OrganizeImports.removeUnused = true
49+
|RemoveUnused.imports = false
50+
|""".stripMargin
51+
52+
val conf = Conf.parseString("test", rawConfig).get
53+
val config =
54+
Configuration().withConf(conf).withScalacOptions(List("-Wunused"))
55+
val rule = new OrganizeImports()
56+
57+
val result = rule.withConfiguration(config)
58+
59+
assert(
60+
result.isOk,
61+
"Expected OrganizeImports to succeed with RemoveUnused.imports=false"
62+
)
63+
}
64+
65+
test("OrganizeImports should succeed when RemoveUnused is not configured") {
66+
val rawConfig =
67+
"""|rules = [OrganizeImports]
68+
|OrganizeImports.removeUnused = true
69+
|""".stripMargin
70+
71+
val conf = Conf.parseString("test", rawConfig).get
72+
val config =
73+
Configuration().withConf(conf).withScalacOptions(List("-Wunused"))
74+
val rule = new OrganizeImports()
75+
76+
val result = rule.withConfiguration(config)
77+
78+
assert(
79+
result.isOk,
80+
"Expected OrganizeImports to succeed when RemoveUnused is not configured"
81+
)
82+
}
83+
}

0 commit comments

Comments
 (0)