-
Notifications
You must be signed in to change notification settings - Fork 20
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #423 from AVSystem/constants-analyzer-rule
Analyzer rule for Scala constant declarations
- Loading branch information
Showing
6 changed files
with
146 additions
and
15 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37 changes: 37 additions & 0 deletions
37
commons-analyzer/src/main/scala/com/avsystem/commons/analyzer/ConstantDeclarations.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 _ => | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
83 changes: 83 additions & 0 deletions
83
commons-analyzer/src/test/scala/com/avsystem/commons/analyzer/ConstantDeclarationsTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters