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

Fix issues with using being added erroneously #31

Merged
merged 7 commits into from
Oct 8, 2024
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
157 changes: 91 additions & 66 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,85 +15,75 @@ on:
tags: [v*]

env:
PGP_PASSPHRASE: ${{ secrets.PGP_PASSPHRASE }}
SONATYPE_PASSWORD: ${{ secrets.SONATYPE_PASSWORD }}
SONATYPE_CREDENTIAL_HOST: ${{ secrets.SONATYPE_CREDENTIAL_HOST }}
SONATYPE_USERNAME: ${{ secrets.SONATYPE_USERNAME }}
PGP_SECRET: ${{ secrets.PGP_SECRET }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}


concurrency:
group: ${{ github.workflow }} @ ${{ github.ref }}
cancel-in-progress: true

jobs:
build:
name: Build and Test
strategy:
matrix:
os: [ubuntu-latest]
scala: [2.12.17]
scala: [2.12]
java: [temurin@8]
runs-on: ${{ matrix.os }}
timeout-minutes: 60
steps:
- name: Install sbt
if: contains(runner.os, 'macos')
run: brew install sbt

- name: Checkout current branch (full)
uses: actions/checkout@v2
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Download Java (temurin@8)
id: download-java-temurin-8
if: matrix.java == 'temurin@8'
uses: typelevel/download-java@v1
with:
distribution: temurin
java-version: 8

- name: Setup Java (temurin@8)
id: setup-java-temurin-8
if: matrix.java == 'temurin@8'
uses: actions/setup-java@v2
uses: actions/setup-java@v4
with:
distribution: jdkfile
distribution: temurin
java-version: 8
jdkFile: ${{ steps.download-java-temurin-8.outputs.jdkFile }}
cache: sbt

- name: Cache sbt
uses: actions/cache@v2
with:
path: |
~/.sbt
~/.ivy2/cache
~/.coursier/cache/v1
~/.cache/coursier/v1
~/AppData/Local/Coursier/Cache/v1
~/Library/Caches/Coursier/v1
key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}
- name: sbt update
if: matrix.java == 'temurin@8' && steps.setup-java-temurin-8.outputs.cache-hit == 'false'
run: sbt +update

- name: Check that workflows are up to date
run: sbt githubWorkflowCheck

- name: Check headers and formatting
if: matrix.java == 'temurin@8'
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
run: sbt '++ ${{ matrix.scala }}' headerCheckAll scalafmtCheckAll 'project /' scalafmtSbtCheck

- name: Test
run: sbt '++ ${{ matrix.scala }}' test

- name: Check binary compatibility
if: matrix.java == 'temurin@8'
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
run: sbt '++ ${{ matrix.scala }}' mimaReportBinaryIssues

- name: Generate API documentation
if: matrix.java == 'temurin@8'
if: matrix.java == 'temurin@8' && matrix.os == 'ubuntu-latest'
run: sbt '++ ${{ matrix.scala }}' doc

- name: Make target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
run: mkdir -p rules/target/jvm-2.12 rules/target/jvm-2.13 target/testsAggregate/target target tests/target/target3-jvm-2.13 output/target/jvm-3 input/target/jvm-3 project/target
run: mkdir -p rules/target/jvm-2.12 rules/target/jvm-2.13 project/target

- name: Compress target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
run: tar cf targets.tar rules/target/jvm-2.12 rules/target/jvm-2.13 target/testsAggregate/target target tests/target/target3-jvm-2.13 output/target/jvm-3 input/target/jvm-3 project/target
run: tar cf targets.tar rules/target/jvm-2.12 rules/target/jvm-2.13 project/target

- name: Upload target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
with:
name: target-${{ matrix.os }}-${{ matrix.java }}-${{ matrix.scala }}
path: targets.tar
Expand All @@ -105,63 +95,98 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
scala: [2.12.17]
java: [temurin@8]
runs-on: ${{ matrix.os }}
steps:
- name: Install sbt
if: contains(runner.os, 'macos')
run: brew install sbt

- name: Checkout current branch (full)
uses: actions/checkout@v2
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Download Java (temurin@8)
id: download-java-temurin-8
- name: Setup Java (temurin@8)
id: setup-java-temurin-8
if: matrix.java == 'temurin@8'
uses: typelevel/download-java@v1
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: 8
cache: sbt

- name: Setup Java (temurin@8)
if: matrix.java == 'temurin@8'
uses: actions/setup-java@v2
with:
distribution: jdkfile
java-version: 8
jdkFile: ${{ steps.download-java-temurin-8.outputs.jdkFile }}
- name: sbt update
if: matrix.java == 'temurin@8' && steps.setup-java-temurin-8.outputs.cache-hit == 'false'
run: sbt +update

- name: Cache sbt
uses: actions/cache@v2
with:
path: |
~/.sbt
~/.ivy2/cache
~/.coursier/cache/v1
~/.cache/coursier/v1
~/AppData/Local/Coursier/Cache/v1
~/Library/Caches/Coursier/v1
key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}

