diff --git a/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala b/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala index 6f5101261..28d1411c2 100644 --- a/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala +++ b/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala @@ -56,7 +56,8 @@ final class AnalyzerPlugin(val global: Global) extends Plugin { plugin => new Any2StringAdd(global), new ThrowableObjects(global), new DiscardedMonixTask(global), - new BadSingletonComponent(global) + new BadSingletonComponent(global), + new ConstantDeclarations(global), ) private lazy val rulesByName = rules.map(r => (r.name, r)).toMap diff --git a/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerRule.scala b/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerRule.scala index 87f306d44..d5b42f0f0 100644 --- a/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerRule.scala +++ b/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerRule.scala @@ -2,8 +2,8 @@ package com.avsystem.commons package analyzer import java.io.{PrintWriter, StringWriter} - import scala.tools.nsc.Global +import scala.tools.nsc.Reporting.WarningCategory import scala.util.control.NonFatal abstract class AnalyzerRule(val global: Global, val name: String, defaultLevel: Level = Level.Warn) { @@ -28,11 +28,16 @@ abstract class AnalyzerRule(val global: Global, val name: String, defaultLevel: private def adjustMsg(msg: String): String = s"[AVS] $msg" - protected def report(pos: Position, message: String): Unit = + protected final def report( + pos: Position, + message: String, + category: WarningCategory = WarningCategory.Lint, + site: Symbol = NoSymbol + ): Unit = level match { case Level.Off => case Level.Info => reporter.echo(pos, adjustMsg(message)) - case Level.Warn => reporter.warning(pos, adjustMsg(message)) + case Level.Warn => currentRun.reporting.warning(pos, adjustMsg(message), category, site) case Level.Error => reporter.error(pos, adjustMsg(message)) } diff --git a/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/ConstantDeclarations.scala b/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/ConstantDeclarations.scala new file mode 100644 index 000000000..7911fb9dd --- /dev/null +++ b/commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/ConstantDeclarations.scala @@ -0,0 +1,37 @@ +package com.avsystem.commons +package analyzer + +import scala.tools.nsc.Global + +class ConstantDeclarations(g: Global) extends AnalyzerRule(g, "constantDeclarations", Level.Off) { + + import global._ + + def analyze(unit: CompilationUnit): Unit = unit.body.foreach { + case t@ValDef(_, name, tpt, rhs) + if t.symbol.hasGetter && t.symbol.owner.isEffectivelyFinal => + + val getter = t.symbol.getterIn(t.symbol.owner) + if (getter.isPublic && getter.isStable && getter.overrides.isEmpty) { + val constantValue = rhs.tpe match { + case ConstantType(_) => true + case _ => false + } + + def doReport(msg: String): Unit = + report(t.pos, msg, site = t.symbol) + + val firstChar = name.toString.charAt(0) + if (constantValue && (firstChar.isLower || !getter.isFinal)) { + doReport("a literal-valued constant should be declared as a `final val` with an UpperCamelCase name") + } + if (!constantValue && firstChar.isUpper && !getter.isFinal) { + doReport("a constant with UpperCamelCase name should be declared as a `final val`") + } + if (getter.isFinal && constantValue && !(tpt.tpe =:= rhs.tpe)) { + doReport("a constant with a literal value should not have an explicit type annotation") + } + } + case _ => + } +} diff --git a/commons-analyzer/src/test/scala/com/avsystem/commons/analyzer/CheckMacroPrivateTest.scala b/commons-analyzer/src/test/scala/com/avsystem/commons/analyzer/CheckMacroPrivateTest.scala index 6363d0dfe..dd5765c62 100644 --- a/commons-analyzer/src/test/scala/com/avsystem/commons/analyzer/CheckMacroPrivateTest.scala +++ b/commons-analyzer/src/test/scala/com/avsystem/commons/analyzer/CheckMacroPrivateTest.scala @@ -50,7 +50,7 @@ class CheckMacroPrivateTest extends AnyFunSuite with AnalyzerTest { |object test { | @macroPrivate def macroPrivateMethod = { println("whatever"); 5 } | @macroPrivate object macroPrivateObject { - | val x = 42 + | final val X = 42 | } |} """.stripMargin diff --git a/commons-analyzer/src/test/scala/com/avsystem/commons/analyzer/ConstantDeclarationsTest.scala b/commons-analyzer/src/test/scala/com/avsystem/commons/analyzer/ConstantDeclarationsTest.scala new file mode 100644 index 000000000..a4e52edd4 --- /dev/null +++ b/commons-analyzer/src/test/scala/com/avsystem/commons/analyzer/ConstantDeclarationsTest.scala @@ -0,0 +1,83 @@ +package com.avsystem.commons +package analyzer + +import org.scalatest.funsuite.AnyFunSuite + +class ConstantDeclarationsTest extends AnyFunSuite with AnalyzerTest { + test("literal-valued constants should be non-lazy final vals with UpperCamelCase and no type annotation") { + assertErrors(4, + """ + |object Whatever { + | // bad + | val a = 10 + | val B = 10 + | final val c = 10 + | final val D: Int = 10 + | + | // good + | final val E = 10 + |} + """.stripMargin) + } + + test("effectively final, non-literal UpperCamelCase vals should be final") { + assertErrors(1, + """ + |object Whatever { + | // bad + | val A = "foo".trim + | + | // good + | final val B = "foo".trim + | val c = "foo".trim + |} + """.stripMargin) + } + + test("no constant checking in traits or non-final classes") { + assertNoErrors( + """ + |trait Whatever { + | val a = 10 + | val B = 10 + | final val c = 10 + | final val D: Int = 10 + | val A = "foo".trim + |} + | + |class Stuff { + | val a = 10 + | val B = 10 + | final val c = 10 + | final val D: Int = 10 + | val A = "foo".trim + |} + """.stripMargin) + } + + test("no constant checking for overrides") { + assertNoErrors( + """ + |trait Whatever { + | def a: Int + |} + | + |object Stuff extends Whatever { + | val a: Int = 42 + |} + """.stripMargin) + } + + test("no constant checking for privates") { + assertNoErrors( + """ + |object Whatever { + | private val a = 10 + | private val B = 10 + | private final val c = 10 + | private final val D: Int = 10 + | private val A = "foo".trim + |} + """.stripMargin) + } +} diff --git a/docs/Analyzer.md b/docs/Analyzer.md index 8c0cbc8b9..bc1cb4c05 100644 --- a/docs/Analyzer.md +++ b/docs/Analyzer.md @@ -16,26 +16,31 @@ and [silencer](https://github.com/ghik/silencer) plugin for warning suppression. Here's a list of currently supported rules: -| Name | Default level | Description | -| --- | --- | --- | -| `importJavaUtil` | warning | Rejects direct imports of `java.util` package. | +| Name | Default level | Description | +|----------------------------| --- |------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `importJavaUtil` | warning | Rejects direct imports of `java.util` package. | | `valueEnumExhaustiveMatch` | warning | Enables (limited) exhaustive pattern match checking for [`ValueEnum`s](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/misc/ValueEnum.scala). | -| `any2stringadd` | off | Disables `any2stringadd` (concatenating arbitrary values with strings using `+`). | -| `bincompat` | warning | Enables [`@bincompat`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/bincompat.scala) checking | -| `showAst` | error | Handles [`@showAst`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/showAst.scala). | -| `findUsages` | warning | Issues a message for every occurrence of any of the symbols listed in the argument of this rule | -| `varargsAtLeast` | warning | Enables [`@atLeast`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/atLeast.scala) checking | -| `macroPrivate` | warning | Enables [`@macroPrivate`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/macroPrivate.scala) checking | -| `explicitGenerics` | warning | Enables [`@explicitGenerics`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/explicitGenerics.scala) checking | +| `any2stringadd` | off | Disables `any2stringadd` (concatenating arbitrary values with strings using `+`). | +| `bincompat` | warning | Enables [`@bincompat`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/bincompat.scala) checking | +| `showAst` | error | Handles [`@showAst`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/showAst.scala). | +| `findUsages` | warning | Issues a message for every occurrence of any of the symbols listed in the argument of this rule | +| `varargsAtLeast` | warning | Enables [`@atLeast`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/atLeast.scala) checking | +| `macroPrivate` | warning | Enables [`@macroPrivate`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/macroPrivate.scala) checking | +| `explicitGenerics` | warning | Enables [`@explicitGenerics`](https://github.com/AVSystem/scala-commons/blob/master/commons-core/src/main/scala/com/avsystem/commons/annotation/explicitGenerics.scala) checking | +| `discardedMonixTask` | warning | Makes sure that expressions evaluating to Monix `Task`s are not accidentally discarded by conversion to `Unit` | +| `throwableObjects` | warning | Makes sure that objects are never used as `Throwable`s (unless they have stack traces disabled) | +| `constantDeclarations` | warning | Checks if constants are always declared as `final val`s with `UpperCamelCase` and no explicit type annotation for literal values | Rules may be enabled and disabled in `build.sbt` with Scala compiler options: ```scala scalacOptions += "-P:AVSystemAnalyzer:,,..." ``` + `` must be one of the rule names listed above or `_` to apply to all rules. `` may be one of: + * `-` to disable rule * `*` for "info" level * _empty_ for "warning" level