Skip to content

Commit

Permalink
improvement: Bump using directives to 1.0.0
Browse files Browse the repository at this point in the history
This includes the changes to simplify using directives:
- remove the possibility of nesting from API
- allow to omit commas in lists of values
- don't allow multiline comments
- remove multiline strings
- remove require and @require syntax support
- allow values without quotes
- remove @using
- remove multiline directives
  • Loading branch information
tgodzik committed May 5, 2023
1 parent 5d33b7d commit 22b5c99
Show file tree
Hide file tree
Showing 15 changed files with 43 additions and 140 deletions.
8 changes: 4 additions & 4 deletions modules/build/src/main/scala/scala/build/CrossSources.scala
Expand Up @@ -212,7 +212,7 @@ object CrossSources {
} yield mainClass

val pathsWithDirectivePositions
: Seq[(WithBuildRequirements[(os.Path, os.RelPath)], Option[DirectivesPositions])] =
: Seq[(WithBuildRequirements[(os.Path, os.RelPath)], Option[Position.File])] =
preprocessedSources.collect {
case d: PreprocessedSource.OnDisk =>
val baseReqs0 = baseReqs(d.scopePath)
Expand All @@ -222,7 +222,7 @@ object CrossSources {
) -> d.directivesPositions
}
val inMemoryWithDirectivePositions
: Seq[(WithBuildRequirements[Sources.InMemory], Option[DirectivesPositions])] =
: Seq[(WithBuildRequirements[Sources.InMemory], Option[Position.File])] =
preprocessedSources.collect {
case m: PreprocessedSource.InMemory =>
val baseReqs0 = baseReqs(m.scopePath)
Expand All @@ -239,7 +239,7 @@ object CrossSources {
WithBuildRequirements(BuildRequirements(), _)
)

lazy val allPathsWithDirectivesByScope: Map[Scope, Seq[(os.Path, DirectivesPositions)]] =
lazy val allPathsWithDirectivesByScope: Map[Scope, Seq[(os.Path, Position.File)]] =
(pathsWithDirectivePositions ++ inMemoryWithDirectivePositions)
.flatMap { (withBuildRequirements, directivesPositions) =>
val scope = withBuildRequirements.scopedValue(Scope.Main).scope
Expand Down Expand Up @@ -270,7 +270,7 @@ object CrossSources {
.foreach { (_, directivesPositions) =>
logger.diagnostic(
s"Using directives detected in multiple files. It is recommended to keep them centralized in the $projectFilePath file.",
positions = Seq(directivesPositions.all.maxBy(_.endPos._1))
positions = Seq(directivesPositions)
)
}
}
Expand Down
@@ -1,13 +1,8 @@
package scala.build.preprocessing

import com.virtuslab.using_directives.config.Settings
import com.virtuslab.using_directives.custom.model.{
UsingDirectiveKind,
UsingDirectiveSyntax,
UsingDirectives
}
import com.virtuslab.using_directives.custom.model.UsingDirectives
import com.virtuslab.using_directives.custom.utils.ast.{UsingDef, UsingDefs}
import com.virtuslab.using_directives.{Context, UsingDirectivesProcessor}
import com.virtuslab.using_directives.UsingDirectivesProcessor

import scala.annotation.targetName
import scala.build.errors.*
Expand All @@ -19,22 +14,13 @@ import scala.jdk.CollectionConverters.*

case class ExtractedDirectives(
directives: Seq[StrictDirective],
positions: Option[DirectivesPositions]
positions: Option[Position.File]
) {
@targetName("append")
def ++(other: ExtractedDirectives): ExtractedDirectives =
ExtractedDirectives(directives ++ other.directives, positions)
}

case class DirectivesPositions(
codeDirectives: Position.File,
specialCommentDirectives: Position.File,
plainCommentDirectives: Position.File
) {
def all: Seq[Position.File] =
Seq(codeDirectives, specialCommentDirectives, plainCommentDirectives)
}

object ExtractedDirectives {

def empty: ExtractedDirectives = ExtractedDirectives(Seq.empty, None)
Expand All @@ -55,14 +41,8 @@ object ExtractedDirectives {
else
errors += diag
}
val processor = {
val settings = new Settings
settings.setAllowStartWithoutAt(true)
settings.setAllowRequire(false)
val context = new Context(reporter, settings)
new UsingDirectivesProcessor(context)
}
val all = processor.extract(contentChars, true, true).asScala
val processor = new UsingDirectivesProcessor(reporter)
val allDirectives = processor.extract(contentChars).asScala
val malformedDirectiveErrors =
errors.map(diag => new MalformedDirectiveError(diag.message, diag.positions)).toSeq
val maybeCompositeMalformedDirectiveError =
Expand All @@ -71,65 +51,21 @@ object ExtractedDirectives {
else None
if (malformedDirectiveErrors.isEmpty || maybeCompositeMalformedDirectiveError.isEmpty) {

def byKind(kind: UsingDirectiveKind) = all.find(_.getKind == kind).get

val codeDirectives = byKind(UsingDirectiveKind.Code)
val specialCommentDirectives = byKind(UsingDirectiveKind.SpecialComment)
val plainCommentDirectives = byKind(UsingDirectiveKind.PlainComment)

val directivesPositionsOpt =
if (
codeDirectives.containsTargetDirectivesOnly &&
specialCommentDirectives.containsTargetDirectivesOnly &&
plainCommentDirectives.containsTargetDirectivesOnly
)
val directives = allDirectives.head
val directivesPositionOpt =
if (directives.containsTargetDirectivesOnly)
None
else
Some(DirectivesPositions(
codeDirectives.getPosition(path),
specialCommentDirectives.getPosition(path),
plainCommentDirectives.getPosition(path)
))

def reportWarning(msg: String, values: Seq[UsingDef], before: Boolean = true): Unit =
values.foreach { v =>
val astPos = v.getPosition
val (start, end) =
if (before) (0, astPos.getColumn)
else (astPos.getColumn, astPos.getColumn + v.getSyntax.getKeyword.length)
val position = Position.File(path, (astPos.getLine, start), (astPos.getLine, end))
logger.diagnostic(msg, positions = Seq(position))
}

if (codeDirectives.nonEmpty) {
val msg =
"This using directive is ignored. Only using directives starting with //> are supported."
reportWarning(msg, getDirectives(codeDirectives))
}

if (plainCommentDirectives.nonEmpty) {
val msg =
s"This using directive is ignored. $changeToSpecialCommentMsg"
reportWarning(msg, getDirectives(plainCommentDirectives))
}

val usedDirectives = specialCommentDirectives

// All using directives should use just `using` keyword, no @using or require
reportWarning(
"Deprecated using directive syntax, please use keyword `using`.",
getDirectives(specialCommentDirectives).filter(_.getSyntax != UsingDirectiveSyntax.Using),
before = false
)
Some(directives.getPosition(path))

val flattened = usedDirectives.getFlattenedMap.asScala.toSeq
val flattened = directives.getFlattenedMap.asScala.toSeq
val strictDirectives =
flattened.map {
case (k, l) =>
StrictDirective(k.getPath.asScala.mkString("."), l.asScala.toSeq)
}

Right(ExtractedDirectives(strictDirectives, directivesPositionsOpt))
Right(ExtractedDirectives(strictDirectives, directivesPositionOpt))
}
else
maybeCompositeMalformedDirectiveError match {
Expand Down
@@ -1,6 +1,5 @@
package scala.build.preprocessing

import com.virtuslab.using_directives.custom.model.UsingDirectiveKind
import coursier.cache.ArchiveCache
import coursier.util.Task

Expand Down
@@ -1,7 +1,5 @@
package scala.build.preprocessing

import com.virtuslab.using_directives.custom.model.UsingDirectiveKind

import scala.build.EitherCps.{either, value}
import scala.build.Logger
import scala.build.errors.BuildException
Expand Down
@@ -1,6 +1,7 @@
package scala.build.preprocessing

import scala.build.options.{BuildOptions, BuildRequirements, WithBuildRequirements}
import scala.build.Position

sealed abstract class PreprocessedSource extends Product with Serializable {
def options: Option[BuildOptions]
Expand All @@ -10,7 +11,7 @@ sealed abstract class PreprocessedSource extends Product with Serializable {

def scopedRequirements: Seq[Scoped[BuildRequirements]]
def scopePath: ScopePath
def directivesPositions: Option[DirectivesPositions]
def directivesPositions: Option[Position.File]
}

object PreprocessedSource {
Expand All @@ -22,7 +23,7 @@ object PreprocessedSource {
requirements: Option[BuildRequirements],
scopedRequirements: Seq[Scoped[BuildRequirements]],
mainClassOpt: Option[String],
directivesPositions: Option[DirectivesPositions]
directivesPositions: Option[Position.File]
) extends PreprocessedSource {
def scopePath: ScopePath =
ScopePath.fromPath(path)
Expand All @@ -38,7 +39,7 @@ object PreprocessedSource {
scopedRequirements: Seq[Scoped[BuildRequirements]],
mainClassOpt: Option[String],
scopePath: ScopePath,
directivesPositions: Option[DirectivesPositions]
directivesPositions: Option[Position.File]
) extends PreprocessedSource {
def reportingPath: Either[String, os.Path] =
originalPath.map(_._2)
Expand Down
@@ -1,6 +1,5 @@
package scala.build.preprocessing

import com.virtuslab.using_directives.custom.model.UsingDirectiveKind
import dependency.AnyDependency
import dependency.parser.DependencyParser

Expand Down Expand Up @@ -33,7 +32,7 @@ case object ScalaPreprocessor extends Preprocessor {
opts: BuildOptions,
optsWithReqs: List[WithBuildRequirements[BuildOptions]],
updatedContent: Option[String],
directivesPositions: Option[DirectivesPositions]
directivesPositions: Option[Position.File]
)

object ProcessingOutput {
Expand Down Expand Up @@ -129,7 +128,7 @@ case object ScalaPreprocessor extends Preprocessor {
options: BuildOptions,
optionsWithTargetRequirements: List[WithBuildRequirements[BuildOptions]],
updatedContentOpt: Option[String],
directivesPositions: Option[DirectivesPositions]
directivesPositions: Option[Position.File]
) =
value(
process(
Expand Down
@@ -1,15 +1,16 @@
package scala.build.preprocessing.directives

import scala.build.options.{BuildOptions, BuildRequirements, WithBuildRequirements}
import scala.build.preprocessing.{DirectivesPositions, Scoped}
import scala.build.preprocessing.Scoped
import scala.build.Position

case class PreprocessedDirectives(
globalReqs: BuildRequirements,
globalUsings: BuildOptions,
usingsWithReqs: List[WithBuildRequirements[BuildOptions]],
scopedReqs: Seq[Scoped[BuildRequirements]],
strippedContent: Option[String],
directivesPositions: Option[DirectivesPositions]
directivesPositions: Option[Position.File]
) {
def isEmpty: Boolean = globalReqs == BuildRequirements.monoid.zero &&
globalUsings == BuildOptions.monoid.zero &&
Expand Down
Expand Up @@ -2,7 +2,6 @@ package scala.build
package tests

import com.eed3si9n.expecty.Expecty.{assert => expect}
import com.virtuslab.using_directives.custom.model.UsingDirectiveKind

import scala.build.errors.Diagnostic
import scala.build.preprocessing.{ExtractedDirectives, ScopePath}
Expand Down
Expand Up @@ -27,8 +27,8 @@ class ScalaPreprocessorTests extends munit.FunSuite {
)
expect(result.nonEmpty)
val Some(directivesPositions) = result.head.directivesPositions
expect(directivesPositions.specialCommentDirectives.startPos == 0 -> 0)
expect(directivesPositions.specialCommentDirectives.endPos == 3 -> 0)
expect(directivesPositions.startPos == 0 -> 0)
expect(directivesPositions.endPos == 3 -> 0)
}
}

Expand All @@ -47,8 +47,8 @@ class ScalaPreprocessorTests extends munit.FunSuite {
)
expect(result.nonEmpty)
val Some(directivesPositions) = result.head.directivesPositions
expect(directivesPositions.specialCommentDirectives.startPos == 0 -> 0)
expect(directivesPositions.specialCommentDirectives.endPos == 2 -> 0)
expect(directivesPositions.startPos == 0 -> 0)
expect(directivesPositions.endPos == 2 -> 0)
}
}
}
@@ -1,12 +1,6 @@
package scala.build.directives

import com.virtuslab.using_directives.custom.model.{
BooleanValue,
EmptyValue,
NumericValue,
StringValue,
Value
}
import com.virtuslab.using_directives.custom.model.{BooleanValue, EmptyValue, StringValue, Value}

import scala.build.Positioned.apply
import scala.build.errors.{
Expand Down Expand Up @@ -106,11 +100,6 @@ object DirectiveValueParser {
case s: BooleanValue => Some(s.get())
case _ => None
}
def asNum: Option[String] =
value match {
case n: NumericValue => Some(n.get())
case _ => None
}

def position(path: Either[String, os.Path]): Position =
DirectiveUtil.position(value, path, skipQuotes = isString)
Expand Down Expand Up @@ -152,10 +141,10 @@ object DirectiveValueParser {

given DirectiveSingleValueParser[MaybeNumericalString] =
(value, scopePath, path) =>
value.asString.orElse(value.asNum).map(MaybeNumericalString(_)).toRight {
value.asString.map(MaybeNumericalString(_)).toRight {
val pos = value.position(path)
new MalformedDirectiveError(
s"Expected a string or a numerical value, got '${value.getRelatedASTNode.toString}'",
s"Expected a string value, got '${value.getRelatedASTNode.toString}'",
Seq(pos)
)
}
Expand Down
Expand Up @@ -10,5 +10,5 @@ final class SingleValueExpectedError(
s"(got ${directive.values.length} values: ${directive.values.map(_.get().toString).mkString(", ")})",
positions = DirectiveUtil.positions(directive.values, path)
) {
assert(directive.numericalOrStringValuesCount > 1)
assert(directive.stringValuesCount > 1)
}
@@ -1,12 +1,6 @@
package scala.build.preprocessing.directives

import com.virtuslab.using_directives.custom.model.{
BooleanValue,
EmptyValue,
NumericValue,
StringValue,
Value
}
import com.virtuslab.using_directives.custom.model.{BooleanValue, EmptyValue, StringValue, Value}

import scala.build.preprocessing.ScopePath
import scala.build.{Position, Positioned}
Expand Down Expand Up @@ -34,9 +28,6 @@ object DirectiveUtil {
case v: StringValue =>
val pos = position(v, scopedDirective.maybePath, skipQuotes = true)
Positioned(pos, v.get)
case v: NumericValue =>
val pos = position(v, scopedDirective.maybePath, skipQuotes = false)
Positioned(pos, v.get)
case v: BooleanValue =>
val pos = position(v, scopedDirective.maybePath, skipQuotes = false)
Positioned(pos, v.get.toString)
Expand Down
@@ -1,6 +1,6 @@
package scala.build.preprocessing.directives

import com.virtuslab.using_directives.custom.model.{NumericValue, StringValue, Value}
import com.virtuslab.using_directives.custom.model.{StringValue, Value}

case class StrictDirective(
key: String,
Expand All @@ -10,10 +10,9 @@ case class StrictDirective(
val suffix = if values.isEmpty then "" else s" \"${values.mkString("\", \"")}\""
s"//> using $key$suffix"
}
def numericalOrStringValuesCount: Int =
def stringValuesCount: Int =
values.count {
case _: NumericValue => true
case _: StringValue => true
case _ => false
case _: StringValue => true
case _ => false
}
}

0 comments on commit 22b5c99

Please sign in to comment.