- name: Download target directories (2.12.17)
uses: actions/download-artifact@v2
- name: Download target directories (2.12)
uses: actions/download-artifact@v4
with:
name: target-${{ matrix.os }}-${{ matrix.java }}-2.12.17
name: target-${{ matrix.os }}-${{ matrix.java }}-2.12

- name: Inflate target directories (2.12.17)
- name: Inflate target directories (2.12)
run: |
tar xf targets.tar
rm targets.tar
- name: Import signing key
if: env.PGP_SECRET != '' && env.PGP_PASSPHRASE == ''
run: echo $PGP_SECRET | base64 -di | gpg --import
env:
PGP_SECRET: ${{ secrets.PGP_SECRET }}
PGP_PASSPHRASE: ${{ secrets.PGP_PASSPHRASE }}
run: echo $PGP_SECRET | base64 -d -i - | gpg --import

- name: Import signing key and strip passphrase
if: env.PGP_SECRET != '' && env.PGP_PASSPHRASE != ''
env:
PGP_SECRET: ${{ secrets.PGP_SECRET }}
PGP_PASSPHRASE: ${{ secrets.PGP_PASSPHRASE }}
run: |
echo "$PGP_SECRET" | base64 -di > /tmp/signing-key.gpg
echo "$PGP_SECRET" | base64 -d -i - > /tmp/signing-key.gpg
echo "$PGP_PASSPHRASE" | gpg --pinentry-mode loopback --passphrase-fd 0 --import /tmp/signing-key.gpg
(echo "$PGP_PASSPHRASE"; echo; echo) | gpg --command-fd 0 --pinentry-mode loopback --change-passphrase $(gpg --list-secret-keys --with-colons 2> /dev/null | grep '^sec:' | cut --delimiter ':' --fields 5 | tail -n 1)
- name: Publish
run: sbt '++ ${{ matrix.scala }}' tlRelease
env:
SONATYPE_USERNAME: ${{ secrets.SONATYPE_USERNAME }}
SONATYPE_PASSWORD: ${{ secrets.SONATYPE_PASSWORD }}
SONATYPE_CREDENTIAL_HOST: ${{ secrets.SONATYPE_CREDENTIAL_HOST }}
run: sbt tlCiRelease

dependency-submission:
name: Submit Dependencies
if: github.event.repository.fork == false && github.event_name != 'pull_request'
strategy:
matrix:
os: [ubuntu-latest]
java: [temurin@8]
runs-on: ${{ matrix.os }}
steps:
- name: Install sbt
if: contains(runner.os, 'macos')
run: brew install sbt

- name: Checkout current branch (full)
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Setup Java (temurin@8)
id: setup-java-temurin-8
if: matrix.java == 'temurin@8'
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: 8
cache: sbt

- name: sbt update
if: matrix.java == 'temurin@8' && steps.setup-java-temurin-8.outputs.cache-hit == 'false'
run: sbt +update

- name: Submit Dependencies
uses: scalacenter/sbt-dependency-submission@v2
with:
modules-ignore: tests_2.12 scala3-scalafix-root_2.12 tests_2.13 output_3 input_3
configs-ignore: test scala-tool scala-doc-tool test-internal
8 changes: 4 additions & 4 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
lazy val V = _root_.scalafix.sbt.BuildInfo

lazy val rulesCrossVersions = Seq(V.scala213, V.scala212)
lazy val scala3Version = "3.2.1"
lazy val scala3Version = "3.3.4"

inThisBuild(
List(
Expand All @@ -26,7 +26,7 @@ inThisBuild(
url("https://github.com/hamnis")
)
),
tlSonatypeUseLegacyHost := true,
sonatypeCredentialHost := xerial.sbt.Sonatype.sonatypeLegacy,
semanticdbEnabled := true,
semanticdbVersion := scalafixSemanticdb.revision
)
Expand All @@ -35,9 +35,9 @@ inThisBuild(
val circeDeps = List(
"io.circe" %% "circe-generic",
"io.circe" %% "circe-core"
).map(_ % "0.14.2")
).map(_ % "0.14.10")

val doobie = "org.tpolecat" %% "doobie-core" % "1.0.0-RC2"
val doobie = "org.tpolecat" %% "doobie-core" % "1.0.0-RC6"

lazy val `scala3-scalafix-root` = (project in file("."))
.enablePlugins(NoPublishPlugin)
Expand Down
9 changes: 9 additions & 0 deletions input/src/main/scala-3/fix/GivenAndUsingTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,13 @@ object ObjectWithApply {
def call1(myClass2: MyClass2) = inner1("")(myClass2)
def call2(myClass2: MyClass2) = inner2(myClass2)("")
}
object WithExplicitUsing {
def test(using i: Int): Int = i
test(using 1)
}
object WithApplyAfterUsing {
given i: Int = 1
def test(using i: Int): String => String = s => s
test("")
}
// format: on
9 changes: 9 additions & 0 deletions output/src/main/scala-3/fix/GivenAndUsingTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,13 @@ object ObjectWithApply {
def call1(myClass2: MyClass2) = inner1("")(using myClass2)
def call2(myClass2: MyClass2) = inner2(using myClass2)("")
}
object WithExplicitUsing {
def test(using i: Int): Int = i
test(using 1)
}
object WithApplyAfterUsing {
given i: Int = 1
def test(using i: Int): String => String = s => s
test("")
}
// format: on
2 changes: 1 addition & 1 deletion project/build.properties
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version=1.8.2
sbt.version=1.10.2
8 changes: 4 additions & 4 deletions project/plugins.sbt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
resolvers ++= Resolver.sonatypeOssRepos("releases")
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.10.4")
addSbtPlugin("com.eed3si9n" % "sbt-projectmatrix" % "0.9.0")
addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.0")
addSbtPlugin("org.typelevel" % "sbt-typelevel" % "0.4.17")
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.13.0")
addSbtPlugin("com.eed3si9n" % "sbt-projectmatrix" % "0.10.0")
addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.2")
addSbtPlugin("org.typelevel" % "sbt-typelevel" % "0.7.3")
36 changes: 18 additions & 18 deletions rules/src/main/scala/fix/GivenAndUsing.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package fix

