Skip to content

Commit d32cce7

Browse files
ghostbuster91ghostbuster91
committed
Address review comments
1 parent 0bac3a9 commit d32cce7

File tree

3 files changed

+41
-48
lines changed

3 files changed

+41
-48
lines changed

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3160,7 +3160,7 @@ extends SyntaxMsg(VolatileOnValID):
31603160
protected def msg(using Context): String = "values cannot be volatile"
31613161
protected def explain(using Context): String = ""
31623162

3163-
class UnusedSymbol(pos: SourcePosition,_msg: String, _actions: List[CodeAction])(using Context)
3163+
class UnusedSymbol(pos: SourcePosition, _msg: String, _actions: List[CodeAction])(using Context)
31643164
extends Message(UnusedSymbolID) {
31653165
def kind = MessageKind.UnusedSymbol
31663166

@@ -3170,10 +3170,8 @@ extends Message(UnusedSymbolID) {
31703170
}
31713171

31723172
object UnusedSymbol {
3173-
private def createRemoveLocalDefAction(tree: tpd.DefTree)(using Context): List[CodeAction] = {
3174-
import scala.language.unsafeNulls
3175-
3176-
val source = tree.sourcePos.source
3173+
private def createRemoveLocalDefAction(tree: SourcePosition)(using Context): List[CodeAction] = {
3174+
val source = tree.source
31773175
val endLine = source.offsetToLine(tree.sourcePos.end - 1)
31783176
val nextLineOffset = source.lineToOffset(endLine + 1)
31793177
val startOffset = source.lineToOffset(tree.sourcePos.startLine)
@@ -3191,10 +3189,10 @@ object UnusedSymbol {
31913189
)
31923190
)
31933191
}
3194-
def imports(pos: SourcePosition)(using Context): UnusedSymbol= new UnusedSymbol(pos, i"unused import", List.empty)
3195-
def localDefs(tree: tpd.NamedDefTree)(using Context): UnusedSymbol = new UnusedSymbol(tree.namePos, i"unused local definition", createRemoveLocalDefAction(tree))
3196-
def explicitParams(tree: tpd.NamedDefTree)(using Context): UnusedSymbol = new UnusedSymbol(tree.namePos, i"unused explicit parameter", List.empty)
3197-
def implicitParams(tree: tpd.NamedDefTree)(using Context): UnusedSymbol = new UnusedSymbol(tree.namePos, i"unused implicit parameter", List.empty)
3198-
def privateMembers(tree: tpd.NamedDefTree)(using Context): UnusedSymbol = new UnusedSymbol(tree.namePos, i"unused private member", List.empty)
3199-
def patVars(tree: tpd.NamedDefTree)(using Context): UnusedSymbol = new UnusedSymbol(tree.namePos, i"unused pattern variable", List.empty)
3192+
def imports(pos: SourcePosition)(using Context): UnusedSymbol = new UnusedSymbol(pos, i"unused import", List.empty)
3193+
def localDefs(tree: SourcePosition)(using Context): UnusedSymbol = new UnusedSymbol(tree, i"unused local definition", createRemoveLocalDefAction(tree))
3194+
def explicitParams(tree: SourcePosition)(using Context): UnusedSymbol = new UnusedSymbol(tree, i"unused explicit parameter", List.empty)
3195+
def implicitParams(tree: SourcePosition)(using Context): UnusedSymbol = new UnusedSymbol(tree, i"unused implicit parameter", List.empty)
3196+
def privateMembers(tree: SourcePosition)(using Context): UnusedSymbol = new UnusedSymbol(tree, i"unused private member", List.empty)
3197+
def patVars(tree: SourcePosition)(using Context): UnusedSymbol = new UnusedSymbol(tree, i"unused pattern variable", List.empty)
32003198
}

‎compiler/src/dotty/tools/dotc/transform/CheckUnused.scala

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -283,26 +283,25 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke
283283
/** Do the actual reporting given the result of the anaylsis */
284284
private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit =
285285
res.warnings.toList.sortBy{
286-
case us: UnusedSymbol.ImportSelector => us.pos.line
287-
case us: UnusedSymbol.Symbol => us.tree.sourcePos.line
286+
case us: UnusedSymbol => us.namePos.sourcePos.line
288287
}(using Ordering[Int]).foreach { s =>
289288
s match
290-
case UnusedSymbol.ImportSelector(srcPos, _) =>
291-
report.warning(UnusedSymbolMessage.imports(srcPos.sourcePos))
292-
case UnusedSymbol.Symbol(t, WarnTypes.LocalDefs) =>
293-
report.warning(UnusedSymbolMessage.localDefs(t), t)
294-
case UnusedSymbol.Symbol(t, WarnTypes.ExplicitParams) =>
295-
report.warning(UnusedSymbolMessage.explicitParams(t), t)
296-
case UnusedSymbol.Symbol(t, WarnTypes.ImplicitParams) =>
297-
report.warning(UnusedSymbolMessage.implicitParams(t), t)
298-
case UnusedSymbol.Symbol(t, WarnTypes.PrivateMembers) =>
299-
report.warning(UnusedSymbolMessage.privateMembers(t), t)
300-
case UnusedSymbol.Symbol(t, WarnTypes.PatVars) =>
301-
report.warning(UnusedSymbolMessage.patVars(t), t)
302-
case UnusedSymbol.Symbol(t, WarnTypes.UnsetLocals) =>
303-
report.warning("unset local variable, consider using an immutable val instead", t)
304-
case UnusedSymbol.Symbol(t, WarnTypes.UnsetPrivates) =>
305-
report.warning("unset private variable, consider using an immutable val instead", t)
289+
case UnusedSymbol(_, namePos, WarnTypes.Imports) =>
290+
report.warning(UnusedSymbolMessage.imports(namePos.sourcePos), namePos)
291+
case UnusedSymbol(treePos, namePos, WarnTypes.LocalDefs) =>
292+
report.warning(UnusedSymbolMessage.localDefs(treePos.sourcePos), namePos)
293+
case UnusedSymbol(treePos, namePos, WarnTypes.ExplicitParams) =>
294+
report.warning(UnusedSymbolMessage.explicitParams(treePos.sourcePos), namePos)
295+
case UnusedSymbol(treePos, namePos, WarnTypes.ImplicitParams) =>
296+
report.warning(UnusedSymbolMessage.implicitParams(treePos.sourcePos), namePos)
297+
case UnusedSymbol(treePos, namePos, WarnTypes.PrivateMembers) =>
298+
report.warning(UnusedSymbolMessage.privateMembers(treePos.sourcePos), namePos)
299+
case UnusedSymbol(treePos, namePos, WarnTypes.PatVars) =>
300+
report.warning(UnusedSymbolMessage.patVars(treePos.sourcePos), namePos)
301+
case UnusedSymbol(_, namePos, WarnTypes.UnsetLocals) =>
302+
report.warning("unset local variable, consider using an immutable val instead", namePos)
303+
case UnusedSymbol(_, namePos, WarnTypes.UnsetPrivates) =>
304+
report.warning("unset private variable, consider using an immutable val instead", namePos)
306305
}
307306

308307
end CheckUnused
@@ -316,6 +315,7 @@ object CheckUnused:
316315
case Report
317316

318317
enum WarnTypes:
318+
case Imports
319319
case LocalDefs
320320
case ExplicitParams
321321
case ImplicitParams
@@ -513,7 +513,7 @@ object CheckUnused:
513513
popScope()
514514
val sortedImp =
515515
if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then
516-
unusedImport.map(d => UnusedSymbol.ImportSelector(d.srcPos, d.name)).toList
516+
unusedImport.map(d => UnusedSymbol(d.srcPos, d.srcPos, WarnTypes.Imports)).toList
517517
else
518518
Nil
519519
// Partition to extract unset local variables from usedLocalDefs
@@ -526,24 +526,24 @@ object CheckUnused:
526526
unusedLocalDefs
527527
.filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
528528
.filterNot(d => containsSyntheticSuffix(d.symbol))
529-
.map(d => UnusedSymbol.Symbol(d, WarnTypes.LocalDefs)).toList
530-
val unsetLocalDefs = usedLocalDefs.filter(isUnsetVarDef).map(d => UnusedSymbol.Symbol(d, WarnTypes.UnsetLocals)).toList
529+
.map(d => UnusedSymbol(d.sourcePos, d.namePos, WarnTypes.LocalDefs)).toList
530+
val unsetLocalDefs = usedLocalDefs.filter(isUnsetVarDef).map(d => UnusedSymbol(d.sourcePos, d.namePos, WarnTypes.UnsetLocals)).toList
531531

532532
val sortedExplicitParams =
533533
if ctx.settings.WunusedHas.explicits then
534534
explicitParamInScope
535535
.filterNot(d => d.symbol.usedDefContains)
536536
.filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
537537
.filterNot(d => containsSyntheticSuffix(d.symbol))
538-
.map(d => UnusedSymbol.Symbol(d, WarnTypes.ExplicitParams)).toList
538+
.map(d => UnusedSymbol(d.sourcePos, d.namePos, WarnTypes.ExplicitParams)).toList
539539
else
540540
Nil
541541
val sortedImplicitParams =
542542
if ctx.settings.WunusedHas.implicits then
543543
implicitParamInScope
544544
.filterNot(d => d.symbol.usedDefContains)
545545
.filterNot(d => containsSyntheticSuffix(d.symbol))
546-
.map(d => UnusedSymbol.Symbol(d, WarnTypes.ImplicitParams)).toList
546+
.map(d => UnusedSymbol(d.sourcePos, d.namePos, WarnTypes.ImplicitParams)).toList
547547
else
548548
Nil
549549
// Partition to extract unset private variables from usedPrivates
@@ -552,15 +552,15 @@ object CheckUnused:
552552
privateDefInScope.partition(d => d.symbol.usedDefContains)
553553
else
554554
(Nil, Nil)
555-
val sortedPrivateDefs = unusedPrivates.filterNot(d => containsSyntheticSuffix(d.symbol)).map(d => UnusedSymbol.Symbol(d, WarnTypes.PrivateMembers)).toList
556-
val unsetPrivateDefs = usedPrivates.filter(isUnsetVarDef).map(d => UnusedSymbol.Symbol(d, WarnTypes.UnsetPrivates)).toList
555+
val sortedPrivateDefs = unusedPrivates.filterNot(d => containsSyntheticSuffix(d.symbol)).map(d => UnusedSymbol(d.sourcePos, d.namePos, WarnTypes.PrivateMembers)).toList
556+
val unsetPrivateDefs = usedPrivates.filter(isUnsetVarDef).map(d => UnusedSymbol(d.sourcePos, d.namePos, WarnTypes.UnsetPrivates)).toList
557557
val sortedPatVars =
558558
if ctx.settings.WunusedHas.patvars then
559559
patVarsInScope
560560
.filterNot(d => d.symbol.usedDefContains)
561561
.filterNot(d => containsSyntheticSuffix(d.symbol))
562562
.filterNot(d => usedInPosition.exists { case (pos, name) => d.span.contains(pos.span) && name == d.symbol.name})
563-
.map(d => UnusedSymbol.Symbol(d, WarnTypes.PatVars)).toList
563+
.map(d => UnusedSymbol(d.sourcePos, d.namePos, WarnTypes.PatVars)).toList
564564
else
565565
Nil
566566
val warnings =
@@ -573,13 +573,9 @@ object CheckUnused:
573573
sortedPatVars :::
574574
unsetLocalDefs :::
575575
unsetPrivateDefs
576-
unsorted.sortBy {
577-
case s : UnusedSymbol.Symbol =>
578-
val pos = s.tree.sourcePos
579-
(pos.line, pos.column)
580-
case s: UnusedSymbol.ImportSelector =>
581-
val pos = s.pos.sourcePos
582-
(pos.line, pos.column)
576+
unsorted.sortBy { s =>
577+
val pos = s.namePos.sourcePos
578+
(pos.line, pos.column)
583579
}
584580
UnusedResult(warnings.toSet)
585581
end getUnused
@@ -810,9 +806,8 @@ object CheckUnused:
810806
case _:tpd.Block => Local
811807
case _ => Other
812808

813-
enum UnusedSymbol:
814-
case Symbol(tree: tpd.NamedDefTree, warnType: WarnTypes)
815-
case ImportSelector(pos: SrcPos, name: Name)
809+
case class UnusedSymbol(defnTree: SrcPos, namePos: SrcPos, warnType: WarnTypes)
810+
816811
/** A container for the results of the used elements analysis */
817812
case class UnusedResult(warnings: Set[UnusedSymbol])
818813
object UnusedResult:

‎compiler/test/dotty/tools/dotc/reporting/CodeActionTest.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import org.junit.Test
1616
* diagnostic for a given code snippet.
1717
*/
1818
class CodeActionTest extends DottyTest:
19-
19+
2020
@Test def convertToFunctionValue =
2121
checkCodeAction(
2222
"""|object Test:

0 commit comments

Comments
 (0)