From c925574ef33efcf61dc13a72d5c3a2f3e74c2502 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 4 May 2023 19:43:34 +0200 Subject: [PATCH] improvement: Bump using directives to 1.0.0 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 --- .../main/scala/scala/build/CrossSources.scala | 8 +- .../preprocessing/ExtractedDirectives.scala | 86 +++---------------- .../preprocessing/JavaPreprocessor.scala | 1 - .../MarkdownCodeBlockProcessor.scala | 2 - .../preprocessing/PreprocessedSource.scala | 7 +- .../preprocessing/ScalaPreprocessor.scala | 5 +- .../directives/PreprocessedDirectives.scala | 5 +- .../build/tests/DirectiveParsingTest.scala | 1 - .../build/tests/ScalaPreprocessorTests.scala | 8 +- .../directives/DirectiveValueParser.scala | 17 +--- .../errors/SingleValueExpectedError.scala | 2 +- .../directives/DirectiveUtil.scala | 11 +-- .../directives/StrictDirective.scala | 9 +- project/deps.sc | 2 +- 14 files changed, 38 insertions(+), 126 deletions(-) diff --git a/modules/build/src/main/scala/scala/build/CrossSources.scala b/modules/build/src/main/scala/scala/build/CrossSources.scala index f2a54ca24c..f74d71cc9e 100644 --- a/modules/build/src/main/scala/scala/build/CrossSources.scala +++ b/modules/build/src/main/scala/scala/build/CrossSources.scala @@ -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) @@ -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) @@ -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 @@ -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) ) } } diff --git a/modules/build/src/main/scala/scala/build/preprocessing/ExtractedDirectives.scala b/modules/build/src/main/scala/scala/build/preprocessing/ExtractedDirectives.scala index e5fe655297..c1742b58f2 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/ExtractedDirectives.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/ExtractedDirectives.scala @@ -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.* @@ -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) @@ -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 = @@ -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 { diff --git a/modules/build/src/main/scala/scala/build/preprocessing/JavaPreprocessor.scala b/modules/build/src/main/scala/scala/build/preprocessing/JavaPreprocessor.scala index 622e295747..c3055c491a 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/JavaPreprocessor.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/JavaPreprocessor.scala @@ -1,6 +1,5 @@ package scala.build.preprocessing -import com.virtuslab.using_directives.custom.model.UsingDirectiveKind import coursier.cache.ArchiveCache import coursier.util.Task diff --git a/modules/build/src/main/scala/scala/build/preprocessing/MarkdownCodeBlockProcessor.scala b/modules/build/src/main/scala/scala/build/preprocessing/MarkdownCodeBlockProcessor.scala index 72db669f48..90294b1f13 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/MarkdownCodeBlockProcessor.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/MarkdownCodeBlockProcessor.scala @@ -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 diff --git a/modules/build/src/main/scala/scala/build/preprocessing/PreprocessedSource.scala b/modules/build/src/main/scala/scala/build/preprocessing/PreprocessedSource.scala index b2d6c86554..f8442b4332 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/PreprocessedSource.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/PreprocessedSource.scala @@ -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] @@ -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 { @@ -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) @@ -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) diff --git a/modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala b/modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala index d903654849..32b1d2dd84 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/ScalaPreprocessor.scala @@ -1,6 +1,5 @@ package scala.build.preprocessing -import com.virtuslab.using_directives.custom.model.UsingDirectiveKind import dependency.AnyDependency import dependency.parser.DependencyParser @@ -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 { @@ -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( diff --git a/modules/build/src/main/scala/scala/build/preprocessing/directives/PreprocessedDirectives.scala b/modules/build/src/main/scala/scala/build/preprocessing/directives/PreprocessedDirectives.scala index a83aa1d29e..c3577d7146 100644 --- a/modules/build/src/main/scala/scala/build/preprocessing/directives/PreprocessedDirectives.scala +++ b/modules/build/src/main/scala/scala/build/preprocessing/directives/PreprocessedDirectives.scala @@ -1,7 +1,8 @@ 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, @@ -9,7 +10,7 @@ case class PreprocessedDirectives( 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 && diff --git a/modules/build/src/test/scala/scala/build/tests/DirectiveParsingTest.scala b/modules/build/src/test/scala/scala/build/tests/DirectiveParsingTest.scala index f66b711d60..8ff8894e22 100644 --- a/modules/build/src/test/scala/scala/build/tests/DirectiveParsingTest.scala +++ b/modules/build/src/test/scala/scala/build/tests/DirectiveParsingTest.scala @@ -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} diff --git a/modules/build/src/test/scala/scala/build/tests/ScalaPreprocessorTests.scala b/modules/build/src/test/scala/scala/build/tests/ScalaPreprocessorTests.scala index 508b69f24c..8e0c8c722a 100644 --- a/modules/build/src/test/scala/scala/build/tests/ScalaPreprocessorTests.scala +++ b/modules/build/src/test/scala/scala/build/tests/ScalaPreprocessorTests.scala @@ -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) } } @@ -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) } } } diff --git a/modules/directives/src/main/scala/scala/build/directives/DirectiveValueParser.scala b/modules/directives/src/main/scala/scala/build/directives/DirectiveValueParser.scala index 1d09c767dc..b3ad5d97c4 100644 --- a/modules/directives/src/main/scala/scala/build/directives/DirectiveValueParser.scala +++ b/modules/directives/src/main/scala/scala/build/directives/DirectiveValueParser.scala @@ -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.{ @@ -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) @@ -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) ) } diff --git a/modules/directives/src/main/scala/scala/build/errors/SingleValueExpectedError.scala b/modules/directives/src/main/scala/scala/build/errors/SingleValueExpectedError.scala index 9d38824533..8f9ffb046c 100644 --- a/modules/directives/src/main/scala/scala/build/errors/SingleValueExpectedError.scala +++ b/modules/directives/src/main/scala/scala/build/errors/SingleValueExpectedError.scala @@ -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) } diff --git a/modules/directives/src/main/scala/scala/build/preprocessing/directives/DirectiveUtil.scala b/modules/directives/src/main/scala/scala/build/preprocessing/directives/DirectiveUtil.scala index ae4de3883f..b7f11242d2 100644 --- a/modules/directives/src/main/scala/scala/build/preprocessing/directives/DirectiveUtil.scala +++ b/modules/directives/src/main/scala/scala/build/preprocessing/directives/DirectiveUtil.scala @@ -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} @@ -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) diff --git a/modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala b/modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala index 5ed9de1cae..2ae1588a5d 100644 --- a/modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala +++ b/modules/directives/src/main/scala/scala/build/preprocessing/directives/StrictDirective.scala @@ -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, @@ -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 } } diff --git a/project/deps.sc b/project/deps.sc index 681d5b599f..13c410723a 100644 --- a/project/deps.sc +++ b/project/deps.sc @@ -172,7 +172,7 @@ object Deps { def swoval = ivy"com.swoval:file-tree-views:2.1.9" def testInterface = ivy"org.scala-sbt:test-interface:1.0" def toolkit = ivy"org.scala-lang:toolkit:0.1.6" - def usingDirectives = ivy"org.virtuslab:using_directives:0.1.0" + def usingDirectives = ivy"org.virtuslab:using_directives:1.0.0" // Lives at https://github.com/scala-cli/no-crc32-zip-input-stream, see #865 // This provides a ZipInputStream that doesn't verify CRC32 checksums, that users // can enable by setting SCALA_CLI_VENDORED_ZIS=true in the environment, to workaround