Skip to content

Commit

Permalink
Fix java compile warning, again (#3762)
Browse files Browse the repository at this point in the history
Follow up to #3711

I swear this annotation didn't work last time, and the `match` on the
`discoveredModuleType` did. But now they don't. Must have been getting
confused by incremental compilation

Turns out the discover macro was incorrectly traversing type members and
picking up their types, which is why it was appeared in the generated
code, so I added a filter to filter them out

Added a test to assert on the full stdout/stderr of a simple mill
command run to ensure we don't have stray logs or warnings turning up
  • Loading branch information
lihaoyi authored Oct 17, 2024
1 parent 59e1bda commit 0fdfdbb
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ jobs:

# Most of these integration tests should not depend on which mode they
# are run in, so just run them in `local`
- java-version: '11'
- java-version: '17'
millargs: "'integration.{failure,feature,ide}.__.local.testCached'"

# These invalidation tests need to be exercised in both execution modes
Expand Down
9 changes: 9 additions & 0 deletions integration/feature/full-run-logs/resources/build.mill
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package build
import mill._, javalib._

object `package` extends RootModule with JavaModule {
def ivyDeps = Agg(
ivy"net.sourceforge.argparse4j:argparse4j:0.9.0",
ivy"org.apache.commons:commons-text:1.12.0"
)
}
33 changes: 33 additions & 0 deletions integration/feature/full-run-logs/resources/src/foo/Foo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package foo;

import org.apache.commons.text.StringEscapeUtils;
import net.sourceforge.argparse4j.ArgumentParsers;
import net.sourceforge.argparse4j.inf.ArgumentParser;
import net.sourceforge.argparse4j.inf.ArgumentParserException;
import net.sourceforge.argparse4j.inf.Namespace;


public class Foo{
public static String generateHtml(String text){
return "<h1>" + StringEscapeUtils.escapeHtml4(text) + "</h1>";
}
public static void main(String[] args) {
ArgumentParser parser = ArgumentParsers.newFor("template").build()
.defaultHelp(true)
.description("Inserts text into a HTML template");

parser.addArgument("-t", "--text")
.required(true)
.help("text to insert");

Namespace ns = null;
try {
ns = parser.parseArgs(args);
}catch(Exception e){
System.out.println(e.getMessage());
System.exit(1);
}

System.out.println(generateHtml(ns.getString("text")));
}
}
54 changes: 54 additions & 0 deletions integration/feature/full-run-logs/src/FullRunLogsTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package mill.integration

import mill.testkit.UtestIntegrationTestSuite
import utest._

// Run simple commands on a simple build and check their entire output,
// ensuring we don't get spurious warnings or logging messages slipping in
object FullRunLogsTests extends UtestIntegrationTestSuite {

def tests: Tests = Tests {
test("noticker") - integrationTest { tester =>
import tester._

val res = eval(("--ticker", "false", "run", "--text", "hello"))
res.isSuccess ==> true
assert(res.out == "<h1>hello</h1>")
assert(
res.err.replace('\\', '/').replaceAll("(\r\n)|\r", "\n") ==
s"""[build.mill] [info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ...
|[build.mill] [info] done compiling
|[info] compiling 1 Java source to ${tester.workspacePath}/out/compile.dest/classes ...
|[info] done compiling""".stripMargin.replace('\\', '/').replaceAll("(\r\n)|\r", "\n")
)
}
test("ticker") - integrationTest { tester =>
import tester._

val res = eval(("--ticker", "true", "run", "--text", "hello"))
res.isSuccess ==> true
assert(res.out == "[46] <h1>hello</h1>")

val expectedErrorRegex =
s"""==================================================== run --text hello ================================================
|======================================================================================================================
|[build.mill-56/60] compile
|[build.mill-56] [info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ...
|[build.mill-56] [info] done compiling
|[40/46] compile
|[40] [info] compiling 1 Java source to ${tester.workspacePath}/out/compile.dest/classes ...
|[40] [info] done compiling
|[46/46] run
|[46/46] ============================================ run --text hello ============================================<seconds-digits>s
|======================================================================================================================"""
.stripMargin
.replaceAll("(\r\n)|\r", "\n")
.replace('\\', '/')
.split("<seconds-digits>")
.map(java.util.regex.Pattern.quote)
.mkString("=? [\\d]+")

assert(expectedErrorRegex.r.matches(res.err.replace('\\', '/').replaceAll("(\r\n)|\r", "\n")))
}
}
}
11 changes: 2 additions & 9 deletions main/define/src/mill/define/Discover.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ object Discover {
seen.add(tpe)
for {
m <- tpe.members.toList.sortBy(_.name.toString)
if !m.isType
memberTpe = m.typeSignature
if memberTpe.resultType <:< typeOf[mill.define.Module] && memberTpe.paramLists.isEmpty
} rec(memberTpe.resultType)
Expand Down Expand Up @@ -141,15 +142,7 @@ object Discover {
}
if overridesRoutes._1.nonEmpty || overridesRoutes._2.nonEmpty || overridesRoutes._3.nonEmpty
} yield {
val lhs0 = discoveredModuleType match {
// Explicitly do not de-alias type refs, so type aliases to deprecated
// types do not result in spurious deprecation warnings appearing
case tr: TypeRef => tr
// Other types are fine
case _ => discoveredModuleType.typeSymbol.asClass.toType
}

val lhs = q"classOf[$lhs0]"
val lhs = q"classOf[${discoveredModuleType.typeSymbol.asClass.toType}]"

// by wrapping the `overridesRoutes` in a lambda function we kind of work around
// the problem of generating a *huge* macro method body that finally exceeds the
Expand Down

0 comments on commit 0fdfdbb

Please sign in to comment.