Skip to content

Commit

Permalink
Warn about transitive using file directive (#2432)
Browse files Browse the repository at this point in the history
Add warning about chaining of 'using file'  directive
Mention in docs that chaining of 'using file' is not supported
  • Loading branch information
MaciejG604 authored Oct 6, 2023
1 parent 89c2758 commit 1d24921
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 6 deletions.
25 changes: 23 additions & 2 deletions modules/build/src/main/scala/scala/build/CrossSources.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import scala.build.errors.{
BuildException,
CompositeBuildException,
ExcludeDefinitionError,
MalformedDirectiveError
MalformedDirectiveError,
Severity
}
import scala.build.input.ElementsUtils.*
import scala.build.input.*
import scala.build.internal.Constants
import scala.build.internal.util.RegexUtils
import scala.build.internal.util.{RegexUtils, WarningMessages}
import scala.build.options.{
BuildOptions,
BuildRequirements,
Expand Down Expand Up @@ -214,6 +215,9 @@ object CrossSources {
value(preprocessSources(inputsElemFromDirectives.pipe(elements =>
value(excludeSources(elements, inputs.workspace, allExclude))
)))

warnAboutChainedUsingFileDirectives(preprocessedSourcesFromDirectives, logger)

val allInputs = inputs.add(inputsElemFromDirectives).pipe(inputs =>
val filteredElements = value(excludeSources(inputs.elements, inputs.workspace, allExclude))
inputs.withElements(elements = filteredElements)
Expand Down Expand Up @@ -462,4 +466,21 @@ object CrossSources {
))
}
}

/** When a source file added by a `using file` directive, itself, contains `using file` directives
* there should be a warning printed that transitive `using file` directives are not supported.
*/
def warnAboutChainedUsingFileDirectives(
sourcesAddedWithDirectives: Seq[PreprocessedSource],
logger: Logger
): Unit = for {
additionalSource <- sourcesAddedWithDirectives
buildOptions <- additionalSource.options
transitiveAdditionalSource <- buildOptions.internal.extraSourceFiles
} do
logger.diagnostic(
WarningMessages.chainingUsingFileDirective,
Severity.Warning,
transitiveAdditionalSource.positions
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,7 @@ object WarningMessages {
"directive",
handler.scalaSpecificationLevel
)

val chainingUsingFileDirective: String =
"Chaining the 'using file' directive is not supported, the source won't be included in the build."
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import scala.util.Try
|`//> using files` _path1_ _path2_ …
|""".stripMargin
)
@DirectiveDescription("Manually add sources to the project")
@DirectiveDescription(
"Manually add sources to the project. Does not support chaining, sources are added only once, not recursively."
)
@DirectiveLevel(SpecificationLevel.SHOULD)
// format: off
final case class Sources(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1663,7 +1663,7 @@ abstract class RunTestDefinitions(val scalaVersionOpt: Option[String])
}

// Credentials tests
test("Repository credentials passed to coursier") {
test("repository credentials passed to coursier") {
val testOrg = "test-org"
val testName = "the-messages"
val testVersion = "0.1.2"
Expand Down Expand Up @@ -1818,4 +1818,67 @@ abstract class RunTestDefinitions(val scalaVersionOpt: Option[String])
}
}
}

test("warn about transitive `using file` directive") {
TestInputs(
os.rel / "Main.scala" ->
"""//> using file "bar/Bar.scala"
|//> using file "abc/Abc.scala"
|object Main extends App {
| println(Bar(42))
|}
|""".stripMargin,
os.rel / "bar" / "Bar.scala" ->
"""//> using file "xyz/Xyz.scala"
|//> using file "xyz/NonExistent.scala"
|case class Bar(x: Int)
|""".stripMargin,
os.rel / "abc" / "Abc.scala" ->
"""//> using file "xyz/Xyz.scala"
|//> using file "xyz/NonExistent.scala"
|case class Abc(x: Int)
|""".stripMargin,
os.rel / "xyz" / "Xyz.scala" ->
"""val xyz = 42
|""".stripMargin
).fromRoot { root =>
val res = os.proc(
TestUtil.cli,
"compile",
"Main.scala",
"--suppress-directives-in-multiple-files-warning"
)
.call(cwd = root, mergeErrIntoOut = true)

val output = TestUtil.removeAnsiColors(res.out.trim())

expect(output.contains(
"""[warn] Chaining the 'using file' directive is not supported, the source won't be included in the build.
|[warn] //> using file "xyz/Xyz.scala"
|[warn] ^^^^^^^^^^^^^
|""".stripMargin
))

expect(output.contains(
"""[warn] Chaining the 'using file' directive is not supported, the source won't be included in the build.
|[warn] //> using file "xyz/NonExistent.scala"
|[warn] ^^^^^^^^^^^^^^^^^^^^^
|""".stripMargin
))

expect(output.contains(
"""[warn] Chaining the 'using file' directive is not supported, the source won't be included in the build.
|[warn] //> using file "xyz/Xyz.scala"
|[warn] ^^^^^^^^^^^^^
|""".stripMargin
))

expect(output.contains(
"""[warn] Chaining the 'using file' directive is not supported, the source won't be included in the build.
|[warn] //> using file "xyz/NonExistent.scala"
|[warn] ^^^^^^^^^^^^^^^^^^^^^
|""".stripMargin
))
}
}
}
2 changes: 1 addition & 1 deletion website/docs/reference/directives.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ Manually add JAR(s) to the class path

### Custom sources

Manually add sources to the project
Manually add sources to the project. Does not support chaining, sources are added only once, not recursively.

`//> using file` _path_

Expand Down
2 changes: 1 addition & 1 deletion website/docs/reference/scala-command/directives.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ Manually add JAR(s) to the class path

### Custom sources

Manually add sources to the project
Manually add sources to the project. Does not support chaining, sources are added only once, not recursively.

`//> using file` _path_

Expand Down

0 comments on commit 1d24921

Please sign in to comment.