Skip to content

Commit 61fadd8

Browse files
philwalkGedochao
philwalk
and
Gedochao
authored
fix for 3725, 3752, 3766 and 3769 (#3726)
* SheBang preserves newlines; add tests simplify newlines fix 3725 3766 3769 * remove unused newLine, as per suggestion Co-authored-by: Piotr Chabelski <[email protected]> --------- Co-authored-by: Piotr Chabelski <[email protected]>
1 parent 628983e commit 61fadd8

File tree

14 files changed

+187
-60
lines changed

14 files changed

+187
-60
lines changed

‎modules/build/src/main/scala/scala/build/internal/AmmUtil.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,8 @@ object AmmUtil {
1818

1919
def encodeScalaSourcePath(path: Seq[Name]) = path.map(_.backticked).mkString(".")
2020

21-
def normalizeNewlines(s: String) = s.replace("\r", "").replace("\n", System.lineSeparator())
21+
def normalizeNewlines(s: String): String =
22+
s.replace("\r", "")
23+
24+
lazy val lineSeparator: String = "\n"
2225
}

‎modules/build/src/main/scala/scala/build/internal/markdown/MarkdownCodeWrapper.scala

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,15 +115,15 @@ object MarkdownCodeWrapper {
115115
val fence: MarkdownCodeBlock = snippets(index)
116116
val classOpener: String =
117117
if (index == 0)
118-
s"object ${scopeObjectName(scopeIndex)} {${System.lineSeparator()}" // first snippet needs to open a class
118+
s"object ${scopeObjectName(scopeIndex)} {${AmmUtil.lineSeparator}" // first snippet needs to open a class
119119
else if (fence.resetScope)
120-
s"}; object ${scopeObjectName(scopeIndex)} {${System.lineSeparator()}" // if scope is being reset, close previous class and open a new one
121-
else System.lineSeparator()
120+
s"}; object ${scopeObjectName(scopeIndex)} {${AmmUtil.lineSeparator}" // if scope is being reset, close previous class and open a new one
121+
else AmmUtil.lineSeparator
122122
val nextScopeIndex = if index == 0 || fence.resetScope then scopeIndex + 1 else scopeIndex
123-
val newAcc = acc + (System.lineSeparator() * (fence.startLine - line - 1)) // padding
123+
val newAcc = acc + (AmmUtil.lineSeparator * (fence.startLine - line - 1)) // padding
124124
.:++(classOpener) // new class opening (if applicable)
125125
.:++(fence.body) // snippet body
126-
.:++(System.lineSeparator()) // padding in place of closing backticks
126+
.:++(AmmUtil.lineSeparator) // padding in place of closing backticks
127127
generateMainScalaLines(
128128
snippets = snippets,
129129
index = index + 1,
@@ -154,9 +154,9 @@ object MarkdownCodeWrapper {
154154
if index >= snippets.length then acc
155155
else {
156156
val fence: MarkdownCodeBlock = snippets(index)
157-
val newAcc = acc + (System.lineSeparator() * (fence.startLine - line)) // padding
157+
val newAcc = acc + (AmmUtil.lineSeparator * (fence.startLine - line)) // padding
158158
.:++(fence.body) // snippet body
159-
.:++(System.lineSeparator()) // padding in place of closing backticks
159+
.:++(AmmUtil.lineSeparator) // padding in place of closing backticks
160160
generateRawScalaLines(snippets, index + 1, fence.endLine + 1, newAcc)
161161
}
162162
}

‎modules/build/src/main/scala/scala/build/internal/markdown/MarkdownOpenFence.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ case class MarkdownOpenFence(
3939
val start: Int = tickStartLine + 1
4040
val bodyLines: Array[String] = lines.slice(start, tickEndLine)
4141
val body = bodyLines.mkString("\n")
42-
val (bodyWithNoSheBang, _) = SheBang.ignoreSheBangLines(body)
42+
43+
val (bodyWithNoSheBang, _, _) = SheBang.ignoreSheBangLines(body)
4344
MarkdownCodeBlock(
4445
info.split("\\s+").toList, // strip info by whitespaces
4546
bodyWithNoSheBang,

‎modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,8 @@ case object ScalaPreprocessor extends Preprocessor {
171171
allowRestrictedFeatures: Boolean,
172172
suppressWarningOptions: SuppressWarningOptions
173173
)(using ScalaCliInvokeData): Either[BuildException, Option[ProcessingOutput]] = either {
174-
val (contentWithNoShebang, _) = SheBang.ignoreSheBangLines(content)
174+
val (contentWithNoShebang, _, _) = SheBang.ignoreSheBangLines(content)
175+
175176
val extractedDirectives: ExtractedDirectives = value(ExtractedDirectives.from(
176177
contentChars = contentWithNoShebang.toCharArray,
177178
path = path,
@@ -210,7 +211,7 @@ case object ScalaPreprocessor extends Preprocessor {
210211
logger
211212
)
212213

213-
val (content0, isSheBang) = SheBang.ignoreSheBangLines(content)
214+
val (content0, isSheBang, _) = SheBang.ignoreSheBangLines(content)
214215
val preprocessedDirectives: PreprocessedDirectives =
215216
value(DirectivesPreprocessor(
216217
path,

‎modules/build/src/main/scala/scala/build/preprocessing/SheBang.scala

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,47 @@
11
package scala.build.preprocessing
22

3-
import scala.util.matching.Regex
4-
53
object SheBang {
6-
private val sheBangRegex: Regex = s"""(^(#!.*(\\X)?)+(\\X*!#.*)?)""".r
7-
8-
def isShebangScript(content: String): Boolean = sheBangRegex.unanchored.matches(content)
4+
def isShebangScript(content: String): Boolean = content.startsWith("#!")
95

10-
/** Returns the shebang section and the content without the shebang section */
11-
def partitionOnShebangSection(content: String): (String, String) =
6+
def partitionOnShebangSection(content: String): (String, String, String) =
127
if (content.startsWith("#!")) {
13-
val regexMatch = sheBangRegex.findFirstMatchIn(content)
14-
regexMatch match {
15-
case Some(firstMatch) =>
16-
(firstMatch.toString(), content.replaceFirst(firstMatch.toString(), ""))
17-
case None => ("", content)
8+
val splitIndex = content.indexOf("\n!#") match {
9+
case -1 =>
10+
val eolIndex = content.indexOf("\n")
11+
content.drop(eolIndex + 1) match {
12+
case s if s.startsWith("#!") =>
13+
eolIndex + s.indexOf("\n") + 1 // skip over #! nix-shell line
14+
case _ =>
15+
eolIndex + 1
16+
}
17+
case index =>
18+
var i = index + 1
19+
while (i < content.length && content(i) != '\n') i += 1
20+
i + 1 // split at start of subsequent line
1821
}
22+
val newLine: String = content.drop(splitIndex - 2).take(2) match {
23+
case CRLF => CRLF
24+
case _ => "\n"
25+
}
26+
val (header, body) = content.splitAt(splitIndex)
27+
(header, body, newLine)
1928
}
2029
else
21-
("", content)
30+
("", content, lineSeparator(content))
2231

23-
def ignoreSheBangLines(content: String): (String, Boolean) =
24-
if (content.startsWith("#!")) {
25-
val regexMatch = sheBangRegex.findFirstMatchIn(content)
26-
regexMatch match {
27-
case Some(firstMatch) =>
28-
content.replace(
29-
firstMatch.toString(),
30-
System.lineSeparator() * firstMatch.toString().split(System.lineSeparator()).length
31-
) -> true
32-
case None => (content, false)
33-
}
34-
}
32+
def ignoreSheBangLines(content: String): (String, Boolean, String) =
33+
if (content.startsWith("#!"))
34+
val (header, body, newLine) = partitionOnShebangSection(content)
35+
val blankHeader = newLine * (header.split("\\R", -1).length - 1)
36+
(s"$blankHeader$body", true, newLine)
3537
else
36-
(content, false)
38+
(content, false, lineSeparator(content))
39+
40+
def lineSeparator(content: String): String = content.indexOf(CRLF) match {
41+
case -1 => "\n"
42+
case _ => CRLF
43+
}
44+
45+
private final val CRLF: String = "\r\n"
3746

3847
}

‎modules/build/src/test/scala/scala/build/tests/ActionableDiagnosticTests.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ class ActionableDiagnosticTests extends TestUtil.ScalaCliBuildSuite {
2323
)
2424
val buildThreads = BuildThreads.create()
2525

26+
def path2url(p: os.Path): String = p.toIO.toURI.toURL.toString
27+
2628
test("using outdated os-lib") {
2729
val dependencyOsLib = "com.lihaoyi::os-lib:0.7.8"
2830
val testInputs = TestInputs(
@@ -160,7 +162,7 @@ class ActionableDiagnosticTests extends TestUtil.ScalaCliBuildSuite {
160162
)
161163
val withRepoBuildOptions = baseOptions.copy(
162164
classPathOptions =
163-
baseOptions.classPathOptions.copy(extraRepositories = Seq(s"file:${repoTmpDir.toString}"))
165+
baseOptions.classPathOptions.copy(extraRepositories = Seq(path2url(repoTmpDir)))
164166
)
165167
testInputs.withBuild(withRepoBuildOptions, buildThreads, None, actionableDiagnostics = true) {
166168
(_, _, maybeBuild) =>
@@ -222,7 +224,7 @@ class ActionableDiagnosticTests extends TestUtil.ScalaCliBuildSuite {
222224
val withRepoBuildOptions = baseOptions.copy(
223225
classPathOptions =
224226
baseOptions.classPathOptions.copy(extraRepositories =
225-
Seq(s"file:${repoTmpDir.toString.replace('\\', '/')}")
227+
Seq(path2url(repoTmpDir))
226228
)
227229
)
228230
testInputs.withBuild(withRepoBuildOptions, buildThreads, None, actionableDiagnostics = true) {

‎modules/build/src/test/scala/scala/build/tests/SourceGeneratorTests.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class SourceGeneratorTests extends TestUtil.ScalaCliBuildSuite {
5454
"val scalaCliVersion = Some\\(\"[^\"]*\"\\)",
5555
"val scalaCliVersion = Some\\(\"1.1.1-SNAPSHOT\"\\)"
5656
)
57+
.replaceAll("\\\\", "\\")
5758
.linesWithSeparators
5859
.filterNot(_.stripLeading().startsWith("/**"))
5960
.mkString

‎modules/build/src/test/scala/scala/build/tests/SourcesTests.scala

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -308,24 +308,31 @@ class SourcesTests extends TestUtil.ScalaCliBuildSuite {
308308
|#! nix-shell -i scala-cli
309309
|
310310
|println("Hello World")""".stripMargin,
311-
os.rel / "something4.scala" ->
311+
os.rel / "something4.sc" ->
312312
"""#!/usr/bin/scala-cli
313313
|#! nix-shell -i scala-cli
314314
|
315315
|!#
316316
|
317317
|println("Hello World")""".stripMargin,
318-
os.rel / "something5.scala" ->
318+
os.rel / "something5.sc" ->
319319
"""#!/usr/bin/scala-cli
320320
|
321321
|println("Hello World #!")""".stripMargin,
322-
os.rel / "multiline.scala" ->
322+
os.rel / "multiline.sc" ->
323323
"""#!/usr/bin/scala-cli
324324
|# comment
325325
|VAL=1
326326
|!#
327327
|
328-
|println("Hello World #!")""".stripMargin
328+
|println("Hello World #!")""".stripMargin,
329+
os.rel / "hasBangHashInComment.sc" ->
330+
"""#!/usr/bin/scala-cli
331+
|
332+
|
333+
|
334+
|
335+
|println("Hello World !#")""".stripMargin
329336
)
330337
val expectedParsedCodes = Seq(
331338
"""
@@ -349,7 +356,13 @@ class SourcesTests extends TestUtil.ScalaCliBuildSuite {
349356
|
350357
|
351358
|
352-
|println("Hello World #!")""".stripMargin
359+
|println("Hello World #!")""".stripMargin,
360+
"""
361+
|
362+
|
363+
|
364+
|
365+
|println("Hello World !#")""".stripMargin
353366
)
354367

355368
testInputs.withInputs { (root, inputs) =>
@@ -375,10 +388,20 @@ class SourcesTests extends TestUtil.ScalaCliBuildSuite {
375388
sources.inMemory.map(_.content).map(s => new String(s, StandardCharsets.UTF_8))
376389

377390
parsedCodes.zip(expectedParsedCodes).foreach { case (parsedCode, expectedCode) =>
391+
showDiff(parsedCode, expectedCode)
378392
expect(parsedCode.contains(expectedCode))
379393
}
380394
}
381395
}
396+
def showDiff(parsed: String, expected: String): Unit = {
397+
if (!parsed.contains(expected))
398+
for (((p, e), i) <- (parsed zip expected).zipWithIndex) {
399+
val ps = TestUtil.c2s(p)
400+
val es = TestUtil.c2s(e)
401+
if (ps != es)
402+
System.err.printf("%2d: [%s]!=[%s]\n", i, ps, es)
403+
}
404+
}
382405

383406
test("dependencies in .sc - using") {
384407
val testInputs = TestInputs(

‎modules/build/src/test/scala/scala/build/tests/TestUtil.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,11 @@ object TestUtil {
9292
)
9393
}
9494

95+
def c2s(c: Char): String = c match {
96+
case '\r' => "\\r"
97+
case '\n' => "\\n"
98+
case s => s"$s"
99+
}
100+
95101
lazy val cs = Constants.cs
96102
}

‎modules/build/src/test/scala/scala/build/tests/markdown/MarkdownCodeBlockTests.scala

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,30 @@ class MarkdownCodeBlockTests extends TestUtil.ScalaCliBuildSuite {
6666
)
6767
val Right(Seq(actualResult: MarkdownCodeBlock)) =
6868
MarkdownCodeBlock.findCodeBlocks(os.sub / "Example.md", markdown)
69+
showDiffs(actualResult, expectedResult)
70+
expect(actualResult == expectedResult)
71+
}
72+
73+
test("end-of-shebang token allowed in scala code") {
74+
val code = """println("Hello !#")""".stripMargin
75+
val markdown =
76+
s"""# Some snippet
77+
|
78+
|```scala
79+
|#!/usr/bin/env -S scala-cli shebang
80+
|$code
81+
|```
82+
|""".stripMargin
83+
val expectedResult =
84+
MarkdownCodeBlock(
85+
info = PlainScalaInfo,
86+
body = "\n" + code,
87+
startLine = 3,
88+
endLine = 4
89+
)
90+
val Right(Seq(actualResult: MarkdownCodeBlock)) =
91+
MarkdownCodeBlock.findCodeBlocks(os.sub / "Example.md", markdown)
92+
showDiffs(actualResult, expectedResult)
6993
expect(actualResult == expectedResult)
7094
}
7195

@@ -114,6 +138,7 @@ class MarkdownCodeBlockTests extends TestUtil.ScalaCliBuildSuite {
114138
)
115139
val Right(Seq(actualResult: MarkdownCodeBlock)) =
116140
MarkdownCodeBlock.findCodeBlocks(os.sub / "Example.md", markdown)
141+
showDiffs(actualResult, expectedResult)
117142
expect(actualResult == expectedResult)
118143
}
119144

@@ -209,4 +234,16 @@ class MarkdownCodeBlockTests extends TestUtil.ScalaCliBuildSuite {
209234
expect(actualError.positions == expectedError.positions)
210235
expect(actualError.message == expectedError.message)
211236
}
237+
238+
def showDiffs(actual: MarkdownCodeBlock, expect: MarkdownCodeBlock): Unit = {
239+
if actual != expect then
240+
for (((a, b), i) <- (actual.body zip expect.body).zipWithIndex)
241+
if (a != b) {
242+
val aa = TestUtil.c2s(a)
243+
val bb = TestUtil.c2s(b)
244+
System.err.printf("== index %d: [%s]!=[%s]\n", i, aa, bb)
245+
}
246+
System.err.printf("actual[%s]\nexpect[%s]\n", actual, expect)
247+
}
248+
212249
}

0 commit comments

Comments
 (0)