Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ object ScapegoatConfig extends App {
new CatchFatal,
new CatchThrowable,
new ClassNames,
new CollectionIndexOnNonIndexedSeq,
new CollectionNamingConfusion,
new CollectionNegativeIndex,
new CollectionPromotionToAny,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.sksamuel.scapegoat.inspections.collections

import com.sksamuel.scapegoat._

/** @author Josh Rosen */
class CollectionIndexOnNonIndexedSeq extends Inspection {

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

import context.global._

private def isSeq(t: Tree) = t.tpe <:< typeOf[Seq[_]]
private def isIndexedSeq(t: Tree) = t.tpe <:< typeOf[IndexedSeq[_]]

override def inspect(tree: Tree): Unit = {
tree match {
case Apply(Select(lhs, TermName("apply")), List(_)) if isSeq(lhs) && !isIndexedSeq(lhs) =>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably worth ignoring the apply(0) / apply(constant) case (like I did in my linter implementation.

context.warn("Seq.apply() on a non-IndexedSeq may cause performance problems",
tree.pos,
Levels.Warning,
tree.toString().take(100),
CollectionIndexOnNonIndexedSeq.this)
case _ => continue(tree)
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.sksamuel.scapegoat.inspections.collections

import com.sksamuel.scapegoat.PluginRunner
import org.scalatest.{ FreeSpec, Matchers, OneInstancePerTest }

/** @author Josh Rosen */
class CollectionIndexOnNonIndexedSeqTest extends FreeSpec with Matchers with PluginRunner with OneInstancePerTest {

override val inspections = Seq(new CollectionIndexOnNonIndexedSeq)

"collection index on non indexed Seq" - {
"should report warning" in {
val code = """object Test {
List(1,2,3)(1)
Seq(1,2,3)(2)
val s: Seq[Int] = Array(1,2,3)
s(2)
} """.stripMargin

compileCodeSnippet(code)
compiler.scapegoat.feedback.warnings.size shouldBe 3
}
"should not report warning" in {
val code = """object Test {
Array(1,2,3)(1)
IndexedSeq(1,2,3)(2)
Vector(1,2,3)(2)
} """.stripMargin

compileCodeSnippet(code)
compiler.scapegoat.feedback.warnings.size shouldBe 0
}
}
}