FIR checker: make unused checker path-sensitive

This commit is contained in:
Jinseong Jeon
2020-11-24 14:42:51 -08:00
committed by Mikhail Glukhikh
parent 3d7d87ace5
commit 168503573a
@@ -36,41 +36,52 @@ object UnusedChecker : FirControlFlowChecker() {
} }
class CfaVisitor( class CfaVisitor(
val data: Map<CFGNode<*>, VariableStatusInfo>, val data: Map<CFGNode<*>, PathAwareVariableStatusInfo>,
val reporter: DiagnosticReporter val reporter: DiagnosticReporter
) : ControlFlowGraphVisitorVoid() { ) : ControlFlowGraphVisitorVoid() {
override fun visitNode(node: CFGNode<*>) {} override fun visitNode(node: CFGNode<*>) {}
override fun visitVariableAssignmentNode(node: VariableAssignmentNode) { override fun visitVariableAssignmentNode(node: VariableAssignmentNode) {
val variableSymbol = (node.fir.calleeReference as? FirResolvedNamedReference)?.resolvedSymbol ?: return val variableSymbol = (node.fir.calleeReference as? FirResolvedNamedReference)?.resolvedSymbol ?: return
val data = data[node]?.get(variableSymbol) ?: return val dataPerNode = data[node] ?: return
if (data == VariableStatus.ONLY_WRITTEN_NEVER_READ) { for (label in dataPerNode.keys) {
// todo: report case like "a += 1" where `a` `doesn't writes` different way (special for Idea) val data = dataPerNode[label]!![variableSymbol] ?: continue
val source = node.fir.lValue.source if (data == VariableStatus.ONLY_WRITTEN_NEVER_READ) {
reporter.report(source, FirErrors.ASSIGNED_VALUE_IS_NEVER_READ) // todo: report case like "a += 1" where `a` `doesn't writes` different way (special for Idea)
val source = node.fir.lValue.source
reporter.report(source, FirErrors.ASSIGNED_VALUE_IS_NEVER_READ)
// To avoid duplicate reports, stop investigating remaining paths once reported.
break
}
} }
} }
override fun visitVariableDeclarationNode(node: VariableDeclarationNode) { override fun visitVariableDeclarationNode(node: VariableDeclarationNode) {
val variableSymbol = node.fir.symbol val variableSymbol = node.fir.symbol
if (variableSymbol.isLoopIterator) return if (variableSymbol.isLoopIterator) return
val data = data[node]?.get(variableSymbol) ?: return val dataPerNode = data[node] ?: return
for (label in dataPerNode.keys) {
val data = dataPerNode[label]!![variableSymbol] ?: continue
val variableSource = variableSymbol.fir.source.takeIf { it?.elementType != KtNodeTypes.DESTRUCTURING_DECLARATION } val variableSource = variableSymbol.fir.source.takeIf { it?.elementType != KtNodeTypes.DESTRUCTURING_DECLARATION }
when { when {
data == VariableStatus.UNUSED -> { data == VariableStatus.UNUSED -> {
if ((node.fir.initializer as? FirFunctionCall)?.isIterator != true) { if ((node.fir.initializer as? FirFunctionCall)?.isIterator != true) {
reporter.report(variableSource, FirErrors.UNUSED_VARIABLE) reporter.report(variableSource, FirErrors.UNUSED_VARIABLE)
break
}
}
data.isRedundantInit -> {
val source = variableSymbol.fir.initializer?.source
reporter.report(source, FirErrors.VARIABLE_INITIALIZER_IS_REDUNDANT)
break
}
data == VariableStatus.ONLY_WRITTEN_NEVER_READ -> {
reporter.report(variableSource, FirErrors.VARIABLE_NEVER_READ)
break
}
else -> {
} }
}
data.isRedundantInit -> {
val source = variableSymbol.fir.initializer?.source
reporter.report(source, FirErrors.VARIABLE_INITIALIZER_IS_REDUNDANT)
}
data == VariableStatus.ONLY_WRITTEN_NEVER_READ -> {
reporter.report(variableSource, FirErrors.VARIABLE_NEVER_READ)
}
else -> {
} }
} }
} }
@@ -122,72 +133,102 @@ object UnusedChecker : FirControlFlowChecker() {
} }
class PathAwareVariableStatusInfo(
map: PersistentMap<EdgeLabel, VariableStatusInfo> = persistentMapOf()
) : PathAwareControlFlowInfo<PathAwareVariableStatusInfo, VariableStatusInfo>(map) {
companion object {
val EMPTY = PathAwareVariableStatusInfo(persistentMapOf(NormalPath to VariableStatusInfo.EMPTY))
}
override val constructor: (PersistentMap<EdgeLabel, VariableStatusInfo>) -> PathAwareVariableStatusInfo =
::PathAwareVariableStatusInfo
override val empty: () -> PathAwareVariableStatusInfo =
::EMPTY
}
private class ValueWritesWithoutReading( private class ValueWritesWithoutReading(
private val localProperties: Set<FirPropertySymbol> private val localProperties: Set<FirPropertySymbol>
) : ControlFlowGraphVisitor<VariableStatusInfo, Collection<VariableStatusInfo>>() { ) : ControlFlowGraphVisitor<PathAwareVariableStatusInfo, Collection<Pair<EdgeLabel, PathAwareVariableStatusInfo>>>() {
fun getData(graph: ControlFlowGraph): Map<CFGNode<*>, VariableStatusInfo> { fun getData(graph: ControlFlowGraph): Map<CFGNode<*>, PathAwareVariableStatusInfo> {
return graph.collectDataForNode(TraverseDirection.Backward, VariableStatusInfo.EMPTY, this) return graph.collectPathAwareDataForNode(TraverseDirection.Backward, PathAwareVariableStatusInfo.EMPTY, this)
} }
override fun visitNode(node: CFGNode<*>, data: Collection<VariableStatusInfo>): VariableStatusInfo { override fun visitNode(
if (data.isEmpty()) return VariableStatusInfo.EMPTY node: CFGNode<*>,
return data.reduce(VariableStatusInfo::merge) data: Collection<Pair<EdgeLabel, PathAwareVariableStatusInfo>>
): PathAwareVariableStatusInfo {
if (data.isEmpty()) return PathAwareVariableStatusInfo.EMPTY
return data.map { (label, info) -> info.applyLabel(node, label) }
.reduce(PathAwareVariableStatusInfo::merge)
} }
override fun visitVariableDeclarationNode(node: VariableDeclarationNode, data: Collection<VariableStatusInfo>): VariableStatusInfo { override fun visitVariableDeclarationNode(
node: VariableDeclarationNode,
data: Collection<Pair<EdgeLabel, PathAwareVariableStatusInfo>>
): PathAwareVariableStatusInfo {
val dataForNode = visitNode(node, data) val dataForNode = visitNode(node, data)
if (node.fir.source?.kind is FirFakeSourceElementKind) return dataForNode if (node.fir.source?.kind is FirFakeSourceElementKind) return dataForNode
val symbol = node.fir.symbol val symbol = node.fir.symbol
return when (dataForNode[symbol]) { return update(dataForNode, symbol) { prev ->
null -> { when (prev) {
dataForNode.put(symbol, VariableStatus.UNUSED) null -> {
} VariableStatus.UNUSED
VariableStatus.ONLY_WRITTEN_NEVER_READ, VariableStatus.WRITTEN_AFTER_READ -> { }
if (node.fir.initializer != null && dataForNode[symbol]?.isRead == true) { VariableStatus.ONLY_WRITTEN_NEVER_READ, VariableStatus.WRITTEN_AFTER_READ -> {
val newData = dataForNode[symbol] ?: VariableStatus.UNUSED if (node.fir.initializer != null && prev.isRead) {
newData.isRedundantInit = true prev.isRedundantInit = true
dataForNode.put(symbol, newData) prev
} else if (node.fir.initializer != null) { } else if (node.fir.initializer != null) {
dataForNode.put(symbol, VariableStatus.ONLY_WRITTEN_NEVER_READ) VariableStatus.ONLY_WRITTEN_NEVER_READ
} else { } else {
dataForNode null
}
}
VariableStatus.READ -> {
VariableStatus.READ
}
else -> {
null
} }
}
VariableStatus.READ -> {
dataForNode.put(symbol, VariableStatus.READ)
}
else -> {
dataForNode
} }
} }
} }
override fun visitVariableAssignmentNode(node: VariableAssignmentNode, data: Collection<VariableStatusInfo>): VariableStatusInfo { override fun visitVariableAssignmentNode(
node: VariableAssignmentNode,
data: Collection<Pair<EdgeLabel, PathAwareVariableStatusInfo>>
): PathAwareVariableStatusInfo {
val dataForNode = visitNode(node, data) val dataForNode = visitNode(node, data)
val reference = node.fir.lValue as? FirResolvedNamedReference ?: return dataForNode val reference = node.fir.lValue as? FirResolvedNamedReference ?: return dataForNode
val symbol = reference.resolvedSymbol as? FirPropertySymbol ?: return dataForNode val symbol = reference.resolvedSymbol as? FirPropertySymbol ?: return dataForNode
val toPut = when { return update(dataForNode, symbol) update@{ prev ->
symbol !in localProperties -> { val toPut = when {
null symbol !in localProperties -> {
} null
dataForNode[symbol] == VariableStatus.READ -> { }
VariableStatus.WRITTEN_AFTER_READ prev == VariableStatus.READ -> {
} VariableStatus.WRITTEN_AFTER_READ
dataForNode[symbol] == VariableStatus.WRITTEN_AFTER_READ -> { }
VariableStatus.ONLY_WRITTEN_NEVER_READ prev == VariableStatus.WRITTEN_AFTER_READ -> {
} VariableStatus.ONLY_WRITTEN_NEVER_READ
else -> { }
VariableStatus.ONLY_WRITTEN_NEVER_READ.merge(dataForNode[symbol] ?: VariableStatus.UNUSED) else -> {
VariableStatus.ONLY_WRITTEN_NEVER_READ.merge(prev ?: VariableStatus.UNUSED)
}
} }
toPut ?: return@update null
toPut.isRead = prev?.isRead ?: false
toPut
} }
toPut ?: return dataForNode
toPut.isRead = dataForNode[symbol]?.isRead ?: false
return dataForNode.put(symbol, toPut)
} }
override fun visitQualifiedAccessNode(node: QualifiedAccessNode, data: Collection<VariableStatusInfo>): VariableStatusInfo { override fun visitQualifiedAccessNode(
node: QualifiedAccessNode,
data: Collection<Pair<EdgeLabel, PathAwareVariableStatusInfo>>
): PathAwareVariableStatusInfo {
val dataForNode = visitNode(node, data) val dataForNode = visitNode(node, data)
if (node.fir.source?.kind is FirFakeSourceElementKind) return dataForNode if (node.fir.source?.kind is FirFakeSourceElementKind) return dataForNode
val reference = node.fir.calleeReference as? FirResolvedNamedReference ?: return dataForNode val reference = node.fir.calleeReference as? FirResolvedNamedReference ?: return dataForNode
@@ -197,7 +238,27 @@ object UnusedChecker : FirControlFlowChecker() {
val status = VariableStatus.READ val status = VariableStatus.READ
status.isRead = true status.isRead = true
return dataForNode.put(symbol, status) return update(dataForNode, symbol) { status }
}
private fun update(
info: PathAwareVariableStatusInfo,
symbol: FirPropertySymbol,
updater: (VariableStatus?) -> VariableStatus?,
): PathAwareVariableStatusInfo {
var resultMap = persistentMapOf<EdgeLabel, VariableStatusInfo>()
var changed = false
for (label in info.keys) {
val dataPerLabel = info[label]!!
val v = updater.invoke(dataPerLabel[symbol])
if (v != null) {
resultMap = resultMap.put(label, dataPerLabel.put(symbol, v))
changed = true
} else {
resultMap = resultMap.put(label, dataPerLabel)
}
}
return if (changed) PathAwareVariableStatusInfo(resultMap) else info
} }
} }