Merge pull request #31 from mrdziuban/no-duplicate-using

Fix issues with `using` being added erroneously
arktekk · Oct 8, 2024 · 295205c · 295205c
2 parents b5dce31 + 40a830a
commit 295205c
Show file tree

Hide file tree

Showing 14 changed files with 191 additions and 110 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -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
@@ -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
diff --git a/build.sbt b/build.sbt
@@ -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(
@@ -26,7 +26,7 @@ inThisBuild(
         url("https://github.com/hamnis")
       )
     ),
-    tlSonatypeUseLegacyHost := true,
+    sonatypeCredentialHost := xerial.sbt.Sonatype.sonatypeLegacy,
     semanticdbEnabled := true,
     semanticdbVersion := scalafixSemanticdb.revision
   )
@@ -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)

diff --git a/input/src/main/scala-3/fix/GivenAndUsingTest.scala b/input/src/main/scala-3/fix/GivenAndUsingTest.scala
@@ -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
diff --git a/output/src/main/scala-3/fix/GivenAndUsingTest.scala b/output/src/main/scala-3/fix/GivenAndUsingTest.scala
@@ -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
diff --git a/project/build.properties b/project/build.properties
@@ -1 +1 @@
-sbt.version=1.8.2
+sbt.version=1.10.2
diff --git a/project/plugins.sbt b/project/plugins.sbt
@@ -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")
diff --git a/rules/src/main/scala/fix/GivenAndUsing.scala b/rules/src/main/scala/fix/GivenAndUsing.scala
@@ -16,7 +16,7 @@
 
 package fix
 
-import fix.matchers.ApplyImplicitArgs
+import fix.matchers.{ApplyImplicitArgs, ImplicitOrUsingMod}
 import scalafix.lint.LintSeverity
 import scalafix.v1._
 
@@ -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
@@ -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
@@ -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 =>
@@ -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)