From 3e249607c33675b6ea277050e482a7eb81102da8 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 3 Sep 2020 22:41:50 -0700 Subject: [PATCH 1/6] Dotty imports are aligned with Scala 2 Remove comment referring to ancient Scala 2 bug. Adjust imports to read: imports relative to packaging, then absolute dotty packages, then scala packages. --- .../src/dotty/tools/dotc/config/Settings.scala | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Settings.scala b/compiler/src/dotty/tools/dotc/config/Settings.scala index f039f68591d6..4e392e75a228 100644 --- a/compiler/src/dotty/tools/dotc/config/Settings.scala +++ b/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 { From 973b285ae3af4b9143a6df63a6d7a5fc80dae84b Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 4 Sep 2020 00:15:54 -0700 Subject: [PATCH 2/6] Quieter tests --- .../tools/dotc/reporting/StoreReporter.scala | 2 +- .../test/dotty/tools/dotc/SettingsTests.scala | 25 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala b/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala index 0d069ee07a1d..02be3aea6e4c 100644 --- a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala +++ b/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 = null) extends Reporter { protected var infos: mutable.ListBuffer[Diagnostic] = null diff --git a/compiler/test/dotty/tools/dotc/SettingsTests.scala b/compiler/test/dotty/tools/dotc/SettingsTests.scala index 626f595e44bd..d71727eaa081 100644 --- a/compiler/test/dotty/tools/dotc/SettingsTests.scala +++ b/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: 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) + Files.createDirectory(outputDir) + val options = Array("-encoding", "-d", outputDir.toString, source.toString) + val reporter = Main.process(options, reporter = StoreReporter()) assertEquals(1, reporter.errorCount) - } - } From 196b3fae452dd1b82ac210070fc517efce709a76 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 4 Sep 2020 12:04:19 -0700 Subject: [PATCH 3/6] Style tweaks --- .../dotty/tools/dotc/config/Settings.scala | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Settings.scala b/compiler/src/dotty/tools/dotc/config/Settings.scala index 4e392e75a228..5fd65ffed287 100644 --- a/compiler/src/dotty/tools/dotc/config/Settings.scala +++ b/compiler/src/dotty/tools/dotc/config/Settings.scala @@ -22,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})" @@ -89,26 +89,26 @@ object Settings { def isDefaultIn(state: SettingsState): Boolean = valueIn(state) == default 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 = { From ae55323d2d491c8cef9edc7a25dc75e8d70f9fa1 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 4 Sep 2020 17:10:20 -0700 Subject: [PATCH 4/6] Multivalued setting can be set multiply Permit `-language:foo -language:bar` to mean `-language:foo,bar`, and similarly for other settings which are multivalued (i.e., have a value which is a list). Such settings include `-Xplugin` and `-Xprint`. There is no mechanism to distinguish phase names from other string settings. --- .../dotty/tools/dotc/config/Settings.scala | 21 +++++--- .../dotc/config/ScalaSettingsTests.scala | 54 +++++++++++++++++++ 2 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 compiler/test/dotty/tools/dotc/config/ScalaSettingsTests.scala diff --git a/compiler/src/dotty/tools/dotc/config/Settings.scala b/compiler/src/dotty/tools/dotc/config/Settings.scala index 5fd65ffed287..9ff659b2a288 100644 --- a/compiler/src/dotty/tools/dotc/config/Settings.scala +++ b/compiler/src/dotty/tools/dotc/config/Settings.scala @@ -88,6 +88,8 @@ object Settings { def isDefaultIn(state: SettingsState): Boolean = valueIn(state) == default + def isMultivalue: Boolean = implicitly[ClassTag[T]] == ListTag + def legalChoices: String = choices match { case xs if xs.isEmpty => "" @@ -114,12 +116,17 @@ object Settings { 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) - } + val (value1, twicely) = + if changed && isMultivalue then + val value0 = value.asInstanceOf[List[String]] + val current = valueIn(sstate).asInstanceOf[List[String]] + val newly = current ++ value0.filterNot(current.contains) + (newly, value0.exists(current.contains)) + else + (value, changed) + val dangers = if twicely then warnings :+ s"Flag $name set repeatedly" else warnings + changed = true + ArgsSummary(updateIn(sstate, value1), args, errors, dangers) def fail(msg: String, args: List[String]) = ArgsSummary(sstate, args, errors :+ msg, warnings) def missingArg = @@ -226,7 +233,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 => diff --git a/compiler/test/dotty/tools/dotc/config/ScalaSettingsTests.scala b/compiler/test/dotty/tools/dotc/config/ScalaSettingsTests.scala new file mode 100644 index 000000000000..60c4bfa57fc1 --- /dev/null +++ b/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 From 22e76e8549c0323f8ee00d0dd6a23879fa77204a Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 14 Sep 2020 03:23:30 -0700 Subject: [PATCH 5/6] Warn on redundant setting Warn but don't ignore redundant setting. Prefer NoReporter to null default reporter. --- compiler/src/dotty/tools/dotc/config/Settings.scala | 11 ++++++----- .../dotty/tools/dotc/reporting/StoreReporter.scala | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Settings.scala b/compiler/src/dotty/tools/dotc/config/Settings.scala index 9ff659b2a288..a351921e8e64 100644 --- a/compiler/src/dotty/tools/dotc/config/Settings.scala +++ b/compiler/src/dotty/tools/dotc/config/Settings.scala @@ -116,15 +116,16 @@ object Settings { def tryToSet(state: ArgsSummary): ArgsSummary = { val ArgsSummary(sstate, arg :: args, errors, warnings) = state def update(value: Any, args: List[String]) = - val (value1, twicely) = + var dangers = warnings + val value1 = if changed && isMultivalue then val value0 = value.asInstanceOf[List[String]] val current = valueIn(sstate).asInstanceOf[List[String]] - val newly = current ++ value0.filterNot(current.contains) - (newly, value0.exists(current.contains)) + value0.filter(current.contains).foreach(s => dangers :+= s"Setting $name set to $s redundantly") + current ++ value0 else - (value, changed) - val dangers = if twicely then warnings :+ s"Flag $name set repeatedly" else warnings + 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]) = diff --git a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala b/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala index 02be3aea6e4c..8b6cd6ea5a0d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala +++ b/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 = null) extends Reporter { +class StoreReporter(outer: Reporter = Reporter.NoReporter) extends Reporter { protected var infos: mutable.ListBuffer[Diagnostic] = null From 0772585800d2b4a5a71c5eacd6c949e0f9eb0b28 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 14 Sep 2020 03:39:28 -0700 Subject: [PATCH 6/6] Add clarifying test name and comment --- compiler/test/dotty/tools/dotc/SettingsTests.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/test/dotty/tools/dotc/SettingsTests.scala b/compiler/test/dotty/tools/dotc/SettingsTests.scala index d71727eaa081..f0e1699358b2 100644 --- a/compiler/test/dotty/tools/dotc/SettingsTests.scala +++ b/compiler/test/dotty/tools/dotc/SettingsTests.scala @@ -29,11 +29,11 @@ class SettingsTests { assertTrue(Files.exists(out)) } - @Test def t8124: Unit = + @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) + 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)