Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scalafmt - pure refactoring #360

Merged
merged 2 commits into from
May 16, 2020
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Scalafmt
mccartney committed May 8, 2020
commit 42ba34e81dcb4132099a3e8bdc1d26827e0c03cf
2 changes: 1 addition & 1 deletion src/main/scala/com/sksamuel/scapegoat/Feedback.scala
Original file line number Diff line number Diff line change
@@ -60,7 +60,7 @@ class Feedback(
if (shouldPrint(warning)) {
val snippetText = snippet.fold("")("\n " + _ + "\n")
val report = s"""|[scapegoat] $text
| $explanation$snippetText""".stripMargin
| $explanation$snippetText""".stripMargin
mccartney marked this conversation as resolved.
Show resolved Hide resolved

adjustedLevel match {
case Levels.Error => reporter.error(pos, report)
16 changes: 9 additions & 7 deletions src/main/scala/com/sksamuel/scapegoat/Inspection.scala
Original file line number Diff line number Diff line change
@@ -55,10 +55,11 @@ case class InspectionContext(global: Global, feedback: Feedback) {
private val SuppressWarnings = typeOf[SuppressWarnings]

@scala.annotation.tailrec
private def inspectionClass(klass: Class[_]): Class[_] = Option(klass.getEnclosingClass) match {
case None => klass
case Some(k) => inspectionClass(k)
}
private def inspectionClass(klass: Class[_]): Class[_] =
Option(klass.getEnclosingClass) match {
case None => klass
case Some(k) => inspectionClass(k)
}

private def isThisDisabled(an: AnnotationInfo): Boolean = {
val cls = inspectionClass(getClass)
@@ -113,8 +114,9 @@ case class InspectionContext(global: Global, feedback: Feedback) {
}
}
protected def isList(t: Tree): Boolean = t.tpe <:< typeOf[scala.collection.immutable.List[Any]]
protected def isMap(tree: Tree): Boolean = tree.tpe.baseClasses.exists {
_.fullName == "scala.collection.Map"
}
protected def isMap(tree: Tree): Boolean =
tree.tpe.baseClasses.exists {
_.fullName == "scala.collection.Map"
}
}
}
13 changes: 7 additions & 6 deletions src/main/scala/com/sksamuel/scapegoat/Level.scala
Original file line number Diff line number Diff line change
@@ -41,10 +41,11 @@ object Levels {
*/
case object Info extends Level

def fromName(name: String): Level = name.toLowerCase() match {
case "error" => Error
case "warning" => Warning
case "info" => Info
case _ => throw new IllegalArgumentException(s"Unrecognised level '$name'")
}
def fromName(name: String): Level =
name.toLowerCase() match {
case "error" => Error
case "warning" => Warning
case "info" => Info
case _ => throw new IllegalArgumentException(s"Unrecognised level '$name'")
}
}
32 changes: 17 additions & 15 deletions src/main/scala/com/sksamuel/scapegoat/inspections/AnyUse.scala
Original file line number Diff line number Diff line change
@@ -13,23 +13,25 @@ class AnyUse
explanation = "Code returning Any is most likely an indication of a programming error."
) {

def inspector(context: InspectionContext): Inspector = new Inspector(context) {
override def postTyperTraverser = new context.Traverser {
def inspector(context: InspectionContext): Inspector =
new Inspector(context) {
override def postTyperTraverser =
new context.Traverser {

import context.global._
import context.global._

override def inspect(tree: Tree): Unit = {
tree match {
case DefDef(mods, _, _, _, _, _) if mods.isSynthetic =>
case DefDef(mods, _, _, _, _, _) if mods.hasFlag(Flags.SetterFlags) =>
case DefDef(mods, _, _, _, _, _) if mods.hasFlag(Flags.GetterFlags) =>
case ValDef(_, _, tpt, _) if tpt.tpe =:= typeOf[Any] =>
context.warn(tree.pos, self, tree.toString.take(200))
case DefDef(_, _, _, _, tpt, _) if tpt.tpe =:= typeOf[Any] =>
context.warn(tree.pos, self, tree.toString.take(200))
case _ => continue(tree)
override def inspect(tree: Tree): Unit = {
tree match {
case DefDef(mods, _, _, _, _, _) if mods.isSynthetic =>
case DefDef(mods, _, _, _, _, _) if mods.hasFlag(Flags.SetterFlags) =>
case DefDef(mods, _, _, _, _, _) if mods.hasFlag(Flags.GetterFlags) =>
case ValDef(_, _, tpt, _) if tpt.tpe =:= typeOf[Any] =>
context.warn(tree.pos, self, tree.toString.take(200))
case DefDef(_, _, _, _, tpt, _) if tpt.tpe =:= typeOf[Any] =>
context.warn(tree.pos, self, tree.toString.take(200))
case _ => continue(tree)
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -13,38 +13,43 @@ class AvoidToMinusOne
explanation = "A range in the following format (j to k - 1) can be simplified to (j until k)."
) {

def inspector(context: InspectionContext): Inspector = new Inspector(context) {
override def postTyperTraverser = new context.Traverser {

import context.global._
import definitions._

private val Foreach = TermName("foreach")
private val Minus = TermName("$minus")
private val To = TermName("to")

private def isIntegral(tree: Tree): Boolean =
tree.tpe <:< IntTpe ||
tree.tpe <:< LongTpe ||
tree.tpe <:< typeOf[RichInt] ||
tree.tpe <:< typeOf[RichLong]

override def inspect(tree: Tree): Unit = {
tree match {
case Apply(
TypeApply(
Select(
Apply(Select(lhs, To), List(Apply(Select(loopvar, Minus), List(Literal(Constant(1)))))),
Foreach
),
_
),
_
) if isIntegral(lhs) && isIntegral(loopvar) =>
context.warn(tree.pos, self, tree.toString.take(200))
case _ => continue(tree)
def inspector(context: InspectionContext): Inspector =
new Inspector(context) {
override def postTyperTraverser =
new context.Traverser {

import context.global._
import definitions._

private val Foreach = TermName("foreach")
private val Minus = TermName("$minus")
private val To = TermName("to")

private def isIntegral(tree: Tree): Boolean =
tree.tpe <:< IntTpe ||
tree.tpe <:< LongTpe ||
tree.tpe <:< typeOf[RichInt] ||
tree.tpe <:< typeOf[RichLong]

override def inspect(tree: Tree): Unit = {
tree match {
case Apply(
TypeApply(
Select(
Apply(
Select(lhs, To),
List(Apply(Select(loopvar, Minus), List(Literal(Constant(1)))))
),
Foreach
),
_
),
_
) if isIntegral(lhs) && isIntegral(loopvar) =>
context.warn(tree.pos, self, tree.toString.take(200))
case _ => continue(tree)
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -11,20 +11,22 @@ class DoubleNegation
explanation = "Double negation can be removed, e.g. !(!b) it equal to just b."
) {

def inspector(context: InspectionContext): Inspector = new Inspector(context) {
override def postTyperTraverser = new context.Traverser {
def inspector(context: InspectionContext): Inspector =
new Inspector(context) {
override def postTyperTraverser =
new context.Traverser {

import context.global._
import context.global._

private val Bang = TermName("unary_$bang")
private val Bang = TermName("unary_$bang")

override def inspect(tree: Tree): Unit = {
tree match {
case Select(Select(_, Bang), Bang) =>
context.warn(tree.pos, self, tree.toString.take(200))
case _ => continue(tree)
override def inspect(tree: Tree): Unit = {
tree match {
case Select(Select(_, Bang), Bang) =>
context.warn(tree.pos, self, tree.toString.take(200))
case _ => continue(tree)
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -11,27 +11,30 @@ class EmptyCaseClass
explanation = "An empty case class can be rewritten as a case object."
) {

def inspector(context: InspectionContext): Inspector = new Inspector(context) {
override def postTyperTraverser = new context.Traverser {
def inspector(context: InspectionContext): Inspector =
new Inspector(context) {
override def postTyperTraverser =
new context.Traverser {

import context.global._
import context.global._

def accessors(trees: List[Tree]): List[ValDef] = {
trees
.collect {
case v: ValDef => v
def accessors(trees: List[Tree]): List[ValDef] = {
trees
.collect {
case v: ValDef => v
}
.filter(_.mods.isCaseAccessor)
}
.filter(_.mods.isCaseAccessor)
}

override def inspect(tree: Tree): Unit = {
tree match {
// body should have constructor only, and with synthetic methods it has 10 in total
case ClassDef(mods, _, List(), Template(_, _, body)) if mods.isCase && accessors(body).isEmpty =>
context.warn(tree.pos, self)
case _ => continue(tree)
override def inspect(tree: Tree): Unit = {
tree match {
// body should have constructor only, and with synthetic methods it has 10 in total
case ClassDef(mods, _, List(), Template(_, _, body))
if mods.isCase && accessors(body).isEmpty =>
context.warn(tree.pos, self)
case _ => continue(tree)
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -10,19 +10,21 @@ class FinalModifierOnCaseClass
explanation = "Using case classes without final modifier can lead to surprising breakage."
) {

override def inspector(context: InspectionContext): Inspector = new Inspector(context) {
override def inspector(context: InspectionContext): Inspector =
new Inspector(context) {

import context.global._
import context.global._

override def postTyperTraverser = new context.Traverser {
override def postTyperTraverser =
new context.Traverser {

override def inspect(tree: Tree): Unit = {
tree match {
case ClassDef(mods, _, _, _) if !mods.hasAbstractFlag && mods.isCase && !mods.isFinal =>
context.warn(tree.pos, self)
case _ => continue(tree)
override def inspect(tree: Tree): Unit = {
tree match {
case ClassDef(mods, _, _, _) if !mods.hasAbstractFlag && mods.isCase && !mods.isFinal =>
context.warn(tree.pos, self)
case _ => continue(tree)
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -13,40 +13,41 @@ class LonelySealedTrait
explanation = "A sealed trait that is not extended is considered dead code."
) {

override def inspector(context: InspectionContext): Inspector = new Inspector(context) {
override def inspector(context: InspectionContext): Inspector =
new Inspector(context) {

import context.global._
import context.global._

private val sealedClasses = mutable.HashMap[String, ClassDef]()
private val implementedClasses = mutable.HashSet[String]()
private val sealedClasses = mutable.HashMap[String, ClassDef]()
private val implementedClasses = mutable.HashSet[String]()

override def postInspection(): Unit = {
for ((name, cdef) <- sealedClasses) {
if (!implementedClasses.contains(name)) {
context.warn(cdef.pos, self)
override def postInspection(): Unit = {
for ((name, cdef) <- sealedClasses) {
if (!implementedClasses.contains(name))
context.warn(cdef.pos, self)
}
}
}

private def inspectParents(parents: List[Tree]): Unit = {
parents.foreach { parent =>
for (c <- parent.tpe.baseClasses)
implementedClasses.add(c.name.toString)
private def inspectParents(parents: List[Tree]): Unit = {
parents.foreach { parent =>
for (c <- parent.tpe.baseClasses)
implementedClasses.add(c.name.toString)
}
}
}

override def postTyperTraverser = new context.Traverser {

override def inspect(tree: Tree): Unit = {
tree match {
case cdef @ ClassDef(mods, _, _, _) if mods.isSealed =>
sealedClasses.put(cdef.name.toString, cdef)
case ClassDef(_, _, _, Template(parents, _, _)) => inspectParents(parents)
case ModuleDef(_, _, Template(parents, _, _)) => inspectParents(parents)
case _ =>
override def postTyperTraverser =
new context.Traverser {

override def inspect(tree: Tree): Unit = {
tree match {
case cdef @ ClassDef(mods, _, _, _) if mods.isSealed =>
sealedClasses.put(cdef.name.toString, cdef)
case ClassDef(_, _, _, Template(parents, _, _)) => inspectParents(parents)
case ModuleDef(_, _, Template(parents, _, _)) => inspectParents(parents)
case _ =>
}
continue(tree)
}
}
continue(tree)
}
}
}
}
Original file line number Diff line number Diff line change
@@ -12,26 +12,28 @@ class MaxParameters
"Methods having a large number of parameters are more difficult to reason about, consider refactoring this code."
) {

def inspector(context: InspectionContext): Inspector = new Inspector(context) {
override def postTyperTraverser = new context.Traverser {
def inspector(context: InspectionContext): Inspector =
new Inspector(context) {
override def postTyperTraverser =
new context.Traverser {

import context.global._
import context.global._

private def count(vparamss: List[List[ValDef]]): Int =
vparamss.foldLeft(0)((a, b) => a + b.size)
private def count(vparamss: List[List[ValDef]]): Int =
vparamss.foldLeft(0)((a, b) => a + b.size)

private def countExceeds(vparamss: List[List[ValDef]], limit: Int) =
count(vparamss) > limit
private def countExceeds(vparamss: List[List[ValDef]], limit: Int) =
count(vparamss) > limit

override def inspect(tree: Tree): Unit = {
tree match {
case DefDef(_, name, _, _, _, _) if name == nme.CONSTRUCTOR =>
case DefDef(mods, _, _, _, _, _) if mods.isSynthetic =>
case DefDef(_, _, _, vparamss, _, _) if countExceeds(vparamss, 10) =>
context.warn(tree.pos, self)
case _ => continue(tree)
override def inspect(tree: Tree): Unit = {
tree match {
case DefDef(_, name, _, _, _, _) if name == nme.CONSTRUCTOR =>
case DefDef(mods, _, _, _, _, _) if mods.isSynthetic =>
case DefDef(_, _, _, vparamss, _, _) if countExceeds(vparamss, 10) =>
context.warn(tree.pos, self)
case _ => continue(tree)
}
}
}
}
}
}
}
Loading