Merge pull request #9731 from som-snytt/issue/9719

Multivalued setting can be set multiply

compiler/src/dotty/tools/dotc/config/Settings.scala

@@ -1,18 +1,15 @@
 package dotty.tools.dotc
 package config
 
-import collection.mutable.ArrayBuffer
-import scala.util.{ Success, Failure }
-import reflect.ClassTag
 import core.Contexts._
-import scala.annotation.tailrec
-import dotty.tools.io.{ AbstractFile, Directory, JarArchive, PlainDirectory }
 
-// import annotation.unchecked
-  // Dotty deviation: Imports take precedence over definitions in enclosing package
-  // (Note that @unchecked is in scala, not annotation, so annotation.unchecked gives
-  // us a package, which is not what was intended anyway).
+import dotty.tools.io.{AbstractFile, Directory, JarArchive, PlainDirectory}
+
+import annotation.tailrec
+import collection.mutable.ArrayBuffer
 import language.existentials
+import reflect.ClassTag
+import scala.util.{Success, Failure}
 
 object Settings {
 
@@ -25,7 +22,7 @@ object Settings {
   val OutputTag: ClassTag[AbstractFile]  = ClassTag(classOf[AbstractFile])
 
   class SettingsState(initialValues: Seq[Any]) {
-    private var values = ArrayBuffer(initialValues: _*)
+    private val values = ArrayBuffer(initialValues: _*)
     private var _wasRead: Boolean = false
 
     override def toString: String = s"SettingsState(values: ${values.toList})"
@@ -91,38 +88,46 @@ object Settings {
 
     def isDefaultIn(state: SettingsState): Boolean = valueIn(state) == default
 
+    def isMultivalue: Boolean = implicitly[ClassTag[T]] == ListTag
+
     def legalChoices: String =
-      if (choices.isEmpty) ""
-      else choices match {
-        case r: Range => s"${r.head}..${r.last}"
-        case xs: List[?] => xs.mkString(", ")
+      choices match {
+        case xs if xs.isEmpty => ""
+        case r: Range         => s"${r.head}..${r.last}"
+        case xs: List[?]      => xs.toString
       }
 
     def isLegal(arg: Any): Boolean =
-      if (choices.isEmpty)
-        arg match {
-          case _: T => true
-          case _ => false
-        }
-      else choices match {
+      choices match {
+        case xs if xs.isEmpty =>
+          arg match {
+            case _: T => true
+            case _ => false
+          }
         case r: Range =>
           arg match {
             case x: Int => r.head <= x && x <= r.last
             case _ => false
           }
         case xs: List[?] =>
-          xs contains arg
+          xs.contains(arg)
       }
 
     def tryToSet(state: ArgsSummary): ArgsSummary = {
       val ArgsSummary(sstate, arg :: args, errors, warnings) = state
       def update(value: Any, args: List[String]) =
-        if (changed)
-          ArgsSummary(updateIn(sstate, value), args, errors, warnings :+ s"Flag $name set repeatedly")
-        else {
-          changed = true
-          ArgsSummary(updateIn(sstate, value), args, errors, warnings)
-        }
+        var dangers = warnings
+        val value1 =
+          if changed && isMultivalue then
+            val value0  = value.asInstanceOf[List[String]]
+            val current = valueIn(sstate).asInstanceOf[List[String]]
+            value0.filter(current.contains).foreach(s => dangers :+= s"Setting $name set to $s redundantly")
+            current ++ value0
+          else
+            if changed then dangers :+= s"Flag $name set repeatedly"
+            value
+        changed = true
+        ArgsSummary(updateIn(sstate, value1), args, errors, dangers)
       def fail(msg: String, args: List[String]) =
         ArgsSummary(sstate, args, errors :+ msg, warnings)
       def missingArg =
@@ -229,7 +234,7 @@ object Settings {
      *
      *  to get their arguments.
      */
-    protected def processArguments(state: ArgsSummary, processAll: Boolean, skipped: List[String]): ArgsSummary = {
+    protected[config] def processArguments(state: ArgsSummary, processAll: Boolean, skipped: List[String]): ArgsSummary = {
       def stateWithArgs(args: List[String]) = ArgsSummary(state.sstate, args, state.errors, state.warnings)
       state.arguments match {
         case Nil =>

compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala

@@ -17,7 +17,7 @@ import Diagnostic._
   * - The reporter is not flushed and the message containers capture a
   *   `Context` (about 4MB)
   */
-class StoreReporter(outer: Reporter) extends Reporter {
+class StoreReporter(outer: Reporter = Reporter.NoReporter) extends Reporter {
 
   protected var infos: mutable.ListBuffer[Diagnostic] = null
 

compiler/test/dotty/tools/dotc/SettingsTests.scala

@@ -1,23 +1,23 @@
 package dotty.tools
 package dotc
 
+import reporting.StoreReporter
 import vulpix.TestConfiguration
 
-import org.junit.Test
-import org.junit.Assert._
+import dotty.tools.vulpix.TestConfiguration.mkClasspath
 
 import java.nio.file._
 
-import dotty.tools.vulpix.TestConfiguration.mkClasspath
+import org.junit.Test
+import org.junit.Assert._
 
 class SettingsTests {
 
-  @Test def missingOutputDir: Unit = {
+  @Test def missingOutputDir: Unit =
     val options = Array("-d", "not_here")
-    val reporter = Main.process(options)
+    val reporter = Main.process(options, reporter = StoreReporter())
     assertEquals(1, reporter.errorCount)
     assertEquals("'not_here' does not exist or is not a directory or .jar file", reporter.allErrors.head.message)
-  }
 
   @Test def jarOutput: Unit = {
     val source = "tests/pos/Foo.scala"
@@ -29,13 +29,12 @@ class SettingsTests {
     assertTrue(Files.exists(out))
   }
 
-  @Test def t8124: Unit = {
-    val source = Paths.get("tests/pos/Foo.scala").normalize
+  @Test def `t8124 Don't crash on missing argument`: Unit =
+    val source    = Paths.get("tests/pos/Foo.scala").normalize
     val outputDir = Paths.get("out/testSettings").normalize
-    if (Files.notExists(outputDir)) Files.createDirectory(outputDir)
-    val options = Array("-encoding", "-d", outputDir.toString, source.toString)
-    val reporter = Main.process(options)
+    if Files.notExists(outputDir) then Files.createDirectory(outputDir)
+    // -encoding takes an arg!
+    val options  = Array("-encoding", "-d", outputDir.toString, source.toString)
+    val reporter = Main.process(options, reporter = StoreReporter())
     assertEquals(1, reporter.errorCount)
-   }
-
 }

compiler/test/dotty/tools/dotc/config/ScalaSettingsTests.scala

@@ -0,0 +1,54 @@
+package dotty.tools.dotc
+package config
+
+import CommandLineParser.tokenize
+import Settings._
+
+import org.junit.Test
+import org.junit.Assert._
+
+class ScalaSettingsTests:
+
+  @Test def `A multistring setting is multivalued`: Unit =
+    class SUT extends SettingGroup:
+      val language: Setting[List[String]] = MultiStringSetting("-language", "feature", "Enable one or more language features.")
+    val sut  = SUT()
+    val args = tokenize("-language:implicitConversions,dynamics")
+    val sumy = ArgsSummary(sut.defaultState, args, errors = Nil, warnings = Nil)
+    val res  = sut.processArguments(sumy, processAll = true, skipped = Nil)
+    val set  = sut.language.valueIn(res.sstate)
+    assertEquals(1, args.length)
+    assertTrue("No warnings!", res.warnings.isEmpty)
+    assertTrue("No errors!", res.errors.isEmpty)
+    assertTrue("Has the feature", set.contains("implicitConversions"))
+    assertTrue("Has the feature", set.contains("dynamics"))
+
+  @Test def `t9719 Apply -language more than once`: Unit =
+    class SUT extends SettingGroup:
+      val language: Setting[List[String]] = MultiStringSetting("-language", "feature", "Enable one or more language features.")
+    val sut  = SUT()
+    val args = tokenize("-language:implicitConversions -language:dynamics")
+    val sumy = ArgsSummary(sut.defaultState, args, errors = Nil, warnings = Nil)
+    val res  = sut.processArguments(sumy, processAll = true, skipped = Nil)
+    val set  = sut.language.valueIn(res.sstate)
+    assertEquals(2, args.length)
+    assertTrue("No warnings!", res.warnings.isEmpty)
+    assertTrue("No errors!", res.errors.isEmpty)
+    assertTrue("Has the feature", set.contains("implicitConversions"))
+    assertTrue("Has the feature", set.contains("dynamics"))
+
+  @Test def `Warn if multistring element is supplied multiply`: Unit =
+    class SUT extends SettingGroup:
+      val language: Setting[List[String]] = MultiStringSetting("-language", "feature", "Enable one or more language features.")
+    val sut  = SUT()
+    val args = tokenize("-language:dynamics -language:implicitConversions -language:dynamics")
+    val sumy = ArgsSummary(sut.defaultState, args, errors = Nil, warnings = Nil)
+    val res  = sut.processArguments(sumy, processAll = true, skipped = Nil)
+    val set  = sut.language.valueIn(res.sstate)
+    assertEquals(3, args.length)
+    assertEquals("Must warn", 1, res.warnings.length)
+    assertTrue("No errors!", res.errors.isEmpty)
+    assertTrue("Has the feature", set.contains("implicitConversions"))
+    assertTrue("Has the feature", set.contains("dynamics"))
+
+end ScalaSettingsTests