From 37e12e19df840b1558da6979544868b5a07259e8 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 21 Feb 2022 20:53:11 -0800 Subject: [PATCH] Add -Wnonunit-statement and -Wnonunit-if Warn about any interesting value in statement position, for some definition of interest. Generalizes the current warning for "pure" expressions. By default, warns about unibranch if, which lack an else in user code. Specify -Wnonunit-if:false to turn off warning about unibranch if. (That also turns off -Wvalue-discard for those cases.) Improve explicit Unit check Accommodate field when method is this.type debug Handle Match --- .../scala/tools/nsc/ast/parser/Parsers.scala | 18 +- .../scala/tools/nsc/settings/Warnings.scala | 4 + .../tools/nsc/typechecker/RefChecks.scala | 83 +++++++- .../nsc/typechecker/StdAttachments.scala | 4 - .../scala/tools/nsc/typechecker/Typers.scala | 16 +- .../reflect/internal/StdAttachments.scala | 4 + .../scala/reflect/internal/TreeInfo.scala | 44 ++++ .../reflect/runtime/JavaUniverseForce.scala | 1 + test/files/neg/nonunit-if.check | 57 ++++++ test/files/neg/nonunit-if.scala | 191 ++++++++++++++++++ test/files/neg/nonunit-statement.check | 39 ++++ test/files/neg/nonunit-statement.scala | 191 ++++++++++++++++++ test/files/neg/value-discard.check | 6 +- test/files/neg/value-discard.scala | 14 +- .../files/neg/virtpatmat_unreach_select.scala | 2 +- 15 files changed, 637 insertions(+), 37 deletions(-) create mode 100644 test/files/neg/nonunit-if.check create mode 100644 test/files/neg/nonunit-if.scala create mode 100644 test/files/neg/nonunit-statement.check create mode 100644 test/files/neg/nonunit-statement.scala diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 37f3f7e9916f..78ad5016d3fb 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 7c7094fc1053..b1ee9f6cd8c0 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1723,19 +1723,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) // strict 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 msg = "unused value" + val where = t match { + case Block(_, res) => res + case If(_, thenpart, Literal(Constant(()))) => + thenpart match { + case Block(_, res) => res + case _ => thenpart + } + case _ => t + } + refchecksWarning(where.pos, msg, WarningCategory.OtherPureStatement) + true + } + } // end checkInterestingResultInStatement + override def transform(tree: Tree): Tree = { val savedLocalTyper = localTyper val savedCurrentApplication = currentApplication @@ -1775,14 +1839,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 @@ -1849,7 +1911,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) @@ -1859,7 +1922,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 "" @@ -1868,8 +1931,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 cd4b0b6580f8..f541113d1d09 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 } } @@ -1114,13 +1114,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, "discarded non-Unit value", WarningCategory.WFlagValueDiscard) - } + @inline def warnValueDiscard(): Unit = + if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !treeInfo.hasExplicitUnit(tree)) + context.warning(tree.pos, "discarded non-Unit value", WarningCategory.WFlagValueDiscard) @inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit = if (!isPastTyper) { val isInharmonic = ( @@ -1245,7 +1241,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)) @@ -4729,7 +4725,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/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..ebd0772ee637 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -351,6 +351,40 @@ 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), _, _) => + tree.tpe match { + case ThisType(sym) => sym == receiver.symbol + case SingleType(_, sym) => sym == receiver.symbol + case _ => + def check(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) => check(sym) + case SingleType(_, sym) => check(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 +780,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 a3a465498ac2..2f246872aaba 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..6d9e0ef63e69 --- /dev/null +++ b/test/files/neg/nonunit-if.check @@ -0,0 +1,57 @@ +nonunit-if.scala:58: warning: discarded non-Unit value + if (!isEmpty) f(a) // warn, check is on + ^ +nonunit-if.scala:62: warning: discarded non-Unit value + f(a) // warn, check is on + ^ +nonunit-if.scala:13: warning: unused value + improved // warn + ^ +nonunit-if.scala:20: warning: unused value + new E().toString // warn + ^ +nonunit-if.scala:26: warning: unused value + Future(42) // warn + ^ +nonunit-if.scala:30: warning: unused value + copy() // warn + ^ +nonunit-if.scala:37: warning: unused value + 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 + if (!isEmpty) f(a) // warn, check is on + ^ +nonunit-if.scala:62: warning: unused value + f(a) // warn, check is on + ^ +nonunit-if.scala:73: warning: unused value + if (!fellback) action(z) // warn, check is on + ^ +nonunit-if.scala:81: warning: unused value + g // warn, check is on + ^ +nonunit-if.scala:79: warning: unused value + g // warn block statement + ^ +nonunit-if.scala:86: warning: unused value + g // warn + ^ +nonunit-if.scala:84: warning: unused value + g // warn + ^ +nonunit-if.scala:96: warning: unused value + if (b) { // warn, at least one branch looks interesting + ^ +nonunit-if.scala:116: warning: unused value + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + ^ +nonunit-if.scala:146: warning: unused value + 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..f98bcd3bc607 --- /dev/null +++ b/test/files/neg/nonunit-statement.check @@ -0,0 +1,39 @@ +nonunit-statement.scala:13: warning: unused value + improved // warn + ^ +nonunit-statement.scala:20: warning: unused value + new E().toString // warn + ^ +nonunit-statement.scala:26: warning: unused value + Future(42) // warn + ^ +nonunit-statement.scala:30: warning: unused value + copy() // warn + ^ +nonunit-statement.scala:37: warning: unused value + 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 + g // warn block statement + ^ +nonunit-statement.scala:86: warning: unused value + g // warn + ^ +nonunit-statement.scala:84: warning: unused value + g // warn + ^ +nonunit-statement.scala:96: warning: unused value + if (b) { // warn, at least one branch looks interesting + ^ +nonunit-statement.scala:116: warning: unused value + set += a // warn because cannot know whether the `set` was supposed to be consumed or assigned + ^ +nonunit-statement.scala:146: warning: unused value + 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..715a7cd3ebf5 --- /dev/null +++ b/test/files/neg/nonunit-statement.scala @@ -0,0 +1,191 @@ +// 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() + } + } +} diff --git a/test/files/neg/value-discard.check b/test/files/neg/value-discard.check index 1133fbe94a84..04814148c199 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 - mutable.Set[String]().remove("") // expected to warn + mutable.Set[String]().remove("") // warn because suspicious receiver ^ value-discard.scala:13: warning: discarded non-Unit value - 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