From 8201ebdb9f7ad0ba70d918cef25423ff8da39a68 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 28 Jun 2024 17:26:23 -0700 Subject: [PATCH 01/30] Respect prefix when checking if selector selects Prefer context functions for brevity. Avoid intermediate collections. --- .../dotty/tools/dotc/reporting/messages.scala | 15 +- .../tools/dotc/transform/CheckUnused.scala | 482 ++++++++---------- .../dotty/tools/vulpix/ParallelTesting.scala | 1 + tests/warn/i15503i.scala | 8 +- tests/warn/i19657.scala | 54 ++ tests/{pos => warn}/i20860.scala | 2 + 6 files changed, 289 insertions(+), 273 deletions(-) create mode 100644 tests/warn/i19657.scala rename tests/{pos => warn}/i20860.scala (78%) diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 3b7fba1cb52d..9819b8576b8d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -3295,14 +3295,13 @@ extends Message(UnusedSymbolID) { override def explain(using Context) = "" } -object UnusedSymbol { - def imports(using Context): UnusedSymbol = new UnusedSymbol(i"unused import") - def localDefs(using Context): UnusedSymbol = new UnusedSymbol(i"unused local definition") - def explicitParams(using Context): UnusedSymbol = new UnusedSymbol(i"unused explicit parameter") - def implicitParams(using Context): UnusedSymbol = new UnusedSymbol(i"unused implicit parameter") - def privateMembers(using Context): UnusedSymbol = new UnusedSymbol(i"unused private member") - def patVars(using Context): UnusedSymbol = new UnusedSymbol(i"unused pattern variable") -} +object UnusedSymbol: + def imports(using Context): UnusedSymbol = UnusedSymbol(i"unused import") + def localDefs(using Context): UnusedSymbol = UnusedSymbol(i"unused local definition") + def explicitParams(using Context): UnusedSymbol = UnusedSymbol(i"unused explicit parameter") + def implicitParams(using Context): UnusedSymbol = UnusedSymbol(i"unused implicit parameter") + def privateMembers(using Context): UnusedSymbol = UnusedSymbol(i"unused private member") + def patVars(using Context): UnusedSymbol = UnusedSymbol(i"unused pattern variable") class NonNamedArgumentInJavaAnnotation(using Context) extends SyntaxMsg(NonNamedArgumentInJavaAnnotationID): diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index d647d50560d3..470ff2a7fcd2 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -4,52 +4,47 @@ import scala.annotation.tailrec import dotty.tools.uncheckedNN import dotty.tools.dotc.ast.tpd -import dotty.tools.dotc.core.Symbols.* import dotty.tools.dotc.ast.tpd.{Inlined, TreeTraverser} -import dotty.tools.dotc.ast.untpd -import dotty.tools.dotc.ast.untpd.ImportSelector +import dotty.tools.dotc.ast.untpd, untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts.* -import dotty.tools.dotc.core.Decorators.{em, i} +import dotty.tools.dotc.core.Decorators.{em, i, toMessage} import dotty.tools.dotc.core.Denotations.SingleDenotation import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.Phases.Phase -import dotty.tools.dotc.core.StdNames -import dotty.tools.dotc.report -import dotty.tools.dotc.reporting.Message -import dotty.tools.dotc.reporting.UnusedSymbol as UnusedSymbolMessage -import dotty.tools.dotc.typer.ImportInfo -import dotty.tools.dotc.util.{Property, SrcPos} -import dotty.tools.dotc.core.Mode -import dotty.tools.dotc.core.Types.{AnnotatedType, ConstantType, NoType, TermRef, Type, TypeTraverser} -import dotty.tools.dotc.core.Flags.flagsString +import dotty.tools.dotc.core.StdNames.nme +import dotty.tools.dotc.core.Symbols.* +import dotty.tools.dotc.core.Types.{AnnotatedType, ClassInfo, ConstantType, NamedType, NoType, TermRef, Type, TypeProxy, TypeTraverser} import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Names.Name import dotty.tools.dotc.core.NameOps.isReplWrapperName -import dotty.tools.dotc.transform.MegaPhase.MiniPhase import dotty.tools.dotc.core.Annotations import dotty.tools.dotc.core.Definitions import dotty.tools.dotc.core.NameKinds.WildcardParamName import dotty.tools.dotc.core.Symbols.Symbol -import dotty.tools.dotc.core.StdNames.nme +import dotty.tools.dotc.report +import dotty.tools.dotc.reporting.{Message, UnusedSymbol as UnusedSymbolMessage} +import dotty.tools.dotc.transform.MegaPhase.MiniPhase +import dotty.tools.dotc.typer.ImportInfo +import dotty.tools.dotc.util.{Property, SrcPos} import dotty.tools.dotc.util.Spans.Span -import scala.math.Ordering +import scala.util.chaining.given - -/** - * A compiler phase that checks for unused imports or definitions +/** A compiler phase that checks for unused imports or definitions. * - * Basically, it gathers definition/imports and their usage. If a - * definition/imports does not have any usage, then it is reported. + * Every construct that introduces a name must have at least one corresponding reference. + * The analysis is restricted to definitions of limited scope, i.e., private and local definitions. */ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _key: Property.Key[CheckUnused.UnusedData]) extends MiniPhase: import CheckUnused.* import UnusedData.* - private inline def unusedDataApply[U](inline f: UnusedData => U)(using Context): Context = + private inline def ud(using ud: UnusedData): UnusedData = ud + + private inline def preparing[U](inline op: UnusedData ?=> U)(using ctx: Context): ctx.type = ctx.property(_key) match - case Some(ud) => f(ud) - case None => () + case Some(ud) => op(using ud) + case None => ctx override def phaseName: String = CheckUnused.phaseNamePrefix + suffix @@ -68,18 +63,15 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke tree.getAttachment(_key).foreach(oldData => data.unusedAggregate = oldData.unusedAggregate ) - val fresh = ctx.fresh.setProperty(_key, data) - tree.putAttachment(_key, data) - fresh + ctx.fresh.setProperty(_key, data).tap(_ => tree.putAttachment(_key, data)) // ========== END + REPORTING ========== override def transformUnit(tree: tpd.Tree)(using Context): tpd.Tree = - unusedDataApply { ud => + preparing: ud.finishAggregation() - if(phaseMode == PhaseMode.Report) then + if phaseMode == PhaseMode.Report then ud.unusedAggregate.foreach(reportUnused) - } tree // ========== MiniPhase Prepare ========== @@ -93,26 +85,23 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke ctx override def prepareForIdent(tree: tpd.Ident)(using Context): Context = - if tree.symbol.exists then - unusedDataApply { ud => - @tailrec + preparing: + if tree.symbol.exists then def loopOnNormalizedPrefixes(prefix: Type, depth: Int): Unit = // limit to 10 as failsafe for the odd case where there is an infinite cycle if depth < 10 && prefix.exists then - ud.registerUsed(prefix.classSymbol, None) + ud.registerUsed(prefix.classSymbol, name = None, prefix) loopOnNormalizedPrefixes(prefix.normalizedPrefix, depth + 1) - - loopOnNormalizedPrefixes(tree.typeOpt.normalizedPrefix, depth = 0) - ud.registerUsed(tree.symbol, Some(tree.name)) - } - else if tree.hasType then - unusedDataApply(_.registerUsed(tree.tpe.classSymbol, Some(tree.name))) - else - ctx + val prefix = tree.typeOpt.normalizedPrefix + loopOnNormalizedPrefixes(prefix, depth = 0) + ud.registerUsed(tree.symbol, Some(tree.name), tree.typeOpt.importPrefix) + else if tree.hasType then + ud.registerUsed(tree.tpe.classSymbol, Some(tree.name), tree.tpe.importPrefix) override def prepareForSelect(tree: tpd.Select)(using Context): Context = - val name = tree.removeAttachment(OriginalName) - unusedDataApply(_.registerUsed(tree.symbol, name, includeForImport = tree.qualifier.span.isSynthetic)) + preparing: + val name = tree.removeAttachment(OriginalName) + ud.registerUsed(tree.symbol, name, tree.qualifier.tpe, includeForImport = tree.qualifier.span.isSynthetic) override def prepareForBlock(tree: tpd.Block)(using Context): Context = pushInBlockTemplatePackageDef(tree) @@ -124,48 +113,45 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke pushInBlockTemplatePackageDef(tree) override def prepareForValDef(tree: tpd.ValDef)(using Context): Context = - unusedDataApply{ud => - // do not register the ValDef generated for `object` + preparing: traverseAnnotations(tree.symbol) + // do not register the ValDef generated for `object` if !tree.symbol.is(Module) then ud.registerDef(tree) - if tree.name.startsWith("derived$") && tree.typeOpt != NoType then - ud.registerUsed(tree.typeOpt.typeSymbol, None, isDerived = true) + if tree.name.startsWith("derived$") && tree.hasType then + ud.registerUsed(tree.tpe.typeSymbol, name = None, tree.tpe.importPrefix, isDerived = true) ud.addIgnoredUsage(tree.symbol) - } override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context = - unusedDataApply: ud => + preparing: if !tree.symbol.is(Private) then - tree.termParamss.flatten.foreach { p => - ud.addIgnoredParam(p.symbol) - } + tree.termParamss.flatten.foreach(p => ud.addIgnoredParam(p.symbol)) ud.registerTrivial(tree) traverseAnnotations(tree.symbol) ud.registerDef(tree) ud.addIgnoredUsage(tree.symbol) override def prepareForTypeDef(tree: tpd.TypeDef)(using Context): Context = - unusedDataApply: ud => + preparing: traverseAnnotations(tree.symbol) if !tree.symbol.is(Param) then // Ignore type parameter (as Scala 2) ud.registerDef(tree) ud.addIgnoredUsage(tree.symbol) override def prepareForBind(tree: tpd.Bind)(using Context): Context = - traverseAnnotations(tree.symbol) - unusedDataApply(_.registerPatVar(tree)) + preparing: + traverseAnnotations(tree.symbol) + ud.registerPatVar(tree) override def prepareForTypeTree(tree: tpd.TypeTree)(using Context): Context = - if !tree.isInstanceOf[tpd.InferredTypeTree] then typeTraverser(unusedDataApply).traverse(tree.tpe) + if !tree.isInstanceOf[tpd.InferredTypeTree] then typeTraverser.traverse(tree.tpe) ctx override def prepareForAssign(tree: tpd.Assign)(using Context): Context = - unusedDataApply{ ud => + preparing: val sym = tree.lhs.symbol if sym.exists then ud.registerSetVar(sym) - } // ========== MiniPhase Transform ========== @@ -182,31 +168,30 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke tree override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = - unusedDataApply(_.removeIgnoredUsage(tree.symbol)) + preparing: + ud.removeIgnoredUsage(tree.symbol) tree override def transformDefDef(tree: tpd.DefDef)(using Context): tpd.Tree = - unusedDataApply(_.removeIgnoredUsage(tree.symbol)) + preparing: + ud.removeIgnoredUsage(tree.symbol) tree override def transformTypeDef(tree: tpd.TypeDef)(using Context): tpd.Tree = - unusedDataApply(_.removeIgnoredUsage(tree.symbol)) + preparing: + ud.removeIgnoredUsage(tree.symbol) tree // ---------- MiniPhase HELPERS ----------- private def pushInBlockTemplatePackageDef(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = - unusedDataApply { ud => + preparing: ud.pushScope(UnusedData.ScopeType.fromTree(tree)) - } - ctx private def popOutBlockTemplatePackageDef()(using Context): Context = - unusedDataApply { ud => + preparing: ud.popScope() - } - ctx /** * This traverse is the **main** component of this phase @@ -218,12 +203,13 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke import tpd.* import UnusedData.ScopeType - /* Register every imports, definition and usage */ + // Register every import, definition and usage override def traverse(tree: tpd.Tree)(using Context): Unit = val newCtx = if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx tree match case imp: tpd.Import => - unusedDataApply(_.registerImport(imp)) + preparing: + ud.registerImport(imp) imp.selectors.filter(_.isGiven).map(_.bound).collect { case untpd.TypedSplice(tree1) => tree1 }.foreach(traverse(_)(using newCtx)) @@ -254,17 +240,17 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke case t: tpd.Bind => prepareForBind(t) traverseChildren(tree)(using newCtx) - case t:tpd.Assign => + case t: tpd.Assign => prepareForAssign(t) traverseChildren(tree) case _: tpd.InferredTypeTree => - case t@tpd.RefinedTypeTree(tpt, refinements) => + case tpd.RefinedTypeTree(tpt, refinements) => //! DIFFERS FROM MINIPHASE - typeTraverser(unusedDataApply).traverse(t.tpe) + typeTraverser.traverse(tree.tpe) traverse(tpt)(using newCtx) - case t@tpd.TypeTree() => + case tpd.TypeTree() => //! DIFFERS FROM MINIPHASE - typeTraverser(unusedDataApply).traverse(t.tpe) + typeTraverser.traverse(tree.tpe) traverseChildren(tree)(using newCtx) case _ => //! DIFFERS FROM MINIPHASE @@ -273,12 +259,15 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke end traverser /** This is a type traverser which catch some special Types not traversed by the term traverser above */ - private def typeTraverser(dt: (UnusedData => Any) => Unit)(using Context) = new TypeTraverser: + private def typeTraverser(using Context) = new TypeTraverser: override def traverse(tp: Type): Unit = - if tp.typeSymbol.exists then dt(_.registerUsed(tp.typeSymbol, Some(tp.typeSymbol.name))) + if tp.typeSymbol.exists then + preparing: + ud.registerUsed(tp.typeSymbol, Some(tp.typeSymbol.name), tp.importPrefix) tp match case AnnotatedType(_, annot) => - dt(_.registerUsed(annot.symbol, None)) + preparing: + ud.registerUsed(annot.symbol, name = None, annot.symbol.info.importPrefix) traverseChildren(tp) case _ => traverseChildren(tp) @@ -287,28 +276,22 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke private def traverseAnnotations(sym: Symbol)(using Context): Unit = sym.denot.annotations.foreach(annot => traverser.traverse(annot.tree)) - /** Do the actual reporting given the result of the anaylsis */ private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit = - res.warnings.toList.sortBy(_.pos.span.point)(using Ordering[Int]).foreach { s => - s match - case UnusedSymbol(t, _, WarnTypes.Imports) => - report.warning(UnusedSymbolMessage.imports, t) - case UnusedSymbol(t, _, WarnTypes.LocalDefs) => - report.warning(UnusedSymbolMessage.localDefs, t) - case UnusedSymbol(t, _, WarnTypes.ExplicitParams) => - report.warning(UnusedSymbolMessage.explicitParams, t) - case UnusedSymbol(t, _, WarnTypes.ImplicitParams) => - report.warning(UnusedSymbolMessage.implicitParams, t) - case UnusedSymbol(t, _, WarnTypes.PrivateMembers) => - report.warning(UnusedSymbolMessage.privateMembers, t) - case UnusedSymbol(t, _, WarnTypes.PatVars) => - report.warning(UnusedSymbolMessage.patVars, t) - case UnusedSymbol(t, _, WarnTypes.UnsetLocals) => - report.warning("unset local variable, consider using an immutable val instead", t) - case UnusedSymbol(t, _, WarnTypes.UnsetPrivates) => - report.warning("unset private variable, consider using an immutable val instead", t) - } + def messageFor(w: WarnTypes): Message = + import WarnTypes.*, UnusedSymbolMessage.* + w match + case Imports => imports + case LocalDefs => localDefs + case ExplicitParams => explicitParams + case ImplicitParams => implicitParams + case PrivateMembers => privateMembers + case PatVars => patVars + case UnsetLocals => "unset local variable, consider using an immutable val instead".toMessage + case UnsetPrivates => "unset private variable, consider using an immutable val instead".toMessage + res.warnings.toArray.sortInPlaceBy(_.pos.span.point).foreach: + case UnusedSymbol(pos, _, warnType) => + report.warning(messageFor(warnType), pos) end CheckUnused @@ -342,65 +325,58 @@ object CheckUnused: class PostInlining extends CheckUnused(PhaseMode.Report, "PostInlining", _key) - /** - * A stateful class gathering the infos on : - * - imports - * - definitions - * - usage + /** Track usages at a Context. + * + * For an ImportContext, which selectors have been used for lookups? + * + * For other contexts, which symbols defined here have been referenced? */ private class UnusedData: - import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack, ListBuffer => MutList} + import collection.mutable as mut, mut.Stack, mut.ListBuffer import UnusedData.* /** The current scope during the tree traversal */ - val currScopeType: MutStack[ScopeType] = MutStack(ScopeType.Other) + val currScopeType: Stack[ScopeType] = Stack(ScopeType.Other) var unusedAggregate: Option[UnusedResult] = None /* IMPORTS */ - private val impInScope = MutStack(MutList[ImportSelectorData]()) - /** - * We store the symbol along with their accessibility without import. - * Accessibility to their definition in outer context/scope - * - * See the `isAccessibleAsIdent` extension method below in the file - */ - private val usedInScope = MutStack(MutSet[(Symbol, Option[Name], Boolean)]()) - private val usedInPosition = MutMap.empty[Name, MutSet[Symbol]] + private val impInScope = Stack(ListBuffer.empty[ImportSelectorData]) + private val usedInScope = Stack(mut.Set.empty[Usage]) + private val usedInPosition = mut.Map.empty[Name, mut.Set[Symbol]] /* unused import collected during traversal */ - private val unusedImport = MutList.empty[ImportSelectorData] + private val unusedImport = ListBuffer.empty[ImportSelectorData] /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */ - private val localDefInScope = MutList.empty[tpd.MemberDef] - private val privateDefInScope = MutList.empty[tpd.MemberDef] - private val explicitParamInScope = MutList.empty[tpd.MemberDef] - private val implicitParamInScope = MutList.empty[tpd.MemberDef] - private val patVarsInScope = MutList.empty[tpd.Bind] + private val localDefInScope = ListBuffer.empty[tpd.MemberDef] + private val privateDefInScope = ListBuffer.empty[tpd.MemberDef] + private val explicitParamInScope = ListBuffer.empty[tpd.MemberDef] + private val implicitParamInScope = ListBuffer.empty[tpd.MemberDef] + private val patVarsInScope = ListBuffer.empty[tpd.Bind] /** All variables sets*/ - private val setVars = MutSet[Symbol]() + private val setVars = mut.Set.empty[Symbol] /** All used symbols */ - private val usedDef = MutSet[Symbol]() + private val usedDef = mut.Set.empty[Symbol] /** Do not register as used */ - private val doNotRegister = MutSet[Symbol]() + private val doNotRegister = mut.Set.empty[Symbol] /** Trivial definitions, avoid registering params */ - private val trivialDefs = MutSet[Symbol]() - - private val paramsToSkip = MutSet[Symbol]() + private val trivialDefs = mut.Set.empty[Symbol] + private val paramsToSkip = mut.Set.empty[Symbol] def finishAggregation(using Context)(): Unit = - val unusedInThisStage = this.getUnused - this.unusedAggregate match { + unusedAggregate = unusedAggregate match case None => - this.unusedAggregate = Some(unusedInThisStage) + Some(getUnused) case Some(prevUnused) => - val intersection = unusedInThisStage.warnings.intersect(prevUnused.warnings) - this.unusedAggregate = Some(UnusedResult(intersection)) - } + Some(UnusedResult(getUnused.warnings.intersect(prevUnused.warnings))) + def registerSelectors(selectors: List[ImportSelectorData]): this.type = + impInScope.top.prependAll(selectors) + this /** * Register a found (used) symbol along with its name @@ -408,10 +384,11 @@ object CheckUnused: * The optional name will be used to target the right import * as the same element can be imported with different renaming */ - def registerUsed(sym: Symbol, name: Option[Name], includeForImport: Boolean = true, isDerived: Boolean = false)(using Context): Unit = + def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true, isDerived: Boolean = false)(using Context): Unit = if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) then if sym.isConstructor then - registerUsed(sym.owner, None, includeForImport) // constructor are "implicitly" imported with the class + // constructors are "implicitly" imported with the class + registerUsed(sym.owner, name = None, prefix, includeForImport = includeForImport) else // If the symbol is accessible in this scope without an import, do not register it for unused import analysis val includeForImport1 = @@ -422,13 +399,13 @@ object CheckUnused: if sym.exists then usedDef += sym if includeForImport1 then - usedInScope.top += ((sym, name, isDerived)) + usedInScope.top += Usage(sym, name, prefix, isDerived) addIfExists(sym) addIfExists(sym.companionModule) addIfExists(sym.companionClass) if sym.sourcePos.exists then for n <- name do - usedInPosition.getOrElseUpdate(n, MutSet.empty) += sym + usedInPosition.getOrElseUpdate(n, mut.Set.empty) += sym /** Register a symbol that should be ignored */ def addIgnoredUsage(sym: Symbol)(using Context): Unit = @@ -452,18 +429,18 @@ object CheckUnused: val qualTpe = imp.expr.tpe // Put wildcard imports at the end, because they have lower priority within one Import - val reorderdSelectors = + val reorderedSelectors = val (wildcardSels, nonWildcardSels) = imp.selectors.partition(_.isWildcard) nonWildcardSels ::: wildcardSels val newDataInScope = - for sel <- reorderdSelectors yield + for sel <- reorderedSelectors yield val data = new ImportSelectorData(qualTpe, sel) - if shouldSelectorBeReported(imp, sel) || isImportExclusion(sel) || isImportIgnored(imp, sel) then + if shouldSelectorBeReported(imp, sel) || sel.isImportExclusion || isImportIgnored(imp, sel) then // Immediately mark the selector as used data.markUsed() data - impInScope.top.prependAll(newDataInScope) + registerSelectors(newDataInScope) end registerImport /** Register (or not) some `val` or `def` according to the context, scope and flags */ @@ -489,8 +466,8 @@ object CheckUnused: def pushScope(newScopeType: ScopeType): Unit = // unused imports : currScopeType.push(newScopeType) - impInScope.push(MutList()) - usedInScope.push(MutSet()) + impInScope.push(ListBuffer.empty) + usedInScope.push(mut.Set.empty) def registerSetVar(sym: Symbol): Unit = setVars += sym @@ -506,30 +483,23 @@ object CheckUnused: val selDatas = impInScope.pop() for usedInfo <- usedInfos do - val (sym, optName, isDerived) = usedInfo - val usedData = selDatas.find { selData => - sym.isInImport(selData, optName, isDerived) - } - usedData match - case Some(data) => - data.markUsed() + val Usage(sym, optName, prefix, isDerived) = usedInfo + selDatas.find(sym.isInImport(_, optName, prefix, isDerived)) match + case Some(sel) => + sel.markUsed() case None => // Propagate the symbol one level up if usedInScope.nonEmpty then usedInScope.top += usedInfo - end for // each in `used` + end for // each in usedInfos for selData <- selDatas do if !selData.isUsed then unusedImport += selData end popScope - /** - * Leave the scope and return a `List` of unused `ImportSelector`s - * - * The given `List` is sorted by line and then column of the position + /** Leave the scope and return a result set of warnings. */ - def getUnused(using Context): UnusedResult = popScope() @@ -538,69 +508,47 @@ object CheckUnused: case Some(syms) => syms.exists(sym => span.contains(sym.span)) case None => false - val sortedImp = - if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then - unusedImport.toList - .map(d => UnusedSymbol(d.selector.srcPos, d.selector.name, WarnTypes.Imports)) - else - Nil + val warnings = Set.newBuilder[UnusedSymbol] + + if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then + warnings.addAll(unusedImport.iterator.map(d => UnusedSymbol(d.selector.srcPos, d.selector.name, WarnTypes.Imports))) + // Partition to extract unset local variables from usedLocalDefs - val (usedLocalDefs, unusedLocalDefs) = - if ctx.settings.WunusedHas.locals then - localDefInScope.toList.partition(d => d.symbol.usedDefContains) - else - (Nil, Nil) - val sortedLocalDefs = - unusedLocalDefs - .filterNot(d => isUsedInPosition(d.symbol.name, d.span)) - .filterNot(d => containsSyntheticSuffix(d.symbol)) - .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs)) - val unsetLocalDefs = usedLocalDefs.filter(isUnsetVarDef).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetLocals)).toList - - val sortedExplicitParams = - if ctx.settings.WunusedHas.explicits then - explicitParamInScope.toList - .filterNot(d => d.symbol.usedDefContains) - .filterNot(d => isUsedInPosition(d.symbol.name, d.span)) - .filterNot(d => containsSyntheticSuffix(d.symbol)) - .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ExplicitParams)) - else - Nil - val sortedImplicitParams = - if ctx.settings.WunusedHas.implicits then - implicitParamInScope.toList - .filterNot(d => d.symbol.usedDefContains) - .filterNot(d => containsSyntheticSuffix(d.symbol)) - .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.ImplicitParams)) - else - Nil + if ctx.settings.WunusedHas.locals then + for d <- localDefInScope do + if d.symbol.usedDefContains then + if isUnsetVarDef(d) then + warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetLocals)) + else + if !isUsedInPosition(d.symbol.name, d.span) && !containsSyntheticSuffix(d.symbol) then + warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs)) + + if ctx.settings.WunusedHas.explicits then + for d <- explicitParamInScope do + if !d.symbol.usedDefContains && !isUsedInPosition(d.symbol.name, d.span) && !containsSyntheticSuffix(d.symbol) then + warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.ExplicitParams)) + + if ctx.settings.WunusedHas.implicits then + for d <- implicitParamInScope do + if !d.symbol.usedDefContains && !containsSyntheticSuffix(d.symbol) then + warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.ImplicitParams)) + // Partition to extract unset private variables from usedPrivates - val (usedPrivates, unusedPrivates) = - if ctx.settings.WunusedHas.privates then - privateDefInScope.toList.partition(d => d.symbol.usedDefContains) - else - (Nil, Nil) - val sortedPrivateDefs = unusedPrivates.filterNot(d => containsSyntheticSuffix(d.symbol)).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PrivateMembers)) - val unsetPrivateDefs = usedPrivates.filter(isUnsetVarDef).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetPrivates)) - val sortedPatVars = - if ctx.settings.WunusedHas.patvars then - patVarsInScope.toList - .filterNot(d => d.symbol.usedDefContains) - .filterNot(d => containsSyntheticSuffix(d.symbol)) - .filterNot(d => isUsedInPosition(d.symbol.name, d.span)) - .map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PatVars)) - else - Nil - val warnings = - sortedImp ::: - sortedLocalDefs ::: - sortedExplicitParams ::: - sortedImplicitParams ::: - sortedPrivateDefs ::: - sortedPatVars ::: - unsetLocalDefs ::: - unsetPrivateDefs - UnusedResult(warnings.toSet) + if ctx.settings.WunusedHas.privates then + for d <- privateDefInScope do + if d.symbol.usedDefContains then + if isUnsetVarDef(d) then + warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetPrivates)) + else + if !containsSyntheticSuffix(d.symbol) then + warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.PrivateMembers)) + + if ctx.settings.WunusedHas.patvars then + for d <- patVarsInScope do + if !d.symbol.usedDefContains && !isUsedInPosition(d.symbol.name, d.span) && !containsSyntheticSuffix(d.symbol) then + warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.PatVars)) + + UnusedResult(warnings.result) end getUnused //============================ HELPERS ==================================== @@ -612,7 +560,7 @@ object CheckUnused: imp.selectors.exists { sel => val qual = imp.expr val importedMembers = qual.tpe.member(sel.name).alternatives.map(_.symbol) - importedMembers.exists(s => s.is(Transparent) && s.is(Inline)) + importedMembers.exists(_.isAllOf(Transparent | Inline)) } /** @@ -653,13 +601,6 @@ object CheckUnused: private def isSyntheticMainParam(sym: Symbol)(using Context): Boolean = sym.exists && ctx.platform.isMainMethod(sym.owner) && sym.owner.is(Synthetic) - /** - * This is used to ignore exclusion imports (i.e. import `qual`.{`member` => _}) - */ - private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match - case untpd.Ident(name) => name == StdNames.nme.WILDCARD - case _ => false - /** * If -Wunused:strict-no-implicit-warn import and this import selector could potentially import implicit. * return true @@ -685,24 +626,23 @@ object CheckUnused: private def isDefIgnored(memDef: tpd.MemberDef)(using Context): Boolean = memDef.symbol.isOneOf(GivenOrImplicit) && memDef.symbol.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)) - extension (tree: ImportSelector) - def boundTpe: Type = tree.bound match { - case untpd.TypedSplice(tree1) => tree1.tpe + extension (sel: ImportSelector) + def boundTpe: Type = sel.bound match + case untpd.TypedSplice(tree) => tree.tpe case _ => NoType - } + /** This is used to ignore exclusion imports of the form import `qual`.{`member` => _} + * because `sel.isUnimport` is too broad for old style `import concurrent._`. + */ + def isImportExclusion: Boolean = sel.renamed match + case untpd.Ident(nme.WILDCARD) => true + case _ => false extension (sym: Symbol) - /** is accessible without import in current context */ - private def isAccessibleAsIdent(using Context): Boolean = - ctx.outersIterator.exists{ c => - c.owner == sym.owner - || sym.owner.isClass && c.owner.isClass - && c.owner.thisType.baseClasses.contains(sym.owner) - && c.owner.thisType.member(sym.name).alternatives.contains(sym) - } - /** Given an import and accessibility, return selector that matches import<->symbol */ - private def isInImport(selData: ImportSelectorData, altName: Option[Name], isDerived: Boolean)(using Context): Boolean = + /** Given an import selector, is the symbol imported from the given prefix, optionally with a specific name? + * If isDerived, then it may be an aliased type in source but we only witness it dealiased. + */ + private def isInImport(selData: ImportSelectorData, altName: Option[Name], prefix: Type, isDerived: Boolean)(using Context): Boolean = assert(sym.exists) val selector = selData.selector @@ -712,11 +652,13 @@ object CheckUnused: // if there is an explicit name, it must match false else - if isDerived then - // See i15503i.scala, grep for "package foo.test.i17156" - selData.allSymbolsDealiasedForNamed.contains(dealias(sym)) - else - selData.allSymbolsForNamed.contains(sym) + (isDerived || prefix.typeSymbol.isPackageObject || selData.qualTpe =:= prefix) && ( + if isDerived then + // See i15503i.scala, grep for "package foo.test.i17156" + selData.allSymbolsDealiasedForNamed.contains(sym.dealiasAsType) + else + selData.allSymbolsForNamed.contains(sym) + ) else // Wildcard if !selData.qualTpe.member(sym.name).hasAltWith(_.symbol == sym) then @@ -727,6 +669,7 @@ object CheckUnused: // Further check that the symbol is a given or implicit and conforms to the bound sym.isOneOf(Given | Implicit) && (selector.bound.isEmpty || sym.info.finalResultType <:< selector.boundTpe) + && selData.qualTpe =:= prefix else // Normal wildcard, check that the symbol is not a given (but can be implicit) !sym.is(Given) @@ -735,10 +678,10 @@ object CheckUnused: /** Annotated with @unused */ private def isUnusedAnnot(using Context): Boolean = - sym.annotations.exists(a => a.symbol == ctx.definitions.UnusedAnnot) + sym.annotations.exists(_.symbol == ctx.definitions.UnusedAnnot) private def shouldNotReportParamOwner(using Context): Boolean = - if sym.exists then + sym.exists && { val owner = sym.owner trivialDefs(owner) || // is a trivial def owner.isPrimaryConstructor || @@ -747,9 +690,8 @@ object CheckUnused: ) || owner.isAllOf(Synthetic | PrivateLocal) || owner.is(Accessor) || - owner.isOverriden - else - false + owner.isOverridden + } private def usedDefContains(using Context): Boolean = sym.everySymbol.exists(usedDef.apply) @@ -757,8 +699,8 @@ object CheckUnused: private def everySymbol(using Context): List[Symbol] = List(sym, sym.companionClass, sym.companionModule, sym.moduleClass).filter(_.exists) - /** A function is overriden. Either has `override flags` or parent has a matching member (type and name) */ - private def isOverriden(using Context): Boolean = + /** A function is overridden. Either has `override flags` or parent has a matching member (type and name) */ + private def isOverridden(using Context): Boolean = sym.is(Flags.Override) || (sym.exists && sym.owner.thisType.parents.exists(p => sym.matchingMember(p).exists)) end extension @@ -812,7 +754,7 @@ object CheckUnused: extension (thisName: Name) private def isWildcard: Boolean = - thisName == StdNames.nme.WILDCARD || thisName.is(WildcardParamName) + thisName == nme.WILDCARD || thisName.is(WildcardParamName) end UnusedData @@ -827,7 +769,7 @@ object CheckUnused: /** return the scope corresponding to the enclosing scope of the given tree */ def fromTree(tree: tpd.Tree)(using Context): ScopeType = tree match case tree: tpd.Template => if tree.symbol.name.isReplWrapperName then ReplWrapper else Template - case _:tpd.Block => Local + case _: tpd.Block => Local case _ => Other final class ImportSelectorData(val qualTpe: Type, val selector: ImportSelector): @@ -849,21 +791,39 @@ object CheckUnused: def allSymbolsDealiasedForNamed(using Context): Set[Symbol] = if myAllSymbolsDealiased == null then - myAllSymbolsDealiased = allSymbolsForNamed.map(sym => dealias(sym)) + myAllSymbolsDealiased = allSymbolsForNamed.map(_.dealiasAsType) myAllSymbolsDealiased.uncheckedNN end ImportSelectorData case class UnusedSymbol(pos: SrcPos, name: Name, warnType: WarnTypes) /** A container for the results of the used elements analysis */ - case class UnusedResult(warnings: Set[UnusedSymbol]) + class UnusedResult(val warnings: Set[UnusedSymbol]) object UnusedResult: val Empty = UnusedResult(Set.empty) - end UnusedData - - private def dealias(symbol: Symbol)(using Context): Symbol = - if symbol.isType && symbol.asType.denot.isAliasType then - symbol.asType.typeRef.dealias.typeSymbol - else - symbol + /** A symbol usage includes the name under which it was observed, + * the prefix from which it was selected, and whether it is in a derived element. + */ + case class Usage(symbol: Symbol, name: Option[Name], prefix: Type, isDerived: Boolean) + end UnusedData + extension (sym: Symbol) + /** is accessible without import in current context */ + def isAccessibleAsIdent(using Context): Boolean = + ctx.outersIterator.exists: c => + c.owner == sym.owner + || sym.owner.isClass && c.owner.isClass + && c.owner.thisType.baseClasses.contains(sym.owner) + && c.owner.thisType.member(sym.name).alternatives.contains(sym) + + def dealiasAsType(using Context): Symbol = + if sym.isType && sym.asType.denot.isAliasType then + sym.asType.typeRef.dealias.typeSymbol + else + sym + extension (tp: Type) + def importPrefix(using Context): Type = tp match + case tp: NamedType => tp.prefix + case tp: ClassInfo => tp.prefix + case tp: TypeProxy => tp.superType.normalizedPrefix + case _ => tp end CheckUnused diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index e7e5936a4b29..5e08f38dd51c 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -860,6 +860,7 @@ trait ParallelTesting extends RunnerOrchestration { self => (map.asScala.keys.toList, (unexpected ++ unpositioned).toList) end getMissingExpectedWarnings + end WarnTest private final class RewriteTest(testSources: List[TestSource], checkFiles: Map[JFile, JFile], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting) extends Test(testSources, times, threadLimit, suppressAllOutput) { diff --git a/tests/warn/i15503i.scala b/tests/warn/i15503i.scala index b7981e0e4206..db2c8158dac2 100644 --- a/tests/warn/i15503i.scala +++ b/tests/warn/i15503i.scala @@ -1,4 +1,4 @@ -//> using options -Wunused:all +//> using options -Wunused:all -Ystop-after:checkUnusedPostInlining import collection.mutable.{Map => MutMap} // warn import collection.mutable.Set // warn @@ -247,7 +247,7 @@ package foo.test.i16679a: import scala.deriving.Mirror object CaseClassByStringName: inline final def derived[A](using inline A: Mirror.Of[A]): CaseClassByStringName[A] = - new CaseClassByStringName[A]: // warn + new CaseClassByStringName[A]: def name: String = A.toString object secondPackage: @@ -263,7 +263,7 @@ package foo.test.i16679b: object CaseClassName: import scala.deriving.Mirror inline final def derived[A](using inline A: Mirror.Of[A]): CaseClassName[A] = - new CaseClassName[A]: // warn + new CaseClassName[A]: def name: String = A.toString object Foo: @@ -279,7 +279,7 @@ package foo.test.i17156: package a: trait Foo[A] object Foo: - inline def derived[T]: Foo[T] = new Foo{} // warn + inline def derived[T]: Foo[T] = new Foo {} package b: import a.Foo diff --git a/tests/warn/i19657.scala b/tests/warn/i19657.scala new file mode 100644 index 000000000000..19ab2626f352 --- /dev/null +++ b/tests/warn/i19657.scala @@ -0,0 +1,54 @@ +//> using options -Wunused:imports -Ystop-after:checkUnusedPostInlining + +trait Schema[A] + +case class Foo() +case class Bar() + +trait SchemaGenerator[A] { + given Schema[A] = new Schema[A]{} +} + +object FooCodec extends SchemaGenerator[Foo] +object BarCodec extends SchemaGenerator[Bar] + +def summonSchemas(using Schema[Foo], Schema[Bar]) = () + +def summonSchema(using Schema[Foo]) = () + +def `i19657 check prefix to pick selector`: Unit = + import FooCodec.given + import BarCodec.given + summonSchemas + +def `i19657 regression test`: Unit = + import FooCodec.given + import BarCodec.given // warn + summonSchema + +def `i19657 check prefix to pick specific selector`: Unit = + import FooCodec.given_Schema_A + import BarCodec.given_Schema_A + summonSchemas + +def `same symbol different names`: Unit = + import FooCodec.given_Schema_A + import FooCodec.given_Schema_A as AThing + summonSchema(using given_Schema_A) + summonSchema(using AThing) + +package i17156: + package a: + trait Foo[A] + object Foo: + class Food[A] extends Foo[A] + inline def derived[T]: Foo[T] = Food() + + package b: + import a.Foo + type Xd[A] = Foo[A] + + package c: + import b.Xd + trait Z derives Xd // checks if dealiased import is prefix a.Foo + class Bar extends Xd[Int] // checks if import qual b is prefix of b.Xd diff --git a/tests/pos/i20860.scala b/tests/warn/i20860.scala similarity index 78% rename from tests/pos/i20860.scala rename to tests/warn/i20860.scala index 1e1ddea11b75..df08b23c3a61 100644 --- a/tests/pos/i20860.scala +++ b/tests/warn/i20860.scala @@ -1,3 +1,5 @@ +//> using options -Wunused:imports + def `i20860 use result to check selector bound`: Unit = import Ordering.Implicits.given Ordering[?] summon[Ordering[Seq[Int]]] From 2d88b9835b85e17bb6c7ebb64f71b7fbc23553e9 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Oct 2024 11:09:20 -0700 Subject: [PATCH 02/30] Check scope type --- .../tools/dotc/transform/CheckUnused.scala | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 470ff2a7fcd2..af592896737d 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -156,15 +156,15 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke // ========== MiniPhase Transform ========== override def transformBlock(tree: tpd.Block)(using Context): tpd.Tree = - popOutBlockTemplatePackageDef() + popOutBlockTemplatePackageDef(tree) tree override def transformTemplate(tree: tpd.Template)(using Context): tpd.Tree = - popOutBlockTemplatePackageDef() + popOutBlockTemplatePackageDef(tree) tree override def transformPackageDef(tree: tpd.PackageDef)(using Context): tpd.Tree = - popOutBlockTemplatePackageDef() + popOutBlockTemplatePackageDef(tree) tree override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = @@ -189,9 +189,9 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke preparing: ud.pushScope(UnusedData.ScopeType.fromTree(tree)) - private def popOutBlockTemplatePackageDef()(using Context): Context = + private def popOutBlockTemplatePackageDef(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = preparing: - ud.popScope() + ud.popScope(UnusedData.ScopeType.fromTree(tree)) /** * This traverse is the **main** component of this phase @@ -200,8 +200,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke * corresponding context property */ private def traverser = new TreeTraverser: - import tpd.* - import UnusedData.ScopeType // Register every import, definition and usage override def traverse(tree: tpd.Tree)(using Context): Unit = @@ -214,17 +212,17 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke case untpd.TypedSplice(tree1) => tree1 }.foreach(traverse(_)(using newCtx)) traverseChildren(tree)(using newCtx) - case ident: Ident => + case ident: tpd.Ident => prepareForIdent(ident) traverseChildren(tree)(using newCtx) - case sel: Select => + case sel: tpd.Select => prepareForSelect(sel) traverseChildren(tree)(using newCtx) case tree: (tpd.Block | tpd.Template | tpd.PackageDef) => //! DIFFERS FROM MINIPHASE pushInBlockTemplatePackageDef(tree) traverseChildren(tree)(using newCtx) - popOutBlockTemplatePackageDef() + popOutBlockTemplatePackageDef(tree) case t: tpd.ValDef => prepareForValDef(t) traverseChildren(tree)(using newCtx) @@ -477,8 +475,8 @@ object CheckUnused: * * - If there are imports in this scope check for unused ones */ - def popScope()(using Context): Unit = - currScopeType.pop() + def popScope(scopeType: ScopeType)(using Context): Unit = + assert(currScopeType.pop() == scopeType) val usedInfos = usedInScope.pop() val selDatas = impInScope.pop() @@ -501,7 +499,7 @@ object CheckUnused: /** Leave the scope and return a result set of warnings. */ def getUnused(using Context): UnusedResult = - popScope() + popScope(ScopeType.Other) // sentinel def isUsedInPosition(name: Name, span: Span): Boolean = usedInPosition.get(name) match From 968c436c72b569571442b9a14e62f34408c9e1c2 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Oct 2024 11:33:14 -0700 Subject: [PATCH 03/30] Use type test of prefix for usages in scope --- .../tools/dotc/transform/CheckUnused.scala | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index af592896737d..ab34d35acba1 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -340,7 +340,7 @@ object CheckUnused: /* IMPORTS */ private val impInScope = Stack(ListBuffer.empty[ImportSelectorData]) - private val usedInScope = Stack(mut.Set.empty[Usage]) + private val usedInScope = Stack(mut.Map.empty[Symbol, ListBuffer[Usage]]) private val usedInPosition = mut.Map.empty[Name, mut.Set[Symbol]] /* unused import collected during traversal */ private val unusedImport = ListBuffer.empty[ImportSelectorData] @@ -397,7 +397,7 @@ object CheckUnused: if sym.exists then usedDef += sym if includeForImport1 then - usedInScope.top += Usage(sym, name, prefix, isDerived) + addUsage(Usage(sym, name, prefix, isDerived)) addIfExists(sym) addIfExists(sym.companionModule) addIfExists(sym.companionClass) @@ -405,6 +405,11 @@ object CheckUnused: for n <- name do usedInPosition.getOrElseUpdate(n, mut.Set.empty) += sym + def addUsage(usage: Usage)(using Context): Unit = + val usages = usedInScope.top.getOrElseUpdate(usage.symbol, ListBuffer.empty) + if !usages.exists(x => x.name == usage.name && x.isDerived == usage.isDerived && x.prefix =:= usage.prefix) + then usages += usage + /** Register a symbol that should be ignored */ def addIgnoredUsage(sym: Symbol)(using Context): Unit = doNotRegister ++= sym.everySymbol @@ -465,7 +470,7 @@ object CheckUnused: // unused imports : currScopeType.push(newScopeType) impInScope.push(ListBuffer.empty) - usedInScope.push(mut.Set.empty) + usedInScope.push(mut.Map.empty) def registerSetVar(sym: Symbol): Unit = setVars += sym @@ -477,18 +482,17 @@ object CheckUnused: */ def popScope(scopeType: ScopeType)(using Context): Unit = assert(currScopeType.pop() == scopeType) - val usedInfos = usedInScope.pop() val selDatas = impInScope.pop() - for usedInfo <- usedInfos do - val Usage(sym, optName, prefix, isDerived) = usedInfo - selDatas.find(sym.isInImport(_, optName, prefix, isDerived)) match + for usedInfos <- usedInScope.pop().valuesIterator; usedInfo <- usedInfos do + import usedInfo.* + selDatas.find(symbol.isInImport(_, name, prefix, isDerived)) match case Some(sel) => sel.markUsed() case None => // Propagate the symbol one level up if usedInScope.nonEmpty then - usedInScope.top += usedInfo + addUsage(usedInfo) end for // each in usedInfos for selData <- selDatas do @@ -802,7 +806,7 @@ object CheckUnused: /** A symbol usage includes the name under which it was observed, * the prefix from which it was selected, and whether it is in a derived element. */ - case class Usage(symbol: Symbol, name: Option[Name], prefix: Type, isDerived: Boolean) + class Usage(val symbol: Symbol, val name: Option[Name], val prefix: Type, val isDerived: Boolean) end UnusedData extension (sym: Symbol) /** is accessible without import in current context */ From 2d3454dae05844246fc87f89c4569a1ae8e82702 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Oct 2024 11:54:04 -0700 Subject: [PATCH 04/30] Style tweaks in CheckUnused --- .../tools/dotc/transform/CheckUnused.scala | 53 +++++++++---------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index ab34d35acba1..b96e43033d8d 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -21,7 +21,7 @@ import dotty.tools.dotc.core.NameOps.isReplWrapperName import dotty.tools.dotc.core.Annotations import dotty.tools.dotc.core.Definitions import dotty.tools.dotc.core.NameKinds.WildcardParamName -import dotty.tools.dotc.core.Symbols.Symbol +import dotty.tools.dotc.core.Symbols.{Symbol, isDeprecated} import dotty.tools.dotc.report import dotty.tools.dotc.reporting.{Message, UnusedSymbol as UnusedSymbolMessage} import dotty.tools.dotc.transform.MegaPhase.MiniPhase @@ -76,7 +76,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke // ========== MiniPhase Prepare ========== override def prepareForOther(tree: tpd.Tree)(using Context): Context = - // A standard tree traverser covers cases not handled by the Mega/MiniPhase traverser.traverse(tree) ctx @@ -104,13 +103,13 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke ud.registerUsed(tree.symbol, name, tree.qualifier.tpe, includeForImport = tree.qualifier.span.isSynthetic) override def prepareForBlock(tree: tpd.Block)(using Context): Context = - pushInBlockTemplatePackageDef(tree) + pushScope(tree) override def prepareForTemplate(tree: tpd.Template)(using Context): Context = - pushInBlockTemplatePackageDef(tree) + pushScope(tree) override def prepareForPackageDef(tree: tpd.PackageDef)(using Context): Context = - pushInBlockTemplatePackageDef(tree) + pushScope(tree) override def prepareForValDef(tree: tpd.ValDef)(using Context): Context = preparing: @@ -156,15 +155,15 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke // ========== MiniPhase Transform ========== override def transformBlock(tree: tpd.Block)(using Context): tpd.Tree = - popOutBlockTemplatePackageDef(tree) + popScope(tree) tree override def transformTemplate(tree: tpd.Template)(using Context): tpd.Tree = - popOutBlockTemplatePackageDef(tree) + popScope(tree) tree override def transformPackageDef(tree: tpd.PackageDef)(using Context): tpd.Tree = - popOutBlockTemplatePackageDef(tree) + popScope(tree) tree override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = @@ -185,11 +184,11 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke // ---------- MiniPhase HELPERS ----------- - private def pushInBlockTemplatePackageDef(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = + private def pushScope(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = preparing: ud.pushScope(UnusedData.ScopeType.fromTree(tree)) - private def popOutBlockTemplatePackageDef(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = + private def popScope(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = preparing: ud.popScope(UnusedData.ScopeType.fromTree(tree)) @@ -198,6 +197,8 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke * * It traverse the tree the tree and gather the data in the * corresponding context property + * + * A standard tree traverser covers cases not handled by the Mega/MiniPhase */ private def traverser = new TreeTraverser: @@ -220,9 +221,9 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke traverseChildren(tree)(using newCtx) case tree: (tpd.Block | tpd.Template | tpd.PackageDef) => //! DIFFERS FROM MINIPHASE - pushInBlockTemplatePackageDef(tree) + pushScope(tree) traverseChildren(tree)(using newCtx) - popOutBlockTemplatePackageDef(tree) + popScope(tree) case t: tpd.ValDef => prepareForValDef(t) traverseChildren(tree)(using newCtx) @@ -335,6 +336,7 @@ object CheckUnused: /** The current scope during the tree traversal */ val currScopeType: Stack[ScopeType] = Stack(ScopeType.Other) + inline def peekScopeType = currScopeType.top var unusedAggregate: Option[UnusedResult] = None @@ -427,7 +429,7 @@ object CheckUnused: !tpd.languageImport(imp.expr).nonEmpty && !imp.isGeneratedByEnum && !isTransparentAndInline(imp) - && currScopeType.top != ScopeType.ReplWrapper // #18383 Do not report top-level import's in the repl as unused + && peekScopeType != ScopeType.ReplWrapper // #18383 Do not report top-level import's in the repl as unused then val qualTpe = imp.expr.tpe @@ -455,7 +457,7 @@ object CheckUnused: implicitParamInScope += memDef else if !paramsToSkip.contains(memDef.symbol) then explicitParamInScope += memDef - else if currScopeType.top == ScopeType.Local then + else if peekScopeType == ScopeType.Local then localDefInScope += memDef else if memDef.shouldReportPrivateDef then privateDefInScope += memDef @@ -653,14 +655,11 @@ object CheckUnused: if altName.exists(explicitName => selector.rename != explicitName.toTermName) then // if there is an explicit name, it must match false - else - (isDerived || prefix.typeSymbol.isPackageObject || selData.qualTpe =:= prefix) && ( - if isDerived then - // See i15503i.scala, grep for "package foo.test.i17156" - selData.allSymbolsDealiasedForNamed.contains(sym.dealiasAsType) - else - selData.allSymbolsForNamed.contains(sym) - ) + else if isDerived then + // See i15503i.scala, grep for "package foo.test.i17156" + selData.allSymbolsDealiasedForNamed.contains(sym.dealiasAsType) + else (prefix.typeSymbol.isPackageObject || selData.qualTpe =:= prefix) && + selData.allSymbolsForNamed.contains(sym) else // Wildcard if !selData.qualTpe.member(sym.name).hasAltWith(_.symbol == sym) then @@ -687,9 +686,7 @@ object CheckUnused: val owner = sym.owner trivialDefs(owner) || // is a trivial def owner.isPrimaryConstructor || - owner.annotations.exists ( // @depreacated - _.symbol == ctx.definitions.DeprecatedAnnot - ) || + owner.isDeprecated || owner.isAllOf(Synthetic | PrivateLocal) || owner.is(Accessor) || owner.isOverridden @@ -743,7 +740,7 @@ object CheckUnused: !sym.shouldNotReportParamOwner private def shouldReportPrivateDef(using Context): Boolean = - currScopeType.top == ScopeType.Template && !memDef.symbol.isConstructor && memDef.symbol.is(Private, butNot = SelfName | Synthetic | CaseAccessor) + peekScopeType == ScopeType.Template && !memDef.symbol.isConstructor && memDef.symbol.is(Private, butNot = SelfName | Synthetic | CaseAccessor) private def isUnsetVarDef(using Context): Boolean = val sym = memDef.symbol @@ -770,7 +767,7 @@ object CheckUnused: object ScopeType: /** return the scope corresponding to the enclosing scope of the given tree */ def fromTree(tree: tpd.Tree)(using Context): ScopeType = tree match - case tree: tpd.Template => if tree.symbol.name.isReplWrapperName then ReplWrapper else Template + case _: tpd.Template => if tree.symbol.name.isReplWrapperName then ReplWrapper else Template case _: tpd.Block => Local case _ => Other @@ -827,5 +824,5 @@ object CheckUnused: case tp: NamedType => tp.prefix case tp: ClassInfo => tp.prefix case tp: TypeProxy => tp.superType.normalizedPrefix - case _ => tp + case _ => NoType end CheckUnused From eca54d2139ff191b9518089a9b2a3347ec38aa75 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Oct 2024 18:05:02 -0700 Subject: [PATCH 05/30] Show more for misplaced warn comment --- compiler/test/dotty/tools/vulpix/ParallelTesting.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index 5e08f38dd51c..e3f5daaa8f65 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -812,7 +812,12 @@ trait ParallelTesting extends RunnerOrchestration { self => |${showLines("Unexpected warnings:", unexpected)} |$showDiagnostics |""".stripMargin.trim.linesIterator.mkString("\n", "\n", "") - else if hasMissingAnnotations then s"\nWarnings found on incorrect row numbers when compiling $testSource\n$showDiagnostics" + else if hasMissingAnnotations then + s"""|Warnings found on incorrect row numbers when compiling $testSource + |${showLines("Unfulfilled expectations:", expected)} + |${showLines("Unexpected warnings:", unexpected)} + |$showDiagnostics + |""".stripMargin.trim.linesIterator.mkString("\n", "\n", "") else if !map.isEmpty then s"\nExpected warnings(s) have {=}: $map" else null end maybeFailureMessage From e745c24e21c8f69fd4a2bb55b857500c8b3c03cf Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 2 Oct 2024 20:47:38 -0700 Subject: [PATCH 06/30] Attachment tracks derivation --- .../tools/dotc/transform/CheckUnused.scala | 106 ++++++++---------- .../src/dotty/tools/dotc/typer/Deriving.scala | 21 ++-- 2 files changed, 60 insertions(+), 67 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index b96e43033d8d..8a2f8fe3fc16 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -3,8 +3,7 @@ package dotty.tools.dotc.transform import scala.annotation.tailrec import dotty.tools.uncheckedNN -import dotty.tools.dotc.ast.tpd -import dotty.tools.dotc.ast.tpd.{Inlined, TreeTraverser} +import dotty.tools.dotc.ast.tpd, tpd.{Ident, Inlined, Select, Tree, TreeTraverser} import dotty.tools.dotc.ast.untpd, untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts.* @@ -112,14 +111,27 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke pushScope(tree) override def prepareForValDef(tree: tpd.ValDef)(using Context): Context = + preparing: + ud.addIgnoredUsage(tree.symbol) + + override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = preparing: traverseAnnotations(tree.symbol) - // do not register the ValDef generated for `object` - if !tree.symbol.is(Module) then + if !tree.symbol.is(Module) then // do not register the ValDef generated for `object` ud.registerDef(tree) if tree.name.startsWith("derived$") && tree.hasType then - ud.registerUsed(tree.tpe.typeSymbol, name = None, tree.tpe.importPrefix, isDerived = true) - ud.addIgnoredUsage(tree.symbol) + def core(t: Tree): (Symbol, Option[Name], Type) = t match + case Ident(name) => (t.tpe.typeSymbol, Some(name), t.tpe.underlyingPrefix) + case Select(t, _) => core(t) + case _ => (NoSymbol, None, NoType) + tree.getAttachment(OriginalTypeClass) match + case Some(orig) => + val (typsym, name, prefix) = core(orig) + ud.registerUsed(typsym, name, prefix.skipPackageObject) + case _ => + ud.removeIgnoredUsage(tree.symbol) + tree + override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context = preparing: @@ -166,11 +178,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke popScope(tree) tree - override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = - preparing: - ud.removeIgnoredUsage(tree.symbol) - tree - override def transformDefDef(tree: tpd.DefDef)(using Context): tpd.Tree = preparing: ud.removeIgnoredUsage(tree.symbol) @@ -312,14 +319,15 @@ object CheckUnused: case UnsetLocals case UnsetPrivates - /** - * The key used to retrieve the "unused entity" analysis metadata, - * from the compilation `Context` - */ + /** The key used to retrieve the "unused entity" analysis metadata from the compilation `Context` */ private val _key = Property.StickyKey[UnusedData] + /** Attachment holding the name of an Ident as written by the user. */ val OriginalName = Property.StickyKey[Name] + /** Attachment holding the name of a type class as written by the user. */ + val OriginalTypeClass = Property.StickyKey[tpd.Tree] + class PostTyper extends CheckUnused(PhaseMode.Aggregate, "PostTyper", _key) class PostInlining extends CheckUnused(PhaseMode.Report, "PostInlining", _key) @@ -384,7 +392,7 @@ object CheckUnused: * The optional name will be used to target the right import * as the same element can be imported with different renaming */ - def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true, isDerived: Boolean = false)(using Context): Unit = + def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true)(using Context): Unit = if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) then if sym.isConstructor then // constructors are "implicitly" imported with the class @@ -399,7 +407,7 @@ object CheckUnused: if sym.exists then usedDef += sym if includeForImport1 then - addUsage(Usage(sym, name, prefix, isDerived)) + addUsage(Usage(sym, name, prefix)) addIfExists(sym) addIfExists(sym.companionModule) addIfExists(sym.companionClass) @@ -409,7 +417,7 @@ object CheckUnused: def addUsage(usage: Usage)(using Context): Unit = val usages = usedInScope.top.getOrElseUpdate(usage.symbol, ListBuffer.empty) - if !usages.exists(x => x.name == usage.name && x.isDerived == usage.isDerived && x.prefix =:= usage.prefix) + if !usages.exists(x => x.name == usage.name && x.prefix =:= usage.prefix) then usages += usage /** Register a symbol that should be ignored */ @@ -477,10 +485,7 @@ object CheckUnused: def registerSetVar(sym: Symbol): Unit = setVars += sym - /** - * leave the current scope and do : - * - * - If there are imports in this scope check for unused ones + /** Leave current scope and mark any used imports; collect unused imports. */ def popScope(scopeType: ScopeType)(using Context): Unit = assert(currScopeType.pop() == scopeType) @@ -488,7 +493,7 @@ object CheckUnused: for usedInfos <- usedInScope.pop().valuesIterator; usedInfo <- usedInfos do import usedInfo.* - selDatas.find(symbol.isInImport(_, name, prefix, isDerived)) match + selDatas.find(symbol.isInImport(_, name, prefix)) match case Some(sel) => sel.markUsed() case None => @@ -646,35 +651,23 @@ object CheckUnused: /** Given an import selector, is the symbol imported from the given prefix, optionally with a specific name? * If isDerived, then it may be an aliased type in source but we only witness it dealiased. */ - private def isInImport(selData: ImportSelectorData, altName: Option[Name], prefix: Type, isDerived: Boolean)(using Context): Boolean = + private def isInImport(selData: ImportSelectorData, altName: Option[Name], prefix: Type)(using Context): Boolean = assert(sym.exists) val selector = selData.selector - if !selector.isWildcard then - if altName.exists(explicitName => selector.rename != explicitName.toTermName) then - // if there is an explicit name, it must match - false - else if isDerived then - // See i15503i.scala, grep for "package foo.test.i17156" - selData.allSymbolsDealiasedForNamed.contains(sym.dealiasAsType) - else (prefix.typeSymbol.isPackageObject || selData.qualTpe =:= prefix) && - selData.allSymbolsForNamed.contains(sym) - else - // Wildcard - if !selData.qualTpe.member(sym.name).hasAltWith(_.symbol == sym) then - // The qualifier does not have the target symbol as a member - false - else - if selector.isGiven then - // Further check that the symbol is a given or implicit and conforms to the bound + if selector.isWildcard then + selData.qualTpe.member(sym.name).hasAltWith(_.symbol == sym) && { // The qualifier must have the target symbol as a member + if selector.isGiven then // Further check that the symbol is a given or implicit and conforms to the bound sym.isOneOf(Given | Implicit) && (selector.bound.isEmpty || sym.info.finalResultType <:< selector.boundTpe) && selData.qualTpe =:= prefix else - // Normal wildcard, check that the symbol is not a given (but can be implicit) - !sym.is(Given) - end if + !sym.is(Given) // Normal wildcard, check that the symbol is not a given (but can be implicit) + } + else + !altName.exists(_.toTermName != selector.rename) && // if there is an explicit name, it must match + selData.qualTpe =:= prefix && selData.allSymbolsForNamed.contains(sym) end isInImport /** Annotated with @unused */ @@ -786,12 +779,6 @@ object CheckUnused: myAllSymbols = allDenots.map(_.symbol).toSet myAllSymbols.uncheckedNN - private var myAllSymbolsDealiased: Set[Symbol] | Null = null - - def allSymbolsDealiasedForNamed(using Context): Set[Symbol] = - if myAllSymbolsDealiased == null then - myAllSymbolsDealiased = allSymbolsForNamed.map(_.dealiasAsType) - myAllSymbolsDealiased.uncheckedNN end ImportSelectorData case class UnusedSymbol(pos: SrcPos, name: Name, warnType: WarnTypes) @@ -801,9 +788,9 @@ object CheckUnused: val Empty = UnusedResult(Set.empty) /** A symbol usage includes the name under which it was observed, - * the prefix from which it was selected, and whether it is in a derived element. + * and the prefix from which it was selected. */ - class Usage(val symbol: Symbol, val name: Option[Name], val prefix: Type, val isDerived: Boolean) + class Usage(val symbol: Symbol, val name: Option[Name], val prefix: Type) end UnusedData extension (sym: Symbol) /** is accessible without import in current context */ @@ -814,15 +801,20 @@ object CheckUnused: && c.owner.thisType.baseClasses.contains(sym.owner) && c.owner.thisType.member(sym.name).alternatives.contains(sym) - def dealiasAsType(using Context): Symbol = - if sym.isType && sym.asType.denot.isAliasType then - sym.asType.typeRef.dealias.typeSymbol - else - sym extension (tp: Type) def importPrefix(using Context): Type = tp match case tp: NamedType => tp.prefix case tp: ClassInfo => tp.prefix case tp: TypeProxy => tp.superType.normalizedPrefix case _ => NoType + def underlyingPrefix(using Context): Type = tp match + case tp: NamedType => tp.prefix + case tp: ClassInfo => tp.prefix + case tp: TypeProxy => tp.underlying.underlyingPrefix + case _ => NoType + def skipPackageObject(using Context): Type = + if tp.typeSymbol.isPackageObject then tp.underlyingPrefix else tp + def underlying(using Context): Type = tp match + case tp: TypeProxy => tp.underlying + case _ => tp end CheckUnused diff --git a/compiler/src/dotty/tools/dotc/typer/Deriving.scala b/compiler/src/dotty/tools/dotc/typer/Deriving.scala index 60148319a61c..c28f2b309cb9 100644 --- a/compiler/src/dotty/tools/dotc/typer/Deriving.scala +++ b/compiler/src/dotty/tools/dotc/typer/Deriving.scala @@ -10,8 +10,9 @@ import Contexts.*, Symbols.*, Types.*, SymDenotations.*, Names.*, NameOps.*, Fla import ProtoTypes.*, ContextOps.* import util.Spans.* import util.SrcPos -import collection.mutable +import collection.mutable.ListBuffer import ErrorReporting.errorTree +import transform.CheckUnused.OriginalTypeClass /** A typer mixin that implements type class derivation functionality */ trait Deriving { @@ -25,8 +26,8 @@ trait Deriving { */ class Deriver(cls: ClassSymbol, codePos: SrcPos)(using Context) { - /** A buffer for synthesized symbols for type class instances */ - private var synthetics = new mutable.ListBuffer[Symbol] + /** A buffer for synthesized symbols for type class instances, with what user asked to synthesize. */ + private val synthetics = ListBuffer.empty[(tpd.Tree, Symbol)] /** A version of Type#underlyingClassRef that works also for higher-kinded types */ private def underlyingClassRef(tp: Type): Type = tp match { @@ -41,7 +42,7 @@ trait Deriving { * an instance with the same name does not exist already. * @param reportErrors Report an error if an instance with the same name exists already */ - private def addDerivedInstance(clsName: Name, info: Type, pos: SrcPos): Unit = { + private def addDerivedInstance(derived: tpd.Tree, clsName: Name, info: Type, pos: SrcPos): Unit = { val instanceName = "derived$".concat(clsName) if (ctx.denotNamed(instanceName).exists) report.error(em"duplicate type class derivation for $clsName", pos) @@ -50,9 +51,8 @@ trait Deriving { // derived instance will have too low a priority to be selected over a freshly // derived instance at the summoning site. val flags = if info.isInstanceOf[MethodOrPoly] then GivenMethod else Given | Lazy - synthetics += - newSymbol(ctx.owner, instanceName, flags, info, coord = pos.span) - .entered + val sym = newSymbol(ctx.owner, instanceName, flags, info, coord = pos.span).entered + synthetics += derived -> sym } /** Check derived type tree `derived` for the following well-formedness conditions: @@ -77,7 +77,8 @@ trait Deriving { * that have the same name but different prefixes through selective aliasing. */ private def processDerivedInstance(derived: untpd.Tree): Unit = { - val originalTypeClassType = typedAheadType(derived, AnyTypeConstructorProto).tpe + val originalTypeClassTree = typedAheadType(derived, AnyTypeConstructorProto) + val originalTypeClassType = originalTypeClassTree.tpe val underlyingClassType = underlyingClassRef(originalTypeClassType) val typeClassType = checkClassType( underlyingClassType.orElse(originalTypeClassType), @@ -100,7 +101,7 @@ trait Deriving { val derivedInfo = if derivedParams.isEmpty then monoInfo else PolyType.fromParams(derivedParams, monoInfo) - addDerivedInstance(originalTypeClassType.typeSymbol.name, derivedInfo, derived.srcPos) + addDerivedInstance(originalTypeClassTree, originalTypeClassType.typeSymbol.name, derivedInfo, derived.srcPos) } def deriveSingleParameter: Unit = { @@ -312,7 +313,7 @@ trait Deriving { else tpd.ValDef(sym.asTerm, typeclassInstance(sym)(Nil)) } - synthetics.map(syntheticDef).toList + synthetics.map((t, s) => syntheticDef(s).withAttachment(OriginalTypeClass, t)).toList } def finalize(stat: tpd.TypeDef): tpd.Tree = { From 532ebde23eee3be871e440864514311ab388e361 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 3 Oct 2024 01:45:21 -0700 Subject: [PATCH 07/30] Sort unfulfilled expectations --- .../dotty/tools/vulpix/ParallelTesting.scala | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index e3f5daaa8f65..996093e936ea 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -793,13 +793,14 @@ trait ParallelTesting extends RunnerOrchestration { self => diffCheckfile(testSource, reporters, logger) override def maybeFailureMessage(testSource: TestSource, reporters: Seq[TestReporter]): Option[String] = - lazy val (map, expCount) = getWarnMapAndExpectedCount(testSource.sourceFiles.toIndexedSeq) + lazy val (expected, expCount) = getWarnMapAndExpectedCount(testSource.sourceFiles.toIndexedSeq) lazy val obtCount = reporters.foldLeft(0)(_ + _.warningCount) - lazy val (expected, unexpected) = getMissingExpectedWarnings(map, reporters.iterator.flatMap(_.diagnostics)) - lazy val diagnostics = reporters.flatMap(_.diagnostics.toSeq.sortBy(_.pos.line).map(e => s" at ${e.pos.line + 1}: ${e.message}")) - def showLines(title: String, lines: Seq[String]) = if lines.isEmpty then "" else title + lines.mkString("\n", "\n", "") - def hasMissingAnnotations = expected.nonEmpty || unexpected.nonEmpty - def showDiagnostics = showLines("-> following the diagnostics:", diagnostics) + lazy val (unfulfilled, unexpected) = getMissingExpectedWarnings(expected, diagnostics.iterator) + lazy val diagnostics = reporters.flatMap(_.diagnostics.toSeq.sortBy(_.pos.line)) + lazy val messages = diagnostics.map(d => s" at ${d.pos.line + 1}: ${d.message}") + def showLines(title: String, lines: Seq[String]) = if lines.isEmpty then "" else lines.mkString(s"$title\n", "\n", "") + def hasMissingAnnotations = unfulfilled.nonEmpty || unexpected.nonEmpty + def showDiagnostics = showLines("-> following the diagnostics:", messages) Option: if reporters.exists(_.errorCount > 0) then s"""Compilation failed for: ${testSource.title} @@ -808,62 +809,61 @@ trait ParallelTesting extends RunnerOrchestration { self => else if expCount != obtCount then s"""|Wrong number of warnings encountered when compiling $testSource |expected: $expCount, actual: $obtCount - |${showLines("Unfulfilled expectations:", expected)} + |${showLines("Unfulfilled expectations:", unfulfilled)} |${showLines("Unexpected warnings:", unexpected)} |$showDiagnostics |""".stripMargin.trim.linesIterator.mkString("\n", "\n", "") else if hasMissingAnnotations then s"""|Warnings found on incorrect row numbers when compiling $testSource - |${showLines("Unfulfilled expectations:", expected)} + |${showLines("Unfulfilled expectations:", unfulfilled)} |${showLines("Unexpected warnings:", unexpected)} |$showDiagnostics |""".stripMargin.trim.linesIterator.mkString("\n", "\n", "") - else if !map.isEmpty then s"\nExpected warnings(s) have {=}: $map" + else if !expected.isEmpty then s"\nExpected warnings(s) have {=}: $expected" else null end maybeFailureMessage def getWarnMapAndExpectedCount(files: Seq[JFile]): (HashMap[String, Integer], Int) = - val comment = raw"//( *)(nopos-)?warn".r - val map = new HashMap[String, Integer]() + val comment = raw"//(?: *)(nopos-)?warn".r + val map = HashMap[String, Integer]() var count = 0 def bump(key: String): Unit = map.get(key) match case null => map.put(key, 1) case n => map.put(key, n+1) count += 1 - files.filter(isSourceFile).foreach { file => - Using(Source.fromFile(file, StandardCharsets.UTF_8.name)) { source => - source.getLines.zipWithIndex.foreach { case (line, lineNbr) => - comment.findAllMatchIn(line).foreach { m => - m.group(2) match - case "nopos-" => - bump("nopos") - case _ => bump(s"${file.getPath}:${lineNbr+1}") - } - } - }.get - } + for file <- files if isSourceFile(file) do + Using.resource(Source.fromFile(file, StandardCharsets.UTF_8.name)) { source => + source.getLines.zipWithIndex.foreach: (line, lineNbr) => + comment.findAllMatchIn(line).foreach: + case comment("nopos-") => bump("nopos") + case _ => bump(s"${file.getPath}:${lineNbr+1}") + } + end for (map, count) - def getMissingExpectedWarnings(map: HashMap[String, Integer], reporterWarnings: Iterator[Diagnostic]): (List[String], List[String]) = - val unexpected, unpositioned = ListBuffer.empty[String] + // return unfulfilled expected warnings and unexpected diagnostics + def getMissingExpectedWarnings(expected: HashMap[String, Integer], reporterWarnings: Iterator[Diagnostic]): (List[String], List[String]) = + val unexpected = ListBuffer.empty[String] def relativize(path: String): String = path.split(JFile.separatorChar).dropWhile(_ != "tests").mkString(JFile.separator) def seenAt(key: String): Boolean = - map.get(key) match + expected.get(key) match case null => false - case 1 => map.remove(key) ; true - case n => map.put(key, n - 1) ; true + case 1 => expected.remove(key); true + case n => expected.put(key, n - 1); true def sawDiagnostic(d: Diagnostic): Unit = val srcpos = d.pos.nonInlined if srcpos.exists then val key = s"${relativize(srcpos.source.file.toString())}:${srcpos.line + 1}" if !seenAt(key) then unexpected += key else - if(!seenAt("nopos")) unpositioned += relativize(srcpos.source.file.toString()) + if !seenAt("nopos") then unexpected += relativize(srcpos.source.file.toString) reporterWarnings.foreach(sawDiagnostic) - (map.asScala.keys.toList, (unexpected ++ unpositioned).toList) + val splitter = raw"(?:[^:]*):(\d+)".r + val unfulfilled = expected.asScala.keys.toList.sortBy { case splitter(n) => n.toInt case _ => -1 } + (unfulfilled, unexpected.toList) end getMissingExpectedWarnings end WarnTest @@ -1000,8 +1000,8 @@ trait ParallelTesting extends RunnerOrchestration { self => def seenAt(key: String): Boolean = errorMap.get(key) match case null => false - case 1 => errorMap.remove(key) ; true - case n => errorMap.put(key, n - 1) ; true + case 1 => errorMap.remove(key); true + case n => errorMap.put(key, n - 1); true def sawDiagnostic(d: Diagnostic): Unit = d.pos.nonInlined match case srcpos if srcpos.exists => From f09e89470aa8e48683f7dc00d08236fc30f188e1 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 3 Oct 2024 00:09:54 -0700 Subject: [PATCH 08/30] No warn serialization methods --- .../tools/dotc/transform/CheckUnused.scala | 9 +- tests/untried/neg/warn-unused-privates.scala | 105 ------- tests/warn/unused-privates.scala | 259 ++++++++++++++++++ 3 files changed, 266 insertions(+), 107 deletions(-) delete mode 100644 tests/untried/neg/warn-unused-privates.scala create mode 100644 tests/warn/unused-privates.scala diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 8a2f8fe3fc16..b76c5cea179e 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -15,7 +15,7 @@ import dotty.tools.dotc.core.StdNames.nme import dotty.tools.dotc.core.Symbols.* import dotty.tools.dotc.core.Types.{AnnotatedType, ClassInfo, ConstantType, NamedType, NoType, TermRef, Type, TypeProxy, TypeTraverser} import dotty.tools.dotc.core.Flags -import dotty.tools.dotc.core.Names.Name +import dotty.tools.dotc.core.Names.{Name, TermName, termName} import dotty.tools.dotc.core.NameOps.isReplWrapperName import dotty.tools.dotc.core.Annotations import dotty.tools.dotc.core.Definitions @@ -733,7 +733,10 @@ object CheckUnused: !sym.shouldNotReportParamOwner private def shouldReportPrivateDef(using Context): Boolean = - peekScopeType == ScopeType.Template && !memDef.symbol.isConstructor && memDef.symbol.is(Private, butNot = SelfName | Synthetic | CaseAccessor) + peekScopeType == ScopeType.Template + && !memDef.symbol.isConstructor + && memDef.symbol.is(Private, butNot = SelfName | Synthetic | CaseAccessor) + && !ignoredNames(memDef.symbol.name.toTermName) private def isUnsetVarDef(using Context): Boolean = val sym = memDef.symbol @@ -764,6 +767,8 @@ object CheckUnused: case _: tpd.Block => Local case _ => Other + val ignoredNames: Set[TermName] = Set("readResolve", "readObject", "writeObject", "writeReplace").map(termName(_)) + final class ImportSelectorData(val qualTpe: Type, val selector: ImportSelector): private var myUsed: Boolean = false diff --git a/tests/untried/neg/warn-unused-privates.scala b/tests/untried/neg/warn-unused-privates.scala deleted file mode 100644 index 64e7679f37ac..000000000000 --- a/tests/untried/neg/warn-unused-privates.scala +++ /dev/null @@ -1,105 +0,0 @@ -class Bippy(a: Int, b: Int) { - private def this(c: Int) = this(c, c) // warn - private def bippy(x: Int): Int = bippy(x) // TODO: could warn - private def boop(x: Int) = x + a + b // warn - final private val MILLIS1 = 2000 // no warn, might have been inlined - final private val MILLIS2: Int = 1000 // warn - final private val HI_COMPANION: Int = 500 // no warn, accessed from companion - def hi() = Bippy.HI_INSTANCE -} -object Bippy { - def hi(x: Bippy) = x.HI_COMPANION - private val HI_INSTANCE: Int = 500 // no warn, accessed from instance - private val HEY_INSTANCE: Int = 1000 // warn -} - -class A(val msg: String) -class B1(msg: String) extends A(msg) -class B2(msg0: String) extends A(msg0) -class B3(msg0: String) extends A("msg") - -/*** Early defs warnings disabled primarily due to SI-6595. - * The test case is here to assure we aren't issuing false positives; - * the ones labeled "warn" don't warn. - ***/ -class Boppy extends { - private val hmm: String = "abc" // no warn, used in early defs - private val hom: String = "def" // no warn, used in body - private final val him = "ghi" // no warn, might have been (was) inlined - final val him2 = "ghi" // no warn, same - final val himinline = him - private val hum: String = "jkl" // warn - final val ding = hmm.length -} with Mutable { - val dinger = hom - private val hummer = "def" // warn - - private final val bum = "ghi" // no warn, might have been (was) inlined - final val bum2 = "ghi" // no warn, same -} - -trait Accessors { - private var v1: Int = 0 // warn - private var v2: Int = 0 // warn, never set - private var v3: Int = 0 // warn, never got - private var v4: Int = 0 // no warn - - def bippy(): Int = { - v3 = 5 - v4 = 6 - v2 + v4 - } -} - -trait DefaultArgs { - // warn about default getters for x2 and x3 - private def bippy(x1: Int, x2: Int = 10, x3: Int = 15): Int = x1 + x2 + x3 - - def boppy() = bippy(5, 100, 200) -} - -class Outer { - class Inner -} - -trait Locals { - def f0 = { - var x = 1 // warn - var y = 2 - y = 3 - y + y - } - def f1 = { - val a = new Outer // no warn - val b = new Outer // warn - new a.Inner - } - def f2 = { - var x = 100 // warn about it being a var - x - } -} - -object Types { - private object Dongo { def f = this } // warn - private class Bar1 // warn - private class Bar2 // no warn - private type Alias1 = String // warn - private type Alias2 = String // no warn - def bippo = (new Bar2).toString - - def f(x: Alias2) = x.length - - def l1() = { - object HiObject { def f = this } // warn - class Hi { // warn - def f1: Hi = new Hi - def f2(x: Hi) = x - } - class DingDongDoobie // warn - class Bippy // no warn - type Something = Bippy // no warn - type OtherThing = String // warn - (new Bippy): Something - } -} diff --git a/tests/warn/unused-privates.scala b/tests/warn/unused-privates.scala new file mode 100644 index 000000000000..e30c00907b4c --- /dev/null +++ b/tests/warn/unused-privates.scala @@ -0,0 +1,259 @@ +// +//> using options -deprecation -Wunused:privates,locals +// +class Bippy(a: Int, b: Int) { + private def this(c: Int) = this(c, c) // NO warn + private def bippy(x: Int): Int = bippy(x) // warn + private def boop(x: Int) = x+a+b // warn + final private val MILLIS1 = 2000 // warn, scala2: no warn, might have been inlined + final private val MILLIS2: Int = 1000 // warn + final private val HI_COMPANION: Int = 500 // no warn, accessed from companion + def hi() = Bippy.HI_INSTANCE +} +object Bippy { + def hi(x: Bippy) = x.HI_COMPANION + private val HI_INSTANCE: Int = 500 // no warn, accessed from instance + private val HEY_INSTANCE: Int = 1000 // warn + private lazy val BOOL: Boolean = true // warn +} + +class A(val msg: String) +class B1(msg: String) extends A(msg) +class B2(msg0: String) extends A(msg0) +class B3(msg0: String) extends A("msg") + +trait Accessors { + private var v1: Int = 0 // warn + private var v2: Int = 0 // warn, never set + private var v3: Int = 0 // NO warn, never got + private var v4: Int = 0 // no warn + + private var v5 = 0 // warn, never set + private var v6 = 0 // NO warn, never got + private var v7 = 0 // no warn + + def bippy(): Int = { + v3 = 3 + v4 = 4 + v6 = 6 + v7 = 7 + v2 + v4 + v5 + v7 + } +} + +class StableAccessors { + private var s1: Int = 0 // warn + private var s2: Int = 0 // warn, never set + private var s3: Int = 0 // NO warn, never got + private var s4: Int = 0 // no warn + + private var s5 = 0 // warn, never set + private var s6 = 0 // no warn, limitation + private var s7 = 0 // no warn + + def bippy(): Int = { + s3 = 3 + s4 = 4 + s6 = 6 + s7 = 7 + s2 + s4 + s5 + s7 + } +} + +trait DefaultArgs { + // NO warn about default getters for x2 and x3 + private def bippy(x1: Int, x2: Int = 10, x3: Int = 15): Int = x1 + x2 + x3 + + def boppy() = bippy(5, 100, 200) +} + +/* scala/bug#7707 Both usages warn default arg because using PrivateRyan.apply, not new. +case class PrivateRyan private (ryan: Int = 42) { def f = PrivateRyan() } +object PrivateRyan { def f = PrivateRyan() } +*/ + +class Outer { + class Inner +} + +trait Locals { + def f0 = { + var x = 1 // warn + var y = 2 + y = 3 + y + y + } + def f1 = { + val a = new Outer // no warn + val b = new Outer // warn + new a.Inner + } + def f2 = { + var x = 100 // warn about it being a var + x + } +} + +object Types { + private object Dongo { def f = this } // NO warn + private class Bar1 // warn + private class Bar2 // no warn + private type Alias1 = String // warn + private type Alias2 = String // no warn + def bippo = (new Bar2).toString + + def f(x: Alias2) = x.length + + def l1() = { + object HiObject { def f = this } // NO warn + class Hi { // warn + def f1: Hi = new Hi + def f2(x: Hi) = x + } + class DingDongDoobie // warn + class Bippy // no warn + type Something = Bippy // no warn + type OtherThing = String // warn + (new Bippy): Something + } +} + +trait Underwarn { + def f(): Seq[Int] + + def g() = { + val Seq(_, _) = f() // no warn + true + } +} + +class OtherNames { + private def x_=(i: Int): Unit = () + private def x: Int = 42 // warn + private def y_=(i: Int): Unit = () + private def y: Int = 42 + + def f = y +} + +case class C(a: Int, b: String, c: Option[String]) +case class D(a: Int) + +// patvars which used to warn as vals in older scala 2 +trait Boundings { + + def c = C(42, "hello", Some("world")) + def d = D(42) + + def f() = { + val C(x, y, Some(z)) = c: @unchecked // no warn + 17 + } + def g() = { + val C(x @ _, y @ _, Some(z @ _)) = c: @unchecked // no warn + 17 + } + def h() = { + val C(x @ _, y @ _, z @ Some(_)) = c: @unchecked // no warn for z? + 17 + } + + def v() = { + val D(x) = d // no warn + 17 + } + def w() = { + val D(x @ _) = d // no warn + 17 + } + +} + +trait Forever { + def f = { + val t = Option((17, 42)) + for { + ns <- t + (i, j) = ns // no warn + } yield (i + j) + } + def g = { + val t = Option((17, 42)) + for { + ns <- t + (i, j) = ns // no warn + } yield 42 // val emitted only if needed, hence nothing unused + } +} + +trait Ignorance { + private val readResolve = 42 // no warn special members +} + +trait CaseyKasem { + def f = 42 match { + case x if x < 25 => "no warn" + case y if toString.nonEmpty => "no warn" + y + case z => "warn" + } +} +trait CaseyAtTheBat { + def f = Option(42) match { + case Some(x) if x < 25 => "no warn" + case Some(y @ _) if toString.nonEmpty => "no warn" + case Some(z) => "warn" + case None => "no warn" + } +} + +class `not even using companion privates` + +object `not even using companion privates` { + private implicit class `for your eyes only`(i: Int) { // no warn deprecated feature + def f = i + } +} + +class `no warn in patmat anonfun isDefinedAt` { + def f(pf: PartialFunction[String, Int]) = pf("42") + def g = f { + case s => s.length // no warn (used to warn case s => true in isDefinedAt) + } +} + +// this is the ordinary case, as AnyRef is an alias of Object +class `nonprivate alias is enclosing` { + class C + type C2 = C + private class D extends C2 // warn +} + +object `classof something` { + private class intrinsically + def f = classOf[intrinsically].toString() +} + +trait `scala 2 short comings` { + def f: Int = { + val x = 42 // warn + 17 + } +} + +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 +} + +class `recursive reference is not a usage` { + private def f(i: Int): Int = // warn + if (i <= 0) i + else f(i-1) + private class P { // warn + def f() = new P() + } +} From bd7ac2fbb393eb850db5c404f081d967fcc9eb64 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 3 Oct 2024 02:38:10 -0700 Subject: [PATCH 09/30] Assume tpd --- .../tools/dotc/transform/CheckUnused.scala | 140 +++++++++--------- 1 file changed, 67 insertions(+), 73 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index b76c5cea179e..337dce9fafd5 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -3,7 +3,7 @@ package dotty.tools.dotc.transform import scala.annotation.tailrec import dotty.tools.uncheckedNN -import dotty.tools.dotc.ast.tpd, tpd.{Ident, Inlined, Select, Tree, TreeTraverser} +import dotty.tools.dotc.ast.tpd.* import dotty.tools.dotc.ast.untpd, untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts.* @@ -12,15 +12,12 @@ import dotty.tools.dotc.core.Denotations.SingleDenotation import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames.nme -import dotty.tools.dotc.core.Symbols.* import dotty.tools.dotc.core.Types.{AnnotatedType, ClassInfo, ConstantType, NamedType, NoType, TermRef, Type, TypeProxy, TypeTraverser} import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Names.{Name, TermName, termName} import dotty.tools.dotc.core.NameOps.isReplWrapperName -import dotty.tools.dotc.core.Annotations -import dotty.tools.dotc.core.Definitions import dotty.tools.dotc.core.NameKinds.WildcardParamName -import dotty.tools.dotc.core.Symbols.{Symbol, isDeprecated} +import dotty.tools.dotc.core.Symbols.{NoSymbol, Symbol, defn, isDeprecated} import dotty.tools.dotc.report import dotty.tools.dotc.reporting.{Message, UnusedSymbol as UnusedSymbolMessage} import dotty.tools.dotc.transform.MegaPhase.MiniPhase @@ -50,14 +47,13 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke override def description: String = CheckUnused.description - override def isRunnable(using Context): Boolean = - super.isRunnable && - ctx.settings.WunusedHas.any && - !ctx.isJava + override def isEnabled(using Context): Boolean = ctx.settings.Wunused.value.nonEmpty + + override def isRunnable(using Context): Boolean = super.isRunnable && ctx.settings.WunusedHas.any && !ctx.isJava // ========== SETUP ============ - override def prepareForUnit(tree: tpd.Tree)(using Context): Context = + override def prepareForUnit(tree: Tree)(using Context): Context = val data = UnusedData() tree.getAttachment(_key).foreach(oldData => data.unusedAggregate = oldData.unusedAggregate @@ -66,23 +62,22 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke // ========== END + REPORTING ========== - override def transformUnit(tree: tpd.Tree)(using Context): tpd.Tree = + override def transformUnit(tree: Tree)(using Context): Tree = preparing: ud.finishAggregation() if phaseMode == PhaseMode.Report then ud.unusedAggregate.foreach(reportUnused) tree - // ========== MiniPhase Prepare ========== - override def prepareForOther(tree: tpd.Tree)(using Context): Context = + override def prepareForOther(tree: Tree)(using Context): Context = traverser.traverse(tree) ctx - override def prepareForInlined(tree: tpd.Inlined)(using Context): Context = + override def prepareForInlined(tree: Inlined)(using Context): Context = traverser.traverse(tree.call) ctx - override def prepareForIdent(tree: tpd.Ident)(using Context): Context = + override def prepareForIdent(tree: Ident)(using Context): Context = preparing: if tree.symbol.exists then def loopOnNormalizedPrefixes(prefix: Type, depth: Int): Unit = @@ -96,25 +91,25 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke else if tree.hasType then ud.registerUsed(tree.tpe.classSymbol, Some(tree.name), tree.tpe.importPrefix) - override def prepareForSelect(tree: tpd.Select)(using Context): Context = + override def prepareForSelect(tree: Select)(using Context): Context = preparing: val name = tree.removeAttachment(OriginalName) ud.registerUsed(tree.symbol, name, tree.qualifier.tpe, includeForImport = tree.qualifier.span.isSynthetic) - override def prepareForBlock(tree: tpd.Block)(using Context): Context = + override def prepareForBlock(tree: Block)(using Context): Context = pushScope(tree) - override def prepareForTemplate(tree: tpd.Template)(using Context): Context = + override def prepareForTemplate(tree: Template)(using Context): Context = pushScope(tree) - override def prepareForPackageDef(tree: tpd.PackageDef)(using Context): Context = + override def prepareForPackageDef(tree: PackageDef)(using Context): Context = pushScope(tree) - override def prepareForValDef(tree: tpd.ValDef)(using Context): Context = + override def prepareForValDef(tree: ValDef)(using Context): Context = preparing: ud.addIgnoredUsage(tree.symbol) - override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = + override def transformValDef(tree: ValDef)(using Context): Tree = preparing: traverseAnnotations(tree.symbol) if !tree.symbol.is(Module) then // do not register the ValDef generated for `object` @@ -133,7 +128,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke tree - override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context = + override def prepareForDefDef(tree: DefDef)(using Context): Context = preparing: if !tree.symbol.is(Private) then tree.termParamss.flatten.foreach(p => ud.addIgnoredParam(p.symbol)) @@ -142,23 +137,23 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke ud.registerDef(tree) ud.addIgnoredUsage(tree.symbol) - override def prepareForTypeDef(tree: tpd.TypeDef)(using Context): Context = + override def prepareForTypeDef(tree: TypeDef)(using Context): Context = preparing: traverseAnnotations(tree.symbol) if !tree.symbol.is(Param) then // Ignore type parameter (as Scala 2) ud.registerDef(tree) ud.addIgnoredUsage(tree.symbol) - override def prepareForBind(tree: tpd.Bind)(using Context): Context = + override def prepareForBind(tree: Bind)(using Context): Context = preparing: traverseAnnotations(tree.symbol) ud.registerPatVar(tree) - override def prepareForTypeTree(tree: tpd.TypeTree)(using Context): Context = - if !tree.isInstanceOf[tpd.InferredTypeTree] then typeTraverser.traverse(tree.tpe) + override def prepareForTypeTree(tree: TypeTree)(using Context): Context = + if !tree.isInstanceOf[InferredTypeTree] then typeTraverser.traverse(tree.tpe) ctx - override def prepareForAssign(tree: tpd.Assign)(using Context): Context = + override def prepareForAssign(tree: Assign)(using Context): Context = preparing: val sym = tree.lhs.symbol if sym.exists then @@ -166,24 +161,24 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke // ========== MiniPhase Transform ========== - override def transformBlock(tree: tpd.Block)(using Context): tpd.Tree = + override def transformBlock(tree: Block)(using Context): Tree = popScope(tree) tree - override def transformTemplate(tree: tpd.Template)(using Context): tpd.Tree = + override def transformTemplate(tree: Template)(using Context): Tree = popScope(tree) tree - override def transformPackageDef(tree: tpd.PackageDef)(using Context): tpd.Tree = + override def transformPackageDef(tree: PackageDef)(using Context): Tree = popScope(tree) tree - override def transformDefDef(tree: tpd.DefDef)(using Context): tpd.Tree = + override def transformDefDef(tree: DefDef)(using Context): Tree = preparing: ud.removeIgnoredUsage(tree.symbol) tree - override def transformTypeDef(tree: tpd.TypeDef)(using Context): tpd.Tree = + override def transformTypeDef(tree: TypeDef)(using Context): Tree = preparing: ud.removeIgnoredUsage(tree.symbol) tree @@ -191,11 +186,11 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke // ---------- MiniPhase HELPERS ----------- - private def pushScope(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = + private def pushScope(tree: Block | Template | PackageDef)(using Context): Context = preparing: ud.pushScope(UnusedData.ScopeType.fromTree(tree)) - private def popScope(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = + private def popScope(tree: Block | Template | PackageDef)(using Context): Context = preparing: ud.popScope(UnusedData.ScopeType.fromTree(tree)) @@ -210,51 +205,51 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke private def traverser = new TreeTraverser: // Register every import, definition and usage - override def traverse(tree: tpd.Tree)(using Context): Unit = + override def traverse(tree: Tree)(using Context): Unit = val newCtx = if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx tree match - case imp: tpd.Import => + case imp: Import => preparing: ud.registerImport(imp) imp.selectors.filter(_.isGiven).map(_.bound).collect { case untpd.TypedSplice(tree1) => tree1 }.foreach(traverse(_)(using newCtx)) traverseChildren(tree)(using newCtx) - case ident: tpd.Ident => + case ident: Ident => prepareForIdent(ident) traverseChildren(tree)(using newCtx) - case sel: tpd.Select => + case sel: Select => prepareForSelect(sel) traverseChildren(tree)(using newCtx) - case tree: (tpd.Block | tpd.Template | tpd.PackageDef) => + case tree: (Block | Template | PackageDef) => //! DIFFERS FROM MINIPHASE pushScope(tree) traverseChildren(tree)(using newCtx) popScope(tree) - case t: tpd.ValDef => + case t: ValDef => prepareForValDef(t) traverseChildren(tree)(using newCtx) transformValDef(t) - case t: tpd.DefDef => + case t: DefDef => prepareForDefDef(t) traverseChildren(tree)(using newCtx) transformDefDef(t) - case t: tpd.TypeDef => + case t: TypeDef => prepareForTypeDef(t) traverseChildren(tree)(using newCtx) transformTypeDef(t) - case t: tpd.Bind => + case t: Bind => prepareForBind(t) traverseChildren(tree)(using newCtx) - case t: tpd.Assign => + case t: Assign => prepareForAssign(t) traverseChildren(tree) - case _: tpd.InferredTypeTree => - case tpd.RefinedTypeTree(tpt, refinements) => + case _: InferredTypeTree => + case RefinedTypeTree(tpt, refinements) => //! DIFFERS FROM MINIPHASE typeTraverser.traverse(tree.tpe) traverse(tpt)(using newCtx) - case tpd.TypeTree() => + case TypeTree() => //! DIFFERS FROM MINIPHASE typeTraverser.traverse(tree.tpe) traverseChildren(tree)(using newCtx) @@ -326,7 +321,7 @@ object CheckUnused: val OriginalName = Property.StickyKey[Name] /** Attachment holding the name of a type class as written by the user. */ - val OriginalTypeClass = Property.StickyKey[tpd.Tree] + val OriginalTypeClass = Property.StickyKey[Tree] class PostTyper extends CheckUnused(PhaseMode.Aggregate, "PostTyper", _key) @@ -356,11 +351,11 @@ object CheckUnused: private val unusedImport = ListBuffer.empty[ImportSelectorData] /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */ - private val localDefInScope = ListBuffer.empty[tpd.MemberDef] - private val privateDefInScope = ListBuffer.empty[tpd.MemberDef] - private val explicitParamInScope = ListBuffer.empty[tpd.MemberDef] - private val implicitParamInScope = ListBuffer.empty[tpd.MemberDef] - private val patVarsInScope = ListBuffer.empty[tpd.Bind] + private val localDefInScope = ListBuffer.empty[MemberDef] + private val privateDefInScope = ListBuffer.empty[MemberDef] + private val explicitParamInScope = ListBuffer.empty[MemberDef] + private val implicitParamInScope = ListBuffer.empty[MemberDef] + private val patVarsInScope = ListBuffer.empty[Bind] /** All variables sets*/ private val setVars = mut.Set.empty[Symbol] @@ -400,8 +395,7 @@ object CheckUnused: else // If the symbol is accessible in this scope without an import, do not register it for unused import analysis val includeForImport1 = - includeForImport - && (name.exists(_.toTermName != sym.name.toTermName) || !sym.isAccessibleAsIdent) + includeForImport && (name.exists(_.toTermName != sym.name.toTermName) || !sym.isAccessibleAsIdent) def addIfExists(sym: Symbol): Unit = if sym.exists then @@ -432,9 +426,9 @@ object CheckUnused: paramsToSkip += sym /** Register an import */ - def registerImport(imp: tpd.Import)(using Context): Unit = + def registerImport(imp: Import)(using Context): Unit = if - !tpd.languageImport(imp.expr).nonEmpty + !languageImport(imp.expr).nonEmpty && !imp.isGeneratedByEnum && !isTransparentAndInline(imp) && peekScopeType != ScopeType.ReplWrapper // #18383 Do not report top-level import's in the repl as unused @@ -457,7 +451,7 @@ object CheckUnused: end registerImport /** Register (or not) some `val` or `def` according to the context, scope and flags */ - def registerDef(memDef: tpd.MemberDef)(using Context): Unit = + def registerDef(memDef: MemberDef)(using Context): Unit = if memDef.isValidMemberDef && !isDefIgnored(memDef) then if memDef.isValidParam then if memDef.symbol.isOneOf(GivenOrImplicit) then @@ -471,8 +465,8 @@ object CheckUnused: privateDefInScope += memDef /** Register pattern variable */ - def registerPatVar(patvar: tpd.Bind)(using Context): Unit = - if !patvar.symbol.isUnusedAnnot then + def registerPatVar(patvar: Bind)(using Context): Unit = + if !patvar.symbol.hasUnusedAnnot then patVarsInScope += patvar /** enter a new scope */ @@ -565,7 +559,7 @@ object CheckUnused: /** * Checks if import selects a def that is transparent and inline */ - private def isTransparentAndInline(imp: tpd.Import)(using Context): Boolean = + private def isTransparentAndInline(imp: Import)(using Context): Boolean = imp.selectors.exists { sel => val qual = imp.expr val importedMembers = qual.tpe.member(sel.name).alternatives.map(_.symbol) @@ -614,7 +608,7 @@ object CheckUnused: * If -Wunused:strict-no-implicit-warn import and this import selector could potentially import implicit. * return true */ - private def shouldSelectorBeReported(imp: tpd.Import, sel: ImportSelector)(using Context): Boolean = + private def shouldSelectorBeReported(imp: Import, sel: ImportSelector)(using Context): Boolean = ctx.settings.WunusedHas.strictNoImplicitWarn && ( sel.isWildcard || imp.expr.tpe.member(sel.name.toTermName).alternatives.exists(_.symbol.isOneOf(GivenOrImplicit)) || @@ -624,7 +618,7 @@ object CheckUnused: /** * Ignore CanEqual imports */ - private def isImportIgnored(imp: tpd.Import, sel: ImportSelector)(using Context): Boolean = + private def isImportIgnored(imp: Import, sel: ImportSelector)(using Context): Boolean = (sel.isWildcard && sel.isGiven && imp.expr.tpe.allMembers.exists(p => p.symbol.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)) && p.symbol.isOneOf(GivenOrImplicit))) || (imp.expr.tpe.member(sel.name.toTermName).alternatives .exists(p => p.symbol.isOneOf(GivenOrImplicit) && p.symbol.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)))) @@ -632,7 +626,7 @@ object CheckUnused: /** * Ignore definitions of CanEqual given */ - private def isDefIgnored(memDef: tpd.MemberDef)(using Context): Boolean = + private def isDefIgnored(memDef: MemberDef)(using Context): Boolean = memDef.symbol.isOneOf(GivenOrImplicit) && memDef.symbol.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)) extension (sel: ImportSelector) @@ -671,7 +665,7 @@ object CheckUnused: end isInImport /** Annotated with @unused */ - private def isUnusedAnnot(using Context): Boolean = + private def hasUnusedAnnot(using Context): Boolean = sym.annotations.exists(_.symbol == ctx.definitions.UnusedAnnot) private def shouldNotReportParamOwner(using Context): Boolean = @@ -697,7 +691,7 @@ object CheckUnused: end extension - extension (defdef: tpd.DefDef) + extension (defdef: DefDef) // so trivial that it never consumes params private def isTrivial(using Context): Boolean = val rhs = defdef.rhs @@ -705,7 +699,7 @@ object CheckUnused: rhs.tpe =:= ctx.definitions.NothingType || defdef.symbol.is(Deferred) || (rhs match { - case _: tpd.Literal => true + case _: Literal => true case _ => rhs.tpe match case ConstantType(_) => true case tp: TermRef => @@ -718,10 +712,10 @@ object CheckUnused: if defdef.isTrivial then trivialDefs += defdef.symbol - extension (memDef: tpd.MemberDef) + extension (memDef: MemberDef) private def isValidMemberDef(using Context): Boolean = memDef.symbol.exists - && !memDef.symbol.isUnusedAnnot + && !memDef.symbol.hasUnusedAnnot && !memDef.symbol.isAllOf(Flags.AccessorCreationFlags) && !memDef.name.isWildcard && !memDef.symbol.owner.is(ExtensionMethod) @@ -742,7 +736,7 @@ object CheckUnused: val sym = memDef.symbol sym.is(Mutable) && !setVars(sym) - extension (imp: tpd.Import) + extension (imp: Import) /** Enum generate an import for its cases (but outside them), which should be ignored */ def isGeneratedByEnum(using Context): Boolean = imp.symbol.exists && imp.symbol.owner.is(Flags.Enum, butNot = Flags.Case) @@ -762,9 +756,9 @@ object CheckUnused: object ScopeType: /** return the scope corresponding to the enclosing scope of the given tree */ - def fromTree(tree: tpd.Tree)(using Context): ScopeType = tree match - case _: tpd.Template => if tree.symbol.name.isReplWrapperName then ReplWrapper else Template - case _: tpd.Block => Local + def fromTree(tree: Tree)(using Context): ScopeType = tree match + case _: Template => if tree.symbol.name.isReplWrapperName then ReplWrapper else Template + case _: Block => Local case _ => Other val ignoredNames: Set[TermName] = Set("readResolve", "readObject", "writeObject", "writeReplace").map(termName(_)) From 9be057e4967c32bd06e70f9a8796235c65cbf0e5 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 3 Oct 2024 03:25:33 -0700 Subject: [PATCH 10/30] Refactor traverser to miniphase callbacks --- .../tools/dotc/transform/CheckUnused.scala | 298 ++++++++---------- tests/warn/i15503i.scala | 2 +- 2 files changed, 128 insertions(+), 172 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 337dce9fafd5..afb36b86ba56 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -47,37 +47,26 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke override def description: String = CheckUnused.description - override def isEnabled(using Context): Boolean = ctx.settings.Wunused.value.nonEmpty + override def isEnabled(using Context): Boolean = ctx.settings.WunusedHas.any override def isRunnable(using Context): Boolean = super.isRunnable && ctx.settings.WunusedHas.any && !ctx.isJava - // ========== SETUP ============ - + // Inherit data from previous phase. override def prepareForUnit(tree: Tree)(using Context): Context = val data = UnusedData() - tree.getAttachment(_key).foreach(oldData => + for oldData <- tree.getAttachment(_key) do data.unusedAggregate = oldData.unusedAggregate - ) ctx.fresh.setProperty(_key, data).tap(_ => tree.putAttachment(_key, data)) - // ========== END + REPORTING ========== - - override def transformUnit(tree: Tree)(using Context): Tree = + // Report if we are last phase + override def transformUnit(tree: Tree)(using Context): tree.type = preparing: ud.finishAggregation() if phaseMode == PhaseMode.Report then ud.unusedAggregate.foreach(reportUnused) tree - override def prepareForOther(tree: Tree)(using Context): Context = - traverser.traverse(tree) - ctx - - override def prepareForInlined(tree: Inlined)(using Context): Context = - traverser.traverse(tree.call) - ctx - - override def prepareForIdent(tree: Ident)(using Context): Context = + override def transformIdent(tree: Ident)(using Context): tree.type = preparing: if tree.symbol.exists then def loopOnNormalizedPrefixes(prefix: Type, depth: Int): Unit = @@ -87,29 +76,65 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke loopOnNormalizedPrefixes(prefix.normalizedPrefix, depth + 1) val prefix = tree.typeOpt.normalizedPrefix loopOnNormalizedPrefixes(prefix, depth = 0) - ud.registerUsed(tree.symbol, Some(tree.name), tree.typeOpt.importPrefix) + ud.registerUsed(tree.symbol, Some(tree.name), tree.typeOpt.importPrefix.skipPackageObject) else if tree.hasType then - ud.registerUsed(tree.tpe.classSymbol, Some(tree.name), tree.tpe.importPrefix) + ud.registerUsed(tree.tpe.classSymbol, Some(tree.name), tree.tpe.importPrefix.skipPackageObject) + tree - override def prepareForSelect(tree: Select)(using Context): Context = + override def transformSelect(tree: Select)(using Context): tree.type = preparing: val name = tree.removeAttachment(OriginalName) ud.registerUsed(tree.symbol, name, tree.qualifier.tpe, includeForImport = tree.qualifier.span.isSynthetic) + tree + + override def transformApply(tree: Apply)(using Context): Tree = + //println(s"APPLY ${tree.show}") + tree + + override def transformTyped(tree: Typed)(using Context): Tree = + tree match + case Typed(expr, tpt) => + tree + + override def transformAssign(tree: Assign)(using Context): tree.type = + preparing: + val sym = tree.lhs.symbol + if sym.exists then + ud.registerSetVar(sym) + tree override def prepareForBlock(tree: Block)(using Context): Context = pushScope(tree) - override def prepareForTemplate(tree: Template)(using Context): Context = - pushScope(tree) + override def transformBlock(tree: Block)(using Context): tree.type = + popScope(tree) + tree - override def prepareForPackageDef(tree: PackageDef)(using Context): Context = - pushScope(tree) + override def transformInlined(tree: Inlined)(using Context): tree.type = + transformAllDeep(tree.call) + tree + + override def prepareForTypeTree(tree: TypeTree)(using Context): Context = ctx + + override def transformTypeTree(tree: TypeTree)(using Context): tree.type = + //println(s"TYPETREE ${tree.getClass} ${tree.show} or $tree") + tree.tpe match + case AnnotatedType(_, annot) => transformAllDeep(annot.tree) + case _ => + tree + + override def transformBind(tree: Bind)(using Context): tree.type = + preparing: + traverseAnnotations(tree.symbol) + ud.registerPatVar(tree) + tree override def prepareForValDef(tree: ValDef)(using Context): Context = preparing: ud.addIgnoredUsage(tree.symbol) - override def transformValDef(tree: ValDef)(using Context): Tree = + override def transformValDef(tree: ValDef)(using Context): tree.type = + //println(s"VAL ${tree.name} ${tree.show} tpt ${tree.tpt.show} is a ${tree.tpt.getClass}") preparing: traverseAnnotations(tree.symbol) if !tree.symbol.is(Module) then // do not register the ValDef generated for `object` @@ -127,65 +152,87 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke ud.removeIgnoredUsage(tree.symbol) tree - override def prepareForDefDef(tree: DefDef)(using Context): Context = preparing: + ud.registerTrivial(tree) if !tree.symbol.is(Private) then tree.termParamss.flatten.foreach(p => ud.addIgnoredParam(p.symbol)) - ud.registerTrivial(tree) - traverseAnnotations(tree.symbol) - ud.registerDef(tree) ud.addIgnoredUsage(tree.symbol) - override def prepareForTypeDef(tree: TypeDef)(using Context): Context = + override def transformDefDef(tree: DefDef)(using Context): tree.type = preparing: traverseAnnotations(tree.symbol) - if !tree.symbol.is(Param) then // Ignore type parameter (as Scala 2) - ud.registerDef(tree) - ud.addIgnoredUsage(tree.symbol) + ud.registerDef(tree) + ud.removeIgnoredUsage(tree.symbol) + tree - override def prepareForBind(tree: Bind)(using Context): Context = + override def prepareForTypeDef(tree: TypeDef)(using Context): Context = preparing: - traverseAnnotations(tree.symbol) - ud.registerPatVar(tree) - - override def prepareForTypeTree(tree: TypeTree)(using Context): Context = - if !tree.isInstanceOf[InferredTypeTree] then typeTraverser.traverse(tree.tpe) - ctx + ud.addIgnoredUsage(tree.symbol) - override def prepareForAssign(tree: Assign)(using Context): Context = + override def transformTypeDef(tree: TypeDef)(using Context): tree.type = preparing: - val sym = tree.lhs.symbol - if sym.exists then - ud.registerSetVar(sym) - - // ========== MiniPhase Transform ========== - - override def transformBlock(tree: Block)(using Context): Tree = - popScope(tree) + traverseAnnotations(tree.symbol) + if !tree.symbol.is(Param) then // type parameter to do? + ud.registerDef(tree) + ud.removeIgnoredUsage(tree.symbol) tree + override def prepareForTemplate(tree: Template)(using Context): Context = + pushScope(tree) + override def transformTemplate(tree: Template)(using Context): Tree = popScope(tree) tree + override def prepareForPackageDef(tree: PackageDef)(using Context): Context = + pushScope(tree) + override def transformPackageDef(tree: PackageDef)(using Context): Tree = popScope(tree) tree - override def transformDefDef(tree: DefDef)(using Context): Tree = - preparing: - ud.removeIgnoredUsage(tree.symbol) - tree + override def prepareForStats(trees: List[Tree])(using Context): Context = ctx + + override def transformStats(trees: List[Tree])(using Context): List[Tree] = + super.transformStats(trees) + trees - override def transformTypeDef(tree: TypeDef)(using Context): Tree = + override def transformOther(tree: Tree)(using Context): tree.type = preparing: - ud.removeIgnoredUsage(tree.symbol) + tree match + case imp: Import => + ud.registerImport(imp) + transformAllDeep(imp.expr) + for selector <- imp.selectors do + if selector.isGiven then + selector.bound match + case untpd.TypedSplice(bound) => transformAllDeep(bound) + case _ => + case _: InferredTypeTree => + //println(s"INF ${tree.getClass} ${tree.show}") + case AppliedTypeTree(tpt, args) => + transformAllDeep(tpt) + args.foreach(transformAllDeep) + case RefinedTypeTree(tpt, refinements) => + transformAllDeep(tpt) + refinements.foreach(transformAllDeep) + case LambdaTypeTree(tparams, body) => + tparams.foreach(transformAllDeep) + transformAllDeep(body) + case SingletonTypeTree(ref) => + transformAllDeep(ref) + case TypeBoundsTree(lo, hi, alias) => + transformAllDeep(lo) + transformAllDeep(hi) + transformAllDeep(alias) + case tree: NamedArg => transformAllDeep(tree.arg) + case _ if tree.isType => + //println(s"OTHER TYPE ${tree.getClass} ${tree.show}") + case _ => + //println(s"OTHER ${tree.getClass} ${tree.show}") tree - - // ---------- MiniPhase HELPERS ----------- - private def pushScope(tree: Block | Template | PackageDef)(using Context): Context = preparing: ud.pushScope(UnusedData.ScopeType.fromTree(tree)) @@ -194,88 +241,9 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke preparing: ud.popScope(UnusedData.ScopeType.fromTree(tree)) - /** - * This traverse is the **main** component of this phase - * - * It traverse the tree the tree and gather the data in the - * corresponding context property - * - * A standard tree traverser covers cases not handled by the Mega/MiniPhase - */ - private def traverser = new TreeTraverser: - - // Register every import, definition and usage - override def traverse(tree: Tree)(using Context): Unit = - val newCtx = if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx - tree match - case imp: Import => - preparing: - ud.registerImport(imp) - imp.selectors.filter(_.isGiven).map(_.bound).collect { - case untpd.TypedSplice(tree1) => tree1 - }.foreach(traverse(_)(using newCtx)) - traverseChildren(tree)(using newCtx) - case ident: Ident => - prepareForIdent(ident) - traverseChildren(tree)(using newCtx) - case sel: Select => - prepareForSelect(sel) - traverseChildren(tree)(using newCtx) - case tree: (Block | Template | PackageDef) => - //! DIFFERS FROM MINIPHASE - pushScope(tree) - traverseChildren(tree)(using newCtx) - popScope(tree) - case t: ValDef => - prepareForValDef(t) - traverseChildren(tree)(using newCtx) - transformValDef(t) - case t: DefDef => - prepareForDefDef(t) - traverseChildren(tree)(using newCtx) - transformDefDef(t) - case t: TypeDef => - prepareForTypeDef(t) - traverseChildren(tree)(using newCtx) - transformTypeDef(t) - case t: Bind => - prepareForBind(t) - traverseChildren(tree)(using newCtx) - case t: Assign => - prepareForAssign(t) - traverseChildren(tree) - case _: InferredTypeTree => - case RefinedTypeTree(tpt, refinements) => - //! DIFFERS FROM MINIPHASE - typeTraverser.traverse(tree.tpe) - traverse(tpt)(using newCtx) - case TypeTree() => - //! DIFFERS FROM MINIPHASE - typeTraverser.traverse(tree.tpe) - traverseChildren(tree)(using newCtx) - case _ => - //! DIFFERS FROM MINIPHASE - traverseChildren(tree)(using newCtx) - end traverse - end traverser - - /** This is a type traverser which catch some special Types not traversed by the term traverser above */ - private def typeTraverser(using Context) = new TypeTraverser: - override def traverse(tp: Type): Unit = - if tp.typeSymbol.exists then - preparing: - ud.registerUsed(tp.typeSymbol, Some(tp.typeSymbol.name), tp.importPrefix) - tp match - case AnnotatedType(_, annot) => - preparing: - ud.registerUsed(annot.symbol, name = None, annot.symbol.info.importPrefix) - traverseChildren(tp) - case _ => - traverseChildren(tp) - - /** This traverse the annotations of the symbol */ private def traverseAnnotations(sym: Symbol)(using Context): Unit = - sym.denot.annotations.foreach(annot => traverser.traverse(annot.tree)) + for annot <- sym.denot.annotations do + transformAllDeep(annot.tree) /** Do the actual reporting given the result of the anaylsis */ private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit = @@ -381,11 +349,10 @@ object CheckUnused: impInScope.top.prependAll(selectors) this - /** - * Register a found (used) symbol along with its name + /** Register a found (used) symbol along with its name. * - * The optional name will be used to target the right import - * as the same element can be imported with different renaming + * The optional name will be used to target the right import + * as the same element can be imported with different renaming. */ def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true)(using Context): Unit = if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) then @@ -444,8 +411,7 @@ object CheckUnused: for sel <- reorderedSelectors yield val data = new ImportSelectorData(qualTpe, sel) if shouldSelectorBeReported(imp, sel) || sel.isImportExclusion || isImportIgnored(imp, sel) then - // Immediately mark the selector as used - data.markUsed() + data.markUsed() // Immediately mark the selector as used data registerSelectors(newDataInScope) end registerImport @@ -471,7 +437,6 @@ object CheckUnused: /** enter a new scope */ def pushScope(newScopeType: ScopeType): Unit = - // unused imports : currScopeType.push(newScopeType) impInScope.push(ListBuffer.empty) usedInScope.push(mut.Map.empty) @@ -553,11 +518,8 @@ object CheckUnused: UnusedResult(warnings.result) end getUnused - //============================ HELPERS ==================================== - - /** - * Checks if import selects a def that is transparent and inline + /** Checks if import selects a def that is transparent and inline. */ private def isTransparentAndInline(imp: Import)(using Context): Boolean = imp.selectors.exists { sel => @@ -566,8 +528,7 @@ object CheckUnused: importedMembers.exists(_.isAllOf(Transparent | Inline)) } - /** - * Heuristic to detect synthetic suffixes in names of symbols + /** Heuristic to detect synthetic suffixes in names of symbols. */ private def containsSyntheticSuffix(symbol: Symbol)(using Context): Boolean = symbol.name.mangledString.contains("$") @@ -597,16 +558,12 @@ object CheckUnused: private def isConstructorOfSynth(sym: Symbol)(using Context): Boolean = sym.exists && sym.isConstructor && sym.owner.isPackageObject && sym.owner.is(Synthetic) - /** - * This is used to avoid reporting the parameters of the synthetic main method - * generated by `@main` + /** This is used to avoid reporting the parameters of the synthetic main method generated by `@main`. */ private def isSyntheticMainParam(sym: Symbol)(using Context): Boolean = sym.exists && ctx.platform.isMainMethod(sym.owner) && sym.owner.is(Synthetic) - /** - * If -Wunused:strict-no-implicit-warn import and this import selector could potentially import implicit. - * return true + /** If -Wunused:strict-no-implicit-warn import and this import selector could potentially import implicit. */ private def shouldSelectorBeReported(imp: Import, sel: ImportSelector)(using Context): Boolean = ctx.settings.WunusedHas.strictNoImplicitWarn && ( @@ -615,19 +572,19 @@ object CheckUnused: imp.expr.tpe.member(sel.name.toTypeName).alternatives.exists(_.symbol.isOneOf(GivenOrImplicit)) ) - /** - * Ignore CanEqual imports + /** Ignore CanEqual imports. */ + private def derivesFromCanEqual(sym: Symbol)(using Context): Boolean = + sym.isOneOf(GivenOrImplicit) && sym.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)) + private def isImportIgnored(imp: Import, sel: ImportSelector)(using Context): Boolean = - (sel.isWildcard && sel.isGiven && imp.expr.tpe.allMembers.exists(p => p.symbol.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)) && p.symbol.isOneOf(GivenOrImplicit))) || - (imp.expr.tpe.member(sel.name.toTermName).alternatives - .exists(p => p.symbol.isOneOf(GivenOrImplicit) && p.symbol.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)))) + sel.isWildcard && sel.isGiven && imp.expr.tpe.allMembers.exists(p => derivesFromCanEqual(p.symbol)) + || + imp.expr.tpe.member(sel.name.toTermName).alternatives.exists(p => derivesFromCanEqual(p.symbol)) - /** - * Ignore definitions of CanEqual given + /** Ignore definitions of CanEqual given. */ - private def isDefIgnored(memDef: MemberDef)(using Context): Boolean = - memDef.symbol.isOneOf(GivenOrImplicit) && memDef.symbol.typeRef.baseClasses.exists(_.derivesFrom(defn.CanEqualClass)) + private def isDefIgnored(memDef: MemberDef)(using Context): Boolean = derivesFromCanEqual(memDef.symbol) extension (sel: ImportSelector) def boundTpe: Type = sel.bound match @@ -698,7 +655,7 @@ object CheckUnused: rhs.symbol == ctx.definitions.Predef_undefined || rhs.tpe =:= ctx.definitions.NothingType || defdef.symbol.is(Deferred) || - (rhs match { + rhs.match case _: Literal => true case _ => rhs.tpe match case ConstantType(_) => true @@ -707,7 +664,6 @@ object CheckUnused: tp.underlying.classSymbol.is(Flags.Module) case _ => false - }) def registerTrivial(using Context): Unit = if defdef.isTrivial then trivialDefs += defdef.symbol @@ -763,7 +719,7 @@ object CheckUnused: val ignoredNames: Set[TermName] = Set("readResolve", "readObject", "writeObject", "writeReplace").map(termName(_)) - final class ImportSelectorData(val qualTpe: Type, val selector: ImportSelector): + final case class ImportSelectorData(val qualTpe: Type, val selector: ImportSelector): private var myUsed: Boolean = false def markUsed(): Unit = myUsed = true @@ -789,7 +745,7 @@ object CheckUnused: /** A symbol usage includes the name under which it was observed, * and the prefix from which it was selected. */ - class Usage(val symbol: Symbol, val name: Option[Name], val prefix: Type) + case class Usage(val symbol: Symbol, val name: Option[Name], val prefix: Type) end UnusedData extension (sym: Symbol) /** is accessible without import in current context */ diff --git a/tests/warn/i15503i.scala b/tests/warn/i15503i.scala index db2c8158dac2..d3701b696009 100644 --- a/tests/warn/i15503i.scala +++ b/tests/warn/i15503i.scala @@ -82,8 +82,8 @@ package foo.test.i16678: def run = println(foo(number => number.toString, value = 5)) // OK println(foo(number => "", value = 5)) // warn - println(foo(func = number => "", value = 5)) // warn println(foo(func = number => number.toString, value = 5)) // OK + println(foo(func = number => "", value = 5)) // warn println(foo(func = _.toString, value = 5)) // OK package foo.test.possibleclasses: From 3934aa41f94ad7f6207615a2f7218aa0ecd9ff47 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 3 Oct 2024 20:25:25 -0700 Subject: [PATCH 11/30] Avoid tracking red herrings --- .../dotty/tools/dotc/transform/CheckUnused.scala | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index afb36b86ba56..45afa1366a46 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -1,6 +1,6 @@ package dotty.tools.dotc.transform -import scala.annotation.tailrec +import scala.annotation.* import dotty.tools.uncheckedNN import dotty.tools.dotc.ast.tpd.* @@ -301,7 +301,7 @@ object CheckUnused: * * For other contexts, which symbols defined here have been referenced? */ - private class UnusedData: + private class UnusedData(using Context @constructorOnly): import collection.mutable as mut, mut.Stack, mut.ListBuffer import UnusedData.* @@ -330,8 +330,13 @@ object CheckUnused: /** All used symbols */ private val usedDef = mut.Set.empty[Symbol] - /** Do not register as used */ - private val doNotRegister = mut.Set.empty[Symbol] + + /** Do not register as used. + * + * Seed with common symbols that are never warnable, as an optimization. + */ + private val doNotRegister = mut.Set[Symbol](defn.SourceFileAnnot, defn.ModuleSerializationProxyClass) + private val doNotRegisterPrefix = mut.Set[Symbol](defn.ScalaRuntimePackageClass) /** Trivial definitions, avoid registering params */ private val trivialDefs = mut.Set.empty[Symbol] @@ -355,7 +360,7 @@ object CheckUnused: * as the same element can be imported with different renaming. */ def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true)(using Context): Unit = - if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) then + if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) && !doNotRegisterPrefix(prefix.typeSymbol) then if sym.isConstructor then // constructors are "implicitly" imported with the class registerUsed(sym.owner, name = None, prefix, includeForImport = includeForImport) From 0cafda1e1039f0ad2f7d8d715ff86227d0f2d905 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 3 Oct 2024 20:26:07 -0700 Subject: [PATCH 12/30] Mono Mega Phase --- compiler/src/dotty/tools/dotc/Compiler.scala | 4 +-- .../tools/dotc/transform/CheckShadowing.scala | 32 +++++++++---------- .../tools/dotc/transform/CheckUnused.scala | 6 +--- tests/warn/i19657-mega.scala | 9 ++++++ 4 files changed, 26 insertions(+), 25 deletions(-) create mode 100644 tests/warn/i19657-mega.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index d8ba1ab5dc2e..b832b5e07019 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -7,7 +7,6 @@ import typer.{TyperPhase, RefChecks} import parsing.Parser import Phases.Phase import transform.* -import dotty.tools.backend import backend.jvm.{CollectSuperCalls, GenBCode} import localopt.StringInterpolatorOpt @@ -34,8 +33,7 @@ class Compiler { protected def frontendPhases: List[List[Phase]] = List(new Parser) :: // Compiler frontend: scanner, parser List(new TyperPhase) :: // Compiler frontend: namer, typer - List(new CheckUnused.PostTyper) :: // Check for unused elements - List(new CheckShadowing) :: // Check shadowing elements + List(new CheckUnused.PostTyper, new CheckShadowing) :: // Check for unused, shadowed elements List(new YCheckPositions) :: // YCheck positions List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks List(new semanticdb.ExtractSemanticDB.ExtractSemanticInfo) :: // Extract info into .semanticdb files diff --git a/compiler/src/dotty/tools/dotc/transform/CheckShadowing.scala b/compiler/src/dotty/tools/dotc/transform/CheckShadowing.scala index 87d652bd9133..795296874ad6 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckShadowing.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckShadowing.scala @@ -49,26 +49,22 @@ class CheckShadowing extends MiniPhase: override def description: String = CheckShadowing.description + override def isEnabled(using Context): Boolean = ctx.settings.Wshadow.value.nonEmpty + override def isRunnable(using Context): Boolean = - super.isRunnable && - ctx.settings.Wshadow.value.nonEmpty && - !ctx.isJava + super.isRunnable && ctx.settings.Wshadow.value.nonEmpty && !ctx.isJava - // Setup before the traversal override def prepareForUnit(tree: tpd.Tree)(using Context): Context = val data = ShadowingData() val fresh = ctx.fresh.setProperty(_key, data) shadowingDataApply(sd => sd.registerRootImports())(using fresh) - // Reporting on traversal's end override def transformUnit(tree: tpd.Tree)(using Context): tpd.Tree = shadowingDataApply(sd => reportShadowing(sd.getShadowingResult) ) tree - // MiniPhase traversal : - override def prepareForPackageDef(tree: tpd.PackageDef)(using Context): Context = shadowingDataApply(sd => sd.inNewScope()) ctx @@ -94,14 +90,14 @@ class CheckShadowing extends MiniPhase: if tree.symbol.isAliasType then // if alias, the parent is the current symbol nestedTypeTraverser(tree.symbol).traverse(tree.rhs) if tree.symbol.is(Param) then // if param, the parent is up - val owner = tree.symbol.owner - val parent = if (owner.isConstructor) then owner.owner else owner - nestedTypeTraverser(parent).traverse(tree.rhs)(using ctx.outer) - shadowingDataApply(sd => sd.registerCandidate(parent, tree)) + val enclosing = + val owner = tree.symbol.ownersIterator.dropWhile(_.is(Param)).next() + if owner.isConstructor then owner.owner else owner + nestedTypeTraverser(enclosing).traverse(tree.rhs)(using ctx.outer) + shadowingDataApply(_.registerCandidate(enclosing, tree)) else ctx - override def transformPackageDef(tree: tpd.PackageDef)(using Context): tpd.Tree = shadowingDataApply(sd => sd.outOfScope()) tree @@ -115,13 +111,16 @@ class CheckShadowing extends MiniPhase: tree override def transformTypeDef(tree: tpd.TypeDef)(using Context): tpd.Tree = - if tree.symbol.is(Param) && isValidTypeParamOwner(tree.symbol.owner) then // Do not register for constructors the work is done for the Class owned equivalent TypeDef - shadowingDataApply(sd => sd.computeTypeParamShadowsFor(tree.symbol.owner)(using ctx.outer)) - if tree.symbol.isAliasType then // No need to start outer here, because the TypeDef reached here it's already the parent + // Do not register for constructors the work is done for the Class owned equivalent TypeDef + if tree.symbol.is(Param) then + val owner = tree.symbol.ownersIterator.dropWhile(_.is(Param)).next() + if isValidTypeParamOwner(owner) then + shadowingDataApply(_.computeTypeParamShadowsFor(owner)(using ctx.outer)) + // No need to start outer here, because the TypeDef reached here it's already the parent + if tree.symbol.isAliasType then shadowingDataApply(sd => sd.computeTypeParamShadowsFor(tree.symbol)(using ctx)) tree - // Helpers : private def isValidTypeParamOwner(owner: Symbol)(using Context): Boolean = !owner.isConstructor && !owner.is(Synthetic) && !owner.is(Exported) @@ -310,4 +309,3 @@ object CheckShadowing: case class ShadowResult(warnings: List[ShadowWarning]) end CheckShadowing - diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 45afa1366a46..9950e77f9833 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -88,7 +88,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke tree override def transformApply(tree: Apply)(using Context): Tree = - //println(s"APPLY ${tree.show}") tree override def transformTyped(tree: Typed)(using Context): Tree = @@ -117,7 +116,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke override def prepareForTypeTree(tree: TypeTree)(using Context): Context = ctx override def transformTypeTree(tree: TypeTree)(using Context): tree.type = - //println(s"TYPETREE ${tree.getClass} ${tree.show} or $tree") tree.tpe match case AnnotatedType(_, annot) => transformAllDeep(annot.tree) case _ => @@ -134,7 +132,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke ud.addIgnoredUsage(tree.symbol) override def transformValDef(tree: ValDef)(using Context): tree.type = - //println(s"VAL ${tree.name} ${tree.show} tpt ${tree.tpt.show} is a ${tree.tpt.getClass}") preparing: traverseAnnotations(tree.symbol) if !tree.symbol.is(Module) then // do not register the ValDef generated for `object` @@ -209,8 +206,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke selector.bound match case untpd.TypedSplice(bound) => transformAllDeep(bound) case _ => - case _: InferredTypeTree => - //println(s"INF ${tree.getClass} ${tree.show}") case AppliedTypeTree(tpt, args) => transformAllDeep(tpt) args.foreach(transformAllDeep) @@ -227,6 +222,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke transformAllDeep(hi) transformAllDeep(alias) case tree: NamedArg => transformAllDeep(tree.arg) + case _: InferredTypeTree => case _ if tree.isType => //println(s"OTHER TYPE ${tree.getClass} ${tree.show}") case _ => diff --git a/tests/warn/i19657-mega.scala b/tests/warn/i19657-mega.scala new file mode 100644 index 000000000000..681db545920c --- /dev/null +++ b/tests/warn/i19657-mega.scala @@ -0,0 +1,9 @@ +//> using options -Wshadow:type-parameter-shadow -Wunused:all + +class F[X, M[N[X]]]: + private def x[X] = toString // warn // warn + +// the first is spurious +// at 3: Type parameter X for type M shadows the type defined by type X in class F +// at 4: Type parameter X for method x shadows the type defined by type X in class F +// at 4: unused private member From 9c00b44a39b5d81144dff051c9883e6a4cb33117 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Thu, 3 Oct 2024 20:34:38 -0700 Subject: [PATCH 13/30] Add test --- tests/warn/i15503a.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/warn/i15503a.scala b/tests/warn/i15503a.scala index df8691c21a13..ff7d054cfb5d 100644 --- a/tests/warn/i15503a.scala +++ b/tests/warn/i15503a.scala @@ -213,7 +213,8 @@ package testImportsInImports: package c: import a.b // OK import b.x // OK - val y = x + import b.x as z // OK + val y = x + z //------------------------------------- package testOnOverloadedMethodsImports: @@ -265,4 +266,4 @@ package foo.test.typeapply.hklamdba.i16680: import foo.IO // OK def f[F[_]]: String = "hello" - def go = f[IO] \ No newline at end of file + def go = f[IO] From 8ddf038b3d97b2e4c908592c9634f77a5d01b107 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 4 Oct 2024 01:14:57 -0700 Subject: [PATCH 14/30] More broken tests --- tests/warn/i19657.scala | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/warn/i19657.scala b/tests/warn/i19657.scala index 19ab2626f352..fb45bf642cb1 100644 --- a/tests/warn/i19657.scala +++ b/tests/warn/i19657.scala @@ -52,3 +52,27 @@ package i17156: import b.Xd trait Z derives Xd // checks if dealiased import is prefix a.Foo class Bar extends Xd[Int] // checks if import qual b is prefix of b.Xd + +object Coll: + class C: + type HM[K, V] = scala.collection.mutable.HashMap[K, V] +object CC extends Coll.C +import CC.* + +def `param type is imported`(map: HM[String, String]): Unit = println(map("hello, world")) + +object Constants: + final val i = 42 +def `old-style constants are usages`: Unit = + object Local: + final val j = 27 + import Constants.i + println(i + Local.j) + +class `scope of super`: + import Constants.i // bad warn + class C(x: Int): + def y = x + class D extends C(i): + import Constants.* // does not resolve i in C(i) + def m = i From 3a07f345bf5d5cbb8a18e545e00f9704ae6744b1 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Sun, 6 Oct 2024 13:02:02 -0700 Subject: [PATCH 15/30] Consider superclass context --- .../tools/dotc/transform/CheckUnused.scala | 42 +++++++++++-------- tests/warn/i19657.scala | 11 ++++- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 9950e77f9833..2979828171a0 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -72,19 +72,19 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke def loopOnNormalizedPrefixes(prefix: Type, depth: Int): Unit = // limit to 10 as failsafe for the odd case where there is an infinite cycle if depth < 10 && prefix.exists then - ud.registerUsed(prefix.classSymbol, name = None, prefix) + ud.registerUsed(prefix.classSymbol, name = None, prefix, tree = tree) loopOnNormalizedPrefixes(prefix.normalizedPrefix, depth + 1) val prefix = tree.typeOpt.normalizedPrefix loopOnNormalizedPrefixes(prefix, depth = 0) - ud.registerUsed(tree.symbol, Some(tree.name), tree.typeOpt.importPrefix.skipPackageObject) + ud.registerUsed(tree.symbol, Some(tree.name), tree.typeOpt.importPrefix.skipPackageObject, tree = tree) else if tree.hasType then - ud.registerUsed(tree.tpe.classSymbol, Some(tree.name), tree.tpe.importPrefix.skipPackageObject) + ud.registerUsed(tree.tpe.classSymbol, Some(tree.name), tree.tpe.importPrefix.skipPackageObject, tree = tree) tree override def transformSelect(tree: Select)(using Context): tree.type = preparing: val name = tree.removeAttachment(OriginalName) - ud.registerUsed(tree.symbol, name, tree.qualifier.tpe, includeForImport = tree.qualifier.span.isSynthetic) + ud.registerUsed(tree.symbol, name, tree.qualifier.tpe, includeForImport = tree.qualifier.span.isSynthetic, tree = tree) tree override def transformApply(tree: Apply)(using Context): Tree = @@ -144,7 +144,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke tree.getAttachment(OriginalTypeClass) match case Some(orig) => val (typsym, name, prefix) = core(orig) - ud.registerUsed(typsym, name, prefix.skipPackageObject) + ud.registerUsed(typsym, name, prefix.skipPackageObject, tree = EmptyTree) case _ => ud.removeIgnoredUsage(tree.symbol) tree @@ -176,7 +176,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke tree override def prepareForTemplate(tree: Template)(using Context): Context = - pushScope(tree) + pushScope(tree, tree.parents) override def transformTemplate(tree: Template)(using Context): Tree = popScope(tree) @@ -229,9 +229,9 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke //println(s"OTHER ${tree.getClass} ${tree.show}") tree - private def pushScope(tree: Block | Template | PackageDef)(using Context): Context = + private def pushScope(tree: Block | Template | PackageDef, parents: List[Tree] = Nil)(using Context): Context = preparing: - ud.pushScope(UnusedData.ScopeType.fromTree(tree)) + ud.pushScope(UnusedData.ScopeType.fromTree(tree), parents) private def popScope(tree: Block | Template | PackageDef)(using Context): Context = preparing: @@ -307,14 +307,17 @@ object CheckUnused: var unusedAggregate: Option[UnusedResult] = None - /* IMPORTS */ + // Trees of superclass constructors, i.e., template.parents when currScopeType is Template. + // Ideally, Context would supply correct context and scope; instead, trees in superclass context + // are promoted to "enclosing scope" by popScope. (This is just for import usage, so class params are ignored.) + private val parents = Stack(List.empty[Tree]) + private val impInScope = Stack(ListBuffer.empty[ImportSelectorData]) private val usedInScope = Stack(mut.Map.empty[Symbol, ListBuffer[Usage]]) private val usedInPosition = mut.Map.empty[Name, mut.Set[Symbol]] /* unused import collected during traversal */ private val unusedImport = ListBuffer.empty[ImportSelectorData] - /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */ private val localDefInScope = ListBuffer.empty[MemberDef] private val privateDefInScope = ListBuffer.empty[MemberDef] private val explicitParamInScope = ListBuffer.empty[MemberDef] @@ -355,11 +358,11 @@ object CheckUnused: * The optional name will be used to target the right import * as the same element can be imported with different renaming. */ - def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true)(using Context): Unit = + def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true, tree: Tree)(using Context): Unit = if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) && !doNotRegisterPrefix(prefix.typeSymbol) then if sym.isConstructor then // constructors are "implicitly" imported with the class - registerUsed(sym.owner, name = None, prefix, includeForImport = includeForImport) + registerUsed(sym.owner, name = None, prefix, includeForImport = includeForImport, tree = tree) else // If the symbol is accessible in this scope without an import, do not register it for unused import analysis val includeForImport1 = @@ -369,7 +372,7 @@ object CheckUnused: if sym.exists then usedDef += sym if includeForImport1 then - addUsage(Usage(sym, name, prefix)) + addUsage(Usage(sym, name, prefix, isSuper = !tree.isEmpty && parents.top.exists(t => t.find(_ eq tree).isDefined))) addIfExists(sym) addIfExists(sym.companionModule) addIfExists(sym.companionClass) @@ -379,7 +382,7 @@ object CheckUnused: def addUsage(usage: Usage)(using Context): Unit = val usages = usedInScope.top.getOrElseUpdate(usage.symbol, ListBuffer.empty) - if !usages.exists(x => x.name == usage.name && x.prefix =:= usage.prefix) + if !usages.exists(x => x.name == usage.name && x.prefix =:= usage.prefix && x.isSuper == usage.isSuper) then usages += usage /** Register a symbol that should be ignored */ @@ -437,10 +440,11 @@ object CheckUnused: patVarsInScope += patvar /** enter a new scope */ - def pushScope(newScopeType: ScopeType): Unit = + def pushScope(newScopeType: ScopeType, parents: List[Tree]): Unit = currScopeType.push(newScopeType) impInScope.push(ListBuffer.empty) usedInScope.push(mut.Map.empty) + this.parents.push(parents) def registerSetVar(sym: Symbol): Unit = setVars += sym @@ -453,7 +457,9 @@ object CheckUnused: for usedInfos <- usedInScope.pop().valuesIterator; usedInfo <- usedInfos do import usedInfo.* - selDatas.find(symbol.isInImport(_, name, prefix)) match + if isSuper then + addUsage(Usage(symbol, name, prefix, isSuper = false)) // approximate superclass context + else selDatas.find(symbol.isInImport(_, name, prefix)) match case Some(sel) => sel.markUsed() case None => @@ -465,6 +471,8 @@ object CheckUnused: for selData <- selDatas do if !selData.isUsed then unusedImport += selData + + this.parents.pop() end popScope /** Leave the scope and return a result set of warnings. @@ -746,7 +754,7 @@ object CheckUnused: /** A symbol usage includes the name under which it was observed, * and the prefix from which it was selected. */ - case class Usage(val symbol: Symbol, val name: Option[Name], val prefix: Type) + class Usage(val symbol: Symbol, val name: Option[Name], val prefix: Type, val isSuper: Boolean) end UnusedData extension (sym: Symbol) /** is accessible without import in current context */ diff --git a/tests/warn/i19657.scala b/tests/warn/i19657.scala index fb45bf642cb1..3e7a774ffc60 100644 --- a/tests/warn/i19657.scala +++ b/tests/warn/i19657.scala @@ -69,10 +69,17 @@ def `old-style constants are usages`: Unit = import Constants.i println(i + Local.j) +object Constantinople: + val k = 42 class `scope of super`: - import Constants.i // bad warn + import Constants.i // was bad warn class C(x: Int): def y = x - class D extends C(i): + class D(j: Int) extends C(i + j): import Constants.* // does not resolve i in C(i) def m = i + def f = + import Constantinople.* + class E(e: Int) extends C(i + k): + def g = e + y + k + 1 + E(0).g From f089c91131e1e5b489d54c2de1f8ccafc8ff88e6 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 8 Oct 2024 15:00:46 -0700 Subject: [PATCH 16/30] Filter member of refinement, handle Annotated --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 4 ++++ tests/pos/i17631.scala | 2 +- tests/warn/i19657.scala | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 2979828171a0..5289401fd90c 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -222,6 +222,9 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke transformAllDeep(hi) transformAllDeep(alias) case tree: NamedArg => transformAllDeep(tree.arg) + case Annotated(arg, annot) => + transformAllDeep(arg) + transformAllDeep(annot) case _: InferredTypeTree => case _ if tree.isType => //println(s"OTHER TYPE ${tree.getClass} ${tree.show}") @@ -684,6 +687,7 @@ object CheckUnused: && !memDef.symbol.isAllOf(Flags.AccessorCreationFlags) && !memDef.name.isWildcard && !memDef.symbol.owner.is(ExtensionMethod) + && !memDef.symbol.owner.isRefinementClass private def isValidParam(using Context): Boolean = val sym = memDef.symbol diff --git a/tests/pos/i17631.scala b/tests/pos/i17631.scala index 7b8a064493df..24858b166540 100644 --- a/tests/pos/i17631.scala +++ b/tests/pos/i17631.scala @@ -1,4 +1,4 @@ -//> using options -Xfatal-warnings -Wunused:all -deprecation -feature +//> using options -Werror -Wunused:all -deprecation -feature object foo { type Bar diff --git a/tests/warn/i19657.scala b/tests/warn/i19657.scala index 3e7a774ffc60..2b4032cfe032 100644 --- a/tests/warn/i19657.scala +++ b/tests/warn/i19657.scala @@ -83,3 +83,8 @@ class `scope of super`: class E(e: Int) extends C(i + k): def g = e + y + k + 1 E(0).g + +import scala.annotation.meta.* +object Alias { + type A = Deprecated @param +} From 1133e42996321a17b20734c64d4e16747c8f3cd8 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 8 Oct 2024 19:04:43 -0700 Subject: [PATCH 17/30] TypeTree is usage of simple name --- .../tools/dotc/transform/CheckUnused.scala | 4 +++ tests/warn/i19657.scala | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 5289401fd90c..160198dd37c5 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -118,6 +118,9 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke override def transformTypeTree(tree: TypeTree)(using Context): tree.type = tree.tpe match case AnnotatedType(_, annot) => transformAllDeep(annot.tree) + case tpt if tpt.typeSymbol.exists => + preparing: + ud.registerUsed(tpt.typeSymbol, Some(tpt.typeSymbol.name), tree = tree) // usage was a simple name case _ => tree @@ -405,6 +408,7 @@ object CheckUnused: !languageImport(imp.expr).nonEmpty && !imp.isGeneratedByEnum && !isTransparentAndInline(imp) + && !doNotRegisterPrefix(imp.expr.tpe.typeSymbol) && peekScopeType != ScopeType.ReplWrapper // #18383 Do not report top-level import's in the repl as unused then val qualTpe = imp.expr.tpe diff --git a/tests/warn/i19657.scala b/tests/warn/i19657.scala index 2b4032cfe032..06297cd60d5c 100644 --- a/tests/warn/i19657.scala +++ b/tests/warn/i19657.scala @@ -88,3 +88,28 @@ import scala.annotation.meta.* object Alias { type A = Deprecated @param } + +// avoid reporting on runtime (nothing to do with transparent inline) +import scala.runtime.EnumValue + +trait Lime + +enum Color(val rgb: Int): + case Red extends Color(0xFF0000) with EnumValue + case Green extends Color(0x00FF00) with Lime + case Blue extends Color(0x0000FF) + +object prefixes: + class C: + object N: + type U + object Test: + val c: C = ??? + def k2: c.N.U = ??? + import c.N.* + def k3: U = ??? // TypeTree if not a select + object Alt: + val c: C = ??? + import c.N + def k4: N.U = ??? +end prefixes From 9d1a40b03b91c080f57f6d770bba2a52d485bd8d Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 8 Oct 2024 19:48:05 -0700 Subject: [PATCH 18/30] Handle quotes and splices --- .../dotty/tools/dotc/transform/CheckUnused.scala | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 160198dd37c5..b8ac7894e5b1 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -228,6 +228,19 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke case Annotated(arg, annot) => transformAllDeep(arg) transformAllDeep(annot) + case Quote(body, tags) => + transformAllDeep(body) + tags.foreach(transformAllDeep) + case Splice(expr) => + transformAllDeep(expr) + case QuotePattern(bindings, body, quotes) => + bindings.foreach(transformAllDeep) + transformAllDeep(body) + transformAllDeep(quotes) + case SplicePattern(body, typeargs, args) => + transformAllDeep(body) + typeargs.foreach(transformAllDeep) + args.foreach(transformAllDeep) case _: InferredTypeTree => case _ if tree.isType => //println(s"OTHER TYPE ${tree.getClass} ${tree.show}") From 7cb02f167279140ee5de5c659c41d263764e250b Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 8 Oct 2024 22:19:34 -0700 Subject: [PATCH 19/30] Tighten allowance for serialization methods --- .../tools/dotc/transform/CheckUnused.scala | 9 ++++-- tests/warn/unused-privates.scala | 30 ++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index b8ac7894e5b1..d3f3b926ae3b 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -716,7 +716,7 @@ object CheckUnused: peekScopeType == ScopeType.Template && !memDef.symbol.isConstructor && memDef.symbol.is(Private, butNot = SelfName | Synthetic | CaseAccessor) - && !ignoredNames(memDef.symbol.name.toTermName) + && !ignoredSignature(memDef.symbol) private def isUnsetVarDef(using Context): Boolean = val sym = memDef.symbol @@ -747,7 +747,12 @@ object CheckUnused: case _: Block => Local case _ => Other - val ignoredNames: Set[TermName] = Set("readResolve", "readObject", "writeObject", "writeReplace").map(termName(_)) + val ignoredNames: Set[TermName] = + Set("readResolve", "readObject", "readObjectNoData", "writeObject", "writeReplace").map(termName(_)) + + def ignoredSignature(m: Symbol)(using Context): Boolean = + m.is(Method) && ignoredNames(m.name.toTermName) && m.owner.isClass + && m.owner.asClass.classDenot.parentSyms.contains(defn.JavaSerializableClass) final case class ImportSelectorData(val qualTpe: Type, val selector: ImportSelector): private var myUsed: Boolean = false diff --git a/tests/warn/unused-privates.scala b/tests/warn/unused-privates.scala index e30c00907b4c..913a6e206ccb 100644 --- a/tests/warn/unused-privates.scala +++ b/tests/warn/unused-privates.scala @@ -187,7 +187,7 @@ trait Forever { } trait Ignorance { - private val readResolve = 42 // no warn special members + private val readResolve = 42 // warn wrong signatured for special members } trait CaseyKasem { @@ -257,3 +257,31 @@ class `recursive reference is not a usage` { def f() = new P() } } + +class `absolve serial framework` extends Serializable: + import java.io.{IOException, ObjectInputStream, ObjectOutputStream, ObjectStreamException} + @throws(classOf[IOException]) + private def writeObject(stream: ObjectOutputStream): Unit = () + @throws(classOf[ObjectStreamException]) + private def writeReplace(): Object = ??? + @throws(classOf[ClassNotFoundException]) + @throws(classOf[IOException]) + private def readObject(stream: ObjectInputStream): Unit = () + @throws(classOf[ObjectStreamException]) + private def readObjectNoData(): Unit = () + @throws(classOf[ObjectStreamException]) + private def readResolve(): Object = ??? + +class `absolve ONLY serial framework`: + import java.io.{IOException, ObjectInputStream, ObjectOutputStream, ObjectStreamException} + @throws(classOf[IOException]) + private def writeObject(stream: ObjectOutputStream): Unit = () // warn + @throws(classOf[ObjectStreamException]) + private def writeReplace(): Object = ??? // warn + @throws(classOf[ClassNotFoundException]) + @throws(classOf[IOException]) + private def readObject(stream: ObjectInputStream): Unit = () // warn + @throws(classOf[ObjectStreamException]) + private def readObjectNoData(): Unit = () // warn + @throws(classOf[ObjectStreamException]) + private def readResolve(): Object = ??? // warn From 75ea482f7935acad021d73dee339950567278751 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 8 Oct 2024 23:08:48 -0700 Subject: [PATCH 20/30] Restore inferred type is not a usage, noprefix is in import --- .../tools/dotc/transform/CheckUnused.scala | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index d3f3b926ae3b..d3c5b7959a29 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -12,7 +12,7 @@ import dotty.tools.dotc.core.Denotations.SingleDenotation import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames.nme -import dotty.tools.dotc.core.Types.{AnnotatedType, ClassInfo, ConstantType, NamedType, NoType, TermRef, Type, TypeProxy, TypeTraverser} +import dotty.tools.dotc.core.Types.{AnnotatedType, ClassInfo, ConstantType, NamedType, NoPrefix, NoType, TermRef, Type, TypeProxy, TypeTraverser} import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Names.{Name, TermName, termName} import dotty.tools.dotc.core.NameOps.isReplWrapperName @@ -87,14 +87,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke ud.registerUsed(tree.symbol, name, tree.qualifier.tpe, includeForImport = tree.qualifier.span.isSynthetic, tree = tree) tree - override def transformApply(tree: Apply)(using Context): Tree = - tree - - override def transformTyped(tree: Typed)(using Context): Tree = - tree match - case Typed(expr, tpt) => - tree - override def transformAssign(tree: Assign)(using Context): tree.type = preparing: val sym = tree.lhs.symbol @@ -113,12 +105,10 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke transformAllDeep(tree.call) tree - override def prepareForTypeTree(tree: TypeTree)(using Context): Context = ctx - override def transformTypeTree(tree: TypeTree)(using Context): tree.type = tree.tpe match case AnnotatedType(_, annot) => transformAllDeep(annot.tree) - case tpt if tpt.typeSymbol.exists => + case tpt if !tree.isInferred && tpt.typeSymbol.exists => preparing: ud.registerUsed(tpt.typeSymbol, Some(tpt.typeSymbol.name), tree = tree) // usage was a simple name case _ => @@ -192,12 +182,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke popScope(tree) tree - override def prepareForStats(trees: List[Tree])(using Context): Context = ctx - - override def transformStats(trees: List[Tree])(using Context): List[Tree] = - super.transformStats(trees) - trees - override def transformOther(tree: Tree)(using Context): tree.type = preparing: tree match @@ -377,7 +361,7 @@ object CheckUnused: * The optional name will be used to target the right import * as the same element can be imported with different renaming. */ - def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoType, includeForImport: Boolean = true, tree: Tree)(using Context): Unit = + def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoPrefix, includeForImport: Boolean = true, tree: Tree)(using Context): Unit = if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) && !doNotRegisterPrefix(prefix.typeSymbol) then if sym.isConstructor then // constructors are "implicitly" imported with the class @@ -647,7 +631,7 @@ object CheckUnused: } else !altName.exists(_.toTermName != selector.rename) && // if there is an explicit name, it must match - selData.qualTpe =:= prefix && selData.allSymbolsForNamed.contains(sym) + (prefix.eq(NoPrefix) || selData.qualTpe =:= prefix) && selData.allSymbolsForNamed.contains(sym) end isInImport /** Annotated with @unused */ From 9467cda63af326bc9806a0a52cfb84cdb3e32363 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 9 Oct 2024 02:30:27 -0700 Subject: [PATCH 21/30] Warn for top level private --- .../src/dotty/tools/dotc/transform/CheckUnused.scala | 8 +++++--- tests/warn/unused-privates.scala | 10 ++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index d3c5b7959a29..1af1f9909c29 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -697,10 +697,12 @@ object CheckUnused: !sym.shouldNotReportParamOwner private def shouldReportPrivateDef(using Context): Boolean = + val sym = memDef.symbol peekScopeType == ScopeType.Template - && !memDef.symbol.isConstructor - && memDef.symbol.is(Private, butNot = SelfName | Synthetic | CaseAccessor) - && !ignoredSignature(memDef.symbol) + && !sym.isConstructor + && (sym.is(Private, butNot = SelfName | Synthetic | CaseAccessor) + || sym.effectiveOwner.is(PackageClass) && sym.denot.privateWithin == sym.effectiveOwner) + && !ignoredSignature(sym) private def isUnsetVarDef(using Context): Boolean = val sym = memDef.symbol diff --git a/tests/warn/unused-privates.scala b/tests/warn/unused-privates.scala index 913a6e206ccb..099f32fa65e3 100644 --- a/tests/warn/unused-privates.scala +++ b/tests/warn/unused-privates.scala @@ -285,3 +285,13 @@ class `absolve ONLY serial framework`: private def readObjectNoData(): Unit = () // warn @throws(classOf[ObjectStreamException]) private def readResolve(): Object = ??? // warn + +@throws(classOf[java.io.ObjectStreamException]) +private def readResolve(): Object = ??? // warn +private def print() = println() // warn +private val printed = false // warn + +package locked: + private[locked] def locker(): Unit = () // warn as we cannot distinguish unqualified private at top level + package basement: + private[locked] def shackle(): Unit = () // no warn as it is not top level at boundary From db2aec16588e84538c61d3baa51c246a554edb54 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 9 Oct 2024 09:59:37 -0700 Subject: [PATCH 22/30] Regression tests --- tests/pos/i17631.scala | 6 ++++++ tests/warn/unused-privates.scala | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/tests/pos/i17631.scala b/tests/pos/i17631.scala index 24858b166540..0ede23dd5524 100644 --- a/tests/pos/i17631.scala +++ b/tests/pos/i17631.scala @@ -32,3 +32,9 @@ object Main { (bad1, bad2) } } + +def `i18388`: Unit = + def func(pred: [A] => A => Boolean): Unit = + val _ = pred + () + val _ = func diff --git a/tests/warn/unused-privates.scala b/tests/warn/unused-privates.scala index 099f32fa65e3..d9e60f04f5cb 100644 --- a/tests/warn/unused-privates.scala +++ b/tests/warn/unused-privates.scala @@ -295,3 +295,12 @@ package locked: private[locked] def locker(): Unit = () // warn as we cannot distinguish unqualified private at top level package basement: private[locked] def shackle(): Unit = () // no warn as it is not top level at boundary + +object `i19998 refinement`: + trait Foo { + type X[a] + } + trait Bar[X[_]] { + private final type SelfX[a] = X[a] // was false positive + val foo: Foo { type X[a] = SelfX[a] } + } From 12f75a9f9cbc3eb29b5bcd3e9b1ca0fc37722948 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Wed, 9 Oct 2024 11:04:51 -0700 Subject: [PATCH 23/30] Import given takes NoPrefix usage --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 1af1f9909c29..e3579b01bdaa 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -625,7 +625,7 @@ object CheckUnused: if selector.isGiven then // Further check that the symbol is a given or implicit and conforms to the bound sym.isOneOf(Given | Implicit) && (selector.bound.isEmpty || sym.info.finalResultType <:< selector.boundTpe) - && selData.qualTpe =:= prefix + && (prefix.eq(NoPrefix) || selData.qualTpe =:= prefix) else !sym.is(Given) // Normal wildcard, check that the symbol is not a given (but can be implicit) } From 1d4db7fd65a1376ce71086a1a7fc5c524ee3a233 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 11 Oct 2024 16:23:06 -0700 Subject: [PATCH 24/30] Regression tests --- tests/pos/i17631.scala | 14 ++++++++++++++ tests/pos/i18366.scala | 15 ++++++++++++--- tests/warn/i17371.scala | 28 ++++++++++++++++++++++++++++ tests/warn/i18313.scala | 14 ++++++++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 tests/warn/i17371.scala create mode 100644 tests/warn/i18313.scala diff --git a/tests/pos/i17631.scala b/tests/pos/i17631.scala index 0ede23dd5524..ddcb71354968 100644 --- a/tests/pos/i17631.scala +++ b/tests/pos/i17631.scala @@ -38,3 +38,17 @@ def `i18388`: Unit = val _ = pred () val _ = func + +trait L[T]: + type E + +def `i19748` = + type Warn1 = [T] => (l: L[T]) => T => l.E + type Warn2 = [T] => L[T] => T + type Warn3 = [T] => T => T + def use(x: (Warn1, Warn2, Warn3)) = x + use + +type NoWarning1 = [T] => (l: L[T]) => T => l.E +type NoWarning2 = [T] => L[T] => T +type NoWarning3 = [T] => T => T diff --git a/tests/pos/i18366.scala b/tests/pos/i18366.scala index 698510ad13a2..b6385b5bbb59 100644 --- a/tests/pos/i18366.scala +++ b/tests/pos/i18366.scala @@ -1,10 +1,19 @@ -//> using options -Xfatal-warnings -Wunused:all +//> using options -Werror -Wunused:all trait Builder { def foo(): Unit } -def repro = +def `i18366` = val builder: Builder = ??? import builder.{foo => bar} - bar() \ No newline at end of file + bar() + +import java.io.DataOutputStream + +val buffer: DataOutputStream = ??? + +import buffer.{write => put} + +def `i17315` = + put(0: Byte) diff --git a/tests/warn/i17371.scala b/tests/warn/i17371.scala new file mode 100644 index 000000000000..9e1c26368b8f --- /dev/null +++ b/tests/warn/i17371.scala @@ -0,0 +1,28 @@ +//> using options -Wunused:all + +class A +class B + +def Test() = + val ordA: Ordering[A] = ??? + val ordB: Ordering[B] = ??? + val a: A = ??? + val b: B = ??? + + import ordA.given + val _ = a > a + + import ordB.given + val _ = b < b + +// unminimized OP +trait Circular[T] extends Ordering[T] +trait Turns[C: Circular, T] extends Ordering[T]: + extension (turns: T) def extract: C + +def f[K, T](start: T, end: T)(using circular: Circular[K], turns: Turns[K, T]): Boolean = + import turns.given + if start > end then throw new IllegalArgumentException("start must be <= end") + + import circular.given + start.extract < end.extract diff --git a/tests/warn/i18313.scala b/tests/warn/i18313.scala new file mode 100644 index 000000000000..967985853fcf --- /dev/null +++ b/tests/warn/i18313.scala @@ -0,0 +1,14 @@ +//> using options -Wunused:imports + +import scala.deriving.Mirror + +case class Test(i: Int, d: Double) +case class Decoder(d: Product => Test) + +// OK, no warning returned +//val ok = Decoder(summon[Mirror.Of[Test]].fromProduct) +// +// returns warning: +// [warn] unused import +// [warn] import scala.deriving.Mirror +val d = Decoder(d = summon[Mirror.Of[Test]].fromProduct) // no warn From 882cb84ea303405e3a1a6a7de4a880b37d2ae033 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 14 Oct 2024 14:06:43 -0700 Subject: [PATCH 25/30] Don't ignore params of public methods --- .../tools/dotc/transform/CheckUnused.scala | 7 +- tests/warn/i15503e.scala | 6 +- tests/warn/i15503h.scala | 8 +- tests/warn/i15503i.scala | 10 +- tests/warn/unused-locals.scala | 43 +++++ tests/warn/unused-params.scala | 153 ++++++++++++++++++ 6 files changed, 212 insertions(+), 15 deletions(-) create mode 100644 tests/warn/unused-locals.scala create mode 100644 tests/warn/unused-params.scala diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index e3579b01bdaa..7138c332b38d 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -145,8 +145,6 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke override def prepareForDefDef(tree: DefDef)(using Context): Context = preparing: ud.registerTrivial(tree) - if !tree.symbol.is(Private) then - tree.termParamss.flatten.foreach(p => ud.addIgnoredParam(p.symbol)) ud.addIgnoredUsage(tree.symbol) override def transformDefDef(tree: DefDef)(using Context): tree.type = @@ -571,10 +569,11 @@ object CheckUnused: private def isConstructorOfSynth(sym: Symbol)(using Context): Boolean = sym.exists && sym.isConstructor && sym.owner.isPackageObject && sym.owner.is(Synthetic) - /** This is used to avoid reporting the parameters of the synthetic main method generated by `@main`. + /** Avoid reporting unused parameter in required main method signature. + * The method may be user-written or generated by `@main`. */ private def isSyntheticMainParam(sym: Symbol)(using Context): Boolean = - sym.exists && ctx.platform.isMainMethod(sym.owner) && sym.owner.is(Synthetic) + sym.exists && ctx.platform.isMainMethod(sym.owner) //&& sym.owner.is(Synthetic) /** If -Wunused:strict-no-implicit-warn import and this import selector could potentially import implicit. */ diff --git a/tests/warn/i15503e.scala b/tests/warn/i15503e.scala index 46d73a4945cd..32267049b3de 100644 --- a/tests/warn/i15503e.scala +++ b/tests/warn/i15503e.scala @@ -1,4 +1,4 @@ -//> using options -Wunused:explicits +//> using options -Wunused:explicits object Foo { /* This goes around the "trivial method" detection */ @@ -31,7 +31,7 @@ package scala3main: package foo.test.lambda.param: val default_val = 1 val a = (i: Int) => i // OK - val b = (i: Int) => default_val // OK + val b = (i: Int) => default_val // warn val c = (_: Int) => default_val // OK package foo.test.trivial: @@ -67,4 +67,4 @@ package foo.test.i16865: def fn(a: Int, b: Int): Int = b + 3 // OK object Ex2 extends Bar: - override def fn(a: Int, b: Int): Int = b + 3 // OK \ No newline at end of file + override def fn(a: Int, b: Int): Int = b + 3 // OK diff --git a/tests/warn/i15503h.scala b/tests/warn/i15503h.scala index 854693981488..a2bf47c843dd 100644 --- a/tests/warn/i15503h.scala +++ b/tests/warn/i15503h.scala @@ -1,4 +1,4 @@ -//> using options -Wunused:linted +//> using options -Wunused:linted import collection.mutable.Set // warn @@ -7,7 +7,7 @@ class A { val b = 2 // OK private def c = 2 // warn - def d(using x:Int): Int = b // ok + def d(using x: Int): Int = b // warn def e(x: Int) = 1 // OK def f = val x = 1 // warn @@ -15,6 +15,6 @@ class A { 3 def g(x: Int): Int = x match - case x:1 => 0 // OK + case x: 1 => 0 // OK case _ => 1 -} \ No newline at end of file +} diff --git a/tests/warn/i15503i.scala b/tests/warn/i15503i.scala index d3701b696009..257447704cfb 100644 --- a/tests/warn/i15503i.scala +++ b/tests/warn/i15503i.scala @@ -17,10 +17,12 @@ class A { private def c2 = 2 // OK def c3 = c2 - def d1(using x:Int): Int = default_int // ok - def d2(using x:Int): Int = x // OK + def d1(using x: Int): Int = default_int // warn param + def d2(using x: Int): Int = x // OK + def d3(using Int): Int = summon[Int] // OK + def d4(using Int): Int = default_int // warn - def e1(x: Int) = default_int // ok + def e1(x: Int) = default_int // warn param def e2(x: Int) = x // OK def f = val x = 1 // warn @@ -44,7 +46,7 @@ package foo.test.scala.annotation: val default_int = 12 def a1(a: Int) = a // OK - def a2(a: Int) = default_int // ok + def a2(a: Int) = default_int // warn def a3(@unused a: Int) = default_int //OK diff --git a/tests/warn/unused-locals.scala b/tests/warn/unused-locals.scala new file mode 100644 index 000000000000..10bf160fb717 --- /dev/null +++ b/tests/warn/unused-locals.scala @@ -0,0 +1,43 @@ +//> using options -Wunused:locals + +class Outer { + class Inner +} + +trait Locals { + def f0 = { + var x = 1 // warn + var y = 2 // no warn + y = 3 + y + y + } + def f1 = { + val a = new Outer // no warn + val b = new Outer // warn + new a.Inner + } + def f2 = { + var x = 100 // warn about it being a var + x + } +} + +object Types { + def l1() = { + object HiObject { def f = this } // warn + class Hi { // warn + def f1: Hi = new Hi + def f2(x: Hi) = x + } + class DingDongDoobie // warn + class Bippy // no warn + type Something = Bippy // no warn + type OtherThing = String // warn + (new Bippy): Something + } +} + +// breakage: local val x$1 in method skolemize is never used +case class SymbolKind(accurate: String, sanitized: String, abbreviation: String) { + def skolemize: SymbolKind = copy(accurate = s"$accurate skolem", abbreviation = s"$abbreviation#SKO") +} diff --git a/tests/warn/unused-params.scala b/tests/warn/unused-params.scala new file mode 100644 index 000000000000..81229f6b45e5 --- /dev/null +++ b/tests/warn/unused-params.scala @@ -0,0 +1,153 @@ +//> using options -Wunused:params -Werror +// + +import Answers._ + +trait InterFace { + /** Call something. */ + def call(a: Int, b: String, c: Double): Int +} + +trait BadAPI extends InterFace { + def f(a: Int, + b: String, // warn + c: Double): Int = { + println(c) + a + } + @deprecated("no warn in deprecated API", since="yesterday") + def g(a: Int, + b: String, // no warn + c: Double): Int = { + println(c) + a + } + override def call(a: Int, + b: String, // no warn, required by superclass + c: Double): Int = { + println(c) + a + } + + def meth(x: Int) = x + + override def equals(other: Any): Boolean = true // no warn + + def i(implicit s: String) = answer // yes, warn + + /* + def future(x: Int): Int = { + val y = 42 + val x = y // maybe option to warn only if shadowed + x + } + */ +} + +// mustn't alter warnings in super +trait PoorClient extends BadAPI { + override def meth(x: Int) = ??? // no warn + override def f(a: Int, b: String, c: Double): Int = a + b.toInt + c.toInt +} + +class Unusing(u: Int) { // warn + def f = ??? +} + +class Valuing(val u: Int) // no warn + +class Revaluing(u: Int) { def f = u } // no warn + +case class CaseyKasem(k: Int) // no warn + +case class CaseyAtTheBat(k: Int)(s: String) // warn + +trait Ignorance { + def f(readResolve: Int) = answer // warn +} + +class Reusing(u: Int) extends Unusing(u) // no warn + +class Main { + def main(args: Array[String]): Unit = println("hello, args") // no warn +} + +trait Unimplementation { + def f(u: Int): Int = ??? // no warn for param in unimplementation +} + +trait DumbStuff { + def f(implicit dummy: DummyImplicit) = answer + def g(dummy: DummyImplicit) = answer +} +trait Proofs { + def f[A, B](implicit ev: A =:= B) = answer + def g[A, B](implicit ev: A <:< B) = answer + def f2[A, B](ev: A =:= B) = answer + def g2[A, B](ev: A <:< B) = answer +} + +trait Anonymous { + def f = (i: Int) => answer // warn + + def f1 = (_: Int) => answer // no warn underscore parameter (a fresh name) + + def f2: Int => Int = _ + 1 // no warn placeholder syntax (a fresh name and synthetic parameter) + + def g = for (i <- List(1)) yield answer // no warn patvar elaborated as map.(i => 42) +} +trait Context[A] { def m(a: A): A = a } +trait Implicits { + def f[A](implicit ctx: Context[A]) = answer + def g[A: Context] = answer +} +class Bound[A: Context] +object Answers { + def answer: Int = 42 +} + +trait BadMix { _: InterFace => + def f(a: Int, + b: String, // warn + c: Double): Int = { + println(c) + a + } + @deprecated("no warn in deprecated API", since="yesterday") + def g(a: Int, + b: String, // no warn + c: Double): Int = { + println(c) + a + } + override def call(a: Int, + b: String, // no warn, required by superclass + c: Double): Int = { + println(c) + a + } + + def meth(x: Int) = x + + override def equals(other: Any): Boolean = true // no warn + + def i(implicit s: String) = answer // yes, warn +} + +class Unequal { + override def equals(other: Any) = toString.nonEmpty // no warn non-trivial RHS, required by universal method +} + +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 +} From e8ca0cc981337cc681da15a798956221aa74152b Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Mon, 14 Oct 2024 22:44:03 -0700 Subject: [PATCH 26/30] Attempt to ignore subtrees --- .../tools/dotc/transform/CheckUnused.scala | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 7138c332b38d..880ee68ca401 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -101,8 +101,16 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke popScope(tree) tree + // Megaphase will feed us Inlined(call, bindings, expansion) bindings and expansion, + // but we care about call only. + override def prepareForInlined(tree: Inlined)(using Context): Context = + preparing: + ud.exclude.top.addAll(tree.bindings) + ud.exclude.top.addOne(tree.expansion) + override def transformInlined(tree: Inlined)(using Context): tree.type = - transformAllDeep(tree.call) + if phaseMode == PhaseMode.Aggregate then + transformAllDeep(tree.call) tree override def transformTypeTree(tree: TypeTree)(using Context): tree.type = @@ -343,6 +351,8 @@ object CheckUnused: private val paramsToSkip = mut.Set.empty[Symbol] + val exclude = Stack(ListBuffer.empty[Tree]) + def finishAggregation(using Context)(): Unit = unusedAggregate = unusedAggregate match case None => @@ -360,7 +370,9 @@ object CheckUnused: * as the same element can be imported with different renaming. */ def registerUsed(sym: Symbol, name: Option[Name], prefix: Type = NoPrefix, includeForImport: Boolean = true, tree: Tree)(using Context): Unit = - if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) && !doNotRegisterPrefix(prefix.typeSymbol) then + if sym.exists && !isConstructorOfSynth(sym) && !doNotRegister(sym) && !doNotRegisterPrefix(prefix.typeSymbol) + && !exclude.top.exists(_ eq tree) + then if sym.isConstructor then // constructors are "implicitly" imported with the class registerUsed(sym.owner, name = None, prefix, includeForImport = includeForImport, tree = tree) @@ -447,6 +459,7 @@ object CheckUnused: impInScope.push(ListBuffer.empty) usedInScope.push(mut.Map.empty) this.parents.push(parents) + exclude.push(ListBuffer.empty) def registerSetVar(sym: Symbol): Unit = setVars += sym @@ -475,6 +488,7 @@ object CheckUnused: unusedImport += selData this.parents.pop() + exclude.pop() end popScope /** Leave the scope and return a result set of warnings. @@ -594,9 +608,10 @@ object CheckUnused: || imp.expr.tpe.member(sel.name.toTermName).alternatives.exists(p => derivesFromCanEqual(p.symbol)) - /** Ignore definitions of CanEqual given. + /** Ignore definitions of CanEqual given, and ignore certain other trees. */ - private def isDefIgnored(memDef: MemberDef)(using Context): Boolean = derivesFromCanEqual(memDef.symbol) + private def isDefIgnored(tree: Tree)(using Context): Boolean = + exclude.top.exists(t => t.find(_ eq tree).isDefined) || derivesFromCanEqual(tree.symbol) extension (sel: ImportSelector) def boundTpe: Type = sel.bound match From 19077758846504de55cb2c2132116def7291fa34 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 5 Nov 2024 12:16:48 -0800 Subject: [PATCH 27/30] Enable more param warnings --- compiler/src/dotty/tools/dotc/Compiler.scala | 2 +- .../dotty/tools/dotc/core/Definitions.scala | 2 ++ .../tools/dotc/transform/CheckUnused.scala | 30 +++++++++++----- tests/warn/i15503b.scala | 4 +-- tests/warn/i15503e.scala | 4 +-- tests/warn/i15503f.scala | 4 +-- tests/warn/i15503g.scala | 6 ++-- tests/warn/unused-params.scala | 34 +++++++++---------- tests/warn/unused-privates.scala | 4 +-- 9 files changed, 53 insertions(+), 37 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index b832b5e07019..45d58d177712 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -48,10 +48,10 @@ class Compiler { List(new sbt.ExtractAPI) :: // Sends a representation of the API of classes to sbt via callbacks List(new Inlining) :: // Inline and execute macros List(new PostInlining) :: // Add mirror support for inlined code - List(new CheckUnused.PostInlining) :: // Check for unused elements List(new Staging) :: // Check staging levels and heal staged types List(new Splicing) :: // Replace level 1 splices with holes List(new PickleQuotes) :: // Turn quoted trees into explicit run-time data structures + List(new CheckUnused.PostInlining) :: // Check for unused elements Nil /** Phases dealing with the transformation from pickled trees to backend trees */ diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 0195a4ddbf34..6ae1435cca7f 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -494,6 +494,8 @@ class Definitions { @tu lazy val Predef_undefined: Symbol = ScalaPredefModule.requiredMethod(nme.???) @tu lazy val ScalaPredefModuleClass: ClassSymbol = ScalaPredefModule.moduleClass.asClass + @tu lazy val SameTypeClass: ClassSymbol = requiredClass("scala.=:=") + @tu lazy val SameType_refl: Symbol = SameTypeClass.companionModule.requiredMethod(nme.refl) @tu lazy val SubTypeClass: ClassSymbol = requiredClass("scala.<:<") @tu lazy val SubType_refl: Symbol = SubTypeClass.companionModule.requiredMethod(nme.refl) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 880ee68ca401..e7c03a63a628 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -16,7 +16,7 @@ import dotty.tools.dotc.core.Types.{AnnotatedType, ClassInfo, ConstantType, Name import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Names.{Name, TermName, termName} import dotty.tools.dotc.core.NameOps.isReplWrapperName -import dotty.tools.dotc.core.NameKinds.WildcardParamName +import dotty.tools.dotc.core.NameKinds.{ContextFunctionParamName, WildcardParamName} import dotty.tools.dotc.core.Symbols.{NoSymbol, Symbol, defn, isDeprecated} import dotty.tools.dotc.report import dotty.tools.dotc.reporting.{Message, UnusedSymbol as UnusedSymbolMessage} @@ -517,13 +517,25 @@ object CheckUnused: warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.LocalDefs)) if ctx.settings.WunusedHas.explicits then + def forgiven(sym: Symbol) = + containsSyntheticSuffix(sym) + || sym.owner.hasAnnotation(defn.UnusedAnnot) + || sym.info.isSingleton for d <- explicitParamInScope do - if !d.symbol.usedDefContains && !isUsedInPosition(d.symbol.name, d.span) && !containsSyntheticSuffix(d.symbol) then + if !d.symbol.usedDefContains && !isUsedInPosition(d.symbol.name, d.span) && !forgiven(d.symbol) then warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.ExplicitParams)) if ctx.settings.WunusedHas.implicits then + def forgiven(sym: Symbol) = + val dd = defn + sym.name.is(ContextFunctionParamName) + || sym.owner.hasAnnotation(defn.UnusedAnnot) + || sym.info.typeSymbol.match + case dd.DummyImplicitClass | dd.SubTypeClass | dd.SameTypeClass => true + case _ => false + || sym.info.isSingleton for d <- implicitParamInScope do - if !d.symbol.usedDefContains && !containsSyntheticSuffix(d.symbol) then + if !d.symbol.usedDefContains && !forgiven(d.symbol) then warnings.addOne(UnusedSymbol(d.namePos, d.name, WarnTypes.ImplicitParams)) // Partition to extract unset private variables from usedPrivates @@ -556,7 +568,9 @@ object CheckUnused: /** Heuristic to detect synthetic suffixes in names of symbols. */ private def containsSyntheticSuffix(symbol: Symbol)(using Context): Boolean = - symbol.name.mangledString.contains("$") + val mangled = symbol.name.mangledString + val index = mangled.indexOf('$') + index >= 0 && !(index == mangled.length - 1 && symbol.is(Module)) /** * Is the constructor of synthetic package object @@ -659,8 +673,7 @@ object CheckUnused: owner.isPrimaryConstructor || owner.isDeprecated || owner.isAllOf(Synthetic | PrivateLocal) || - owner.is(Accessor) || - owner.isOverridden + owner.is(Accessor) } private def usedDefContains(using Context): Boolean = @@ -669,9 +682,10 @@ object CheckUnused: private def everySymbol(using Context): List[Symbol] = List(sym, sym.companionClass, sym.companionModule, sym.moduleClass).filter(_.exists) - /** A function is overridden. Either has `override flags` or parent has a matching member (type and name) */ + /** A function is overridden. Either has `override flags` or parent has a matching member (type and name) private def isOverridden(using Context): Boolean = sym.is(Flags.Override) || (sym.exists && sym.owner.thisType.parents.exists(p => sym.matchingMember(p).exists)) + */ end extension @@ -688,7 +702,7 @@ object CheckUnused: case ConstantType(_) => true case tp: TermRef => // Detect Scala 2 SingleType - tp.underlying.classSymbol.is(Flags.Module) + tp.underlying.classSymbol.is(Module) case _ => false def registerTrivial(using Context): Unit = diff --git a/tests/warn/i15503b.scala b/tests/warn/i15503b.scala index 7ab86026ff00..0ea110f833f1 100644 --- a/tests/warn/i15503b.scala +++ b/tests/warn/i15503b.scala @@ -117,7 +117,7 @@ package foo.scala2.tests: object Types { def l1() = { - object HiObject { def f = this } // OK + object HiObject { def f = this } // warn class Hi { // warn def f1: Hi = new Hi def f2(x: Hi) = x @@ -141,4 +141,4 @@ package test.foo.twisted.i16682: } isInt - def f = myPackage("42") \ No newline at end of file + def f = myPackage("42") diff --git a/tests/warn/i15503e.scala b/tests/warn/i15503e.scala index 32267049b3de..01d4ddd266d6 100644 --- a/tests/warn/i15503e.scala +++ b/tests/warn/i15503e.scala @@ -64,7 +64,7 @@ package foo.test.i16865: trait Bar extends Foo object Ex extends Bar: - def fn(a: Int, b: Int): Int = b + 3 // OK + def fn(a: Int, b: Int): Int = b + 3 // warn object Ex2 extends Bar: - override def fn(a: Int, b: Int): Int = b + 3 // OK + override def fn(a: Int, b: Int): Int = b + 3 // warn diff --git a/tests/warn/i15503f.scala b/tests/warn/i15503f.scala index ccf0b7e74065..4656771980ae 100644 --- a/tests/warn/i15503f.scala +++ b/tests/warn/i15503f.scala @@ -6,8 +6,8 @@ val default_int = 1 object Xd { private def f1(a: Int) = a // OK private def f2(a: Int) = 1 // OK - private def f3(a: Int)(using Int) = a // OK - private def f4(a: Int)(using Int) = default_int // OK + private def f3(a: Int)(using Int) = a // warn + private def f4(a: Int)(using Int) = default_int // warn private def f6(a: Int)(using Int) = summon[Int] // OK private def f7(a: Int)(using Int) = summon[Int] + a // OK private def f8(a: Int)(using foo: Int) = a // warn diff --git a/tests/warn/i15503g.scala b/tests/warn/i15503g.scala index fbd9f3c1352c..13f8b56b8654 100644 --- a/tests/warn/i15503g.scala +++ b/tests/warn/i15503g.scala @@ -6,8 +6,8 @@ object Foo { private def f1(a: Int) = a // OK private def f2(a: Int) = default_int // warn - private def f3(a: Int)(using Int) = a // OK - private def f4(a: Int)(using Int) = default_int // warn + private def f3(a: Int)(using Int) = a // warn + private def f4(a: Int)(using Int) = default_int // warn // warn private def f6(a: Int)(using Int) = summon[Int] // warn private def f7(a: Int)(using Int) = summon[Int] + a // OK /* --- Trivial method check --- */ @@ -20,4 +20,4 @@ package foo.test.i17101: extension[A] (x: Test[A]) { // OK def value: A = x def causesIssue: Unit = println("oh no") - } \ No newline at end of file + } diff --git a/tests/warn/unused-params.scala b/tests/warn/unused-params.scala index 81229f6b45e5..e157764b62dc 100644 --- a/tests/warn/unused-params.scala +++ b/tests/warn/unused-params.scala @@ -1,4 +1,4 @@ -//> using options -Wunused:params -Werror +//> using options -Wunused:params // import Answers._ @@ -23,7 +23,7 @@ trait BadAPI extends InterFace { a } override def call(a: Int, - b: String, // no warn, required by superclass + b: String, // warn c: Double): Int = { println(c) a @@ -33,7 +33,7 @@ trait BadAPI extends InterFace { override def equals(other: Any): Boolean = true // no warn - def i(implicit s: String) = answer // yes, warn + def i(implicit s: String) = answer // warn /* def future(x: Int): Int = { @@ -60,7 +60,7 @@ class Revaluing(u: Int) { def f = u } // no warn case class CaseyKasem(k: Int) // no warn -case class CaseyAtTheBat(k: Int)(s: String) // warn +case class CaseyAtTheBat(k: Int)(s: String) // NO warn trait Ignorance { def f(readResolve: Int) = answer // warn @@ -78,13 +78,13 @@ trait Unimplementation { trait DumbStuff { def f(implicit dummy: DummyImplicit) = answer - def g(dummy: DummyImplicit) = answer + def g(dummy: DummyImplicit) = answer // warn } trait Proofs { def f[A, B](implicit ev: A =:= B) = answer def g[A, B](implicit ev: A <:< B) = answer - def f2[A, B](ev: A =:= B) = answer - def g2[A, B](ev: A <:< B) = answer + def f2[A, B](ev: A =:= B) = answer // warn + def g2[A, B](ev: A <:< B) = answer // warn } trait Anonymous { @@ -94,19 +94,19 @@ trait Anonymous { def f2: Int => Int = _ + 1 // no warn placeholder syntax (a fresh name and synthetic parameter) - def g = for (i <- List(1)) yield answer // no warn patvar elaborated as map.(i => 42) + def g = for (i <- List(1)) yield answer // warn // TODO no warn patvar elaborated as map.(i => 42) } trait Context[A] { def m(a: A): A = a } trait Implicits { - def f[A](implicit ctx: Context[A]) = answer - def g[A: Context] = answer + def f[A](implicit ctx: Context[A]) = answer // warn + def g[A: Context] = answer // warn } -class Bound[A: Context] +class Bound[A: Context] // warn object Answers { def answer: Int = 42 } -trait BadMix { _: InterFace => +trait BadMix { self: InterFace => def f(a: Int, b: String, // warn c: Double): Int = { @@ -121,7 +121,7 @@ trait BadMix { _: InterFace => a } override def call(a: Int, - b: String, // no warn, required by superclass + XXXX: String, // warn no longer excused because required by superclass c: Double): Int = { println(c) a @@ -131,11 +131,11 @@ trait BadMix { _: InterFace => override def equals(other: Any): Boolean = true // no warn - def i(implicit s: String) = answer // yes, warn + def i(implicit s: String) = answer // warn } class Unequal { - override def equals(other: Any) = toString.nonEmpty // no warn non-trivial RHS, required by universal method + override def equals(other: Any) = toString.nonEmpty // warn } class Seriously { @@ -143,11 +143,11 @@ class Seriously { } class TryStart(start: String) { - def FINALLY(end: END.type) = start + def FINALLY(end: END.type) = start // no warn for DSL taking a singleton } object END class Nested { - @annotation.unused private def actuallyNotUsed(fresh: Int, stale: Int) = fresh + @annotation.unused private def actuallyNotUsed(fresh: Int, stale: Int) = fresh // no warn if owner is unused } diff --git a/tests/warn/unused-privates.scala b/tests/warn/unused-privates.scala index d9e60f04f5cb..963fa3093e48 100644 --- a/tests/warn/unused-privates.scala +++ b/tests/warn/unused-privates.scala @@ -95,7 +95,7 @@ trait Locals { } object Types { - private object Dongo { def f = this } // NO warn + private object Dongo { def f = this } // warn private class Bar1 // warn private class Bar2 // no warn private type Alias1 = String // warn @@ -105,7 +105,7 @@ object Types { def f(x: Alias2) = x.length def l1() = { - object HiObject { def f = this } // NO warn + object HiObject { def f = this } // warn class Hi { // warn def f1: Hi = new Hi def f2(x: Hi) = x From d5ea0e9996afb16d7f95e7a8c78791fd870fc7ce Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 5 Nov 2024 13:09:44 -0800 Subject: [PATCH 28/30] Tweak test warns --- tests/warn/i16639a.scala | 6 +++--- tests/warn/i17371.scala | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/warn/i16639a.scala b/tests/warn/i16639a.scala index 9fe4efe57d7b..47dd431c62bf 100644 --- a/tests/warn/i16639a.scala +++ b/tests/warn/i16639a.scala @@ -91,7 +91,7 @@ trait Locals { } object Types { - private object Dongo { def f = this } // no more warn since #17061 + private object Dongo { def f = this } // warn private class Bar1 // warn warn private class Bar2 // no warn private type Alias1 = String // warn warn @@ -101,7 +101,7 @@ object Types { def f(x: Alias2) = x.length def l1() = { - object HiObject { def f = this } // no more warn since #17061 + object HiObject { def f = this } // warn class Hi { // warn warn def f1: Hi = new Hi def f2(x: Hi) = x @@ -202,4 +202,4 @@ trait `short comings` { val x = 42 // warn /Dotty only triggers in dotty 17 } -} \ No newline at end of file +} diff --git a/tests/warn/i17371.scala b/tests/warn/i17371.scala index 9e1c26368b8f..c7e321882f62 100644 --- a/tests/warn/i17371.scala +++ b/tests/warn/i17371.scala @@ -17,7 +17,7 @@ def Test() = // unminimized OP trait Circular[T] extends Ordering[T] -trait Turns[C: Circular, T] extends Ordering[T]: +trait Turns[C: Circular, T] extends Ordering[T]: // warn Circular is not a marker interface extension (turns: T) def extract: C def f[K, T](start: T, end: T)(using circular: Circular[K], turns: Turns[K, T]): Boolean = From 57347d8996073a9946614de651d7fbbecf76acd1 Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 5 Nov 2024 15:23:14 -0800 Subject: [PATCH 29/30] Accept updated semanticdb output --- tests/semanticdb/metac.expect | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tests/semanticdb/metac.expect b/tests/semanticdb/metac.expect index 26221899035b..94dc55ca165e 100644 --- a/tests/semanticdb/metac.expect +++ b/tests/semanticdb/metac.expect @@ -575,7 +575,7 @@ Text => empty Language => Scala Symbols => 108 entries Occurrences => 127 entries -Diagnostics => 6 entries +Diagnostics => 10 entries Synthetics => 2 entries Symbols: @@ -830,6 +830,10 @@ See: https://docs.scala-lang.org/scala3/reference/dropped-features/this-qualifie This construct can be rewritten automatically under -rewrite -source 3.4-migration. [22:27..22:28): [warning] unused explicit parameter [24:10..24:11): [warning] unused explicit parameter +[36:11..36:12): [warning] unused explicit parameter +[39:11..39:12): [warning] unused explicit parameter +[39:19..39:20): [warning] unused explicit parameter +[51:30..51:31): [warning] unused explicit parameter Synthetics: [51:16..51:27):List(1).map => *[Int] @@ -3533,6 +3537,7 @@ Text => empty Language => Scala Symbols => 62 entries Occurrences => 165 entries +Diagnostics => 2 entries Synthetics => 39 entries Symbols: @@ -3766,6 +3771,10 @@ Occurrences: [68:18..68:24): impure -> local20 [68:30..68:31): s -> local16 +Diagnostics: +[19:21..19:22): [warning] unused explicit parameter +[41:4..41:5): [warning] unused explicit parameter + Synthetics: [5:2..5:13):List(1).map => *[Int] [5:2..5:6):List => *.apply[Int] @@ -4235,6 +4244,7 @@ Text => empty Language => Scala Symbols => 6 entries Occurrences => 11 entries +Diagnostics => 2 entries Symbols: example/Vararg# => class Vararg extends Object { self: Vararg => +3 decls } @@ -4257,6 +4267,10 @@ Occurrences: [4:18..4:21): Int -> scala/Int# [4:26..4:30): Unit -> scala/Unit# +Diagnostics: +[3:11..3:12): [warning] unused explicit parameter +[4:11..4:12): [warning] unused explicit parameter + expect/dep-match.scala ---------------------- @@ -4873,6 +4887,7 @@ Text => empty Language => Scala Symbols => 36 entries Occurrences => 48 entries +Diagnostics => 5 entries Symbols: local0 => type N$1 <: Nat @@ -4962,6 +4977,13 @@ Occurrences: [23:35..23:39): Zero -> recursion/Nats.Zero. [23:40..23:42): ++ -> recursion/Nats.Nat#`++`(). +Diagnostics: +[10:24..10:24): [warning] unused local definition +[10:33..11:5): [warning] unused local definition +[23:19..23:19): [warning] unused local definition +[23:24..23:32): [warning] unused local definition +[23:40..25:2): [warning] unused local definition + expect/semanticdb-Definitions.scala ----------------------------------- @@ -5006,7 +5028,7 @@ Text => empty Language => Scala Symbols => 50 entries Occurrences => 78 entries -Diagnostics => 4 entries +Diagnostics => 5 entries Synthetics => 2 entries Symbols: @@ -5142,6 +5164,7 @@ Occurrences: [25:33..25:36): ??? -> scala/Predef.`???`(). Diagnostics: +[5:19..5:20): [warning] unused private member [9:30..9:31): [warning] unused explicit parameter [9:36..9:37): [warning] unused explicit parameter [9:42..9:43): [warning] unused explicit parameter @@ -5161,7 +5184,7 @@ Text => empty Language => Scala Symbols => 143 entries Occurrences => 246 entries -Diagnostics => 4 entries +Diagnostics => 5 entries Synthetics => 1 entries Symbols: @@ -5565,6 +5588,7 @@ This construct can be rewritten automatically under -rewrite -source 3.4-migrati This construct can be rewritten automatically under -rewrite -source 3.4-migration. [71:31..71:31): [warning] `_` is deprecated for wildcard arguments of types: use `?` instead This construct can be rewritten automatically under -rewrite -source 3.4-migration. +[96:13..96:14): [warning] unused explicit parameter Synthetics: [68:20..68:24):@ann => *[Int] From ac3f5dc0479c06245b9b17c4cd742561fd13b30c Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Tue, 5 Nov 2024 16:02:02 -0800 Subject: [PATCH 30/30] Adjust more tests --- tests/{pos => warn}/i17314.scala | 6 ++---- tests/{pos => warn}/i17314a.scala | 2 +- tests/warn/i17314b.scala | 2 +- tests/warn/scala2-t11681.scala | 22 +++++++++++----------- 4 files changed, 15 insertions(+), 17 deletions(-) rename tests/{pos => warn}/i17314.scala (84%) rename tests/{pos => warn}/i17314a.scala (70%) diff --git a/tests/pos/i17314.scala b/tests/warn/i17314.scala similarity index 84% rename from tests/pos/i17314.scala rename to tests/warn/i17314.scala index 8ece4a3bd7ac..cff90d843c38 100644 --- a/tests/pos/i17314.scala +++ b/tests/warn/i17314.scala @@ -1,4 +1,4 @@ -//> using options -Xfatal-warnings -Wunused:all -deprecation -feature +//> using options -Wunused:all -deprecation -feature import java.net.URI @@ -10,9 +10,7 @@ object circelike { type Configuration trait ConfiguredCodec[T] object ConfiguredCodec: - inline final def derived[A](using conf: Configuration)(using - inline mirror: Mirror.Of[A] - ): ConfiguredCodec[A] = + inline final def derived[A](using conf: Configuration)(using inline mirror: Mirror.Of[A]): ConfiguredCodec[A] = // warn // warn class InlinedConfiguredCodec extends ConfiguredCodec[A]: val codec = summonInline[Codec[URI]] // simplification new InlinedConfiguredCodec diff --git a/tests/pos/i17314a.scala b/tests/warn/i17314a.scala similarity index 70% rename from tests/pos/i17314a.scala rename to tests/warn/i17314a.scala index 4bce56d8bbed..4593fc92274d 100644 --- a/tests/pos/i17314a.scala +++ b/tests/warn/i17314a.scala @@ -1,4 +1,4 @@ -//> using options -Xfatal-warnings -Wunused:all -deprecation -feature +//> using options -Wunused:all -deprecation -feature package foo: class Foo[T] diff --git a/tests/warn/i17314b.scala b/tests/warn/i17314b.scala index e1500028ca93..ad4c8f1e4a31 100644 --- a/tests/warn/i17314b.scala +++ b/tests/warn/i17314b.scala @@ -1,4 +1,4 @@ -//> using options -Wunused:all +//> using options -Wunused:all package foo: class Foo[T] diff --git a/tests/warn/scala2-t11681.scala b/tests/warn/scala2-t11681.scala index ae2187181ceb..bddd18855f2e 100644 --- a/tests/warn/scala2-t11681.scala +++ b/tests/warn/scala2-t11681.scala @@ -23,7 +23,7 @@ trait BadAPI extends InterFace { a } override def call(a: Int, - b: String, // OK + b: String, // warn now c: Double): Int = { println(c) a @@ -33,7 +33,7 @@ trait BadAPI extends InterFace { override def equals(other: Any): Boolean = true // OK - def i(implicit s: String) = answer // ok + def i(implicit s: String) = answer // warn now /* def future(x: Int): Int = { @@ -62,7 +62,7 @@ case class CaseyKasem(k: Int) // OK case class CaseyAtTheBat(k: Int)(s: String) // ok trait Ignorance { - def f(readResolve: Int) = answer // ok + def f(readResolve: Int) = answer // warn now } class Reusing(u: Int) extends Unusing(u) // OK @@ -78,30 +78,30 @@ trait Unimplementation { trait DumbStuff { def f(implicit dummy: DummyImplicit) = answer // ok - def g(dummy: DummyImplicit) = answer // ok + def g(dummy: DummyImplicit) = answer // warn now } trait Proofs { def f[A, B](implicit ev: A =:= B) = answer // ok def g[A, B](implicit ev: A <:< B) = answer // ok - def f2[A, B](ev: A =:= B) = answer // ok - def g2[A, B](ev: A <:< B) = answer // ok + def f2[A, B](ev: A =:= B) = answer // warn now + def g2[A, B](ev: A <:< B) = answer // warn now } trait Anonymous { - def f = (i: Int) => answer // ok + def f = (i: Int) => answer // warn now def f1 = (_: Int) => answer // OK def f2: Int => Int = _ + 1 // OK - def g = for (i <- List(1)) yield answer // ok + def g = for (i <- List(1)) yield answer // warn } trait Context[A] trait Implicits { - def f[A](implicit ctx: Context[A]) = answer // ok - def g[A: Context] = answer // OK + def f[A](implicit ctx: Context[A]) = answer // warn + def g[A: Context] = answer // warn } -class Bound[A: Context] // OK +class Bound[A: Context] // warn object Answers { def answer: Int = 42 }