Skip to content

Commit 1f82de9

Browse files
authored
Re-enable compile-time errors (#6119)
These were converted to runtime errors in #6107 to try and work around problems in intellij highlighting, but we can get the best of both worlds by turning them into compile errors only in the presence of a `sys.env`/`sys.props` that Mill itself provides. So people using these APIs in Mill will get the stricter compile-time checks, but IntelliJ trying to compile this code will not have these env/props and not result in (spurious) compile errors
1 parent 32485d4 commit 1f82de9

File tree

12 files changed

+114
-10
lines changed

12 files changed

+114
-10
lines changed

build.mill

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//| mill-version: 1.1.0-RC1
22
//| mill-jvm-opts: ["-XX:NonProfiledCodeHeapSize=250m", "-XX:ReservedCodeCacheSize=500m"]
3-
//| mill-opts: ["--jobs=0.5C"]
3+
//| mill-opts: ["--jobs=0.5C", "-DMILL_ENABLE_STATIC_CHECKS=true"]
44

55
package build
66

core/api/src/mill/api/ModuleCtx.scala

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package mill.api
33
import mill.api.internal.OverrideMapping
44

55
import scala.annotation.implicitNotFound
6+
import scala.quoted.*
67

78
/**
89
* The contextual information provided to a [[mill.api.Module]] or [[mill.api.Task]]
@@ -163,10 +164,27 @@ object ModuleCtx extends LowPriCtx {
163164
}
164165

165166
trait LowPriCtx {
166-
// Dummy `Ctx` available in implicit scope but must not be actually used.
167+
// Binary compatibility stub
168+
def dummyInfo: ModuleCtx = throw new Exception(LowPriCtx.errorMessage)
169+
170+
// Dummy `Ctx` available in implicit scope but never actually used.
167171
// as it is provided by the codegen. Defined for IDEs to think that one is available
168172
// and not show errors in build.mill/package.mill even though they can't see the codegen
169-
implicit def dummyInfo: ModuleCtx = sys.error(
173+
inline implicit def dummyInfo2: ModuleCtx = ${ LowPriCtx.dummyInfoImpl }
174+
}
175+
176+
object LowPriCtx {
177+
private def errorMessage =
170178
"Modules and Tasks can only be defined within a mill Module (in `build.mill` or `package.mill` files)"
171-
)
179+
180+
def dummyInfoImpl(using quotes: Quotes): Expr[ModuleCtx] = {
181+
import quotes.reflect.*
182+
183+
if (
184+
sys.env.contains(mill.constants.EnvVars.MILL_ENABLE_STATIC_CHECKS) ||
185+
sys.props.contains(mill.constants.EnvVars.MILL_ENABLE_STATIC_CHECKS)
186+
) {
187+
report.errorAndAbort(errorMessage)
188+
} else '{ throw new Exception(${ Expr(errorMessage) }) }
189+
}
172190
}

core/api/src/mill/api/internal/Cacher.scala

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,20 @@ private[mill] object Cacher {
6565
owner.owner.isClassDef &&
6666
owner.owner.typeRef.baseClasses.contains(CacherSym)
6767

68+
val errorMessage = "Task{} members must be defs defined in a Module class/trait/object body"
6869
if (ownerIsCacherClass && owner.flags.is(Flags.Method)) {
6970
val enclosingCtx = Expr.summon[sourcecode.Enclosing].getOrElse(
7071
report.errorAndAbort("Cannot find enclosing context", Position.ofMacroExpansion)
7172
)
7273

7374
val thisSel = This(owner.owner).asExprOf[Cacher]
7475
'{ $thisSel.cachedTask[T](${ t })(using $enclosingCtx) }
75-
} else '{
76+
} else if (
77+
sys.env.contains(mill.constants.EnvVars.MILL_ENABLE_STATIC_CHECKS) ||
78+
sys.props.contains(mill.constants.EnvVars.MILL_ENABLE_STATIC_CHECKS)
79+
) {
80+
report.errorAndAbort(errorMessage, Position.ofMacroExpansion)
7681
// Use a runtime exception to prevent false error highlighting in IntelliJ
77-
throw new Exception("Task{} members must be defs defined in a Module class/trait/object body")
78-
}
82+
} else '{ throw new Exception(${ Expr(errorMessage) }) }
7983
}
8084
}

core/api/test/src/mill/api/MacroErrorTests.scala

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@ import scala.annotation.unused
88
object MacroErrorTests extends TestSuite {
99

1010
val tests = Tests {
11+
12+
test("errors") {
13+
val expectedMsg =
14+
"Task{} members must be defs defined in a Module class/trait/object body"
15+
16+
val err = assertCompileError("object Foo extends TestRootModule{ val x = Task {1} }")
17+
assert(err.msg == expectedMsg)
18+
}
19+
1120
test("badParameterSets") {
1221
test("command") {
1322
val e = assertCompileError("""
@@ -110,7 +119,19 @@ object MacroErrorTests extends TestSuite {
110119
lazy val millDiscover = Discover[this.type]
111120
}
112121
}
122+
test("neg1") {
123+
val e = assertCompileError("""def a = Task { 1 }""")
124+
assert(e.msg.contains(
125+
"Task{} members must be defs defined in a Module class/trait/object body"
126+
))
127+
}
113128

129+
test("neg2") {
130+
val e = assertCompileError("object foo extends TestRootModule{ val a = Task { 1 } }")
131+
assert(e.msg.contains(
132+
"Task{} members must be defs defined in a Module class/trait/object body"
133+
))
134+
}
114135
test("neg3") {
115136

116137
val expectedMsg =

core/constants/src/mill/constants/EnvVars.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,10 @@ public class EnvVars {
5353
* The path to the currently executing Mill launcher executable
5454
*/
5555
public static final String MILL_EXECUTABLE_PATH = "MILL_EXECUTABLE_PATH";
56+
57+
/**
58+
* Env var used to enable certain compile-time checks only in the actual Mill process,
59+
* so that they don't cause false positives in IDEs like IntelliJ.
60+
*/
61+
public static final String MILL_ENABLE_STATIC_CHECKS = "MILL_ENABLE_STATIC_CHECKS";
5662
}

