Skip to content

Commit

Permalink
Added DivideByOne, DoubleNegation, DuplicateMapKey, DuplicateSetValue…
Browse files Browse the repository at this point in the history
… and EmptyCaseClass
  • Loading branch information
t1b00 committed Sep 12, 2024
1 parent 6842afb commit 3b5a8f8
Show file tree
Hide file tree
Showing 11 changed files with 274 additions and 1 deletion.
31 changes: 31 additions & 0 deletions input/src/main/scala/fix/DivideByOne.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
rule = DivideByOne
*/
package fix

object DivideByOne {
var a = 14
val b = a / 1 // assert: DivideByOne
a /= 1 // assert: DivideByOne
a = a / 2 // scalafix: ok;
a /= 2 // scalafix: ok;

var c = 10.0
val d = c / 1 // assert: DivideByOne
c /= 1 // assert: DivideByOne
c = c / 2 // scalafix: ok;
c /= 2 // scalafix: ok;

var e = 100L
val f = e / 1 // assert: DivideByOne
e /= 1 // assert: DivideByOne
e = e / 2 // scalafix: ok;
e /= 2 // scalafix: ok;

var g = 5.0d
val h = g / 1 // assert: DivideByOne
g /= 1 // assert: DivideByOne
g = g / 2 // scalafix: ok;
g /= 2 // scalafix: ok;

}
12 changes: 12 additions & 0 deletions input/src/main/scala/fix/DoubleNegation.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
rule = DoubleNegation
*/
package fix

object DoubleNegation {
val b = true
val c = !(!b) // assert: DoubleNegation
val d = !(!(!b)) // assert: DoubleNegation
val f = !b // scalafix: ok;

}
12 changes: 12 additions & 0 deletions input/src/main/scala/fix/DuplicateMapKey.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
rule = DuplicateMapKey
*/
package fix

object DuplicateMapKey {
Map("name" -> "sam", "location" -> "aylesbury", "name" -> "bob") // assert: DuplicateMapKey
Map("name" -> "sam", "location" -> "aylesbury", "name2" -> "bob") // scalafix: ok;

val tuples = List((1, 2), (3, 4))
Map(tuples: _*) // scalafix: ok;
}
21 changes: 21 additions & 0 deletions input/src/main/scala/fix/DuplicateSetValue.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
rule = DuplicateSetValue
*/
package fix

object DuplicateSetValue {
Set("sam", "aylesbury", "sam") // assert: DuplicateSetValue
Set("name", "location", "aylesbury", "bob")

// Works for different types of literals
Set(1, 2, 1) // assert: DuplicateSetValue
Set(2.4, 3.5, 2.4) // assert: DuplicateSetValue
Set(2.4f, 3.5f, 2.4f) // assert: DuplicateSetValue
Set(true, false, true) // assert: DuplicateSetValue
Set('a', 'b', 'a') // assert: DuplicateSetValue
Set("olivia", 2, "olivia") // assert: DuplicateSetValue
Set(2L, 3, 2L) // assert: DuplicateSetValue

def name = "could be random" // We could have def name calling random.nextInt, hence we exclude variables and only check literals
Set(name, "middle", name) // scalafix: ok;
}
31 changes: 31 additions & 0 deletions input/src/main/scala/fix/EmptyCaseClass.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
rule = EmptyCaseClass
*/
package fix

object EmptyCaseClass {
case class Empty() // assert: EmptyCaseClass
case class Empty2() {} // assert: EmptyCaseClass

case class Empty3() { // assert: EmptyCaseClass
def foo = "boo"
}

case class NonEmpty() { // scalafix: ok;
def foo = "boo"
val myVal = 42
}
class NonEmpty2() // scalafix: ok;

case class Empty4(name: String) // scalafix: ok;
case object Empty5 // scalafix: ok;

abstract class Attr(val name: String) {
override def toString: String = name
override def hashCode: Int = name.hashCode
override def equals(obj: Any): Boolean = false
}
// We don't look at case classes extending something because we could have some fields in the parent class
case class TestClass() extends Attr("test") // scalafix: ok;
case class TestClass2(bool: Boolean) extends Attr("test") // scalafix: ok;
}
7 changes: 6 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 @@ -62,4 +62,9 @@ fix.BrokenOddness
fix.ClassNames
fix.CollectionNamingConfusion
fix.ComparisonToEmptyList
fix.ComparisonToEmptySet
fix.ComparisonToEmptySet
fix.DivideByOne
fix.DoubleNegation
fix.DuplicateMapKey
fix.DuplicateSetValue
fix.EmptyCaseClass
27 changes: 27 additions & 0 deletions rules/src/main/scala/fix/DivideByOne.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
rule = DivideByOne
*/
package fix

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

import scala.meta._

