Skip to content

Commit

Permalink
Added VarCouldBeVal
Browse files Browse the repository at this point in the history
  • Loading branch information
t1b00 committed Aug 20, 2024
1 parent 2a84785 commit 94a81af
Show file tree
Hide file tree
Showing 3 changed files with 225 additions and 1 deletion.
132 changes: 132 additions & 0 deletions input/src/main/scala/fix/VarCouldBeVal.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
rule = VarCouldBeVal
*/
package fix

import scala.util.Try

object VarCouldBeVal {
def foo = {
var count = 1 // assert: VarCouldBeVal
println(count)
for (k <- 1 until 10) {
println(k + count)
}
}

def foo2 = {
var count = 1 // assert: VarCouldBeVal
println(count)
def boo(n: Int): Unit = {
println(n + count)
}
for (k <- 1 until 10) {
println(k + count)
}
}

def foo3 = {
var bar = 1 // assert: VarCouldBeVal
val myValue = 2 // scalafix: ok;
var baz = 3 // assert: VarCouldBeVal
}

def foo4 = {
var count = 1 // scalafix: ok;
if (true) count = 2
}

def foo5 = {
var count = 1 // scalafix: ok;
println(count)
for (k <- 1 until 10) {
count = count + 1
println(k + count)
}
}

def foo6 = {
var count = 1 // scalafix: ok;
println(count)
def boo(n: Int): Unit = {
count = n
println(count)
}
for (k <- 1 until 10) {
println(k + count)
}
}

def foo7 = {
var count = 1 // scalafix: ok;
try {
println("sam")
} finally {
count = 2
}
}

def foo8 = {
var count = 1 // scalafix: ok;
if (count == 10) {
println("sam")
} else {
count = count + 1
}
}

def foo9 = {
var count = 1 // scalafix: ok;
count match {
case 10 => println("sam")
case _ => count = count + 1
}
}

def foo10(b: Boolean): Int = {
var count = 0 // scalafix: ok;
if (b) count += 1
count
}

def something(): List[String] = List("a")
def foo11(): Unit = {
var items = List.empty[String] // scalafix: ok;
while ({
items = something()
items.size
} < 10) {
println(items)
}
}

trait Iterator {
def next: Int
}
object foo12 {
val iterator = new Iterator {
var last = -1 // scalafix: ok;

def next: Int = {
last = last + 1
last
}
}
}

def foo13 = {
val l = List(1, 2, 3)
var modif = 0 // scalafix: ok;
l.foreach {
case i if i % 2 == 0 => modif = i
case _ => ()
}
}

def foo14 = {
val l = List(1, 2, 3)
var modif = 0 // scalafix: ok;
l.foreach(e => modif = e)
}

}
3 changes: 2 additions & 1 deletion rules/src/main/resources/META-INF/services/scalafix.v1.Rule
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,5 @@ fix.RepeatedIfElseBody
fix.SwallowedException
fix.UnnecessaryConversion
fix.UnreachableCatch
fix.UnusedMethodParameter
fix.UnusedMethodParameter
fix.VarCouldBeVal
91 changes: 91 additions & 0 deletions rules/src/main/scala/fix/VarCouldBeVal.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
rule = VarCouldBeVal
*/
package fix

import scalafix.lint.LintSeverity
import scalafix.v1._

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

class VarCouldBeVal extends SemanticRule("VarCouldBeVal") {

private def diag(pos: Position) = Diagnostic(
"",
"Finds catch blocks that don't handle caught exceptions.",
pos,
"If you use a try/catch block to deal with an exception, you should handle all of the caught exceptions or name it ignore(d). If you're throwing another exception in the result, you should include the original exception as the cause.",
LintSeverity.Warning
)
override def fix(implicit doc: SemanticDocument): Patch = {

// See: https://scala-lang.org/files/archive/spec/2.13/06-expressions.html#assignment-operators
val notAssignmentsSet = Set("<=", ">=", "!=")
def isAssignment(op: String) = op.endsWith("=") && !notAssignmentsSet.contains(op)

def collect(block: List[Stat], vars: mutable.HashMap[String, Position]): List[Patch] = {
// Extracts the stats from blocks and otherwise converts them so that we can do a foreach on them
def normalize(term: Stat) = term match {
case Term.Block(stats) => stats
case _ => List(term)
}

@nowarn // this is because in Scala 2, t.children at line 47 is a List[Tree], but in Scala 3 it is a List[Stat], so we need to suppress the warning for the unreachable case case _ in Scala 3
def collectInner(block: List[Stat], vars: mutable.HashMap[String, Position]): Unit = {
block.foreach {
// Corresponds to var a = ...
case Defn.Var.After_4_7_2(_, List(Pat.Var(name)), _, _) => vars.put(name.value, name.pos)
// Corresponds to a = ...
case Term.Assign(Term.Name(name), _) if vars.exists(v => v._1 == name) => vars.remove(name)
// Corresponds to compound assignments e.g. +=, -=, etc.
case Term.ApplyInfix.After_4_6_0(Term.Name(name), Term.Name(op), _, _) if isAssignment(op) => vars.remove(name)

// Blocks {} and templates
case Term.Block(stats) => collectInner(stats, vars)
case Template.After_4_4_0(_, _, _, stats, _) => collectInner(stats, vars)

// Examine for loop body recursively. Condition cannot be a block so no need to be examined
case Term.For(_, body) => collectInner(normalize(body), vars)
// Examine the body of the if condition and the body of the else condition recursively
case Term.If.After_4_4_0(_, thenBody, elseBody, _) => collectInner(normalize(thenBody), vars); collectInner(normalize(elseBody), vars)
// Examine the body of the match cases recursively
case Term.Match.After_4_4_5(_, cases, _) => cases.foreach(c => collectInner(normalize(c.body), vars))
// Examine body, catches and finally condition of the try block recursively
case Term.Try(body, catches, finallyp) =>
collectInner(normalize(body), vars)
catches.foreach(c => collectInner(normalize(c.body), vars))
if (finallyp.isDefined) collectInner(normalize(finallyp.get), vars)
// Examine the condition and body of the while loop recursively, condition can have block / assignment inside
case Term.While(condition, body) => collectInner(normalize(condition), vars); collectInner(normalize(body), vars)

// Examine partial functions and functions (e.g. l.collect { ... } or l.foreach(e => ...)
case Term.PartialFunction(cases) => cases.foreach(c => collectInner(normalize(c.body), vars))
case Term.Function.After_4_6_0(_, body) => collectInner(normalize(body), vars)

// Examine argclauses e.g. l.foreach(e => ...), e => ... is the argclause
case Term.ArgClause(args, _) => args.foreach(a => collectInner(normalize(a), vars))

// Try to handle everything else
case t: Stat =>
t.children.foreach { // t.children is Tree, not compatible with normalize
case c: Stat => collectInner(normalize(c), vars)
case Term.ArgClause(args, _) => args.foreach(a => collectInner(normalize(a), vars)) // For some reason, ArgClause is not considered as a Stat even though it inherits from it
case _ => ()
}
case _ => ()
}
}

collectInner(block, vars)
vars.map(name => Patch.lint(diag(name._2))).toList
}

// We first look at the templates (start of a definition) or the blocks and then use our collector
doc.tree.collect {
case Template.After_4_4_0(_, _, _, stats, _) => collect(stats, mutable.HashMap.empty)
case Term.Block(stats) => collect(stats, mutable.HashMap.empty)
}.flatten.asPatch
}
}

0 comments on commit 94a81af

Please sign in to comment.