dist/scripts/test/src/ScriptTests.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -347,13 +347,11 @@ object ScriptTests extends TestSuite {
347347
)
348348

349349
val Seq(downloadUrl, downloadDest) = res.out.lines()
350-
println("downloadUrl: " + downloadUrl)
351-
println("downloadDest: " + downloadDest)
350+
352351
assert(downloadUrl == versionPaths.downloadUrl)
353352
if (!scala.util.Properties.isWin) assert(downloadDest == versionPaths.macLinuxDownloadPath)
354353
else assert(downloadDest == versionPaths.windowsDownloadPath)
355354
}
356355
}
357356
}
358-
359357
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package build
2+
import mill._
3+
4+
object invalidModule extends Module
5+
6+
object `package` extends mill.Module
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package mill.integration
2+
3+
import mill.testkit.UtestIntegrationTestSuite
4+
5+
import utest._
6+
7+
object ModuleOutsideTopLevelModuleTests extends UtestIntegrationTestSuite {
8+
val tests: Tests = Tests {
9+
test("success") - integrationTest { tester =>
10+
import tester._
11+
val res = eval(("resolve", "_"))
12+
assert(!res.isSuccess)
13+
assert(
14+
res.err.contains(
15+
"Modules and Tasks can only be defined within a mill Module (in `build.mill` or `package.mill` files)"
16+
)
17+
)
18+
assert(
19+
res.err.contains(
20+
"object invalidModule extends Module"
21+
)
22+
)
23+
}
24+
}
25+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package build
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package build
2+
import mill._
3+
object foo extends Module

0 commit comments

Comments
 (0)