hairyfotr / linter Goto Github PK
View Code? Open in Web Editor NEWStatic Analysis Compiler Plugin for Scala
License: Apache License 2.0
Static Analysis Compiler Plugin for Scala
License: Apache License 2.0
Reported by @harshad-deo on gitter:
Unused method parameter check does not consider usage of types defined in the parameter
specifically, this throws a warning:
def buildT = new Witness[EmptyDenseSet#Include[f.ValueHash]]
def build[T](f: LiteralHash[T]) = new Witness[EmptyDenseSet#Include[f.ValueHash]]
This should cause a warning:
def test : Option[Type] = null
The user probably meant to return None.
Use of the parboiled2 library which uses scala macros causes a MergeNestedIfs warnings at every rule declaration
Parser.scala:6: warning: [MergeNestedIfs] These two nested ifs can be merged into one.
def Input = rule {
^
sealed trait HttpStatusCode {
val intValue: Int
}
object HttpStatusCode {
private type T = HttpStatusCode
case object BadRequest extends T { val intValue = 400 }
case object Unauthorized extends T { val intValue = 403 }
case object InternalServerError extends T { val intValue = 500 }
}
gives
warning: [UnextendedSealedTrait] This sealed trait is never extended
sealed trait HttpStatusCode {
I tried your project in our play app (2.1.1, scala 2.10) and am getting a StackOverflowError when compiling, specifically on the generated reverse routes.
The relevant output:
[error]
[error] while compiling: something/routes_reverseRouting.scala
[error] during phase: linter-typed
[error] library version: version 2.10.2
[error] compiler version: version 2.10.2
[error] reconstructed args: -encoding utf8 -deprecation -feature -bootclasspath something -Xplugin:lib/linter.jar
[error]
[error] last tree to typer: Select(This(package txt), foo)
[error] symbol: object foo in package txt (flags: )
[error] symbol definition: object foo
[error] tpe: views.txt.foo.type
[error] symbol owners: object foo -> package txt
[error] context owners: method readResolve -> object foo -> package txt
[error]
[error] == Enclosing template or block ==
[error]
[error] DefDef( // private def readResolve(): Object in object foo
[error] private
[error] "readResolve"
[error] []
[error] List(Nil)
[error] // tree.tpe=Object
[error] txt.this."foo" // object foo in package txt, tree.tpe=views.txt.foo.type
[error] )
[error]
[error] == Expanded type of tree ==
[error]
[error] TypeRef(
[error] TypeSymbol(
[error] class foo extends BaseScalaTemplate[play.api.templates.Txt,play.templates.Format[play.api.templates.Txt]] with Template4[Option[String],Option[String],Boolean,util.EnhancedRequest[_],play.api.templates.Txt]
[error]
[error] )
[error] )
[error]
[error] uncaught exception during compilation: java.lang.StackOverflowError
error java.lang.StackOverflowError
Would it be possible to publish an official release? Often I'm hacking without a network connection and have to comment out adding the linter compiler plugin because sbt tries to update the snapshot release ...
I downloaded the .jar for Scala 2.11 from the link provided in the README.md and running it with java -jar linter_2.11-0.1-SNAPSHOT.jar
gives me the error no main manifest attribute, in linter_2.11-0.1-SNAPSHOT.jar
.
Hi, I'm getting a warning
Unless there are side-effects, this col.foldLeft can be replaced by col.exists.
which in principle would be correct, except there is no exists
on my collection (scalaz.NonEmptyList
).
There are two legitimately nested if
so not sure what this is about:
[warn] /home/johannes/git/telfish/spray/spray/spray-can/src/main/scala/spray/can/server/StatsSupport.scala:45: These two ifs can be merged into one.
[warn] if (currentMoc > moc)
[warn] ^
Here's the complete method:
@tailrec
final def adjustMaxOpenConnections(): Unit = {
val co = connectionsOpened.get
val cc = connectionsClosed.get
val moc = maxOpenConnections.get
val currentMoc = co - cc
if (currentMoc > moc)
if (!maxOpenConnections.compareAndSet(moc, currentMoc)) adjustMaxOpenConnections()
}
First of all, thanks for maintaining linter! I've ran it over the current spray code base (jrudolph/spray@bc7bbb7) to see what it catches. I think I've found several issues and I'm putting them into here one by one.
[warn] /home/johannes/git/telfish/spray/spray/spray-io/src/main/scala/akka/io/Tcp.scala:91: This sealed trait is never extended.
[warn] sealed trait Message
but it definitely is
sealed trait Message
/// COMMANDS
/**
* This is the common trait for all commands understood by TCP actors.
*/
trait Command extends Message with SelectionHandler.HasFailureMessage {
def failureMessage = CommandFailed(this)
}
Not sure what the exact condition is.
The analyser incorrectly thinks that assignments in anonymous inner classes happen directly. Adding an assert(called == 0)
between the two add
s here works around the problem.
"remove" should "be idempotent" in {
val event = new Event
var called = 0
val id1 = event.add(new Runnable {
override def run(): Unit = called = 1
})
val id2 = event.add(new Runnable {
override def run(): Unit = called = 2 // Variable called has an unused value before this reassign.
})
event.remove(id1)
event.remove(id1)
event.run()
assert(called == 2)
}
It's not enough to use a string constant with BigDecimal and a large decimal expansion, you have to actually provide your own MathContext:
scala> BigDecimal("0.12958129581958395829583295823958329852395832958295382")
res0: scala.math.BigDecimal = 0.1295812958195839582958329582395833
scala> BigDecimal("0.12958129581958395829583295823958329852395832958295382", new java.math.MathContext(200))
res1: scala.math.BigDecimal = 0.12958129581958395829583295823958329852395832958295382
I'm not sure the best way to word the warning, but I think the current warning is misleading.
Thanks for your Linter!
Is it possible to have it published on a more default repository such as Maven Central? This way it will be possible to get it into our corporate Nexus.
val base = scala.collection.mutable.Map[String, Any](
"review_id" -> "abcd"
)
linter reports:
Inferred type scala.collection.mutable.Map[String,Any]. (This might not be what you've intended)
I did certainly intend to have a Map[String, Any].
is there some mechanism to integrate the finding result with sonar/jenkins or any other reporting mechenism?
Moreover, can it be configured to stop the build for any or some warning/error?
Thanks
-Chris L.
The UnnecessaryElseBranch
warning is issued when
"the
then
branch always returns"
What is the reasoning behind issuing this warning? When the if
condition fails, the else
branch is executed, so it doesn't seem unnecessary to me.
This is one of the unit tests for UnnecessaryElseBranch
:
should("""
def test(): Any = {
if(util.Random.nextBoolean) {
println("foo"); return 5; println("foo2");
} else {
println("foo3"); println("foo4");
}
}""")
I don't see why the else
branch would be unnecessary here.
Linting the following code:
package pkg
sealed trait Base
case object Sub extends pkg.Base
would incorrectly yield:
[warn] /tmp/linter/src/main/scala/Test.scala:3: [UnextendedSealedTrait] This sealed trait is never extended
[warn] sealed trait Base
[warn] ^
[warn] one warning found
Changing extends pkg.Base
to extends Base
would fix the lint error. Using the full name is sometimes necessary to prevent ambiguity.
I've found a false positive with the ContainsTypeMismatch:
val foo = Some("foo")
val bar = Some("bar")
fails with the following:
foo.exists(bar.contains(_))
foo.exists(bar.contains)
... but works with:
foo.exists(str => bar.contains(str))
To demonstrate the problem:
scala> class A; class B extends A
defined class A
defined class Bscala> val b = new B; val a: A = b
b: B = B@3358f363
a: A = B@3358f363scala> List(a).contains(b)
res9: Boolean = truescala> List(b).contains(a)
:16: warning: [ContainsTypeMismatch] List[B].contains(A) will probably return false, since the collection and target element are of unrelated types.
List(b).contains(a)
^
res10: Boolean = true
The warning shouldn't be raised because it's possible for a List of B to contain an element of type A which is a supertype (not subtype) of B- it's obvious that it does return true.
When a project has converted some data structures to e.g. shapeless records, compilation time has naturally increased. However, using linter, it spends a disproportionate amount of time in the linter-typed and linter-typed-inter. I've done some measurements using "-verbose" and the scalac-aspects project (https://github.com/gkossakowski/scalac-aspects) and for a particular file:
So while linter phases took about 35% of the time without sqltyped, they took a whopping 83% with sqltyped. So while the time in typer increased 8-9 times, linter phases increased 43 times!
I guess this is because shapeless generates a very deep type tree, but I didn't expect the linter time to grow so drastically. Do you have an idea how this could be improved? Perhaps a way to indicate to the traverser to prune a type branch and not process it at all? For instance, if there is a shapeless HList, this might indicate a very deep type structure and could be skipped, as there isn't much value in statically checking it. Maybe if shapeless indicates in some way that some of the members are synthetically generated?
Any tips are welcome.
[warn] /home/johannes/git/telfish/spray/spray/spray-can/src/main/scala/spray/can/server/PipeliningLimiter.scala:33: Parameter context is not used in method apply. (Add override if that's the reason)
[warn] def apply(context: PipelineContext, commandPL: CPL, eventPL: EPL): Pipelines =
[warn] ^
You could use typing information instead of relying on the modifiers to find out if a method is an implementation of an abstract method.
On the other hand, it's arguable if you should always use override in these cases but there are a lot of cases where adding override
just adds noise (like SAMs). Maybe make it configurable if you want all method implementations to use override and other wise don't warn when you can detect that a method is an implementation.
src/scala/com/stripe/streamish/Streamish.scala:33: warning: Parameter $this is not used in method copy$extension.
case class InvariantEvents[T](evs: Events[T]) extends AnyVal {
^
src/scala/com/stripe/streamish/Streamish.scala:33: warning: Parameter $this is not used in method productPrefix$extension.
case class InvariantEvents[T](evs: Events[T]) extends AnyVal {
^
src/scala/com/stripe/streamish/Streamish.scala:33: warning: Parameter $this is not used in method productArity$extension.
case class InvariantEvents[T](evs: Events[T]) extends AnyVal {
^
src/scala/com/stripe/streamish/Streamish.scala:33: warning: Parameter $this is not used in method canEqual$extension.
case class InvariantEvents[T](evs: Events[T]) extends AnyVal {
^
warning: there were three feature warnings; re-run with -feature for details
5 warnings found
those methods are all compiler generated when using AnyVal
. -P:linter:disable:UnusedParameter
fixes it, but that is a pretty heavy hammer to drop on default settings.
the plugin impacts compilation time quite severely
for me it goes up from about 300 secs to about 600
even enabling just one check, eg enable-only:UnlikelyEquality still gives me about 600 secs
val x = 1/0 // linter:ignore DivideByZeor
should issue a warning about unrecognized check (notice the typo).
Switch for all linter checks to be errors, and switch for info,warning,error sets of checks
I have got the following go function
import (
"fmt"
"github.com/northwesternmutual/grammes"
"github.com/stretchr/testify/assert"
"testing"
"time"
)
func connect(t *testing.T) *grammes.Client {
client, err := grammes.DialWithWebSocket("ws://localhost:8182", grammes.WithAuthUserPass("root", "root"), grammes.WithTimeout(time.Second))
assert.NoError(t, err)
return client
}
and I get the following linter error for the first function line with Google Style client, err := grammes.DialWithWebSocket("ws://localhost:8182", grammes.WithAuthUserPass("root", "root"), grammes.WithTimeout(time.Second))
:
13:73 error '.W' should have one space. google.Spacing
13:91 error Commas and periods go inside quotation marks. google.Quotes
13:115 error '.W' should have one space. google.Spacing
13:132 error '.S' should have one space. google.Spacing
I assume that code lines are skipped.
Linter gives a false positive for "method parameter not used" if there's a java file in project.
full file http://pastebin.com/uRZcR0e5
it says this method doesn't use it's parameters
public void setAcceptedAttributeNameRegex(String acceptedAttributeNameRegex) {
this.acceptedAttributeNameRegex = Pattern.compile(acceptedAttributeNameRegex);
}
and also this one
private boolean isAcceptedAttribute(Object key) {
if (acceptedAttributeNameRegex == null) {
return true;
}
if (!(key instanceof String)) {
return true;
}
return acceptedAttributeNameRegex.matcher((String)key).matches();
}
since this is clearly false...is there a way to check if file being compiled is a java source and then just skip it(no linting)?
Often I'll see code that does _.size == 0
or _.length == 0
when _.isEmpty
would be better. It's good to get people into this habit since for some collections (notably List[_]
) getting the size is an O(n) operation whereas checking emptiness is O(1).
It seems Sonatype has some problems... Use 0.1.14 or 0.1-SNAPSHOT(currently same as 0.1.15) in the meantime.
Can't write this code using foreach since it is using tailrec. Can't think of another way to write this without using null.
@tailrec
final def tick(name: String, time: FiniteDuration): Unit =
Option(times.putIfAbsent(name, time)) match { // linter:ignore UseOptionForeachNotPatMatch
case None =>
case Some(oldValue) =>
if (!times.replace(name, oldValue, oldValue + time)) {
tick(name, time)
}
}
Don't run the compiler phases that don't need to run (anything after refchecks, etc), check out Global.Run instead of IMain interpreter, where you can possibly also get positions and strings, allowing mixed tests. Non-compiling negative tests pass right now!
Say we have a macro for debugging:
@inline def debug(msg: => String): Unit = macro debugImpl
with debugImpl being equivalent to:
if (Logger.isDebugEnabled) Logger.debug(msg)
Then errors are reported on the call sites of this method:
if (someCondition) debug(msg)
<<<
Tasks:
With version 0.1.17, I get a false positive with the following code:
def test1(s: StringBuilder): Boolean = {
if (s != null) {
val trimmed = s.toString.trim
trimmed.nonEmpty
} else {
false
}
}
Warning:(123, 15) [UnnecessaryStringNonEmpty] This string will never be empty.
trimmed.nonEmpty
If I remove the intermediate variable trimmed
, the false positive disappears:
def test2(s: StringBuilder): Boolean = {
if (s != null) {
s.toString.trim.nonEmpty
} else {
false
}
}
It is also not reported if we replace StringBuilder by String as the argument:
def test3(s: String): Boolean = {
if (s != null) {
val trimmed = s.trim
trimmed.nonEmpty
} else {
false
}
}
I get this error when I try to generate scaladocs: [warn] dropping dependency on node with no phase object: refchecks
sbt doc
huitseeker➜huitseeker/tmp/TEst» sbt lint:compile [18:43:17]
[warn] /home/huitseeker/tmp/TEst/src/main/scala/Test.scala:2: Taking the modulo by one will return zero.
[warn] def f(x: Double) = x % 1
[warn] ^
[warn] one warning found
[success] Total time: 3 s, completed Nov 6, 2015 6:43:25 PM
huitseeker➜huitseeker/tmp/TEst» cat src/main/scala/Test.scala [18:43:25]
object Test {
def f(x: Double) = x % 1
}
Taking the modulo by 1 on a float or double is actually a perfectly valid way to take the fractional part.ty o
Note : I assume Division by 1 is guilty of the same problem, but didn't have time to check.
I am using SBT 0.13.11, Scala 2.11.8 and 0.1.14 version of the compiler plugin, and the ContainsTypeMismatch check does not catch any of the following:
Some("foo").contains(1)
Some(1).contains("foo")
List(4).contains("foo")
I haven't found a case where it is working properly. The UnlikelyEquality check is working fine, so I don't believe it's an issue with my build.
Imo that's almost always a bug. Also M[Any] and M[Nothing]
Thanks for the great work, btw!
Is there a version that works with Scala 2.13 ?
It would be very useful to prospective users to have a section in the README discussing the other Scala linters that are available and explaining how this project relates to those and when one might choose each.
This SO answer has links to the current Scala linters: http://stackoverflow.com/a/25117125/8261
I see that you have a list of links to other Scala linters in a section starting "Rule lists from other static analysis tools", but I can't tell if this project is more or less appropriate for my needs than the alternatives.
Thanks
When you have a range of values, this is the preffered style, and Linter shouldn't warn:
if (0 < x && x < 10) ...
Consider
@tailrec
private def f(x: Widget, token: Option[String] = None): Boolean = {
if (foo) true
else Option(x.getNextToken) match {
case None => false
case Some(tok) => f(x, Some(tok))
}
}
When a method is marked @tailrec, the linter should not trigger, since rewriting creates a non-tail-recursive function
Option(x.getNextToken).exists(t => f(x, Some(t))
This is "morally" tail recursive, but the type checker can't tell. (I didn't look at the bytecode to see if it gets compiled to a tail recursive function.)
Hi! Thanks for your work on the Linter library!
This method does not raise an unused method parameter by Linter:
def foo(param1: String, param2: Int) = {
param2 + 1
}
version: linter_2.11-0.1-SNAPSHOT
For example, the below cause the error "Inferred type Map[String,Any]. (This might not be what you've intended)". Since the method is defined as returning Map[String, Any] I would think this is acceptable code.
val exifMap = parseExif()
def parseExif(): Map[String, Any] = {}
val x = 1/0 // linter:ignore DivideByZero,YodaConditions
should cause a warning about YodaConditions
not present on the line.
I am kinda new to Scala and this might be be an SBT thing, but I get the linter warnings only the first time I compile the code. Every subsequent compile goes thorough without any warnings. This might be because the compiled class files are not compiled again unless there are any changes, but is there a way to show the warnings every time nonetheless?
looks like linter do not find conditions like
if (false) "not linted"
if (true) { "not linted" } else { "not linted" }
Here should be a warning, that conditional expression should be omitted.
My stack is:
Scala 2.10.2, sbt 0.13.0, Play 2.2.1
Linter is added as jar and scalacOpt to Build.scala file.
[warn] /home/johannes/git/telfish/spray/spray/spray-io/src/main/scala/akka/io/DirectByteBufferPool.scala:43: This condition will never hold.
[warn] try if (buffersInPool > 0)
buffersInPool
is a mutable field of the class this condition lies in.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.