diff --git a/input/src/main/scala/fix/DivideByOne.scala b/input/src/main/scala/fix/DivideByOne.scala new file mode 100644 index 0000000..72bcf9e --- /dev/null +++ b/input/src/main/scala/fix/DivideByOne.scala @@ -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; + +} diff --git a/input/src/main/scala/fix/DoubleNegation.scala b/input/src/main/scala/fix/DoubleNegation.scala new file mode 100644 index 0000000..144e50f --- /dev/null +++ b/input/src/main/scala/fix/DoubleNegation.scala @@ -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; + +} diff --git a/input/src/main/scala/fix/DuplicateMapKey.scala b/input/src/main/scala/fix/DuplicateMapKey.scala new file mode 100644 index 0000000..756e81f --- /dev/null +++ b/input/src/main/scala/fix/DuplicateMapKey.scala @@ -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; +} diff --git a/input/src/main/scala/fix/DuplicateSetValue.scala b/input/src/main/scala/fix/DuplicateSetValue.scala new file mode 100644 index 0000000..e11c3ed --- /dev/null +++ b/input/src/main/scala/fix/DuplicateSetValue.scala @@ -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; +} diff --git a/input/src/main/scala/fix/EmptyCaseClass.scala b/input/src/main/scala/fix/EmptyCaseClass.scala new file mode 100644 index 0000000..028939a --- /dev/null +++ b/input/src/main/scala/fix/EmptyCaseClass.scala @@ -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; +} diff --git a/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index 5ab490d..b88dfbd 100644 --- a/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -62,4 +62,9 @@ fix.BrokenOddness fix.ClassNames fix.CollectionNamingConfusion fix.ComparisonToEmptyList -fix.ComparisonToEmptySet \ No newline at end of file +fix.ComparisonToEmptySet +fix.DivideByOne +fix.DoubleNegation +fix.DuplicateMapKey +fix.DuplicateSetValue +fix.EmptyCaseClass \ No newline at end of file diff --git a/rules/src/main/scala/fix/DivideByOne.scala b/rules/src/main/scala/fix/DivideByOne.scala new file mode 100644 index 0000000..d900e79 --- /dev/null +++ b/rules/src/main/scala/fix/DivideByOne.scala @@ -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 + } + +} diff --git a/rules/src/main/scala/fix/DoubleNegation.scala b/rules/src/main/scala/fix/DoubleNegation.scala new file mode 100644 index 0000000..0f9304f --- /dev/null +++ b/rules/src/main/scala/fix/DoubleNegation.scala @@ -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 + } + +} diff --git a/rules/src/main/scala/fix/DuplicateMapKey.scala b/rules/src/main/scala/fix/DuplicateMapKey.scala new file mode 100644 index 0000000..c8aa679 --- /dev/null +++ b/rules/src/main/scala/fix/DuplicateMapKey.scala @@ -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 + } + +} diff --git a/rules/src/main/scala/fix/DuplicateSetValue.scala b/rules/src/main/scala/fix/DuplicateSetValue.scala new file mode 100644 index 0000000..6e32aa0 --- /dev/null +++ b/rules/src/main/scala/fix/DuplicateSetValue.scala @@ -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 + } + +} diff --git a/rules/src/main/scala/fix/EmptyCaseClass.scala b/rules/src/main/scala/fix/EmptyCaseClass.scala new file mode 100644 index 0000000..678e586 --- /dev/null +++ b/rules/src/main/scala/fix/EmptyCaseClass.scala @@ -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 + } +}