From 80362f9fc2dfa1c9d5560ad2aa8b2eef320e5fd6 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 20 Feb 2015 16:46:00 -0800 Subject: SI-9167 Clarify ScalaVersion parsing Probably it was unintended to accept "2.." and "2.-11.7". This commit makes it a bit more regular and also accepts arbitrary text after the dash in the build string, including newlines. Since the number parts must be numbers, accept only digits. That also disallows "2.+11.7", which could be misconstrued as some sort of range. As before, the special build string prefixes "rc" and "m" are case-insensitive, but "FINAL" is not. --- .../scala/tools/nsc/settings/ScalaVersion.scala | 50 ++++++++++------------ test/files/run/classfile-format-51.scala | 1 - test/files/run/classfile-format-52.scala | 1 - .../tools/nsc/settings/ScalaVersionTest.scala | 44 ++++++++++++++++++- 4 files changed, 65 insertions(+), 31 deletions(-) diff --git a/src/compiler/scala/tools/nsc/settings/ScalaVersion.scala b/src/compiler/scala/tools/nsc/settings/ScalaVersion.scala index 43bdad5882..7e67b7bec6 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaVersion.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaVersion.scala @@ -68,45 +68,39 @@ case object AnyScalaVersion extends ScalaVersion { * Factory methods for producing ScalaVersions */ object ScalaVersion { - private val dot = "\\." - private val dash = "\\-" - private def not(s:String) = s"[^${s}]" - private val R = s"((${not(dot)}*)(${dot}(${not(dot)}*)(${dot}(${not(dash)}*)(${dash}(.*))?)?)?)".r - - def apply(versionString : String, errorHandler: String => Unit): ScalaVersion = { - def errorAndValue() = { - errorHandler( - s"There was a problem parsing ${versionString}. " + - "Versions should be in the form major[.minor[.revision]] " + - "where each part is a positive number, as in 2.10.1. " + - "The minor and revision parts are optional." - ) - AnyScalaVersion - } + private val dot = """\.""" + private val dash = "-" + private val vchar = """\d""" //"[^-+.]" + private val vpat = s"(?s)($vchar+)(?:$dot($vchar+)(?:$dot($vchar+)(?:$dash(.*))?)?)?".r + private val rcpat = """(?i)rc(\d*)""".r + private val mspat = """(?i)m(\d*)""".r + + def apply(versionString: String, errorHandler: String => Unit): ScalaVersion = { + def error() = errorHandler( + s"There was a problem parsing ${versionString}. " + + "Versions should be in the form major[.minor[.revision]] " + + "where each part is a positive number, as in 2.10.1. " + + "The minor and revision parts are optional." + ) def toInt(s: String) = s match { case null | "" => 0 - case _ => s.toInt + case _ => s.toInt } - def isInt(s: String) = util.Try(toInt(s)).isSuccess - def toBuild(s: String) = s match { case null | "FINAL" => Final - case s if (s.toUpperCase.startsWith("RC") && isInt(s.substring(2))) => RC(toInt(s.substring(2))) - case s if (s.toUpperCase.startsWith("M") && isInt(s.substring(1))) => Milestone(toInt(s.substring(1))) - case _ => Development(s) + case rcpat(i) => RC(toInt(i)) + case mspat(i) => Milestone(toInt(i)) + case _ /* | "" */ => Development(s) } - try versionString match { + versionString match { case "none" => NoScalaVersion - case "any" => AnyScalaVersion - case R(_, majorS, _, minorS, _, revS, _, buildS) => + case "any" => AnyScalaVersion + case vpat(majorS, minorS, revS, buildS) => SpecificScalaVersion(toInt(majorS), toInt(minorS), toInt(revS), toBuild(buildS)) - case _ => - errorAndValue() - } catch { - case e: NumberFormatException => errorAndValue() + case _ => error() ; AnyScalaVersion } } diff --git a/test/files/run/classfile-format-51.scala b/test/files/run/classfile-format-51.scala index 24b1ee8397..4351757a64 100644 --- a/test/files/run/classfile-format-51.scala +++ b/test/files/run/classfile-format-51.scala @@ -1,6 +1,5 @@ import java.io.{File, FileOutputStream} -import scala.tools.nsc.settings.ScalaVersion import scala.tools.partest._ import scala.tools.asm import asm.{AnnotationVisitor, ClassWriter, FieldVisitor, Handle, MethodVisitor, Opcodes} diff --git a/test/files/run/classfile-format-52.scala b/test/files/run/classfile-format-52.scala index e12c84124c..6646e081c4 100644 --- a/test/files/run/classfile-format-52.scala +++ b/test/files/run/classfile-format-52.scala @@ -1,6 +1,5 @@ import java.io.{File, FileOutputStream} -import scala.tools.nsc.settings.ScalaVersion import scala.tools.partest._ import scala.tools.asm import asm.{AnnotationVisitor, ClassWriter, FieldVisitor, Handle, MethodVisitor, Opcodes} diff --git a/test/junit/scala/tools/nsc/settings/ScalaVersionTest.scala b/test/junit/scala/tools/nsc/settings/ScalaVersionTest.scala index 77a2da828e..acbf39fe23 100644 --- a/test/junit/scala/tools/nsc/settings/ScalaVersionTest.scala +++ b/test/junit/scala/tools/nsc/settings/ScalaVersionTest.scala @@ -13,6 +13,48 @@ class ScalaVersionTest { @Test def versionUnparse() { val v = "2.11.3" - assertEquals(ScalaVersion(v).unparse, v) + assertEquals(v, ScalaVersion(v).unparse) + assertEquals("2.11.3-RC4", ScalaVersion("2.11.3-rc4").unparse) + } + + // SI-9167 + @Test def `version parses with rigor`() { + import settings.{ SpecificScalaVersion => V } + import ScalaVersion._ + + // no-brainers + assertEquals(V(2,11,7,Final), ScalaVersion("2.11.7")) + assertEquals(V(2,11,7,Final), ScalaVersion("2.11.7-FINAL")) + assertEquals(V(2,11,7,Milestone(3)), ScalaVersion("2.11.7-M3")) + assertEquals(V(2,11,7,RC(3)), ScalaVersion("2.11.7-RC3")) + assertEquals(V(2,11,7,Development("devbuild")), ScalaVersion("2.11.7-devbuild")) + + // partial-brainers + assertEquals(V(2,11,7,Milestone(3)), ScalaVersion("2.11.7-m3")) + assertEquals(V(2,11,7,RC(3)), ScalaVersion("2.11.7-rc3")) + assertEquals(V(2,11,7,Development("maybegood")), ScalaVersion("2.11.7-maybegood")) + assertEquals(V(2,11,7,Development("RCCola")), ScalaVersion("2.11.7-RCCola")) + assertEquals(V(2,11,7,Development("RC1.5")), ScalaVersion("2.11.7-RC1.5")) + assertEquals(V(2,11,7,Development("")), ScalaVersion("2.11.7-")) + assertEquals(V(2,11,7,Development("0.5")), ScalaVersion("2.11.7-0.5")) + assertEquals(V(2,11,7,Development("devbuild\nSI-9167")), ScalaVersion("2.11.7-devbuild\nSI-9167")) + assertEquals(V(2,11,7,Development("final")), ScalaVersion("2.11.7-final")) + + // oh really + assertEquals(NoScalaVersion, ScalaVersion("none")) + assertEquals(AnyScalaVersion, ScalaVersion("any")) + + assertThrows[NumberFormatException] { ScalaVersion("2.11.7.2") } + assertThrows[NumberFormatException] { ScalaVersion("2.11.7.beta") } + assertThrows[NumberFormatException] { ScalaVersion("2.x.7") } + assertThrows[NumberFormatException] { ScalaVersion("2.-11.7") } + assertThrows[NumberFormatException] { ScalaVersion("2. ") } + assertThrows[NumberFormatException] { ScalaVersion("2.1 .7") } + assertThrows[NumberFormatException] { ScalaVersion("2.") } + assertThrows[NumberFormatException] { ScalaVersion("2..") } + assertThrows[NumberFormatException] { ScalaVersion("2...") } + assertThrows[NumberFormatException] { ScalaVersion("2-") } + assertThrows[NumberFormatException] { ScalaVersion("2-.") } // scalacheck territory + assertThrows[NumberFormatException] { ScalaVersion("any.7") } } } -- cgit v1.2.3