Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn about transitive using file directive #2432

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 23 additions & 2 deletions modules/build/src/main/scala/scala/build/CrossSources.scala
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,
Gedochao marked this conversation as resolved.
Show resolved Hide resolved
Severity.Warning,
transitiveAdditionalSource.positions
)
}
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."
}
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
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
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
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