Skip to content

Commit e92ae4f

Browse files
schuetzcarlWojciechMazur
authored andcommitted
Implement -Xlint:private-shadow, type-parameter-shadow
Respectively warn about : - a private field or a class parameter that shadows a superclass field - a local type parameter that shadows a type already in the scope Fixes : #17612 and #17613 [Cherry-picked e5fd477]
1 parent 739b319 commit e92ae4f

File tree

10 files changed

+547
-2
lines changed

10 files changed

+547
-2
lines changed

compiler/src/dotty/tools/dotc/Compiler.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class Compiler {
3535
protected def frontendPhases: List[List[Phase]] =
3636
List(new Parser) :: // Compiler frontend: scanner, parser
3737
List(new TyperPhase) :: // Compiler frontend: namer, typer
38-
List(new CheckUnused.PostTyper) :: // Check for unused elements
38+
List(new CheckUnused.PostTyper, new CheckShadowing) :: // Check for unused and shadowing elements
3939
List(new YCheckPositions) :: // YCheck positions
4040
List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks
4141
List(new semanticdb.ExtractSemanticDB) :: // Extract info into .semanticdb files

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

+25-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import dotty.tools.dotc.config.SourceVersion
99
import dotty.tools.dotc.core.Contexts._
1010
import dotty.tools.dotc.rewrites.Rewrites
1111
import dotty.tools.io.{AbstractFile, Directory, JDK9Reflectors, PlainDirectory}
12+
import Setting.ChoiceWithHelp
1213

1314
import scala.util.chaining._
1415

@@ -156,7 +157,6 @@ private sealed trait VerboseSettings:
156157
*/
157158
private sealed trait WarningSettings:
158159
self: SettingGroup =>
159-
import Setting.ChoiceWithHelp
160160

161161
val Whelp: Setting[Boolean] = BooleanSetting("-W", "Print a synopsis of warning options.")
162162
val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Werror", "Fail the compilation if there are any warnings.", aliases = List("-Xfatal-warnings"))
@@ -307,6 +307,30 @@ private sealed trait XSettings:
307307
}
308308

309309
val XmacroSettings: Setting[List[String]] = MultiStringSetting("-Xmacro-settings", "setting1,setting2,..settingN", "List of settings which exposed to the macros")
310+
311+
val Xlint: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
312+
name = "-Xlint",
313+
helpArg = "advanced warning",
314+
descr = "Enable or disable specific `lint` warnings",
315+
choices = List(
316+
ChoiceWithHelp("nowarn", ""),
317+
ChoiceWithHelp("all", ""),
318+
ChoiceWithHelp("private-shadow", "Warn if a private field or class parameter shadows a superclass field"),
319+
ChoiceWithHelp("type-parameter-shadow", "Warn when a type parameter shadows a type already in the scope"),
320+
),
321+
default = Nil
322+
)
323+
324+
object XlintHas:
325+
def isChoiceSet(s: String)(using Context) = Xlint.value.pipe(us => us.contains(s))
326+
def allOr(s: String)(using Context) = Xlint.value.pipe(us => us.contains("all") || us.contains(s))
327+
def nowarn(using Context) = allOr("nowarn")
328+
329+
def privateShadow(using Context) =
330+
allOr("private-shadow")
331+
def typeParameterShadow(using Context) =
332+
allOr("type-parameter-shadow")
333+
310334
end XSettings
311335

