Skip to content

Commit

Permalink
Improved UnusedMethodParameter
Browse files Browse the repository at this point in the history
  • Loading branch information
@t1b00
t1b00 committed Aug 28, 2024
1 parent 152329b commit 3c3c197
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 67 deletions.
17 changes: 16 additions & 1 deletion input/src/main/scala/fix/UnusedMethodParameter.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ object UnusedMethodParameter {

class Test1 {
val initstuff = "sammy" // assert: UnusedMethodParameter
val test = ??? // scalafix: ok;
val test = ??? // assert: UnusedMethodParameter

def foo(a: String, b: Int, c: Int): Unit = { // assert: UnusedMethodParameter
println(b)
Expand Down Expand Up @@ -124,4 +124,19 @@ object UnusedMethodParameter {
def foo(x: Int) = 42 // assert: UnusedMethodParameter
def fooUnused(@unused x: Int) = 42 // scalafix: ok;
}

trait A
class MessageHandler {
val dbActor: A = ??? // assert: UnusedMethodParameter
}
class FederationHandler(dbRef: => A) extends MessageHandler { // scalafix: ok;
class B {}

override final val dbActor: A = dbRef // assert: UnusedMethodParameter

def olivia(a: Int) = { // assert: UnusedMethodParameter
println("test")
}
}

}
134 changes: 68 additions & 66 deletions rules/src/main/scala/fix/UnusedMethodParameter.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package fix
import scalafix.lint.LintSeverity
import scalafix.v1._

import scala.annotation.nowarn
import scala.collection.mutable
import scala.meta._

Expand All @@ -21,73 +22,45 @@ class UnusedMethodParameter extends SemanticRule("UnusedMethodParameter") {

override def fix(implicit doc: SemanticDocument): Patch = {

def isNothing(sym: Symbol) = {
val symbolMatcher = SymbolMatcher.exact("scala/Nothing#")
sym.info.get.signature match {
case MethodSignature(_, _, TypeRef(_, tpe, _)) => symbolMatcher.matches(tpe) // Scala 2
case ValueSignature(TypeRef(_, symbol, _)) => symbolMatcher.matches(symbol) // Scala 3
case _ => false
}
def normalize(term: Stat): List[Stat] = term match {
case Term.Block(stats) => stats
case _ => List(term)
}
def getFields(stats: List[Stat]): Map[String, Position] = stats.collect {
case Defn.Var.After_4_7_2(_, List(Pat.Var(name)), _, _) => (name.value, name.pos)
case Defn.Val(_, List(Pat.Var(name)), _, _) => (name.value, name.pos)
}.toMap

def analyzeStats(stats: List[Stat], ctorNotVals: Seq[String], possiblyUnusedFields: List[String], positions: mutable.HashMap[String, Position]): Patch = {
stats.collect {
// Ignore main methods and overrides
case Defn.Def.After_4_6_0(_, Term.Name("main"), Some(Member.ParamClauseGroup(_, List(Term.ParamClause(List(Term.Param(_, Term.Name("args"), Some(Type.Apply.After_4_6_0(Type.Name("Array"), Type.ArgClause(List(Type.Name("String"))))), _)), _)))), _, _) => Patch.empty
// Looking at mods directly e.g. @main def hello = () or override def hello = ()
case Defn.Def.After_4_6_0(mods, _, _, _, _) if mods.exists(m => m.toString == "@main" || m.toString == "override") => Patch.empty
// Some overridden methods are not marked as overridden but this is stored in symbol information
case d @ Defn.Def.After_4_6_0(_, _, _, _, _) if d.symbol.info.exists(_.overriddenSymbols.nonEmpty) => Patch.empty

// Ignore nothing methods
case Defn.Def.After_4_6_0(_, _, _, Some(Type.Name("Nothing")), _) => Patch.empty // Methods declared returning Nothing
case t @ Defn.Def.After_4_6_0(_, _, _, _, _) if t.symbol.info.map(_.signature).exists {
case MethodSignature(_, _, TypeRef(_, tpe, _)) => SymbolMatcher.exact("scala/Nothing#").matches(tpe) // Methods inferred to return Nothing
case _ => false
} => Patch.empty

// Any other method
case Defn.Def.After_4_6_0(_, _, Some(Member.ParamClauseGroup(_, List(Term.ParamClause(params, _)))), _, body) =>
val paramNames = params.collect {
// Collect the parameter of the functions which are not marked as unused
case Term.Param(mods, p @ Term.Name(name), _, _) if !mods.exists(_.toString == "@unused") => positions.put(name, p.pos); name
}
val usedParams = body.collect {
// Collect any parameter that is used in the body
case p @ Term.Name(name) if paramNames.contains(name) => positions.put(name, p.pos); name
}
@nowarn
def unusedParams(stats: List[Stat], params: Set[String]): Set[String] = {

// Find constructor parameters that are not used in the body
(ctorNotVals.filterNot(usedParams.contains).map { e =>
Patch.lint(diag(positions(e)))
} ++ paramNames.filterNot(usedParams.contains).map { e => // Find method parameters not used in the body
Patch.lint(diag(positions(e)))
} ++ possiblyUnusedFields.filterNot(usedParams.contains).map { e => // Find fields that are not used in the body
Patch.lint(diag(positions(e)))
}).asPatch
// The three are combined into a patch

case _ => Patch.empty
}.asPatch
}
def processArgClause(args: List[Term], acc: Set[String]): Set[String] = args.foldLeft(acc) {
case (acc, e: Stat) => unusedParams(normalize(e), acc)
case (acc, _) => acc
}

def getPossiblyUnusedFields(stats: List[Stat], positions: mutable.HashMap[String, Position]) = stats.collect {
case v @ Defn.Var.After_4_7_2(_, List(Pat.Var(name)), _, _) if !isNothing(v.symbol) => positions.put(name.value, v.pos); name.value
case v @ Defn.Val(_, List(Pat.Var(name)), _, _) if !isNothing(v.symbol) => positions.put(name.value, v.pos); name.value
if (stats.isEmpty) params
else stats.foldLeft(params) {
case (acc, Term.Name(name)) if acc.contains(name) => acc - name
case (acc, others: Stat) => others.children.foldLeft(acc) {
case (acc, e: Stat) => unusedParams(normalize(e), acc)
// Argclause is not a stat, we recurse on arguments
case (acc, Term.ArgClause(args, _)) => processArgClause(args, acc)
case (acc, _) => acc
}
case (acc, _) => acc
}
}

doc.tree.collect {
case Defn.Trait.After_4_6_0(_, _, _, _, _) => Patch.empty
case Defn.Object(_, _, Template.After_4_4_0(_, _, _, stats, _)) =>
val positions = mutable.HashMap[String, Position]()
val possiblyUnusedFields = getPossiblyUnusedFields(stats, positions)
analyzeStats(stats, Nil, possiblyUnusedFields, positions) // no constructor for objects
val fields = getFields(stats)
unusedParams(stats, fields.keySet).map(e =>
Patch.lint(diag(fields(e)))
).asPatch
case Defn.Class.After_4_6_0(mods, _, _, ctor, Template.After_4_4_0(_, _, _, stats, _))
if !mods.exists(m => m.toString == "abstract") =>
// Easier to collect positions in a separate HashMap than to collect them in the three later collections and combine them
val positions = mutable.HashMap[String, Position]()
val possiblyUnusedFields = getPossiblyUnusedFields(stats, positions)

/*
* For constructor params, some params become vals / fields of the class (and should be ignored when unused):
* 1. all params in the first argument list for case classes
Expand All @@ -98,15 +71,44 @@ class UnusedMethodParameter extends SemanticRule("UnusedMethodParameter") {
// Else we consider everything
val consideredCtorVals = if (mods.exists(_.toString == "case")) ctor.paramClauses.drop(1) else ctor.paramClauses

val ctorNotVals = consideredCtorVals
val ctorNotVals: Map[String, Position] = consideredCtorVals
.flatMap(_.values) // get the values of paramclauses
.collect { case Term.Param(mods, name, _, _) if !mods.exists(m => m.toString() == "var" || m.toString() == "val" || m.toString() == "@unused") => positions.put(name.value, name.pos); name.value } // get the names of the parameters, when they are not vals / vars or unused

if (stats.isEmpty && ctorNotVals.nonEmpty) { // If we do not have stats (i.e. no class body), we should check constructor parameters nonetheless
ctorNotVals.map { e =>
Patch.lint(diag(positions(e)))
}.asPatch
} else analyzeStats(stats, ctorNotVals, possiblyUnusedFields, positions)
}.asPatch
}
.collect {
case t @ Term.Param(mods, name, _, _) if !mods.exists(m => m.toString() == "var" || m.toString() == "val" || m.toString() == "@unused") => (name.value, t.pos)
}.toMap
val fields = getFields(stats)
unusedParams(stats, fields.keySet ++ ctorNotVals.keySet).map(e =>
Patch.lint(diag(fields.getOrElse(e, ctorNotVals(e))))
).asPatch

case Defn.Def.After_4_6_0(_, Term.Name("main"), Some(Member.ParamClauseGroup(_, List(Term.ParamClause(List(Term.Param(_, Term.Name("args"), Some(Type.Apply.After_4_6_0(Type.Name("Array"), Type.ArgClause(List(Type.Name("String"))))), _)), _)))), _, _) => Patch.empty
// Looking at mods directly e.g. @main def hello = () or override def hello = ()
case Defn.Def.After_4_6_0(mods, _, _, _, _) if mods.exists(m => m.toString == "@main" || m.toString == "override") => Patch.empty
// Some overridden methods are not marked as overridden but this is stored in symbol information
case d @ Defn.Def.After_4_6_0(_, _, _, _, _) if d.symbol.info.exists(_.overriddenSymbols.nonEmpty) => Patch.empty

// Ignore nothing methods
case Defn.Def.After_4_6_0(_, _, _, Some(Type.Name("Nothing")), _) => Patch.empty // Methods declared returning Nothing
case t @ Defn.Def.After_4_6_0(_, _, _, _, _) if t.symbol.info.map(_.signature).exists {
case MethodSignature(_, _, TypeRef(_, tpe, _)) => SymbolMatcher.exact("scala/Nothing#").matches(tpe) // Methods inferred to return Nothing
case _ => false
} => Patch.empty

// Any other method
case Defn.Def.After_4_6_0(_, _, paramClause, _, body) =>
// Method might not have params
val params = paramClause match {
case Some(Member.ParamClauseGroup(_, List(Term.ParamClause(paramsValue, _)))) => paramsValue
case _ => Nil
}
val paramNames = params.collect {
// Collect the parameter of the functions which are not marked as unused
case Term.Param(mods, p @ Term.Name(name), _, _) if !mods.exists(_.toString == "@unused") => (name, p.pos)
}.toMap

unusedParams(normalize(body), paramNames.keySet).map(e =>
Patch.lint(diag(paramNames(e)))
).asPatch
}
}.asPatch
}

0 comments on commit 3c3c197

Please sign in to comment.