import fix.matchers.ApplyImplicitArgs
import fix.matchers.{ApplyImplicitArgs, ImplicitOrUsingMod}
import scalafix.lint.LintSeverity
import scalafix.v1._

Expand All @@ -35,9 +35,9 @@ class GivenAndUsing extends SemanticRule("GivenAndUsing") {
if (onlyImplicitOrUsingParams(d)) replaceWithGiven(d, "def")
else APatch.Lint(GivenFunctionWithArgs(d))
else APatch.Empty
) ++ replaceWithUsing(d.paramss)
) ++ replaceWithUsing(d.paramClauseGroups.flatMap(_.paramClauses).map(_.values))
case c: Defn.Class =>
replaceWithUsing(c.ctor.paramss)
replaceWithUsing(c.ctor.paramClauses.map(_.values).toList)
}

val givenImports = givenAndUsingPass.flatMap(_.collect { case APatch.Given(_, symbol) => symbol.owner }).toSet
Expand All @@ -50,7 +50,7 @@ class GivenAndUsing extends SemanticRule("GivenAndUsing") {
else if (importees.exists(_.is[Importee.GivenAll])) Patch.empty
else Patch.addLeft(importees.head, "given")

case ApplyImplicitArgs(symbol, args) if usingRefs.contains(symbol) =>
case ApplyImplicitArgs(symbol, args) if usingRefs.contains(symbol) && !args.mod.exists(_.is[Mod.Using]) =>
args.headOption.map(h => Patch.addLeft(h, "using ")).getOrElse(Patch.empty)

}.asPatch
Expand All @@ -59,25 +59,24 @@ class GivenAndUsing extends SemanticRule("GivenAndUsing") {
}

private def onlyImplicitOrUsingParams(d: Defn.Def): Boolean =
d.paramss.forall(_.forall(_.mods.exists(m => m.is[Mod.Implicit] || m.is[Mod.Using])))
d.paramClauseGroups.forall(_.paramClauses.forall(_.mod.exists {
case ImplicitOrUsingMod(_) => true
case _ => false
}))

private def replaceWithUsing(paramss: List[List[Term.Param]])(implicit doc: SemanticDocument): List[APatch] = {
val usingPatch = paramss.reverse.flatten
.collectFirst {
case p: Term.Param if p.mods.exists(mod => mod.is[Mod.Implicit] || mod.is[Mod.Using]) =>
val pp = p.mods
.find(_.is[Mod.Implicit])
.toList
.flatMap(_.tokens)
.headOption
.collectFirst { case p @ ImplicitOrUsingMod(mod) =>
if (mod.is[Mod.Using])
APatch.Empty
else {
val pp = mod.tokens.headOption
.map(t => Patch.replaceToken(t, "using"))
.asPatch
APatch.Using(pp, p.symbol.owner)
}
}
val lintPatchers = paramss.flatMap(_.collectFirst {
case p: Term.Param if p.mods.exists(mod => mod.is[Mod.Implicit] || mod.is[Mod.Using]) =>
p.mods.find(mod => mod.is[Mod.Implicit] || mod.is[Mod.Using])
}.toList.flatten) match {
val lintPatchers = paramss.flatMap(_.collectFirst { case ImplicitOrUsingMod(mod) => mod }.toList) match {
case Nil => List.empty
case _ :: Nil => List.empty
case other =>
Expand Down Expand Up @@ -131,9 +130,10 @@ case class GivenFunctionWithArgs(func: Defn.Def) extends Diagnostic {
"""Unable to rewrite to `given` syntax because we found a function with a non implicit argument.""".stripMargin

override def position: Position = {
val paramss = func.paramClauseGroups.flatMap(_.paramClauses).flatMap(_.values)
val fromArgs = for {
hPos <- func.paramss.headOption.flatMap(_.headOption).map(_.pos)
lPos <- func.paramss.lastOption.flatMap(_.lastOption).map(_.pos)
hPos <- paramss.headOption.map(_.pos)
lPos <- paramss.lastOption.map(_.pos)
} yield
if (hPos == lPos) hPos
else Position.Range(hPos.input, hPos.startLine, hPos.startColumn, lPos.startLine, lPos.endColumn)
Expand Down
Loading