fix for 3725

[philwalk]

The Regex that is used to partition the shebang section from script code is too permissive.
The end marker must appear as the first 2 characters of the line it appears on.

The cause of bug #3725 is that the partitionShebangSection grabs part of the scala code here.
The fix is to require that the Regex substring !# is anchored at the beginning of a line.

[philwalk]

Am working on an integration test to verify the fix.

[philwalk]

I added two tests, and I assume I can run them with the following command line:

 ./mill integration.test.jvm 'scala.build.tests.SourcesTests' 'scala.build.tests.markdown.MarkdownCodeBlockTests'

The output shows no error messages, but it doesn't report pass/fail either:

[1237/1339] test-runner[3.3.6].publishLocalNoFluff
[1237] Publishing Artifact(org.virtuslab.scala-cli,test-runner_3,0.0.1-SNAPSHOT) to ivy repo /home/philwalk/workspace/scala-cli-phil/out/repo/0.0.1-SNAPSHOT
[1238/1339] runner[3.3.6].publishLocalNoFluff
[1238] Publishing Artifact(org.virtuslab.scala-cli,runner_3,0.0.1-SNAPSHOT) to ivy repo /home/philwalk/workspace/scala-cli-phil/out/repo/0.0.1-SNAPSHOT
[1239/1339] runner[2.13.16].publishLocalNoFluff
[1239] Publishing Artifact(org.virtuslab.scala-cli,runner_2.13,0.0.1-SNAPSHOT) to ivy repo /home/philwalk/workspace/scala-cli-phil/out/repo/0.0.1-SNAPSHOT
[1240/1339] test-runner[2.13.16].publishLocalNoFluff
[1240] Publishing Artifact(org.virtuslab.scala-cli,test-runner_2.13,0.0.1-SNAPSHOT) to ivy repo /home/philwalk/workspace/scala-cli-phil/out/repo/0.0.1-SNAPSHOT
[1241/1339] runner[2.12.20].publishLocalNoFluff
[1241] Publishing Artifact(org.virtuslab.scala-cli,runner_2.12,0.0.1-SNAPSHOT) to ivy repo /home/philwalk/workspace/scala-cli-phil/out/repo/0.0.1-SNAPSHOT
[1242/1339] test-runner[2.12.20].publishLocalNoFluff
[1242] Publishing Artifact(org.virtuslab.scala-cli,test-runner_2.12,0.0.1-SNAPSHOT) to ivy repo /home/philwalk/workspace/scala-cli-phil/out/repo/0.0.1-SNAPSHOT
[1111/1339] integration.compile
[1111] [info] compiling 1 Scala source to /home/philwalk/workspace/scala-cli-phil/out/integration/compile.dest/classes ...
[1111] [info] done compiling
[1332/1339] integration.test.compile
[1332] [info] compiling 154 Scala sources to /home/philwalk/workspace/scala-cli-phil/out/integration/test/compile.dest/classes ...
[1332] [warn] /home/philwalk/workspace/scala-cli-phil/modules/integration/src/test/scala/scala/cli/integration/ExportScalaOrientedBuildToolsTestDefinitions.scala:28:42: parameter testArgs in method testZioTest is never used
[1332] [warn]   def testZioTest(testClassName: String, testArgs: Seq[String] = Nil): Unit = {
[1332] [warn]                                          ^
[1332] [warn] /home/philwalk/workspace/scala-cli-phil/modules/integration/src/test/scala/scala/cli/integration/RunTestDefinitions.scala:2358:57: Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method exitCode,
[1332] [warn] or remove the empty argument list from its definition (Java-defined methods are exempt).
[1332] [warn] In Scala 3, an unapplied method like this will be eta-expanded into a function. [quickfixable]
[1332] [warn]             processes.foreach { case (p, _) => expect(p.exitCode == 0) }
[1332] [warn]                                                         ^
[1332] [warn] /home/philwalk/workspace/scala-cli-phil/modules/integration/src/test/scala/scala/cli/integration/BspSuite.scala:170:14: method destroyForcibly in class SubProcess is deprecated: Use destroy(shutdownGracePeriod = 0)
[1332] [warn]         proc.destroyForcibly()
[1332] [warn]              ^
[1332] [warn] /home/philwalk/workspace/scala-cli-phil/modules/integration/src/test/scala/scala/cli/integration/TestUtil.scala:254:14: method destroyForcibly in class SubProcess is deprecated: Use destroy(shutdownGracePeriod = 0)
[1332] [warn]         proc.destroyForcibly()
[1332] [warn]              ^
[1332] [warn] /home/philwalk/workspace/scala-cli-phil/modules/integration/src/test/scala/scala/cli/integration/TestUtil.scala:371:34: method destroyForcibly in class SubProcess is deprecated: Use destroy(shutdownGracePeriod = 0)
[1332] [warn]         if (proc.isAlive()) proc.destroyForcibly()
[1332] [warn]                                  ^
[1332] [warn] 5 warnings found
[1332] [info] done compiling
[1339/1339] =============== integration.test.jvm scala.build.tests.Sourc....build.tests.markdown.MarkdownCodeBlockTests =============== 58s
(base) philwalk@d5:~/workspace/scala-cli-phil$ 

[philwalk]

I'm not sure why the SourcesTests is failing, AFAICT I left it the way I found it and appended an additional test to the testInputs Seqand a corresponding additionalexpectedParseCodes` value.

It takes 6 minutes to run ./mill 'build-module[3.7.1].test.test', if someone can suggest a shortcut that runs minumum number of tests, it would be welcome. I'll see if I can figure this out tomorrow.

[philwalk]

A more subtle fix is needed, working on it.

[Gedochao]

It takes 6 minutes to run ./mill 'build-module[3.7.1].test.test', if someone can suggest a shortcut that runs minumum number of tests, it would be welcome. I'll see if I can figure this out tomorrow.

I'm guessing you need something like

./mill 'build-module[].test.test' 'scala.build.tests.SourcesTests.*'

(you can tweak the regex for a specific test)

[philwalk]

Here's a working version of the Regex:

  private val sheBangRegex: Regex = s"""(?m)^(#![^\\r\\n]*\\R){1,2}(.*?\\R)*?(^!#[^\\r\\n]*\\R)""".r

However, there are a handful of failing tests, some of which are "off-by-one" errors, so it will be awhile before I can push the fix.

[philwalk]

Also, I have a non-regex equivalent version that is 3 to 5 times as fast, so let me know if there's a preference.

[Gedochao]

Also, I have a non-regex equivalent version that is 3 to 5 times as fast, so let me know if there's a preference.

I mean, you make it sound like the non-regex version is worth a look 🤷

[philwalk]

I need to benchmark it again ...
I'll fix the SheBang.scala formatting before Monday.

[philwalk]

Of the 3 or 4 test failures from the Friday morning run that I've tried to duplicate, none are failing on my system. I won't be able to do a deep dive until tomorrow.

[tgodzik]

No worries, @Gedochao is out until next Monday, so if you need help I can also try and help out.