class DivideByOne extends SemanticRule("DivideByOne") {

private def diag(pos: Position) = Diagnostic(
"",
"Checks for division by one.",
pos,
"Divide by one will always return the original value.",
LintSeverity.Warning
)

override def fix(implicit doc: SemanticDocument): Patch = {
doc.tree.collect {
case t @ Term.ApplyInfix.After_4_6_0(_, Term.Name("/" | "/="), _, Term.ArgClause(List(Lit.Int(1)), _)) => Patch.lint(diag(t.pos))
}.asPatch
}

}
27 changes: 27 additions & 0 deletions rules/src/main/scala/fix/DoubleNegation.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
rule = DoubleNegation
*/
package fix

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

import scala.meta._

class DoubleNegation extends SemanticRule("DoubleNegation") {

private def diag(pos: Position) = Diagnostic(
"",
"Checks for code like !(!b).",
pos,
"Double negation can be removed, e.g. !(!b) it equal to just b.",
LintSeverity.Info
)

override def fix(implicit doc: SemanticDocument): Patch = {
doc.tree.collect {
case t @ Term.ApplyUnary(Term.Name("!"), Term.ApplyUnary(Term.Name("!"), _)) => Patch.lint(diag(t.pos))
}.asPatch
}

}
34 changes: 34 additions & 0 deletions rules/src/main/scala/fix/DuplicateMapKey.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
rule = DuplicateMapKey
*/
package fix

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

import scala.collection.mutable
import scala.meta._

class DuplicateMapKey extends SemanticRule("DuplicateMapKey") {

private def diag(pos: Position) = Diagnostic(
"",
"Checks for duplicate key names in Map literals.",
pos,
"A map key is overwritten by a later entry.",
LintSeverity.Warning
)

override def fix(implicit doc: SemanticDocument): Patch = {
doc.tree.collect {
case t @ Term.Apply.After_4_6_0(Term.Name("Map"), Term.ArgClause(args, _)) =>
val seen = mutable.HashSet.empty[String]
args.collect {
// Simply look at the args, try to add them in the sent, will fail if already present.
// We chose not to support the unicode arrow (→) because it has been deprecated in Scala 2.13.
case Term.ApplyInfix.After_4_6_0(Lit.String(key), Term.Name("->"), _, _) if !seen.add(key) => Patch.lint(diag(t.pos))
}
}.flatten.asPatch
}

}
35 changes: 35 additions & 0 deletions rules/src/main/scala/fix/DuplicateSetValue.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
rule = DuplicateSetValue
*/
package fix

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

import scala.collection.mutable
import scala.meta._

class DuplicateSetValue extends SemanticRule("DuplicateSetValue") {

private def diag(pos: Position) = Diagnostic(
"",
"Checks for duplicate values in set literals.",
pos,
"A set value is overwritten by a later entry.",
LintSeverity.Warning
)

override def fix(implicit doc: SemanticDocument): Patch = {
doc.tree.collect {
case t @ Term.Apply.After_4_6_0(Term.Name("Set"), Term.ArgClause(args, _)) =>
val seen = mutable.HashSet.empty[Any]
args.collect {
// Simply look at the args, try to add them in the sent, will fail if already present.
// We limit ourselves to literals, as variables could change at runtime, see tests.
// We work with the value, not the literal, as otherwise comparisons do not function.
case value: Lit if !seen.add(value.value) => Patch.lint(diag(t.pos))
}
}.flatten.asPatch
}

}
38 changes: 38 additions & 0 deletions rules/src/main/scala/fix/EmptyCaseClass.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
rule = EmptyCaseClass
*/
package fix

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

import scala.meta._

class EmptyCaseClass extends SemanticRule("EmptyCaseClass") {

private def diag(pos: Position) = Diagnostic(
"",
"Checks for empty case classes like, e.g. case class Faceman().",
pos,
"An empty case class can be rewritten as a case object.",
LintSeverity.Info
)

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

// For a case class to be empty, it should either have an empty body or no accessors (i.e. just methods, in which case we can have a case object)
def hasAccessor(body: List[Stat]): Boolean = body.exists {
case _: Defn.Val => true
case _: Defn.Var => true
case _ => false
}

def isCaseClass(mods: List[Mod]) = mods.exists { case Mod.Case() => true; case _ => false }

doc.tree.collect {
// Corresponds to case class Empty() {} or only with methods, see above
// We should consider only cases class with empty constructors, no body / accessors and no extending classes
case c @ Defn.Class.After_4_6_0(mods, _, _, Ctor.Primary.After_4_6_0(_, _, List(Term.ParamClause(Nil, _))), Template.After_4_4_0(_, Nil, _, body, _)) if isCaseClass(mods) && (body.isEmpty || !hasAccessor(body)) => Patch.lint(diag(c.pos))
}.asPatch
}
}

0 comments on commit 3b5a8f8

Please sign in to comment.