diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index d32082e3fd31..f809182abdcb 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -1617,8 +1617,22 @@ self => val cond = condExpr() newLinesOpt() val thenp = expr() - val elsep = if (in.token == ELSE) { in.nextToken(); expr() } - else literalUnit + val elsep = + if (in.token == ELSE) { + in.nextToken() + expr() + } + else { + // user asked to silence warnings on unibranch if; also suppresses value discard + if (settings.warnNonUnitIf.isSetByUser && !settings.warnNonUnitIf.value) { + thenp match { + case Block(_, res) => res.updateAttachment(TypedExpectingUnitAttachment) + case _ => () + } + thenp.updateAttachment(TypedExpectingUnitAttachment) + } + literalUnit + } If(cond, thenp, elsep) } parseIf diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 5c38ba7dcb60..b5d12f83683a 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -113,6 +113,10 @@ trait Warnings { ) ) withAbbreviation "-Ywarn-macros" val warnDeadCode = BooleanSetting("-Wdead-code", "Warn when dead code is identified.") withAbbreviation "-Ywarn-dead-code" + val warnNonUnitIf = BooleanSetting("-Wnonunit-if", "Warn when if statements are non-Unit expressions, enabled by -Wnonunit-statement.") + import scala.language.existentials + val warnNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.") + .enablingIfNotSetByUser(warnNonUnitIf :: Nil) val warnValueDiscard = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.") withAbbreviation "-Ywarn-value-discard" val warnNumericWiden = BooleanSetting("-Wnumeric-widen", "Warn when numerics are widened.") withAbbreviation "-Ywarn-numeric-widen" val warnOctalLiteral = BooleanSetting("-Woctal-literal", "Warn on obsolete octal syntax.") withAbbreviation "-Ywarn-octal-literal" diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index d8cd90876047..2b455e38e17f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1726,19 +1726,83 @@ abstract class RefChecks extends Transform { } // Verify classes extending AnyVal meet the requirements - private def checkAnyValSubclass(clazz: Symbol) = { + private def checkAnyValSubclass(clazz: Symbol) = if (clazz.isDerivedValueClass) { if (clazz.isTrait) reporter.error(clazz.pos, "Only classes (not traits) are allowed to extend AnyVal") else if (clazz.hasAbstractFlag) reporter.error(clazz.pos, "`abstract` modifier cannot be used with value classes") } - } private def checkUnexpandedMacro(t: Tree) = if (!t.isDef && t.hasSymbolField && t.symbol.isTermMacro) reporter.error(t.pos, "macro has not been expanded") + // if expression in statement position (of template or block) + // looks like a useful value that should not be ignored, warn and return true + // User specifies that an expression is boring by ascribing `e: Unit`. + // The subtree `e` will bear an attachment, but may be wrapped in adaptations. + private def checkInterestingResultInStatement(t: Tree): Boolean = { + def isUninterestingSymbol(sym: Symbol): Boolean = + sym != null && ( + sym.isConstructor || + sym.hasPackageFlag || + sym.isPackageObjectOrClass || + sym == BoxedUnitClass || + sym == AnyClass || + sym == AnyRefClass || + sym == AnyValClass + ) + def isUninterestingType(tpe: Type): Boolean = + tpe != null && ( + isUnitType(tpe) || + tpe.typeSymbol.isBottomClass || + tpe =:= UnitTpe || + tpe =:= BoxedUnitTpe || + isTrivialTopType(tpe) + ) + // java lacks this.type idiom to distinguish side-effecting method, so ignore result of invoking java method. + def isJavaApplication(t: Tree): Boolean = t match { + case Apply(f, _) => f.symbol.isJavaDefined && !isUniversalMember(f.symbol) + case _ => false + } + // The quirk of typechecking if is that the LUB often results in boring types. + // Parser adds suppressing attachment on `if (b) expr` when user has `-Wnonunit-if:false`. + def checkInterestingShapes(t: Tree): Boolean = + t match { + case If(_, thenpart, elsepart) => checkInterestingShapes(thenpart) || checkInterestingShapes(elsepart) // either or + //case Block(_, Apply(label, Nil)) if label.symbol != null && nme.isLoopHeaderLabel(label.symbol.name) => false + case Block(_, res) => checkInterestingShapes(res) + case Match(_, cases) => cases.exists(k => checkInterestingShapes(k.body)) + case _ => checksForInterestingResult(t) + } + // tests for various flavors of blandness in expressions. + def checksForInterestingResult(t: Tree): Boolean = ( + !t.isDef && !treeInfo.isPureDef(t) // ignore defs + && !isUninterestingSymbol(t.symbol) // ctors, package, Unit, Any + && !isUninterestingType(t.tpe) // bottom types, Unit, Any + && !treeInfo.isThisTypeResult(t) // buf += x + && !treeInfo.isSuperConstrCall(t) // just a thing + && !treeInfo.hasExplicitUnit(t) // suppressed by explicit expr: Unit + && !isJavaApplication(t) // Java methods are inherently side-effecting + ) + // begin checkInterestingResultInStatement + settings.warnNonUnitStatement.value && checkInterestingShapes(t) && { + val where = t match { + case Block(_, res) => res + case If(_, thenpart, Literal(Constant(()))) => + thenpart match { + case Block(_, res) => res + case _ => thenpart + } + case _ => t + } + def msg = s"unused value of type ${where.tpe} (add `: Unit` to discard silently)" + refchecksWarning(where.pos, msg, WarningCategory.OtherPureStatement) + true + } + } // end checkInterestingResultInStatement + override def transform(tree: Tree): Tree = { val savedLocalTyper = localTyper val savedCurrentApplication = currentApplication @@ -1778,14 +1842,12 @@ abstract class RefChecks extends Transform { case Template(parents, self, body) => localTyper = localTyper.atOwner(tree, currentOwner) - for (stat <- body) { - if (treeInfo.isPureExprForWarningPurposes(stat)) { + for (stat <- body) + if (!checkInterestingResultInStatement(stat) && treeInfo.isPureExprForWarningPurposes(stat)) { val msg = "a pure expression does nothing in statement position" val clause = if (body.lengthCompare(1) > 0) "; multiline expressions may require enclosing parentheses" else "" refchecksWarning(stat.pos, s"$msg$clause", WarningCategory.OtherPureStatement) } - } - validateBaseTypes(currentOwner) checkOverloadedRestrictions(currentOwner, currentOwner) // scala/bug#7870 default getters for constructors live in the companion module @@ -1852,7 +1914,8 @@ abstract class RefChecks extends Transform { // probably not, until we allow parameterised extractors tree - case Block(stats, expr) => + case blk @ Block(stats, expr) => + // diagnostic info val (count, result0, adapted) = expr match { case Block(expr :: Nil, Literal(Constant(()))) => (1, expr, true) @@ -1862,7 +1925,7 @@ abstract class RefChecks extends Transform { val isMultiline = stats.lengthCompare(1 - count) > 0 def checkPure(t: Tree, supple: Boolean): Unit = - if (!analyzer.explicitlyUnit(t) && treeInfo.isPureExprForWarningPurposes(t)) { + if (!treeInfo.hasExplicitUnit(t) && treeInfo.isPureExprForWarningPurposes(t)) { val msg = "a pure expression does nothing in statement position" val parens = if (isMultiline) "multiline expressions might require enclosing parentheses" else "" val discard = if (adapted) "; a value can be silently discarded when Unit is expected" else "" @@ -1871,8 +1934,8 @@ abstract class RefChecks extends Transform { else if (!parens.isEmpty) s"$msg; $parens" else msg refchecksWarning(t.pos, text, WarningCategory.OtherPureStatement) } - // check block for unintended expr placement - stats.foreach(checkPure(_, supple = false)) + // check block for unintended "expression in statement position" + stats.foreach { t => if (!checkInterestingResultInStatement(t)) checkPure(t, supple = false) } if (result0.nonEmpty) checkPure(result0, supple = true) def checkImplicitlyAdaptedBlockResult(t: Tree): Unit = diff --git a/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala b/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala index 45b701a33c3f..88f96e34c474 100644 --- a/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala +++ b/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala @@ -196,10 +196,6 @@ trait StdAttachments { * track of other adapted trees. */ case class OriginalTreeAttachment(original: Tree) - - /** Marks a Typed tree with Unit tpt. */ - case object TypedExpectingUnitAttachment - def explicitlyUnit(tree: Tree): Boolean = tree.hasAttachment[TypedExpectingUnitAttachment.type] } diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index ad493817d11a..4e9a7fb8ece5 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -750,7 +750,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper if (immediate) { action() } else { - unit.toCheck += (() => action()) + unit.toCheck += (() => action(): Unit) true } } @@ -1112,15 +1112,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper @inline def tpdPos(transformed: Tree) = typedPos(tree.pos, mode, pt)(transformed) @inline def tpd(transformed: Tree) = typed(transformed, mode, pt) - @inline def warnValueDiscard(): Unit = if (!isPastTyper && settings.warnValueDiscard.value) { - def isThisTypeResult = (tree, tree.tpe) match { - case (Apply(Select(receiver, _), _), SingleType(_, sym)) => sym == receiver.symbol - case _ => false - } - if (!isThisTypeResult && !explicitlyUnit(tree)) - context.warning(tree.pos, s"discarded non-Unit value of type ${tree.tpe}", - WarningCategory.WFlagValueDiscard) - } + @inline def warnValueDiscard(): Unit = + if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !treeInfo.hasExplicitUnit(tree)) + context.warning(tree.pos, s"discarded non-Unit value of type ${tree.tpe}", WarningCategory.WFlagValueDiscard) @inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit = if (!isPastTyper) { val isInharmonic = ( @@ -1245,7 +1239,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper case ct @ FoldableConstantType(value) if mode.inNone(TYPEmode | FUNmode) && (ct <:< pt) && canAdaptConstantTypeToLiteral => // (0) adaptConstant(value) case OverloadedType(_, _) if !mode.inFunMode => // (1) - inferExprAlternative(tree, pt) + inferExprAlternative(tree, pt): Unit adaptAfterOverloadResolution(tree, mode, pt, original) case NullaryMethodType(restpe) => // (2) if (hasUndets && settings.lintUniversalMethods && (isCastSymbol(tree.symbol) || isTypeTestSymbol(tree.symbol)) && context.undetparams.exists(_.owner == tree.symbol)) @@ -4722,7 +4716,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper def typedIf(tree: If): If = { val cond1 = checkDead(context, typedByValueExpr(tree.cond, BooleanTpe)) - // One-legged ifs don't need a lot of analysis + // Unibranch if normally has unit value else, but synthetic code may emit empty else. if (tree.elsep.isEmpty) return treeCopy.If(tree, cond1, typed(tree.thenp, UnitTpe), tree.elsep) setType UnitTpe diff --git a/src/reflect/scala/reflect/api/Internals.scala b/src/reflect/scala/reflect/api/Internals.scala index 93c4fe85118e..1478f55f897f 100644 --- a/src/reflect/scala/reflect/api/Internals.scala +++ b/src/reflect/scala/reflect/api/Internals.scala @@ -530,7 +530,7 @@ trait Internals { self: Universe => /** Set symbol's type signature to given type. * @return the symbol itself */ - def setInfo[S <: Symbol](sym: S, tpe: Type): S + def setInfo[S <: Symbol](sym: S, tpe: Type): sym.type /** Set symbol's annotations to given annotations `annots`. */ diff --git a/src/reflect/scala/reflect/internal/ReificationSupport.scala b/src/reflect/scala/reflect/internal/ReificationSupport.scala index b47a09ea13b2..4a02e4eba0c3 100644 --- a/src/reflect/scala/reflect/internal/ReificationSupport.scala +++ b/src/reflect/scala/reflect/internal/ReificationSupport.scala @@ -60,7 +60,7 @@ trait ReificationSupport { self: SymbolTable => def setAnnotations[S <: Symbol](sym: S, annots: List[AnnotationInfo]): S = sym.setAnnotations(annots) - def setInfo[S <: Symbol](sym: S, tpe: Type): S = + def setInfo[S <: Symbol](sym: S, tpe: Type): sym.type = sym.setInfo(tpe).markAllCompleted() def mkThis(sym: Symbol): Tree = self.This(sym) diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index 926e2927b6a4..6045c42adec3 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -145,4 +145,8 @@ trait StdAttachments { // Use of _root_ is in correct leading position of selection case object RootSelection extends PlainAttachment + + /** Marks a Typed tree with Unit tpt. */ + case object TypedExpectingUnitAttachment + def explicitlyUnit(tree: Tree): Boolean = tree.hasAttachment[TypedExpectingUnitAttachment.type] } diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index 3ba0f488e27a..97865f121ddb 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -351,6 +351,42 @@ abstract class TreeInfo { case _ => false } + /** Is tree an application with result `this.type`? + * Accept `b.addOne(x)` and also `xs(i) += x` + * where the op is an assignment operator. + */ + def isThisTypeResult(tree: Tree): Boolean = tree match { + case Applied(fun @ Select(receiver, op), _, argss) => + tree.tpe match { + case ThisType(sym) => + sym == receiver.symbol + case SingleType(p, sym) => + sym == receiver.symbol || argss.exists(_.exists(sym == _.symbol)) + case _ => + def checkSingle(sym: Symbol): Boolean = + (sym == receiver.symbol) || { + receiver match { + case Apply(_, _) => Precedence(op.decoded).level == 0 // xs(i) += x + case _ => receiver.symbol != null && + (receiver.symbol.isGetter || receiver.symbol.isField) // xs.addOne(x) for var xs + } + } + @tailrec def loop(mt: Type): Boolean = mt match { + case MethodType(_, restpe) => + restpe match { + case ThisType(sym) => checkSingle(sym) + case SingleType(_, sym) => checkSingle(sym) + case _ => loop(restpe) + } + case PolyType(_, restpe) => loop(restpe) + case _ => false + } + fun.symbol != null && loop(fun.symbol.info) + } + case _ => + tree.tpe.isInstanceOf[ThisType] + } + /** * Named arguments can transform a constructor call into a block, e.g. * (b = foo, a = bar) @@ -746,6 +782,16 @@ abstract class TreeInfo { ((sym ne null) && sym.initialize.isTrait) } + def hasExplicitUnit(tree: Tree): Boolean = + explicitlyUnit(tree) || { + tree match { + case Apply(f, _) => hasExplicitUnit(f) + case TypeApply(f, _) => hasExplicitUnit(f) + case AppliedTypeTree(f, _) => hasExplicitUnit(f) + case _ => false + } + } + /** Applications in Scala can have one of the following shapes: * * 1) naked core: Ident(_) or Select(_, _) or basically anything else diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala index 27901be7c301..ead2054e9092 100644 --- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala +++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala @@ -77,6 +77,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse => this.ChangeOwnerAttachment this.InterpolatedString this.RootSelection + this.TypedExpectingUnitAttachment this.noPrint this.typeDebug // inaccessible: this.posAssigner diff --git a/test/files/neg/nonunit-if.check b/test/files/neg/nonunit-if.check new file mode 100644 index 000000000000..d1ffb1337ef3 --- /dev/null +++ b/test/files/neg/nonunit-if.check @@ -0,0 +1,57 @@ +nonunit-if.scala:58: warning: discarded non-Unit value of type U + if (!isEmpty) f(a) // warn, check is on + ^ +nonunit-if.scala:62: warning: discarded non-Unit value of type Boolean + f(a) // warn, check is on + ^ +nonunit-if.scala:13: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently) + improved // warn + ^ +nonunit-if.scala:20: warning: unused value of type String (add `: Unit` to discard silently) + new E().toString // warn + ^ +nonunit-if.scala:26: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently) + Future(42) // warn + ^ +nonunit-if.scala:30: warning: unused value of type K (add `: Unit` to discard silently) + copy() // warn + ^ +nonunit-if.scala:37: warning: unused value of type List[Int] (add `: Unit` to discard silently) + 27 +: xs // warn + ^ +nonunit-if.scala:44: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses + null // warn for purity + ^ +nonunit-if.scala:58: warning: unused value of type U (add `: Unit` to discard silently) + if (!isEmpty) f(a) // warn, check is on + ^ +nonunit-if.scala:62: warning: unused value of type Boolean (add `: Unit` to discard silently) + f(a) // warn, check is on + ^ +nonunit-if.scala:73: warning: unused value of type U (add `: Unit` to discard silently) + if (!fellback) action(z) // warn, check is on + ^ +nonunit-if.scala:81: warning: unused value of type Int (add `: Unit` to discard silently) + g // warn, check is on + ^ +nonunit-if.scala:79: warning: unused value of type Int (add `: Unit` to discard silently) + g // warn block statement + ^ +nonunit-if.scala:86: warning: unused value of type Int (add `: Unit` to discard silently) + g // warn + ^ +nonunit-if.scala:84: warning: unused value of type Int (add `: Unit` to discard silently) + g // warn + ^ +nonunit-if.scala:96: warning: unused value of type Int (add `: Unit` to discard silently) + if (b) { // warn, at least one branch looks interesting + ^ +nonunit-if.scala:116: warning: unused value of type scala.collection.mutable.LinkedHashSet[A] (add `: Unit` to discard silently) + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + ^ +nonunit-if.scala:146: warning: unused value of type String (add `: Unit` to discard silently) + while (it.hasNext) it.next() // warn + ^ +error: No warnings can be incurred under -Werror. +18 warnings +1 error diff --git a/test/files/neg/nonunit-if.scala b/test/files/neg/nonunit-if.scala new file mode 100644 index 000000000000..17aee82cc15c --- /dev/null +++ b/test/files/neg/nonunit-if.scala @@ -0,0 +1,191 @@ +// scalac: -Werror -Wnonunit-statement -Wnonunit-if:true -Wvalue-discard +// debug: -Vprint:refchecks -Yprint-trees:format +import collection.ArrayOps +import collection.mutable.{ArrayBuilder, LinkedHashSet, ListBuffer} +import concurrent._ +import scala.reflect.ClassTag + +class C { + import ExecutionContext.Implicits._ + def c = { + def improved = Future(42) + def stale = Future(27) + improved // warn + stale + } +} +class D { + def d = { + class E + new E().toString // warn + new E().toString * 2 + } +} +class F { + import ExecutionContext.Implicits._ + Future(42) // warn +} +// unused template expression uses synthetic method of class +case class K(s: String) { + copy() // warn +} +// mutations returning this are ok +class Mutate { + val b = ListBuffer.empty[Int] + b += 42 // nowarn, returns this.type + val xs = List(42) + 27 +: xs // warn + + def f(x: Int): this.type = this + def g(): Unit = f(42) // nowarn +} +// some uninteresting expressions may warn for other reasons +class WhoCares { + null // warn for purity + ??? // nowarn for impurity +} +// explicit Unit ascription to opt out of warning, even for funky applies +class Absolution { + def f(i: Int): Int = i+1 + import ExecutionContext.Implicits._ + Future(42): Unit // nowarn { F(42)(ctx) }: Unit where annot is on F(42) + f(42): Unit // nowarn +} +// warn uni-branched unless user disables it with -Wnonunit-if:false +class Boxed[A](a: A) { + def isEmpty = false + def foreach[U](f: A => U): Unit = + if (!isEmpty) f(a) // warn, check is on + def forall(f: A => Boolean): Unit = + if (!isEmpty) { + println(".") + f(a) // warn, check is on + } + def take(p: A => Boolean): Option[A] = { + while (isEmpty || !p(a)) () + Some(a).filter(p) + } +} +class Unibranch[A, B] { + def runWith[U](action: B => U): A => Boolean = { x => + val z = null.asInstanceOf[B] + val fellback = false + if (!fellback) action(z) // warn, check is on + !fellback + } + def f(i: Int): Int = { + def g = 17 + if (i < 42) { + g // warn block statement + println("uh oh") + g // warn, check is on + } + while (i < 42) { + g // warn + println("uh oh") + g // warn + } + 42 + } +} +class Dibranch { + def i: Int = ??? + def j: Int = ??? + def f(b: Boolean): Int = { + // if-expr might have an uninteresting LUB + if (b) { // warn, at least one branch looks interesting + println("true") + i + } + else { + println("false") + j + } + 42 + } +} +class Next[A] { + val all = ListBuffer.empty[A] + def f(it: Iterator[A], g: A => A): Unit = + while (it.hasNext) + all += g(it.next()) // nowarn +} +class Setting[A] { + def set = LinkedHashSet.empty[A] + def f(a: A): Unit = { + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + println(set) + } +} +// neither StringBuilder warns, because either append is Java method or returns this.type +// while loop looks like if branch with block1(block2, jump to label), where block2 typed as non-unit +class Strung { + def iterator = Iterator.empty[String] + def addString(b: StringBuilder, start: String, sep: String, end: String): StringBuilder = { + val jsb = b.underlying + if (start.length != 0) jsb.append(start) + val it = iterator + if (it.hasNext) { + jsb.append(it.next()) + while (it.hasNext) { + jsb.append(sep) + jsb.append(it.next()) + } + } + if (end.length != 0) jsb.append(end) + b + } + def f(b: java.lang.StringBuilder, it: Iterator[String]): String = { + while (it.hasNext) { + b.append("\n") + b.append(it.next()) + } + b.toString + } + def g(b: java.lang.StringBuilder, it: Iterator[String]): String = { + while (it.hasNext) it.next() // warn + b.toString + } +} +class J { + import java.util.Collections + def xs: java.util.List[Int] = ??? + def f(): Int = { + Collections.checkedList[Int](xs, classOf[Int]) + 42 + } +} +class Variant { + var bs = ListBuffer.empty[Int] + val xs = ListBuffer.empty[Int] + private[this] val ys = ListBuffer.empty[Int] + private[this] var zs = ListBuffer.empty[Int] + def f(i: Int): Unit = { + bs.addOne(i) + xs.addOne(i) + ys.addOne(i) + zs.addOne(i) + println("done") + } +} +final class ArrayOops[A](private val xs: Array[A]) extends AnyVal { + def other: ArrayOps[A] = ??? + def transpose[B](implicit asArray: A => Array[B]): Array[Array[B]] = { + val aClass = xs.getClass.getComponentType + val bb = new ArrayBuilder.ofRef[Array[B]]()(ClassTag[Array[B]](aClass)) + if (xs.length == 0) bb.result() + else { + def mkRowBuilder() = ArrayBuilder.make[B](ClassTag[B](aClass.getComponentType)) + val bs = new ArrayOps(asArray(xs(0))).map((x: B) => mkRowBuilder()) + for (xs <- other) { + var i = 0 + for (x <- new ArrayOps(asArray(xs))) { + bs(i) += x + i += 1 + } + } + for (b <- new ArrayOps(bs)) bb += b.result() + bb.result() + } + } +} diff --git a/test/files/neg/nonunit-statement.check b/test/files/neg/nonunit-statement.check new file mode 100644 index 000000000000..b15ae34fb7fd --- /dev/null +++ b/test/files/neg/nonunit-statement.check @@ -0,0 +1,39 @@ +nonunit-statement.scala:13: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently) + improved // warn + ^ +nonunit-statement.scala:20: warning: unused value of type String (add `: Unit` to discard silently) + new E().toString // warn + ^ +nonunit-statement.scala:26: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently) + Future(42) // warn + ^ +nonunit-statement.scala:30: warning: unused value of type K (add `: Unit` to discard silently) + copy() // warn + ^ +nonunit-statement.scala:37: warning: unused value of type List[Int] (add `: Unit` to discard silently) + 27 +: xs // warn + ^ +nonunit-statement.scala:44: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses + null // warn for purity + ^ +nonunit-statement.scala:79: warning: unused value of type Int (add `: Unit` to discard silently) + g // warn block statement + ^ +nonunit-statement.scala:86: warning: unused value of type Int (add `: Unit` to discard silently) + g // warn + ^ +nonunit-statement.scala:84: warning: unused value of type Int (add `: Unit` to discard silently) + g // warn + ^ +nonunit-statement.scala:96: warning: unused value of type Int (add `: Unit` to discard silently) + if (b) { // warn, at least one branch looks interesting + ^ +nonunit-statement.scala:116: warning: unused value of type scala.collection.mutable.LinkedHashSet[A] (add `: Unit` to discard silently) + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + ^ +nonunit-statement.scala:146: warning: unused value of type String (add `: Unit` to discard silently) + while (it.hasNext) it.next() // warn + ^ +error: No warnings can be incurred under -Werror. +12 warnings +1 error diff --git a/test/files/neg/nonunit-statement.scala b/test/files/neg/nonunit-statement.scala new file mode 100644 index 000000000000..192091dbd034 --- /dev/null +++ b/test/files/neg/nonunit-statement.scala @@ -0,0 +1,199 @@ +// scalac: -Werror -Wnonunit-statement -Wnonunit-if:false -Wvalue-discard +// debug: -Vprint:refchecks -Yprint-trees:format +import collection.ArrayOps +import collection.mutable.{ArrayBuilder, LinkedHashSet, ListBuffer} +import concurrent._ +import scala.reflect.ClassTag + +class C { + import ExecutionContext.Implicits._ + def c = { + def improved = Future(42) + def stale = Future(27) + improved // warn + stale + } +} +class D { + def d = { + class E + new E().toString // warn + new E().toString * 2 + } +} +class F { + import ExecutionContext.Implicits._ + Future(42) // warn +} +// unused template expression uses synthetic method of class +case class K(s: String) { + copy() // warn +} +// mutations returning this are ok +class Mutate { + val b = ListBuffer.empty[Int] + b += 42 // nowarn, returns this.type + val xs = List(42) + 27 +: xs // warn + + def f(x: Int): this.type = this + def g(): Unit = f(42) // nowarn +} +// some uninteresting expressions may warn for other reasons +class WhoCares { + null // warn for purity + ??? // nowarn for impurity +} +// explicit Unit ascription to opt out of warning, even for funky applies +class Absolution { + def f(i: Int): Int = i+1 + import ExecutionContext.Implicits._ + Future(42): Unit // nowarn { F(42)(ctx) }: Unit where annot is on F(42) + f(42): Unit // nowarn +} +// warn uni-branched unless user disables it with -Wnonunit-if:false +class Boxed[A](a: A) { + def isEmpty = false + def foreach[U](f: A => U): Unit = + if (!isEmpty) f(a) // nowarn, check is off + def forall(f: A => Boolean): Unit = + if (!isEmpty) { + println(".") + f(a) // nowarn, check is off + } + def take(p: A => Boolean): Option[A] = { + while (isEmpty || !p(a)) () + Some(a).filter(p) + } +} +class Unibranch[A, B] { + def runWith[U](action: B => U): A => Boolean = { x => + val z = null.asInstanceOf[B] + val fellback = false + if (!fellback) action(z) // nowarn, check is off + !fellback + } + def f(i: Int): Int = { + def g = 17 + if (i < 42) { + g // warn block statement + println("uh oh") + g // nowarn, check is off + } + while (i < 42) { + g // warn + println("uh oh") + g // warn + } + 42 + } +} +class Dibranch { + def i: Int = ??? + def j: Int = ??? + def f(b: Boolean): Int = { + // if-expr might have an uninteresting LUB + if (b) { // warn, at least one branch looks interesting + println("true") + i + } + else { + println("false") + j + } + 42 + } +} +class Next[A] { + val all = ListBuffer.empty[A] + def f(it: Iterator[A], g: A => A): Unit = + while (it.hasNext) + all += g(it.next()) // nowarn +} +class Setting[A] { + def set = LinkedHashSet.empty[A] + def f(a: A): Unit = { + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + println(set) + } +} +// neither StringBuilder warns, because either append is Java method or returns this.type +// while loop looks like if branch with block1(block2, jump to label), where block2 typed as non-unit +class Strung { + def iterator = Iterator.empty[String] + def addString(b: StringBuilder, start: String, sep: String, end: String): StringBuilder = { + val jsb = b.underlying + if (start.length != 0) jsb.append(start) + val it = iterator + if (it.hasNext) { + jsb.append(it.next()) + while (it.hasNext) { + jsb.append(sep) + jsb.append(it.next()) + } + } + if (end.length != 0) jsb.append(end) + b + } + def f(b: java.lang.StringBuilder, it: Iterator[String]): String = { + while (it.hasNext) { + b.append("\n") + b.append(it.next()) + } + b.toString + } + def g(b: java.lang.StringBuilder, it: Iterator[String]): String = { + while (it.hasNext) it.next() // warn + b.toString + } +} +class J { + import java.util.Collections + def xs: java.util.List[Int] = ??? + def f(): Int = { + Collections.checkedList[Int](xs, classOf[Int]) + 42 + } +} +class Variant { + var bs = ListBuffer.empty[Int] + val xs = ListBuffer.empty[Int] + private[this] val ys = ListBuffer.empty[Int] + private[this] var zs = ListBuffer.empty[Int] + def f(i: Int): Unit = { + bs.addOne(i) + xs.addOne(i) + ys.addOne(i) + zs.addOne(i) + println("done") + } +} +final class ArrayOops[A](private val xs: Array[A]) extends AnyVal { + def other: ArrayOps[A] = ??? + def transpose[B](implicit asArray: A => Array[B]): Array[Array[B]] = { + val aClass = xs.getClass.getComponentType + val bb = new ArrayBuilder.ofRef[Array[B]]()(ClassTag[Array[B]](aClass)) + if (xs.length == 0) bb.result() + else { + def mkRowBuilder() = ArrayBuilder.make[B](ClassTag[B](aClass.getComponentType)) + val bs = new ArrayOps(asArray(xs(0))).map((x: B) => mkRowBuilder()) + for (xs <- other) { + var i = 0 + for (x <- new ArrayOps(asArray(xs))) { + bs(i) += x + i += 1 + } + } + for (b <- new ArrayOps(bs)) bb += b.result() + bb.result() + } + } +} +class Depends { + def f[A](a: A): a.type = a + def g() = { + val d = new Depends + f(d) + () + } +} diff --git a/test/files/neg/value-discard.check b/test/files/neg/value-discard.check index 642a6739b60a..92e63c680954 100644 --- a/test/files/neg/value-discard.check +++ b/test/files/neg/value-discard.check @@ -1,11 +1,11 @@ value-discard.scala:6: warning: discarded non-Unit value of type Boolean - mutable.Set[String]().remove("") // expected to warn + mutable.Set[String]().remove("") // warn because suspicious receiver ^ value-discard.scala:13: warning: discarded non-Unit value of type scala.collection.mutable.Set[String] - def subtract(): Unit = mutable.Set.empty[String].subtractOne("") // warn + def subtract(): Unit = mutable.Set.empty[String].subtractOne("") // warn because suspicious receiver ^ value-discard.scala:17: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses - "" // warn + "" // warn pure expr ^ error: No warnings can be incurred under -Werror. 3 warnings diff --git a/test/files/neg/value-discard.scala b/test/files/neg/value-discard.scala index 2306bc917da4..eed8a29a4d12 100644 --- a/test/files/neg/value-discard.scala +++ b/test/files/neg/value-discard.scala @@ -1,21 +1,21 @@ -// scalac: -Ywarn-value-discard -Xfatal-warnings +// scalac: -Wvalue-discard -Werror final class UnusedTest { import scala.collection.mutable def remove(): Unit = { - mutable.Set[String]().remove("") // expected to warn + mutable.Set[String]().remove("") // warn because suspicious receiver } def removeAscribed(): Unit = { - mutable.Set[String]().remove(""): Unit // no warn + mutable.Set[String]().remove(""): Unit // nowarn } - def subtract(): Unit = mutable.Set.empty[String].subtractOne("") // warn + def subtract(): Unit = mutable.Set.empty[String].subtractOne("") // warn because suspicious receiver def warnings(): Unit = { val s: mutable.Set[String] = mutable.Set.empty[String] - "" // warn - "": Unit // no warn - s.subtractOne("") // no warn + "" // warn pure expr + "": Unit // nowarn + s.subtractOne("") // nowarn } } diff --git a/test/files/neg/virtpatmat_unreach_select.scala b/test/files/neg/virtpatmat_unreach_select.scala index b3ca30e2eab0..db6caf85b1a3 100644 --- a/test/files/neg/virtpatmat_unreach_select.scala +++ b/test/files/neg/virtpatmat_unreach_select.scala @@ -1,4 +1,4 @@ -// scalac: -Xfatal-warnings +// scalac: -Werror // class Test { object severity extends Enumeration diff --git a/test/files/pos/skunky-expansion.scala b/test/files/pos/skunky-expansion.scala new file mode 100644 index 000000000000..d3f26c83a210 --- /dev/null +++ b/test/files/pos/skunky-expansion.scala @@ -0,0 +1,31 @@ +// scalac: -Werror -Wnonunit-statement +// +import scala.reflect.macros._ +import scala.reflect.api.TypeCreator + +abstract trait Encoder[A] extends scala.AnyRef; +object StringContextOps extends scala.AnyRef { + class StringOpsMacros(c: scala.reflect.macros.whitebox.Context) extends scala.AnyRef { + def sql_impl(argSeq: StringOpsMacros.this.c.universe.Tree*): AnyRef = { + val EncoderType: StringOpsMacros.this.c.universe.Type = StringOpsMacros.this.c.universe.typeOf[Encoder[_]](({ + val $u: StringOpsMacros.this.c.universe.type = StringOpsMacros.this.c.universe; + val $m: $u.Mirror = StringOpsMacros.this.c.universe.rootMirror; + $u.TypeTag.apply[Encoder[_]]($m, { + final class $typecreator1 extends TypeCreator { + def apply[U <: scala.reflect.api.Universe with Singleton]($m$untyped: scala.reflect.api.Mirror[U]): U#Type = { + val $u: U = $m$untyped.universe; + val $m: $u.Mirror = $m$untyped.asInstanceOf[$u.Mirror]; + val symdef$EncoderType1: $u.Symbol = $u.internal.reificationSupport.newNestedSymbol($u.internal.reificationSupport.selectTerm($u.internal.reificationSupport.selectType($m.staticModule("StringContextOps").asModule.moduleClass, "StringOpsMacros"), "sql_impl"), $u.TermName.apply("EncoderType"), $u.NoPosition, $u.internal.reificationSupport.FlagsRepr.apply(549755813888L), false); + val symdef$_$11: $u.Symbol = $u.internal.reificationSupport.newNestedSymbol(symdef$EncoderType1, $u.TypeName.apply("_$1"), $u.NoPosition, $u.internal.reificationSupport.FlagsRepr.apply(34359738384L), false); + $u.internal.reificationSupport.setInfo[$u.Symbol](symdef$EncoderType1, $u.NoType); + $u.internal.reificationSupport.setInfo[$u.Symbol](symdef$_$11, $u.internal.reificationSupport.TypeBounds($m.staticClass("scala.Nothing").asType.toTypeConstructor, $m.staticClass("scala.Any").asType.toTypeConstructor)); + $u.internal.reificationSupport.ExistentialType(scala.collection.immutable.List.apply[$u.Symbol](symdef$_$11), $u.internal.reificationSupport.TypeRef($u.internal.reificationSupport.thisPrefix($m.EmptyPackageClass), $m.staticClass("Encoder"), scala.collection.immutable.List.apply[$u.Type]($u.internal.reificationSupport.TypeRef($u.NoPrefix, symdef$_$11, scala.collection.immutable.Nil)))) + } + }; + new $typecreator1() + }) + }: StringOpsMacros.this.c.universe.TypeTag[Encoder[_]])); + argSeq.head + } + } +} diff --git a/test/files/pos/skunky.scala b/test/files/pos/skunky.scala new file mode 100644 index 000000000000..1c5f5730a0ab --- /dev/null +++ b/test/files/pos/skunky.scala @@ -0,0 +1,15 @@ +// scalac: -Werror -Wnonunit-statement + +import scala.reflect.macros._ + +trait Encoder[A] + +object StringContextOps { + class StringOpsMacros(val c: whitebox.Context) { + import c.universe._ + def sql_impl(argSeq: Tree*): Tree = { + val EncoderType = typeOf[Encoder[_]] + argSeq.head + } + } +}