Skip to content

Commit

Permalink
feat: Initial support for Scala 3 (#858)
Browse files Browse the repository at this point in the history
* split source into scala 2 and 3

* First Scala 3 inspection

* Rewire plugin and feedback flow

* Bootstrap compiler differently to allow to extract the raw feedback

* Clean up OptionGet and set up for more re-use

* Set up cross ci and fix readme test

* Apply suggestions from code review

Co-authored-by: Bendix Sältz <[email protected]>

* Fix scalafix and configure Scala 3 compiler options

* sbt fix

* Build dotty feedback per compile run, not compilation unit

* Improve test coverage

* Add coverage on plugin control flows

* Add coverage on feedback reporting flow

---------

Co-authored-by: Bendix Sältz <[email protected]>
  • Loading branch information
Johnnei and saeltz authored Aug 6, 2024
1 parent 483d6eb commit b282044
Show file tree
Hide file tree
Showing 301 changed files with 991 additions and 257 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/pr-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ jobs:
- 2.12.19
- 2.13.13
- 2.13.14
- 3.3.3
- 3.4.2
steps:
- uses: actions/checkout@v4
- uses: actions/setup-java@v4
Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
*.class
*.tasty
*.log

# sbt specific
Expand All @@ -22,4 +23,4 @@ target
.metals
metals.sbt

.bsp
.bsp
23 changes: 23 additions & 0 deletions .scalafix-2.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
rules = [
DisableSyntax,
LeakingImplicitClassVal,
NoAutoTupling,
NoValInForComprehension,
ProcedureSyntax,
RemoveUnused
]

DisableSyntax.noVars = false
DisableSyntax.noThrows = false
DisableSyntax.noNulls = false
DisableSyntax.noReturns = true
DisableSyntax.noWhileLoops = true
DisableSyntax.noFinalize = true
DisableSyntax.noValPatterns = true
DisableSyntax.noUniversalEquality = false
DisableSyntax.noUniversalEqualityMessage = "== and != are unsafe since they allow comparing two unrelated types"
SortImports.blocks = [
"java.",
"scala.",
"*"
]
1 change: 0 additions & 1 deletion .scalafix.conf
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ DisableSyntax,
LeakingImplicitClassVal,
NoAutoTupling,
NoValInForComprehension,
ProcedureSyntax,
RemoveUnused
]

Expand Down
2 changes: 1 addition & 1 deletion .scalafmt.conf
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
version = "3.8.3"
runner.dialect = scala213source3
runner.dialect = scala3
maxColumn = 110
docstrings.style = Asterisk
assumeStandardLibraryStripMargin = true
Expand Down
258 changes: 130 additions & 128 deletions README.md

Large diffs are not rendered by default.

60 changes: 42 additions & 18 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// compiler plugins
addCompilerPlugin("org.scalameta" % "semanticdb-scalac" % "4.9.9" cross CrossVersion.full)

name := "scalac-scapegoat-plugin"
organization := "com.sksamuel.scapegoat"
description := "Scala compiler plugin for static code analysis"
Expand All @@ -22,34 +19,50 @@ developers := List(
)
)

scalaVersion := "2.13.14"
crossScalaVersions := Seq("2.12.18", "2.12.19", "2.13.13", "2.13.14")
scalaVersion := "3.4.2"
crossScalaVersions := Seq("2.12.18", "2.12.19", "2.13.13", "2.13.14", "3.3.3", "3.4.2")
autoScalaLibrary := false
crossVersion := CrossVersion.full
crossTarget := {
// workaround for https://github.com/sbt/sbt/issues/5097
target.value / s"scala-${scalaVersion.value}"
}
versionScheme := Some("early-semver")
semanticdbEnabled := (scalaBinaryVersion.value == "3")
scalafixConfig := Some(file(if (scalaBinaryVersion.value == "3") ".scalafix.conf" else ".scalafix-2.conf"))

// https://github.com/sksamuel/scapegoat/issues/298
ThisBuild / useCoursier := false

