Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configuring the dialect in Scalafix #1373

Merged
merged 6 commits into from
Apr 29, 2021
Merged
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
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ jobs:
- "ci-211"
- "ci-212"
- "ci-213"
- "ci-3"
steps:
- uses: actions/checkout@v2
- uses: olafurpg/setup-scala@v10
Expand Down
20 changes: 15 additions & 5 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import Dependencies._
import sbt.Keys.scalacOptions

inThisBuild(
List(
onLoadMessage := s"Welcome to scalafix ${version.value}",
scalaVersion := scala213,
crossScalaVersions := List(scala213, scala212, scala211),
fork := true,
scalacOptions ++= List("-P:semanticdb:synthetics:on"),
semanticdbEnabled := true,
semanticdbVersion := scalametaV,
scalacOptions ++= List("-P:semanticdb:synthetics:on"),
scalafixScalaBinaryVersion := scalaBinaryVersion.value,
scalafixDependencies += "com.github.liancheng" %% "organize-imports" % "0.5.0"
)
Expand Down Expand Up @@ -128,14 +129,19 @@ lazy val testsInput = project
.in(file("scalafix-tests/input"))
.settings(
noPublishAndNoMima,
crossScalaVersions := List(scala3, scala213, scala212, scala211),
scalacOptions --= (if (isScala3.value)
Seq("-P:semanticdb:synthetics:on")
else Nil),
scalacOptions ~= (_.filterNot(_ == "-Yno-adapted-args")),
scalacOptions += warnAdaptedArgs.value, // For NoAutoTupling
scalacOptions += warnUnusedImports.value, // For RemoveUnused
scalacOptions += "-Ywarn-unused", // For RemoveUnusedTerms
scalacOptions ++= maxwarns.value, // Increase the maximum warnings to print
logLevel := Level.Error, // avoid flood of compiler warnings
libraryDependencies ++= testsDeps,
libraryDependencies += "org.scala-lang" % "scala-reflect" % scalaVersion.value,
libraryDependencies ++= (if (isScala2.value)
testsDeps :+ "org.scala-lang" % "scala-reflect" % scalaVersion.value
else Nil),
coverageEnabled := false
)
.disablePlugins(ScalafixPlugin)
Expand All @@ -144,9 +150,13 @@ lazy val testsOutput = project
.in(file("scalafix-tests/output"))
.settings(
noPublishAndNoMima,
scalacOptions --= (if (scalaVersion.value.startsWith("3"))
Seq("-P:semanticdb:synthetics:on")
else Nil),
scalacOptions -= warnUnusedImports.value,
libraryDependencies ++= testsDeps,
libraryDependencies += "org.scala-lang" % "scala-reflect" % scalaVersion.value,
libraryDependencies ++= (if (scalaVersion.value.startsWith("2"))
testsDeps :+ "org.scala-lang" % "scala-reflect" % scalaVersion.value
else Nil),
coverageEnabled := false
)
.disablePlugins(ScalafixPlugin)
Expand Down
1 change: 1 addition & 0 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ object Dependencies {
val scala211 = "2.11.12"
val scala212 = "2.12.13"
val scala213 = "2.13.5"
val scala3 = "3.0.0-RC3"
// we support 3 last binary versions of scala212 and scala213
val testedPreviousScalaVersions: Map[String, List[String]] =
List(scala213, scala212).map(version => version -> previousVersions(version)).toMap
Expand Down
4 changes: 3 additions & 1 deletion project/Mima.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ object Mima {
ProblemFilters.exclude[ReversedMissingMethodProblem]("scalafix.interfaces.ScalafixFileEvaluation.getError"),
ProblemFilters.exclude[DirectMissingMethodProblem]("scalafix.v1.SemanticDocument.fromPath"),
ProblemFilters.exclude[Problem]("scala.meta.internal.*"),
ProblemFilters.exclude[Problem]("scala.tools.nsc.interactive.*")
ProblemFilters.exclude[Problem]("scala.tools.nsc.interactive.*"),
ProblemFilters.exclude[Problem]("scalafix.syntax.package.XtensionDocument"),
ProblemFilters.exclude[MissingClassProblem]("scalafix.syntax.package$XtensionDocument")
)
}
}
11 changes: 11 additions & 0 deletions project/ScalafixBuild.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
lazy val isFullCrossVersion = Seq(
crossVersion := CrossVersion.full
)
lazy val isScala3 = Def.setting {
scalaVersion.value.startsWith("3")
}
lazy val isScala2 = Def.setting {
scalaVersion.value.startsWith("2")
}
lazy val isScala213 = Def.setting {
scalaVersion.value.startsWith("2.13")
}
Expand Down Expand Up @@ -108,6 +114,11 @@ object ScalafixBuild extends AutoPlugin with GhpagesKeys {
"unit/test:runMain scalafix.tests.util.SaveExpect" ::
s
},
commands += Command.command("ci-3") { s =>
s"""set testsInput/scalaVersion := "$scala3"""" ::
s"""set testsOutput/scalaVersion := "$scala3"""" ::
"unit/testOnly scalafix.tests.rule.RuleSuite" :: s
},
commands += Command.command("ci-213") { s =>
s"""set ThisBuild/scalaVersion := "$scala213"""" ::
"unit/test" ::
Expand Down
57 changes: 43 additions & 14 deletions scalafix-cli/src/main/scala/scalafix/internal/v1/Args.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import scala.util.Failure
import scala.util.Success
import scala.util.Try

import scala.meta.Dialect
import scala.meta.internal.io.PathIO
import scala.meta.internal.symtab.SymbolTable
import scala.meta.io.AbsolutePath
Expand All @@ -33,6 +34,7 @@ import scalafix.interfaces.ScalafixMainCallback
import scalafix.internal.config.FilterMatcher
import scalafix.internal.config.PrintStreamReporter
import scalafix.internal.config.ScalafixConfig
import scalafix.internal.config.ScalafixMetaconfigReaders
import scalafix.internal.diff.DiffDisable
import scalafix.internal.interfaces.MainCallbackImpl
import scalafix.internal.jgit.JGitDiff
Expand All @@ -54,6 +56,11 @@ case class Args(
@ExtraName("remainingArgs")
@ExtraName("f")
files: List[AbsolutePath] = Nil,
@Description(
"default Scala2" +
"Possibilities: Scala2, Scala3"
)
dialect: Option[Dialect] = None,
@Description(
"File path to a .scalafix.conf configuration file. " +
"Defaults to .scalafix.conf in the current working directory, if any."
Expand Down Expand Up @@ -214,6 +221,16 @@ case class Args(
}
}

def configureDialect(
scalafixConfig: ScalafixConfig
): Configured[ScalafixConfig] = {
dialect match {
case Some(value) =>
Configured.ok(scalafixConfig.copy(dialect = value))
case None => Configured.ok(scalafixConfig)
}
}

def fileConfig: Configured[Conf] = {
val toRead: Option[AbsolutePath] = config.orElse {
val defaultPath = cwd.resolve(".scalafix.conf")
Expand Down Expand Up @@ -377,20 +394,25 @@ case class Args(
configuredSymtab |@|
configuredRules(base, scalafixConfig) |@|
resolvedPathReplace |@|
configuredDiffDisable
).map { case ((((root, symtab), rulez), pathReplace), diffDisable) =>
ValidatedArgs(
this,
symtab,
rulez,
scalafixConfig,
classLoader,
root,
pathReplace,
diffDisable,
delegator,
semanticdbFilterMatcher
)
configuredDiffDisable |@|
configureDialect(scalafixConfig)
).map {
case (
((((root, symtab), rulez), pathReplace), diffDisable),
scalafixConfig
) =>
ValidatedArgs(
this,
symtab,
rulez,
scalafixConfig,
classLoader,
root,
pathReplace,
diffDisable,
delegator,
semanticdbFilterMatcher
)
}
}
}
Expand Down Expand Up @@ -430,6 +452,12 @@ object Args {
ConfDecoder.stringConfDecoder.map(glob =>
FileSystems.getDefault.getPathMatcher("glob:" + glob)
)
implicit val dialectDecoder: ConfDecoder[Dialect] =
ScalafixMetaconfigReaders.dialectReader
implicit val dialectEncoder: ConfEncoder[Dialect] =
ConfEncoder.StringEncoder.contramap(_.toString)
implicit val dialectPrint: TPrint[Dialect] =
TPrint.make[Dialect](_ => "dialect")
implicit val callbackDecoder: ConfDecoder[ScalafixMainCallback] =
ConfDecoder.stringConfDecoder.map(_ => MainCallbackImpl.default)

Expand Down Expand Up @@ -457,6 +485,7 @@ object Args {
TPrint.make[PathMatcher](_ => "<glob>")
implicit val confPrint: TPrint[Conf] =
TPrint.make[Conf](implicit cfg => TPrint.implicitly[ScalafixConfig].render)

implicit def optionPrint[T](implicit
ev: pprint.TPrint[T]
): TPrint[Option[T]] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import scala.util.Try
import scala.util.control.NoStackTrace
import scala.util.control.NonFatal

import scala.meta.Tree
import scala.meta.inputs.Input
import scala.meta.internal.semanticdb.TextDocument
import scala.meta.io.AbsolutePath
Expand Down Expand Up @@ -293,10 +292,7 @@ object MainOps {
input: Input,
file: AbsolutePath
): PatchInternals.ResultWithContext = {
val tree = LazyValue.later { () =>
args.parse(input).get: Tree
}
val doc = SyntacticDocument(input, tree, args.diffDisable, args.config)
val doc = SyntacticDocument(input, args.diffDisable, args.config)
if (args.rules.isSemantic) {
val relpath = file.toRelative(args.sourceroot)
val sdoc = SemanticDocument.fromPath(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ case class ValidatedArgs(

def parse(input: Input): Parsed[Source] = {
import scala.meta._
val dialect = config.parser.dialectForFile(input.syntax)
val dialect = config.dialectForFile(input.syntax)
dialect(input).parse[Source]
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
package scala.meta.internal.scalafix

import scala.meta.Dialect
import scala.meta.Tree
import scala.meta.dialects
import scala.meta.internal.semanticdb.Scala.Names
import scala.meta.internal.trees.Origin

object ScalafixScalametaHacks {
def dialect(language: String): Dialect =
if (language == "Scala") dialects.Scala212
else if (language.isEmpty) dialects.Scala212
else Dialect.standards(language)
def resetOrigin(tree: Tree): Tree = tree.withOrigin(Origin.None)
def encode(name: String): String = Names.encode(name)
}
Original file line number Diff line number Diff line change
@@ -1,37 +1 @@
package scalafix.internal.config

import java.nio.file.FileSystems
import java.nio.file.PathMatcher

import scala.meta.Dialect

import metaconfig._

case class ParserConfig(
literalTypes: Boolean = true,
trailingCommas: Boolean = true,
inlineKeyword: Boolean = false
) {

private val dialect = scala.meta.dialects.Scala213.copy(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see the point of writing our own definition of dialect. this one is not correct.
implicit val Scala213 = Scala212
.withAllowImplicitByNameParameters(true)
.withAllowLiteralTypes(true)
.withAllowNumericLiteralUnderscoreSeparators(true)
.withAllowTryWithAnyExpr(true)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure either - does it make sense to keep ParserConfig at all?

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, the motivation for the custom dialect was to allow as much as possible of Scala 3 syntax without introducing regressions for Scala 2 users. Some Scala 3 syntax can be enabled safely while other syntax like significant indent may change the parsed structure of Scala 2 programs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, thanks! I understand better! Clever idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, thanks! I understand better! Clever idea.

allowLiteralTypes = literalTypes,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

already true in 2.12

allowTrailingCommas = trailingCommas,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

already true in 2.12

allowInlineMods = inlineKeyword,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we set allowInlineMods to false, and it's true in scala3

allowInlineIdents = !inlineKeyword
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

allowInlineIdent is true since 2.10

)
private val sbtDialect = dialect.copy(allowToplevelTerms = true)

def dialectForFile(path: String): Dialect =
if (path.endsWith(".sbt")) sbtDialect
else dialect

}

object ParserConfig {
val sbtMatcher: PathMatcher =
FileSystems.getDefault.getPathMatcher("glob:*.sbt")
implicit val surface: generic.Surface[ParserConfig] =
generic.deriveSurface
implicit val codec: ConfCodec[ParserConfig] =
generic.deriveCodec(ParserConfig())
}
Original file line number Diff line number Diff line change
@@ -1,27 +1,29 @@
package scalafix.internal.config

import scala.meta._
import scala.meta.dialects.Scala212

import metaconfig._
import metaconfig.generic.Surface
import scalafix.Versions
import scalafix.internal.config.ScalafixConfig._

case class ScalafixConfig(
version: String = Versions.version,
parser: ParserConfig = ParserConfig(),
debug: DebugConfig = DebugConfig(),
groupImportsByPrefix: Boolean = true,
fatalWarnings: Boolean = true,
reporter: ScalafixReporter = ScalafixReporter.default,
patches: ConfigRulePatches = ConfigRulePatches.default,
dialect: Dialect = ScalafixConfig.DefaultDialect,
dialect: Dialect = ScalafixConfig.Scala2,
lint: LintConfig = LintConfig.default
) {

def dialectForFile(path: String): Dialect =
if (path.endsWith(".sbt")) DefaultSbtDialect
else dialect

val reader: ConfDecoder[ScalafixConfig] =
ScalafixConfig.decoder(this)

}

object ScalafixConfig {
Expand All @@ -34,33 +36,6 @@ object ScalafixConfig {
implicit lazy val ScalafixConfigDecoder: ConfDecoder[ScalafixConfig] =
decoder(default)

val DefaultDialect: Dialect = Scala212.copy(
// Are `&` intersection types supported by this dialect?
allowAndTypes = true,
// Are extractor varargs specified using ats, i.e. is `case Extractor(xs @ _*)` legal or not?
allowAtForExtractorVarargs = true,
// Are extractor varargs specified using colons, i.e. is `case Extractor(xs: _*)` legal or not?
allowColonForExtractorVarargs = true,
// Are inline vals and defs supported by this dialect?
allowInlineMods = false,
// Are literal types allowed, i.e. is `val a : 42 = 42` legal or not?
allowLiteralTypes = true,
// Are `|` (union types) supported by this dialect?
allowOrTypes = true,
// Are trailing commas allowed? SIP-27.
allowTrailingCommas = true,
// Are trait allowed to have parameters?
// They are in Dotty, but not in Scala 2.12 or older.
allowTraitParameters = true,
// Are view bounds supported by this dialect?
// Removed in Dotty.
allowViewBounds = true,
// Are `with` intersection types supported by this dialect?
allowWithTypes = true,
// Are XML literals supported by this dialect?
// We plan to deprecate XML literal syntax, and some dialects
// might go ahead and drop support completely.
allowXmlLiterals = true
)

val Scala2: Dialect = scala.meta.dialects.Scala213
val DefaultSbtDialect: Dialect = scala.meta.dialects.Sbt1
}
Loading