Skip to content

Commit

Permalink
Merge pull request #10772 from som-snytt/issue/12992-less-unused-params
Browse files Browse the repository at this point in the history
No warn in unused elements
  • Loading branch information
lrytz committed May 6, 2024
2 parents 3fb0c39 + d1520b5 commit 7feda5a
Show file tree
Hide file tree
Showing 11 changed files with 234 additions and 136 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -544,11 +544,12 @@ trait TypeDiagnostics extends splain.SplainDiagnostics {
case ValDef(mods@_, name@_, tpt@_, rhs@_) if wasPatVarDef(t) =>
if (settings.warnUnusedPatVars && !atBounded(t)) patvars += sym
case DefDef(mods@_, name@_, tparams@_, vparamss, tpt@_, rhs@_) if !sym.isAbstract && !sym.isDeprecated && !sym.isMacro =>
if (isSuppressed(sym)) return
if (sym.isPrimaryConstructor)
for (cpa <- sym.owner.constrParamAccessors if cpa.isPrivateLocal) params += cpa
else if (sym.isSynthetic && sym.isImplicit) return
else if (!sym.isConstructor && !sym.isVar && !isTrivial(rhs))
for (vs <- vparamss) params ++= vs.map(_.symbol)
for (vs <- vparamss; v <- vs) if (!isSingleType(v.symbol.tpe)) params += v.symbol
defnTrees += m
case TypeDef(mods@_, name@_, tparams@_, rhs@_) =>
if (!sym.isAbstract && !sym.isDeprecated)
Expand Down
1 change: 1 addition & 0 deletions src/repl-frontend/scala/tools/nsc/Interpreter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class InterpreterLoop {
}

// for 2.8 compatibility
@annotation.unused
final class Compat {
def bindValue(id: String, value: Any) =
interpreter.bind(id, value.asInstanceOf[AnyRef].getClass.getName, value)
Expand Down
10 changes: 10 additions & 0 deletions test/files/neg/warn-unused-params.scala
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,13 @@ class Unequal {
class Seriously {
def f(s: Serializable) = toString.nonEmpty // warn explicit param of marker trait
}

class TryStart(start: String) {
def FINALLY(end: END.type) = start
}

object END

class Nested {
@annotation.unused private def actuallyNotUsed(fresh: Int, stale: Int) = fresh
}
5 changes: 4 additions & 1 deletion test/files/neg/warn-unused-privates.check
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ warn-unused-privates.scala:154: warning: private method x in class OtherNames is
warn-unused-privates.scala:155: warning: private method y_= in class OtherNames is never used
private def y_=(i: Int): Unit = ()
^
warn-unused-privates.scala:269: warning: private val n in class t12992 enclosing def is unused is never used
private val n = 42
^
warn-unused-privates.scala:121: warning: private class Bar1 in object Types is never used
private class Bar1 // warn
^
Expand All @@ -80,5 +83,5 @@ warn-unused-privates.scala:114: warning: local var x in method f2 is never updat
var x = 100 // warn about it being a var
^
error: No warnings can be incurred under -Werror.
27 warnings
28 warnings
1 error
5 changes: 5 additions & 0 deletions test/files/neg/warn-unused-privates.scala
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,8 @@ trait `short comings` {
class `issue 12600 ignore abstract types` {
type Abs
}

class `t12992 enclosing def is unused` {
private val n = 42
@annotation.unused def f() = n + 2 // unused code uses n
}
80 changes: 50 additions & 30 deletions test/junit/scala/collection/BuildFromTest.scala
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package scala.collection

import org.junit.Test
import org.junit.Assert.{assertEquals, assertTrue}

import scala.annotation.unused
import scala.collection.mutable.Builder
import scala.math.Ordering

import mutable.{ArrayBuffer, Builder}

class BuildFromTest {

// You can either overload methods for IterableOps and Iterable with SortedOps (if you want to support constrained collection types)
Expand Down Expand Up @@ -43,57 +44,68 @@ class BuildFromTest {

@Test
def optionSequence1Test(): Unit = {
val xs1 = immutable.List(Some(1), None, Some(2))
val xs1 = List(Some(1), None, Some(2))
val o1 = optionSequence1(xs1)
@unused val o1t: Option[immutable.List[Int]] = o1
val o1t: Option[List[Int]] = o1
assertTrue(o1t.isEmpty)

val xs2: immutable.TreeSet[Option[String]] = immutable.TreeSet(Some("foo"), Some("bar"), None)
val o2 = optionSequence1(xs2)
@unused val o2t: Option[immutable.Set[String]] = o2
val o2t: Option[immutable.Set[String]] = o2
assertTrue(o2t.isEmpty)

val xs4 = immutable.List[Option[(Int, String)]](Some((1 -> "a")), Some((2 -> "b")))
val xs4 = List[Option[(Int, String)]](Some((1 -> "a")), Some((2 -> "b")))
val o4 = optionSequence1(xs4)
@unused val o4t: Option[immutable.List[(Int, String)]] = o4
@unused val o5: Option[immutable.TreeMap[Int, String]] = o4.map(_.to(immutable.TreeMap))
val o4t: Option[List[(Int, String)]] = o4
assertEquals(Some(List(1 -> "a", 2 -> "b")), o4t)
val o5: Option[immutable.TreeMap[Int, String]] = o4.map(_.to(immutable.TreeMap))
assertEquals(Some(immutable.TreeMap(1 -> "a", 2 -> "b")), o5)
}

@Test
def optionSequence2Test(): Unit = {
val xs1 = immutable.List(Some(1), None, Some(2))
val xs1 = List(Some(1), None, Some(2))
val o1 = optionSequence2(xs1)
@unused val o1t: Option[immutable.List[Int]] = o1
val o1t: Option[immutable.List[Int]] = o1
assertTrue(o1t.isEmpty)

val xs2 = immutable.TreeSet(Some("foo"), Some("bar"), None)
val o2 = optionSequence2(xs2)
@unused val o2t: Option[immutable.TreeSet[String]] = o2
val o2t: Option[immutable.TreeSet[String]] = o2
assertTrue(o2t.isEmpty)

// Breakout-like use case from https://github.com/scala/scala/pull/5233:
val xs4 = immutable.List[Option[(Int, String)]](Some((1 -> "a")), Some((2 -> "b")))
val xs4 = List[Option[(Int, String)]](Some((1 -> "a")), Some((2 -> "b")))
val o4 = optionSequence2(xs4)(immutable.TreeMap) // same syntax as in `.to`
@unused val o4t: Option[immutable.TreeMap[Int, String]] = o4
val o4t: Option[immutable.TreeMap[Int, String]] = o4
assertEquals(Some(immutable.TreeMap(1 -> "a", 2 -> "b")), o4t)
}

@Test
def optionSequence3Test(): Unit = {
val xs1 = immutable.List(Some(1), None, Some(2))
val o1 = optionSequence3(xs1)
@unused val o1t: Option[immutable.List[Int]] = o1
val o1t: Option[immutable.List[Int]] = o1
assertTrue(o1t.isEmpty)

val xs2 = immutable.TreeSet(Some("foo"), Some("bar"), None)
val o2 = optionSequence3(xs2)
@unused val o2t: Option[immutable.TreeSet[String]] = o2
val o2t: Option[immutable.TreeSet[String]] = o2
assertTrue(o2t.isEmpty)

// Breakout-like use case from https://github.com/scala/scala/pull/5233:
val xs4 = immutable.List[Option[(Int, String)]](Some((1 -> "a")), Some((2 -> "b")))
val o4 = optionSequence3(xs4)(immutable.TreeMap) // same syntax as in `.to`
@unused val o4t: Option[immutable.TreeMap[Int, String]] = o4
val o4t: Option[immutable.TreeMap[Int, String]] = o4
assertEquals(Some(immutable.TreeMap(1 -> "a", 2 -> "b")), o4t)
}

@Test
def eitherSequenceTest(): Unit = {
val xs3 = mutable.ListBuffer(Right("foo"), Left(0), Right("bar"))
val e1 = eitherSequence(xs3)
@unused val e1t: Either[Int, mutable.ListBuffer[String]] = e1
val e1t: Either[Int, mutable.ListBuffer[String]] = e1
assertTrue(e1t.isLeft)
}

// From https://github.com/scala/collection-strawman/issues/44
Expand All @@ -118,34 +130,42 @@ class BuildFromTest {

@Test
def flatCollectTest(): Unit = {
val xs1 = immutable.List(1, 2, 3)
val xs2 = flatCollect(xs1) { case 2 => mutable.ArrayBuffer("foo", "bar") }
@unused val xs3: immutable.List[String] = xs2
val xs1 = List(1, 2, 3)
val xs2 = flatCollect(xs1) { case 2 => ArrayBuffer("foo", "bar") }
val xs3: List[String] = xs2
assertEquals(List("foo", "bar"), xs3)

val xs4 = immutable.TreeMap((1, "1"), (2, "2"))
val xs5 = flatCollect(xs4) { case (2, v) => immutable.List((v, v)) }
@unused val xs6: immutable.TreeMap[String, String] = xs5
val xs6: immutable.TreeMap[String, String] = xs5
assertEquals(immutable.TreeMap("2" -> "2"), xs6)

val xs7 = immutable.HashMap((1, "1"), (2, "2"))
val xs8 = flatCollect(xs7) { case (2, v) => immutable.List((v, v)) }
@unused val xs9: immutable.HashMap[String, String] = xs8
val xs9: immutable.HashMap[String, String] = xs8
assertEquals(immutable.HashMap("2" -> "2"), xs9)

val xs10 = immutable.TreeSet(1, 2, 3)
val xs11 = flatCollect(xs10) { case 2 => immutable.List("foo", "bar") }
@unused val xs12: immutable.TreeSet[String] = xs11
val xs12: immutable.TreeSet[String] = xs11
assertEquals(immutable.TreeSet("foo", "bar"), xs12)
}

@Test
def partitionMapTest(): Unit = {
val xs1 = immutable.List(1, 2, 3)
val xs1 = List(1, 2, 3)
val (xs2, xs3) = partitionMap(xs1)(x => if (x % 2 == 0) Left(x) else Right(x.toString))
@unused val xs4: immutable.List[Int] = xs2
@unused val xs5: immutable.List[String] = xs3
val xs4: List[Int] = xs2
assertEquals(List(2), xs4)
val xs5: List[String] = xs3
assertEquals(List("1", "3"), xs5)

val xs6 = immutable.TreeMap((1, "1"), (2, "2"))
val (xs7, xs8) = partitionMap(xs6) { case (k, v) => Left[(String, Int), (Int, Boolean)]((v, k)) }
@unused val xs9: immutable.TreeMap[String, Int] = xs7
@unused val xs10: immutable.TreeMap[Int, Boolean] = xs8
val xs9: immutable.TreeMap[String, Int] = xs7
assertEquals(immutable.TreeMap(("1", 1), ("2", 2)), xs9)
val xs10: immutable.TreeMap[Int, Boolean] = xs8
assertTrue(xs10.isEmpty)
}

@Test
Expand Down Expand Up @@ -190,8 +210,8 @@ class BuildFromTest {
implicitly[collection.BuildFrom[Seq[Any], (Int, HCons[ExtendsOrdered, HNil]), Seq[(Int, HCons[ExtendsOrdered, HNil])]]]

//
// In Scala 2.12, buildFromSortedSetOps is not a candidate because if it is in the companion object of
// the SortedSet heirarchy, which is not part of the implicit scope for this search.
// In Scala 2.12, buildFromSortedSetOps is not a candidate because it is in the companion object of
// the SortedSet hierarchy, which is not part of the implicit scope for this search.
// In 2.13, the implicit was moved to `object BuildFrom`, so _is_ considered
//
// The dependent implicit search:
Expand Down
45 changes: 26 additions & 19 deletions test/junit/scala/collection/MinByMaxByTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package scala.collection
import org.junit.Assert._
import org.junit.Test

import scala.annotation.unused
import scala.tools.testkit.AssertUtil.assertThrows
import scala.util.Random

Expand Down Expand Up @@ -35,52 +34,60 @@ class MinByMaxByTest {
def testReturnTheFirstMatch() = {
val d = List(1, 2, 3, 4, 5, 6, 7, 8)
def f(x: Int) = x % 3;
assert(d.maxBy(f) == 2, "If multiple elements evaluated to the largest value, maxBy should return the first one.")
assert(d.minBy(f) == 3, "If multiple elements evaluated to the largest value, minBy should return the first one.")
assertEquals("If multiple elements evaluated to the largest value, maxBy should return the first one.",
2, d.maxBy(f))
assertEquals("If multiple elements evaluated to the largest value, minBy should return the first one.",
3, d.minBy(f))
}

// Make sure it evaluates f no more than list.length times.
@Test
def testOnlyEvaluateOnce() = {
var evaluatedCountOfMaxBy = 0

@unused val max = list.maxBy(x => {
val max = list.maxBy { x =>
evaluatedCountOfMaxBy += 1
x * 10
})
assert(evaluatedCountOfMaxBy == list.length, s"maxBy: should evaluate f only ${list.length} times, but it evaluated $evaluatedCountOfMaxBy times.")
}
assertTrue(!list.exists(_ > max))
assertEquals(s"maxBy: should evaluate f only ${list.length} times, but it evaluated $evaluatedCountOfMaxBy times.",
list.length,
evaluatedCountOfMaxBy)

var evaluatedCountOfMinBy = 0

@unused val min = list.minBy(x => {
val min = list.minBy { x =>
evaluatedCountOfMinBy += 1
x * 10
})
assert(evaluatedCountOfMinBy == list.length, s"minBy: should evaluate f only ${list.length} times, but it evaluated $evaluatedCountOfMinBy times.")
}
assertTrue(!list.exists(_ < min))
assertEquals(s"minBy: should evaluate f only ${list.length} times, but it evaluated $evaluatedCountOfMinBy times.",
list.length,
evaluatedCountOfMinBy)
}

@Test
def checkEmptyOption(): Unit = {
assert(Seq.empty[Int].maxOption == None, "maxOption on a Empty Iterable is None")
assert(Seq.empty[Int].minOption == None, "minOption on a Empty Iterable is None")
assert(Seq.empty[Int].maxByOption(identity) == None, "maxByOption on a Empty Iterable is None")
assert(Seq.empty[Int].minByOption(identity) == None, "minByOption on a Empty Iterable is None")
assertTrue("maxOption on a Empty Iterable is None", Seq.empty[Int].maxOption.isEmpty)
assertTrue("minOption on a Empty Iterable is None", Seq.empty[Int].minOption.isEmpty)
assertTrue("maxByOption on a Empty Iterable is None", Seq.empty[Int].maxByOption(identity).isEmpty)
assertTrue("minByOption on a Empty Iterable is None", Seq.empty[Int].minByOption(identity).isEmpty)
}

@Test
def checkNonEmptyOption(): Unit = {
assert(Seq(1).maxOption == Some(1), "maxOption on a Non Empty Iterable has value")
assert(Seq(1).minOption == Some(1), "minOption on a Non Empty Iterable has value")
assert(Seq(1).maxByOption(identity) == Some(1), "maxByOption on a Non Empty Iterable has value")
assert(Seq(1).minByOption(identity) == Some(1), "minByOption on a Non Empty Iterable has value")
assertEquals("maxOption on a Non Empty Iterable has value", Some(1), Seq(1).maxOption)
assertEquals("minOption on a Non Empty Iterable has value", Some(1), Seq(1).minOption)
assertEquals("maxByOption on a Non Empty Iterable has value", Some(1), Seq(1).maxByOption(identity))
assertEquals("minByOption on a Non Empty Iterable has value", Some(1), Seq(1).minByOption(identity))
}

@Test
def testMinMaxCorrectness(): Unit = {
import Ordering.Double.IeeeOrdering
val seq = Seq(5.0, 3.0, Double.NaN, 4.0)
assert(seq.min.isNaN)
assert(seq.max.isNaN)
assertTrue(seq.min.isNaN)
assertTrue(seq.max.isNaN)
}

}
Loading

0 comments on commit 7feda5a

Please sign in to comment.