312336
/** -Y "Forking" as in forked tongue or "Private" settings */
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,314 @@
1+
package dotty.tools.dotc.transform
2+
3+
import dotty.tools.dotc.ast.tpd
4+
import dotty.tools.dotc.ast.Trees.EmptyTree
5+
import dotty.tools.dotc.transform.MegaPhase
6+
import dotty.tools.dotc.transform.MegaPhase.MiniPhase
7+
import dotty.tools.dotc.report
8+
import dotty.tools.dotc.core.Contexts.*
9+
import dotty.tools.dotc.core.Flags.*
10+
import dotty.tools.dotc.util.{Property, SrcPos}
11+
import dotty.tools.dotc.core.Symbols.ClassSymbol
12+
import dotty.tools.dotc.core.Names.Name
13+
import dotty.tools.dotc.core.Symbols.Symbol
14+
import dotty.tools.dotc.core.Flags.EmptyFlags
15+
import dotty.tools.dotc.ast.tpd.TreeTraverser
16+
import dotty.tools.dotc.core.Types.watchList
17+
import dotty.tools.dotc.core.Types.NoType
18+
import dotty.tools.dotc.core.Types.Type
19+
import dotty.tools.dotc.core.Types
20+
import dotty.tools.dotc.semanticdb.TypeOps
21+
import dotty.tools.dotc.cc.boxedCaptureSet
22+
import dotty.tools.dotc.core.Symbols.NoSymbol
23+
import dotty.tools.dotc.transform.SymUtils.isParamOrAccessor
24+
import scala.collection.mutable
25+
import dotty.tools.dotc.core.Scopes.Scope
26+
import scala.collection.immutable.HashMap
27+
import dotty.tools.dotc.core.Symbols
28+
import dotty.tools.dotc.typer.ImportInfo
29+
import dotty.tools.dotc.ast.untpd.ImportSelector
30+
import dotty.tools.dotc.core.StdNames.nme
31+
import dotty.tools.dotc.ast.untpd
32+
import dotty.tools.dotc.core.Denotations.SingleDenotation
33+
import dotty.tools.dotc.ast.Trees.Ident
34+
import dotty.tools.dotc.core.Names.TypeName
35+
import dotty.tools.dotc.core.Names.TermName
36+
import dotty.tools.dotc.core.Mode.Type
37+
import dotty.tools.dotc.core.Names.SimpleName
38+
39+
class CheckShadowing extends MiniPhase:
40+
import CheckShadowing.*
41+
import ShadowingData.*
42+
43+
private val _key = Property.Key[ShadowingData]
44+
45+
private def shadowingDataApply[U](f: ShadowingData => U)(using Context): Context =
46+
ctx.property(_key).foreach(f)
47+
ctx
48+
49+
override def phaseName: String = CheckShadowing.name
50+
51+
override def description: String = CheckShadowing.description
52+
53+
override def isRunnable(using Context): Boolean =
54+
super.isRunnable &&
55+
ctx.settings.Xlint.value.nonEmpty &&
56+
!ctx.isJava
57+
58+
// Setup before the traversal
59+
override def prepareForUnit(tree: tpd.Tree)(using Context): Context =
60+
val data = ShadowingData()
61+
val fresh = ctx.fresh.setProperty(_key, data)
62+
shadowingDataApply(sd => sd.registerRootImports())(using fresh)
63+
64+
// Reporting on traversal's end
65+
override def transformUnit(tree: tpd.Tree)(using Context): tpd.Tree =
66+
shadowingDataApply(sd =>
67+
reportShadowing(sd.getShadowingResult)
68+
)
69+
tree
70+
71+
// MiniPhase traversal :
72+
73+
override def prepareForPackageDef(tree: tpd.PackageDef)(using Context): Context =
74+
shadowingDataApply(sd => sd.inNewScope())
75+
ctx
76+
77+
override def prepareForTemplate(tree: tpd.Template)(using Context): Context =
78+
shadowingDataApply(sd => sd.inNewScope())
79+
ctx
80+
81+
override def prepareForBlock(tree: tpd.Block)(using Context): Context =
82+
shadowingDataApply(sd => sd.inNewScope())
83+
ctx
84+
85+
override def prepareForOther(tree: tpd.Tree)(using Context): Context =
86+
importTraverser(tree.symbol).traverse(tree)
87+
ctx
88+
89+
override def prepareForValDef(tree: tpd.ValDef)(using Context): Context =
90+
shadowingDataApply(sd =>
91+
sd.registerPrivateShadows(tree)
92+
)
93+
94+
override def prepareForTypeDef(tree: tpd.TypeDef)(using Context): Context =
95+
if tree.symbol.isAliasType then // if alias, the parent is the current symbol
96+
nestedTypeTraverser(tree.symbol).traverse(tree.rhs)
97+
if tree.symbol.is(Param) then // if param, the parent is up
98+
val owner = tree.symbol.owner
99+
val parent = if (owner.isConstructor) then owner.owner else owner
100+
nestedTypeTraverser(parent).traverse(tree.rhs)(using ctx.outer)
101+
shadowingDataApply(sd => sd.registerCandidate(parent, tree))
102+
else
103+
ctx
104+
105+
106+
override def transformPackageDef(tree: tpd.PackageDef)(using Context): tpd.Tree =
107+
shadowingDataApply(sd => sd.outOfScope())
108+
tree
109+
110+
override def transformBlock(tree: tpd.Block)(using Context): tpd.Tree =
111+
shadowingDataApply(sd => sd.outOfScope())
112+
tree
113+
114+
override def transformTemplate(tree: tpd.Template)(using Context): tpd.Tree =
115+
shadowingDataApply(sd => sd.outOfScope())
116+
tree
117+
118+
override def transformTypeDef(tree: tpd.TypeDef)(using Context): tpd.Tree =
119+
if tree.symbol.is(Param) && !tree.symbol.owner.isConstructor then // Do not register for constructors the work is done for the Class owned equivalent TypeDef
120+
shadowingDataApply(sd => sd.computeTypeParamShadowsFor(tree.symbol.owner)(using ctx.outer))
121+
if tree.symbol.isAliasType then // No need to start outer here, because the TypeDef reached here it's already the parent
122+
shadowingDataApply(sd => sd.computeTypeParamShadowsFor(tree.symbol)(using ctx))
123+
tree
124+
125+
// Helpers :
126+
127+
private def reportShadowing(res: ShadowingData.ShadowResult)(using Context): Unit =
128+
res.warnings.sortBy(w => (w.pos.line, w.pos.startPos.column))(using Ordering[(Int, Int)]).foreach { s =>
129+
s match
130+
case PrivateShadowWarning(pos, shadow, shadowed) =>
131+
report.warning(s"${shadow.showLocated} shadows field ${shadowed.name} inherited from ${shadowed.owner}", pos)
132+
case TypeParamShadowWarning(pos, shadow, parent, shadowed) =>
133+
if shadowed.exists then
134+
report.warning(s"Type parameter ${shadow.name} for $parent shadows the type defined by ${shadowed.showLocated}", pos)
135+
else
136+
report.warning(s"Type parameter ${shadow.name} for $parent shadows an explicitly renamed type : ${shadow.name}", pos)
137+
}
138+
139+
private def nestedTypeTraverser(parent: Symbol) = new TreeTraverser:
140+
import tpd._
141+
142+
override def traverse(tree: tpd.Tree)(using Context): Unit =
143+
tree match
144+
case t:tpd.TypeDef =>
145+
val newCtx = shadowingDataApply(sd =>
146+
sd.registerCandidate(parent, t)
147+
)
148+
traverseChildren(tree)(using newCtx)
149+
case _ =>
150+
traverseChildren(tree)
151+
end traverse
152+
end nestedTypeTraverser
153+
154+
// To reach the imports during a miniphase traversal
155+
private def importTraverser(parent: Symbol) = new TreeTraverser:
156+
import tpd._
157+
158+
override def traverse(tree: tpd.Tree)(using Context): Unit =
159+
tree match
160+
case t:tpd.Import =>
161+
shadowingDataApply(sd => sd.registerImport(t))
162+
traverseChildren(tree)
163+
case _ =>
164+
traverseChildren(tree)
165+
166+
end CheckShadowing
167+
168+
169+
object CheckShadowing:
170+
171+
val name = "checkShadowing"
172+
val description = "check for elements shadowing other elements in scope"
173+
174+
private class ShadowingData:
175+
import dotty.tools.dotc.transform.CheckShadowing.ShadowingData._
176+
import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack}
177+
178+
private val rootImports = MutSet[SingleDenotation]()
179+
private val explicitsImports = MutStack[MutSet[tpd.Import]]()
180+
private val renamedImports = MutStack[MutMap[SimpleName, Name]]() // original name -> renamed name
181+
182+
private val typeParamCandidates = MutMap[Symbol, Seq[tpd.TypeDef]]().withDefaultValue(Seq())
183+
private val shadowedTypeDefs = MutSet[TypeParamShadowWarning]()
184+
185+
private val shadowedPrivateDefs = MutSet[PrivateShadowWarning]()
186+
187+
def inNewScope()(using Context) =
188+
explicitsImports.push(MutSet())
189+
renamedImports.push(MutMap())
190+
191+
def outOfScope()(using Context) =
192+
explicitsImports.pop()
193+
renamedImports.pop()
194+
195+
/** Register the Root imports (at once per compilation unit)*/
196+
def registerRootImports()(using Context) =
197+
ctx.definitions.rootImportTypes.foreach(rimp => println())
198+
val langPackageName = ctx.definitions.JavaLangPackageVal.name.toSimpleName // excludes lang package
199+
rootImports.addAll(ctx.definitions.rootImportTypes.withFilter(_.name.toSimpleName != langPackageName).flatMap(_.typeMembers))
200+
201+
/* Register an import encountered in the current scope **/
202+
def registerImport(imp: tpd.Import)(using Context) =
203+
val renamedImps = imp.selectors.collect(sel => { sel.renamed match
204+
case Ident(rename) =>
205+
(sel.name.toSimpleName, rename)
206+
}).toMap
207+
explicitsImports.top += imp
208+
renamedImports.top.addAll(renamedImps)
209+
210+
/** Register a potential type definition which could shadows a Type already defined */
211+
def registerCandidate(parent: Symbol, typeDef: tpd.TypeDef) =
212+
val actual = typeParamCandidates.getOrElseUpdate(parent, Seq())
213+
typeParamCandidates.update(parent, actual.+:(typeDef))
214+
215+
/** Compute if there is some TypeParam shadowing and register if it is the case*/
216+
def computeTypeParamShadowsFor(parent: Symbol)(using Context): Unit =
217+
typeParamCandidates(parent).foreach(typeDef => {
218+
val sym = typeDef.symbol
219+
val shadowedType =
220+
lookForRootShadowedType(sym)
221+
.orElse(lookForImportedShadowedType(sym))
222+
.orElse(lookForUnitShadowedType(sym))
223+
shadowedType.foreach(shadowed =>
224+
if !renamedImports.exists(_.contains(shadowed.name.toSimpleName)) then
225+
shadowedTypeDefs += TypeParamShadowWarning(typeDef.srcPos, typeDef.symbol, parent, shadowed)
226+
)
227+
})
228+
229+
private def lookForRootShadowedType(symbol: Symbol)(using Context): Option[Symbol] =
230+
rootImports.find(p => p.name.toSimpleName == symbol.name.toSimpleName).map(_.symbol)
231+
232+
private def lookForImportedShadowedType(symbol: Symbol)(using Context): Option[Symbol] =
233+
explicitsImports
234+
.flatMap(_.flatMap(imp => symbol.isInImport(imp)))
235+
.headOption
236+
237+
private def lookForUnitShadowedType(symbol: Symbol)(using Context): Option[Symbol] =
238+
if !ctx.owner.exists then
239+
None
240+
else
241+
val declarationScope = ctx.effectiveScope
242+
val res = declarationScope.lookup(symbol.name)
243+
res match
244+
case s: Symbol if s.isType => Some(s)
245+
case _ => lookForUnitShadowedType(symbol)(using ctx.outer)
246+
247+
/** Register if the valDef is a private declaration that shadows an inherited field */
248+
def registerPrivateShadows(valDef: tpd.ValDef)(using Context): Unit =
249+
lookForShadowedField(valDef.symbol).foreach(shadowedField =>
250+
shadowedPrivateDefs += PrivateShadowWarning(valDef.startPos, valDef.symbol, shadowedField)
251+
)
252+
253+
private def lookForShadowedField(symDecl: Symbol)(using Context): Option[Symbol] =
254+
if symDecl.isPrivate then
255+
val symDeclType = symDecl.info
256+
val bClasses = symDecl.owner.info.baseClasses
257+
bClasses match
258+
case _ :: inherited =>
259+
inherited
260+
.map(classSymbol => symDecl.denot.matchingDecl(classSymbol, symDeclType))
261+
.find(sym => sym.name == symDecl.name)
262+
case Nil =>
263+
None
264+
else
265+
None
266+
267+
/** Get the shadowing analysis's result */
268+
def getShadowingResult(using Context): ShadowResult =
269+
270+
val privateShadowWarnings: List[ShadowWarning] =
271+
if ctx.settings.XlintHas.privateShadow then
272+
shadowedPrivateDefs.toList
273+
else
274+
Nil
275+
val typeParamShadowWarnings: List[ShadowWarning] =
276+
if ctx.settings.XlintHas.typeParameterShadow then
277+
shadowedTypeDefs.toList
278+
else
279+
Nil
280+
ShadowResult(privateShadowWarnings ++ typeParamShadowWarnings)
281+
282+
extension (sym: Symbol)
283+
/** Given an import and accessibility, return the import's symbol that matches import<->this symbol */
284+
private def isInImport(imp: tpd.Import)(using Context): Option[Symbol] =
285+
val tpd.Import(qual, sels) = imp
286+
val simpleSelections = qual.tpe.member(sym.name).alternatives
287+
val typeSelections = sels.flatMap(n => qual.tpe.member(n.name.toTypeName).alternatives)
288+
289+
sels.find(is => is.rename.toSimpleName == sym.name.toSimpleName).map(_.symbol)
290+
.orElse(typeSelections.map(_.symbol).find(sd => sd.name == sym.name))
291+
.orElse(simpleSelections.map(_.symbol).find(sd => sd.name == sym.name))
292+
293+
end ShadowingData
294+
295+
private object ShadowingData:
296+
sealed abstract class ShadowWarning(val pos: SrcPos, val shadow: Symbol, val shadowed: Symbol)
297+
298+
case class PrivateShadowWarning(
299+
override val pos: SrcPos,
300+
override val shadow: Symbol,
301+
override val shadowed: Symbol
302+
) extends ShadowWarning(pos, shadow, shadowed)
303+
304+
case class TypeParamShadowWarning(
305+
override val pos: SrcPos,
306+
override val shadow: Symbol,
307+
val shadowParent: Symbol,
308+
override val shadowed: Symbol,
309+
) extends ShadowWarning(pos, shadow, shadowed)
310+
311+
/** A container for the results of the shadow elements analysis */
312+
case class ShadowResult(warnings: List[ShadowWarning])
313+
314+
end CheckShadowing

0 commit comments

Comments
 (0)