val scalac13Options = Seq(
val scala2Options = Seq(
"-Xlint",
"-Xlint:adapted-args",
"-Xlint:nullary-unit",
)

val scalac13Options = scala2Options ++ Seq(
"-Xlint:inaccessible",
"-Xlint:infer-any",
"-Xlint:strict-unsealed-patmat",
"-Yrangepos",
"-Ywarn-unused",
"-Xsource:3"
)
val scalac12Options = Seq(
val scalac12Options = scala2Options ++ Seq(
"-Ywarn-inaccessible",
"-Ywarn-infer-any",
"-Xlint:nullary-override",
"-Xmax-classfile-name",
"254"
)
val scala3Options = Seq(
"-Ysafe-init",
"-Wnonunit-statement",
"-Wunused:all",
"-Wvalue-discard",
// Unused locals seem to wrong on Scala XML syntax
"-Wconf:msg=unused value of type scala.xml.NodeBuffer:silent",
)

scalacOptions := {
val common = Seq(
Expand All @@ -58,13 +71,11 @@ scalacOptions := {
"-feature",
"-encoding",
"utf8",
"-Xlint",
"-Xlint:adapted-args",
"-Xlint:nullary-unit"
)
common ++ (scalaBinaryVersion.value match {
case "2.12" => scalac12Options
case "2.13" => scalac13Options
case "3" => scala3Options
})
}
javacOptions ++= Seq("-source", "1.8", "-target", "1.8")
Expand All @@ -85,20 +96,33 @@ def check(code: String) = {
"""

libraryDependencies ++= Seq(
"org.scala-lang" % "scala-reflect" % scalaVersion.value % "provided",
"org.scala-lang" % "scala-compiler" % scalaVersion.value % "provided",
"org.scala-lang.modules" %% "scala-xml" % "2.3.0" excludeAll ExclusionRule(organization = "org.scala-lang"),
"org.scala-lang.modules" %% "scala-collection-compat" % "2.12.0" excludeAll ExclusionRule(organization =
"org.scala-lang"
),
"org.scala-lang" % "scala-compiler" % scalaVersion.value % "test",
"org.scalatest" %% "scalatest" % "3.2.19" % "test",
"org.mockito" % "mockito-all" % "1.10.19" % "test",
"joda-time" % "joda-time" % "2.12.7" % "test",
"org.joda" % "joda-convert" % "2.2.3" % "test",
"org.slf4j" % "slf4j-api" % "2.0.13" % "test"
"org.scalatest" %% "scalatest" % "3.2.19" % "test",
"org.mockito" % "mockito-all" % "1.10.19" % "test",
"joda-time" % "joda-time" % "2.12.7" % "test",
"org.joda" % "joda-convert" % "2.2.3" % "test",
"org.slf4j" % "slf4j-api" % "2.0.13" % "test"
)

libraryDependencies ++= (if (scalaBinaryVersion.value == "3") {
Seq(
"org.scala-lang" %% "scala3-compiler" % scalaVersion.value % "provided",
"org.scala-lang" %% "scala3-compiler" % scalaVersion.value % "test"
)
} else {
Seq(
"org.scala-lang" % "scala-reflect" % scalaVersion.value % "provided",
"org.scala-lang" % "scala-compiler" % scalaVersion.value % "provided",
"org.scala-lang" % "scala-compiler" % scalaVersion.value % "test",
compilerPlugin(
"org.scalameta" % "semanticdb-scalac" % "4.9.9" cross CrossVersion.full
)
)
})

// Test
Test / run / fork := true
Test / logBuffered := false
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/plugin.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pluginClass=com.sksamuel.scapegoat.ScapegoatPlugin
22 changes: 22 additions & 0 deletions src/main/scala-2/com/sksamuel/scapegoat/FeedbackScala2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.sksamuel.scapegoat

import scala.tools.nsc.reporters.Reporter
import scala.reflect.internal.util.Position

class FeedbackScala2(
reporter: Reporter,
configuration: Configuration
) extends Feedback[Position](configuration) {

protected def toSourcePath(pos: Position): String = pos.source.file.path
protected def toSourceLine(pos: Position): Int = pos.line

override protected def report(pos: Position, level: Level, message: String): Unit = {
level match {
case Levels.Error => reporter.error(pos, message)
case Levels.Warning => reporter.warning(pos, message)
case Levels.Info => reporter.echo(pos, message)
case Levels.Ignore => ()
}
}
}
18 changes: 18 additions & 0 deletions src/main/scala-2/com/sksamuel/scapegoat/Inspection.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.sksamuel.scapegoat

/**
* @author
* Stephen Samuel
*/
abstract class Inspection(
override val text: String,
override val defaultLevel: Level,
override val description: String,
override val explanation: String
) extends InspectionBase {

val self: Inspection = this

def inspector(ctx: InspectionContext): Inspector

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,6 @@ package com.sksamuel.scapegoat
import scala.reflect.internal.util.Position
import scala.tools.nsc.Global

/**
* @author
* Stephen Samuel
*/
abstract class Inspection(
val text: String,
val defaultLevel: Level,
val description: String,
val explanation: String
) {

val self: Inspection = this

def inspector(ctx: InspectionContext): Inspector

def isEnabled: Boolean = true

def name: String = getClass.getSimpleName
}

abstract class Inspector(val context: InspectionContext) {

/**
Expand All @@ -37,7 +17,7 @@ abstract class Inspector(val context: InspectionContext) {
def postInspection(): Unit = ()
}

final case class InspectionContext(global: Global, feedback: Feedback) {
final case class InspectionContext(global: Global, feedback: Feedback[Position]) {

def warn(pos: Position, inspection: Inspection): Unit =
feedback.warn(pos, inspection, None, None)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ class ScapegoatPlugin(val global: Global) extends Plugin {
override val optionsHelp: Option[String] = Some(Configuration.optionsHelp)
}

class ScapegoatComponent(val global: Global, inspections: Seq[Inspection])
class ScapegoatComponent(val global: Global, override val inspections: Seq[Inspection])
extends PluginComponent
with ScapegoatBasePlugin
with TypingTransformers
with Transform {

require(inspections != null)

import global._

var configuration: Configuration = null
override var configuration: Configuration = null

val debug: Boolean = false
var summary: Boolean = true
Expand All @@ -47,19 +48,9 @@ class ScapegoatComponent(val global: Global, inspections: Seq[Inspection])
override val runsAfter: List[String] = List("typer")
override val runsBefore = List[String]("patmat")

def disableAll: Boolean = configuration.disabledInspections.exists(_.compareToIgnoreCase("all") == 0)
lazy val feedback = new FeedbackScala2(global.reporter, configuration)

def activeInspections: Seq[Inspection] = {
if (configuration.enabledInspections.isEmpty)
(inspections ++ configuration.customInspectors)
.filterNot(inspection => configuration.disabledInspections.contains(inspection.name))
else
(inspections ++ configuration.customInspectors)
.filter(inspection => configuration.enabledInspections.contains(inspection.name))
}
lazy val feedback = new Feedback(global.reporter, configuration)

def writeReport(isDisabled: Boolean, reportName: String, writer: (File, Feedback) => File): Unit = {
def writeReport(isDisabled: Boolean, reportName: String, writer: (File, Feedback[_]) => File): Unit = {
if (!isDisabled) {
configuration.dataDir.foreach { outputDir =>
val output = writer(outputDir, feedback)
Expand Down
22 changes: 22 additions & 0 deletions src/main/scala-3/com/sksamuel/scapegoat/FeedbackDotty.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package com.sksamuel.scapegoat

import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.Decorators.toMessage
import dotty.tools.dotc.reporting.Diagnostic
import dotty.tools.dotc.util.SourcePosition

class FeedbackDotty(configuration: Configuration)(using ctx: Context)
extends Feedback[SourcePosition](configuration) {

protected def toSourcePath(pos: SourcePosition): String = pos.source().path()
protected def toSourceLine(pos: SourcePosition): Int = pos.line
protected def report(pos: SourcePosition, level: Level, message: String): Unit = {
level match {
case Levels.Error => ctx.reporter.report(Diagnostic.Error(message, pos))
case Levels.Warning => ctx.reporter.report(Diagnostic.Warning(message.toMessage, pos))
case Levels.Info => ctx.reporter.report(Diagnostic.Info(message, pos))
case Levels.Ignore => ()
}
}

}
18 changes: 18 additions & 0 deletions src/main/scala-3/com/sksamuel/scapegoat/Inspection.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.sksamuel.scapegoat

import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.util.SourcePosition

abstract class Inspection(
val text: String,
val defaultLevel: Level,
val description: String,
val explanation: String
) extends InspectionBase {

val self: Inspection = this

def inspect(feedback: Feedback[SourcePosition], tree: tpd.Tree)(using Context): Unit

}
14 changes: 14 additions & 0 deletions src/main/scala-3/com/sksamuel/scapegoat/InspectionTraverser.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.sksamuel.scapegoat

import dotty.tools.dotc.ast.tpd.*
import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.util.NoSource

abstract class InspectionTraverser extends TreeTraverser {

extension (tree: Tree)(using Context)
def asSnippet: Option[String] = tree.source match
case NoSource => None
case _ => Some(tree.source.content().slice(tree.sourcePos.start, tree.sourcePos.end).mkString)

}
11 changes: 11 additions & 0 deletions src/main/scala-3/com/sksamuel/scapegoat/Inspections.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.sksamuel.scapegoat

import com.sksamuel.scapegoat.inspections.option._

object Inspections {

final val inspections: List[Inspection] = List(
new OptionGet
)

}
Loading

0 comments on commit b282044

Please sign in to comment.