diff options
21 files changed, 200 insertions, 42 deletions
diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala index 177a89d9ae..ad9961fc49 100644 --- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala +++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala @@ -24,7 +24,7 @@ abstract class TreeGen extends reflect.internal.TreeGen { if (tree.tpe != null || !tree.hasSymbol) tree.tpe else tree.symbol.tpe - if (!global.phase.erasedTypes && settings.Xchecknull.value && + if (!global.phase.erasedTypes && settings.warnSelectNullable.value && tpe <:< NotNullClass.tpe && !tpe.isNotNull) mkRuntimeCall(nme.checkInitialized, List(tree)) else diff --git a/src/compiler/scala/tools/nsc/reporters/AbstractReporter.scala b/src/compiler/scala/tools/nsc/reporters/AbstractReporter.scala index 49d23b1ab3..5127c2eb19 100644 --- a/src/compiler/scala/tools/nsc/reporters/AbstractReporter.scala +++ b/src/compiler/scala/tools/nsc/reporters/AbstractReporter.scala @@ -31,7 +31,7 @@ abstract class AbstractReporter extends Reporter { protected def info0(pos: Position, msg: String, _severity: Severity, force: Boolean) { val severity = - if (settings.Xwarnfatal.value && _severity == WARNING) ERROR + if (settings.fatalWarnings.value && _severity == WARNING) ERROR else _severity if (severity == INFO) { diff --git a/src/compiler/scala/tools/nsc/settings/AestheticSettings.scala b/src/compiler/scala/tools/nsc/settings/AestheticSettings.scala index 0908ea60b6..136e03d9e2 100644 --- a/src/compiler/scala/tools/nsc/settings/AestheticSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/AestheticSettings.scala @@ -22,7 +22,7 @@ trait AestheticSettings { def declsOnly = false def deprecation = settings.deprecation.value def experimental = settings.Xexperimental.value - def fatalWarnings = settings.Xwarnfatal.value + def fatalWarnings = settings.fatalWarnings.value def logClasspath = settings.Ylogcp.value def printStats = settings.Ystatistics.value def richExes = settings.YrichExes.value || sys.props.traceSourcePath.isSet diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala index 65ef421b65..da66efe1fa 100644 --- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala @@ -15,8 +15,11 @@ import scala.io.Source /** A mutable Settings object. */ -class MutableSettings(val errorFn: String => Unit) extends scala.reflect.internal.settings.MutableSettings - with AbsSettings with ScalaSettings with Mutable { +class MutableSettings(val errorFn: String => Unit) + extends scala.reflect.internal.settings.MutableSettings + with AbsSettings + with ScalaSettings + with Mutable { type ResultOfTryToSet = List[String] /** Iterates over the arguments applying them to settings where applicable. diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala index 70f6ea0b89..03e6e508f6 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala @@ -12,7 +12,9 @@ import annotation.elidable import scala.tools.util.PathResolver.Defaults import scala.collection.mutable.HashSet -trait ScalaSettings extends AbsScalaSettings with StandardScalaSettings { +trait ScalaSettings extends AbsScalaSettings + with StandardScalaSettings + with Warnings { self: MutableSettings => import Defaults.scalaUserClassPath @@ -79,9 +81,6 @@ trait ScalaSettings extends AbsScalaSettings with StandardScalaSettings { val showPhases = BooleanSetting ("-Xshow-phases", "Print a synopsis of compiler phases.") val sourceReader = StringSetting ("-Xsource-reader", "classname", "Specify a custom method for reading source files.", "") - val Xwarnfatal = BooleanSetting ("-Xfatal-warnings", "Fail the compilation if there are any warnings.") - val Xchecknull = BooleanSetting ("-Xcheck-null", "Emit warning on selection of nullable reference.") - // Experimental Extensions val Xexperimental = BooleanSetting ("-Xexperimental", "Enable experimental extensions.") . withPostSetHook(set => List(YdepMethTpes, YmethodInfer) foreach (_.value = set.value)) //YvirtClasses, @@ -163,11 +162,6 @@ trait ScalaSettings extends AbsScalaSettings with StandardScalaSettings { def stop = stopAfter /** - * Warnings - */ - val Ywarndeadcode = BooleanSetting ("-Ywarn-dead-code", "Emit warnings for dead code") - - /** * IDE-specific settings */ val YpresentationVerbose = BooleanSetting("-Ypresentation-verbose", "Print information about presentation compiler tasks.") diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala new file mode 100644 index 0000000000..6fbd9892c7 --- /dev/null +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -0,0 +1,57 @@ +/* NSC -- new Scala compiler + * Copyright 2005-2011 LAMP/EPFL + * @author Paul Phillips + */ + +package scala.tools +package nsc +package settings + +import annotation.elidable +import scala.tools.util.PathResolver.Defaults +import scala.collection.mutable.HashSet + +/** Settings influencing the printing of warnings. + */ +trait Warnings { + self: MutableSettings => + + // Warning semantics. + val fatalWarnings = BooleanSetting("-Xfatal-warnings", "Fail the compilation if there are any warnings.") + + // These warnings are all so noisy as to be useless in their + // present form, but have the potential to offer useful info. + protected def allWarnings = lintWarnings ++ List( + warnSelectNullable, + warnValueDiscard, + warnNumericWiden + ) + // These warnings should be pretty quiet unless you're doing + // something inadvisable. + protected def lintWarnings = List( + warnDeadCode, + warnInaccessible, + warnNullaryOverride, + warnNullaryUnit + ) + + // Warning groups. + val lint = ( + BooleanSetting("-Xlint", "Enable recommended additional warnings.") + withPostSetHook (_ => lintWarnings foreach (_.value = true)) + ) + val warnEverything = ( + BooleanSetting("-Ywarn-all", "Enable all -Y warnings.") + withPostSetHook (_ => lintWarnings foreach (_.value = true)) + ) + + // Individual warnings. + val warnSelectNullable = BooleanSetting ("-Xcheck-null", "Warn upon selection of nullable reference.") + val warnDeadCode = BooleanSetting ("-Ywarn-dead-code", "Warn when dead code is identified.") + val warnValueDiscard = BooleanSetting ("-Ywarn-value-discard", "Warn when non-Unit expression results are unused.") + val warnNumericWiden = BooleanSetting ("-Ywarn-numeric-widen", "Warn when numerics are widened.") + val warnNullaryUnit = BooleanSetting ("-Ywarn-nullary-unit", "Warn when nullary methods return Unit.") + val warnInaccessible = BooleanSetting ("-Ywarn-inaccessible", "Warn about inaccessible types in method signatures.") + val warnNullaryOverride = BooleanSetting ("-Ywarn-nullary-override", + "Warn when non-nullary overrides nullary, e.g. `def foo()` over `def foo`.") +} diff --git a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala index 83b6168c03..49b67bb29f 100644 --- a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala +++ b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala @@ -98,7 +98,7 @@ abstract class SymbolLoaders { private var ok = false private def setSource(sym: Symbol) { - sourcefile map (sf => sym match { + sourcefile foreach (sf => sym match { case cls: ClassSymbol => cls.sourceFile = sf case mod: ModuleSymbol => mod.moduleClass.sourceFile = sf case _ => () diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index d7aa7bc527..d4ec27a6c0 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -75,6 +75,11 @@ abstract class RefChecks extends InfoTransform { } } + def accessFlagsToString(sym: Symbol) = flagsToString( + sym getFlag (PRIVATE | PROTECTED), + if (sym.hasAccessBoundary) "" + sym.privateWithin.name else "" + ) + class RefCheckTransformer(unit: CompilationUnit) extends Transformer { var localTyper: analyzer.Typer = typer; @@ -272,11 +277,6 @@ abstract class RefChecks extends InfoTransform { } } - def accessFlagsToString(sym: Symbol) = flagsToString( - sym getFlag (PRIVATE | PROTECTED), - if (sym.hasAccessBoundary) "" + sym.privateWithin.name else "" - ) - def overrideAccessError() { val otherAccess = accessFlagsToString(other) overrideError("has weaker access privileges; it should be "+ (if (otherAccess == "") "public" else "at least "+otherAccess)) @@ -356,6 +356,11 @@ abstract class RefChecks extends InfoTransform { overrideError("must be declared lazy to override a concrete lazy value") } else { checkOverrideTypes() + if (settings.warnNullaryOverride.value) { + if (other.paramss.isEmpty && !member.paramss.isEmpty) { + unit.warning(member.pos, "non-nullary method overrides nullary method") + } + } } } @@ -502,6 +507,7 @@ abstract class RefChecks extends InfoTransform { ) } else if (underlying.isMethod) { + // If there is a concrete method whose name matches the unimplemented // abstract method, and a cursory examination of the difference reveals // something obvious to us, let's make it more obvious to them. @@ -1173,6 +1179,60 @@ abstract class RefChecks extends InfoTransform { unit.warning(pos, sym.fullLocationString + " has changed semantics:\n" + msg) } + private def lessAccessible(otherSym: Symbol, memberSym: Symbol): Boolean = ( + (otherSym != NoSymbol) + && !otherSym.isTypeParameterOrSkolem + && !otherSym.isExistentiallyBound + && (otherSym isLessAccessibleThan memberSym) + && (otherSym isLessAccessibleThan memberSym.enclClass) + ) + private def lessAccessibleSymsInType(other: Type, memberSym: Symbol): List[Symbol] = { + val extras = other match { + case TypeRef(pre, _, args) => + // checking the prefix here gives us spurious errors on e.g. a private[process] + // object which contains a type alias, which normalizes to a visible type. + args filterNot (_ eq NoPrefix) flatMap (tp => lessAccessibleSymsInType(tp, memberSym)) + case _ => + Nil + } + if (lessAccessible(other.typeSymbol, memberSym)) other.typeSymbol :: extras + else extras + } + private def warnLessAccessible(otherSym: Symbol, memberSym: Symbol) { + val comparison = accessFlagsToString(memberSym) match { + case "" => "" + case acc => " is " + acc + " but" + } + val cannot = + if (memberSym.isDeferred) "may be unable to provide a concrete implementation of" + else "may be unable to override" + + unit.warning(memberSym.pos, + "%s%s references %s %s.".format( + memberSym.fullLocationString, comparison, + accessFlagsToString(otherSym), otherSym + ) + "\nClasses which cannot access %s %s %s.".format( + otherSym.decodedName, cannot, memberSym.decodedName) + ) + } + /** Warn about situations where a method signature will include a type which + * has more restrictive access than the method itself. + */ + private def checkAccessibilityOfReferencedTypes(tree: Tree) { + val member = tree.symbol + + // types of the value parameters + member.paramss.flatten foreach { p => + val normalized = p.tpe.normalize + if ((normalized ne p.tpe) && lessAccessibleSymsInType(normalized, member).isEmpty) () + else lessAccessibleSymsInType(p.tpe, member) foreach (sym => warnLessAccessible(sym, member)) + } + // upper bounds of type parameters + member.typeParams.map(_.info.bounds.hi.widen) foreach { tp => + lessAccessibleSymsInType(tp, member) foreach (sym => warnLessAccessible(sym, member)) + } + } + /** Check that a deprecated val or def does not override a * concrete, non-deprecated method. If it does, then * deprecation is meaningless. @@ -1197,6 +1257,7 @@ abstract class RefChecks extends InfoTransform { case _ => false } + private def checkTypeRef(tp: Type, pos: Position) = tp match { case TypeRef(pre, sym, args) => checkDeprecated(sym, pos) @@ -1335,6 +1396,19 @@ abstract class RefChecks extends InfoTransform { } } + // Warning about nullary methods returning Unit. + private def checkNullaryMethodReturnType(sym: Symbol) = sym.tpe match { + case NullaryMethodType(restpe) if restpe.typeSymbol == UnitClass => + // this may be the implementation of e.g. a generic method being parameterized + // on Unit, in which case we had better let it slide. + if (sym.isGetter || sym.allOverriddenSymbols.exists(over => !(over.tpe.resultType =:= sym.tpe.resultType))) () + else unit.warning(sym.pos, + "side-effecting nullary methods are discouraged: suggest defining as `def %s()` instead".format( + sym.name.decode) + ) + case _ => () + } + override def transform(tree: Tree): Tree = { val savedLocalTyper = localTyper val savedCurrentApplication = currentApplication @@ -1355,6 +1429,12 @@ abstract class RefChecks extends InfoTransform { case ValDef(_, _, _, _) | DefDef(_, _, _, _, _, _) => checkDeprecatedOvers(tree) + if (settings.warnNullaryUnit.value) + checkNullaryMethodReturnType(sym) + if (settings.warnInaccessible.value) { + if (!sym.isConstructor && !sym.isEffectivelyFinal && !sym.isSynthetic) + checkAccessibilityOfReferencedTypes(tree) + } tree case Template(parents, self, body) => diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index 2abecfa572..45663c8bee 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -421,7 +421,7 @@ trait TypeDiagnostics { // Error suppression will squash some of these warnings unless we circumvent it. // It is presumed if you are using a -Y option you would really like to hear // the warnings you've requested. - if (settings.Ywarndeadcode.value && context.unit != null && treeOK(tree) && exprOK) { + if (settings.warnDeadCode.value && context.unit != null && treeOK(tree) && exprOK) { val saved = context.reportGeneralErrors try { context.reportGeneralErrors = true diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index a470e80449..ba965290fe 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -892,10 +892,16 @@ trait Typers extends Modes { case TypeRef(_, sym, _) => // note: was if (pt.typeSymbol == UnitClass) but this leads to a potentially // infinite expansion if pt is constant type () - if (sym == UnitClass && tree.tpe <:< AnyClass.tpe) // (12) + if (sym == UnitClass && tree.tpe <:< AnyClass.tpe) { // (12) + if (settings.warnValueDiscard.value) + context.unit.warning(tree.pos, "discarded non-Unit value") return typed(atPos(tree.pos)(Block(List(tree), Literal(()))), mode, pt) - else if (isNumericValueClass(sym) && isNumericSubType(tree.tpe, pt)) + } + else if (isNumericValueClass(sym) && isNumericSubType(tree.tpe, pt)) { + if (settings.warnNumericWiden.value) + context.unit.warning(tree.pos, "implicit numeric widening") return typed(atPos(tree.pos)(Select(tree, "to"+sym.name)), mode, pt) + } case AnnotatedType(_, _, _) if canAdaptAnnotations(tree, mode, pt) => // (13) return typed(adaptAnnotations(tree, mode, pt), mode, pt) case _ => @@ -3616,7 +3622,7 @@ trait Typers extends Modes { !(List(Any_isInstanceOf, Any_asInstanceOf) contains result.symbol) // null.is/as is not a dereference } // unit is null here sometimes; how are we to know when unit might be null? (See bug #2467.) - if (settings.Xchecknull.value && isPotentialNullDeference && unit != null) + if (settings.warnSelectNullable.value && isPotentialNullDeference && unit != null) unit.warning(tree.pos, "potential null pointer dereference: "+tree) val selection = result match { diff --git a/src/library/scala/io/BufferedSource.scala b/src/library/scala/io/BufferedSource.scala index fafb57da55..60d67a0b58 100644 --- a/src/library/scala/io/BufferedSource.scala +++ b/src/library/scala/io/BufferedSource.scala @@ -31,8 +31,7 @@ class BufferedSource(inputStream: InputStream, bufferSize: Int)(implicit val cod val bufReader = BufferedSource.this.bufferedReader() var nextLine = bufReader.readLine - override def hasNext() = nextLine != null - + override def hasNext = nextLine != null override def next(): String = { val result = nextLine nextLine = bufReader.readLine diff --git a/src/library/scala/xml/pull/XMLEventReader.scala b/src/library/scala/xml/pull/XMLEventReader.scala index 51a2fda6aa..8b7137eed1 100755 --- a/src/library/scala/xml/pull/XMLEventReader.scala +++ b/src/library/scala/xml/pull/XMLEventReader.scala @@ -132,7 +132,7 @@ trait ProducerConsumerIterator[T >: Null] extends Iterator[T] { // consumer/iterator interface - we need not synchronize access to buffer // because we required there to be only one consumer. - def hasNext() = !eos && (buffer != null || fillBuffer) + def hasNext = !eos && (buffer != null || fillBuffer) def next() = { if (eos) throw new NoSuchElementException("ProducerConsumerIterator") if (buffer == null) fillBuffer diff --git a/src/swing/scala/swing/RichWindow.scala b/src/swing/scala/swing/RichWindow.scala index 5cb6dfdf33..721172f333 100644 --- a/src/swing/scala/swing/RichWindow.scala +++ b/src/swing/scala/swing/RichWindow.scala @@ -36,13 +36,13 @@ sealed trait RichWindow extends Window { def peer: AWTWindow with InterfaceMixin trait InterfaceMixin extends super.InterfaceMixin { - def getJMenuBar: JMenuBar + def getJMenuBar(): JMenuBar def setJMenuBar(b: JMenuBar) def setUndecorated(b: Boolean) def setTitle(s: String) - def getTitle: String + def getTitle(): String def setResizable(b: Boolean) - def isResizable: Boolean + def isResizable(): Boolean } def title: String = peer.getTitle diff --git a/test/files/neg/abstract-inaccessible.check b/test/files/neg/abstract-inaccessible.check new file mode 100644 index 0000000000..42b98ac026 --- /dev/null +++ b/test/files/neg/abstract-inaccessible.check @@ -0,0 +1,13 @@ +abstract-inaccessible.scala:5: error: method implementMe in trait YourTrait references private[foo] trait Bippy. +Classes which cannot access Bippy may be unable to provide a concrete implementation of implementMe. + def implementMe(f: Int => (String, Bippy)): Unit + ^ +abstract-inaccessible.scala:6: error: method overrideMe in trait YourTrait references private[foo] trait Bippy. +Classes which cannot access Bippy may be unable to override overrideMe. + def overrideMe[T <: Bippy](x: T): T = x + ^ +abstract-inaccessible.scala:7: error: method overrideMeAlso in trait YourTrait references private[foo] trait Bippy. +Classes which cannot access Bippy may be unable to override overrideMeAlso. + def overrideMeAlso(x: Map[Int, Set[Bippy]]) = 5 + ^ +three errors found diff --git a/test/files/neg/abstract-inaccessible.flags b/test/files/neg/abstract-inaccessible.flags new file mode 100644 index 0000000000..6c1dd108ae --- /dev/null +++ b/test/files/neg/abstract-inaccessible.flags @@ -0,0 +1 @@ +-Xfatal-warnings -Xlint
\ No newline at end of file diff --git a/test/files/neg/abstract-inaccessible.scala b/test/files/neg/abstract-inaccessible.scala new file mode 100644 index 0000000000..3c80f30522 --- /dev/null +++ b/test/files/neg/abstract-inaccessible.scala @@ -0,0 +1,9 @@ +package foo { + private[foo] trait Bippy { } + + trait YourTrait { + def implementMe(f: Int => (String, Bippy)): Unit + def overrideMe[T <: Bippy](x: T): T = x + def overrideMeAlso(x: Map[Int, Set[Bippy]]) = 5 + } +} diff --git a/test/files/neg/bug4533.check b/test/files/neg/bug4533.check deleted file mode 100644 index 0f50b5adb1..0000000000 --- a/test/files/neg/bug4533.check +++ /dev/null @@ -1,4 +0,0 @@ -bug4533.scala:6: error: wrong number of type arguments for scala.collection.GenSetLike, should be 2 - def statusByAlarms(alarms: GenSetLike[FooAlarm]) = println("hello") - ^ -one error found diff --git a/test/files/neg/bug4533.scala b/test/files/neg/bug4533.scala deleted file mode 100644 index 425c958328..0000000000 --- a/test/files/neg/bug4533.scala +++ /dev/null @@ -1,8 +0,0 @@ -package demo - -import scala.collection._ - -class CrashDemo { - def statusByAlarms(alarms: GenSetLike[FooAlarm]) = println("hello") -} -class FooAlarm { } diff --git a/test/files/neg/nullary-override.check b/test/files/neg/nullary-override.check new file mode 100644 index 0000000000..6b2ded2d4a --- /dev/null +++ b/test/files/neg/nullary-override.check @@ -0,0 +1,4 @@ +nullary-override.scala:2: error: non-nullary method overrides nullary method +class B extends A { override def x(): Int = 4 } + ^ +one error found diff --git a/test/files/neg/nullary-override.flags b/test/files/neg/nullary-override.flags new file mode 100644 index 0000000000..6c1dd108ae --- /dev/null +++ b/test/files/neg/nullary-override.flags @@ -0,0 +1 @@ +-Xfatal-warnings -Xlint
\ No newline at end of file diff --git a/test/files/neg/nullary-override.scala b/test/files/neg/nullary-override.scala new file mode 100644 index 0000000000..3eb4784a0c --- /dev/null +++ b/test/files/neg/nullary-override.scala @@ -0,0 +1,3 @@ +class A { def x: Int = 3 } +class B extends A { override def x(): Int = 4 } + |