From a68b37027aa97b1c471464581f235f47bb1b9ccf Mon Sep 17 00:00:00 2001 From: farzad-sedghi <109164663+farzad-sedghi@users.noreply.github.com> Date: Wed, 25 Jan 2023 10:31:11 -0500 Subject: [PATCH] scalafix for the rest of the BQ API changes (#4625) * scalafix for the rest of the BQ API changes * rebase to master * avoid using strings for pattern matching * PR feedbacks * format * merge master --- scalafix/build.sbt | 10 ++- .../src/main/scala/fix/FixBqSaveAsTable.scala | 23 ++++-- .../main/scala/fix/LintBqSaveAsTable.scala | 30 ------- .../src/main/scala/fix/FixBqSaveAsTable.scala | 24 ++++-- .../META-INF/services/scalafix.v1.Rule | 1 - .../src/main/scala/fix/FixBqSaveAsTable.scala | 78 ++++++++++++------- .../main/scala/fix/LintBqSaveAsTable.scala | 60 -------------- .../migrations/v0.12.0-Migration-Guide.md | 4 +- 8 files changed, 93 insertions(+), 137 deletions(-) delete mode 100644 scalafix/input-0_12/src/main/scala/fix/LintBqSaveAsTable.scala delete mode 100644 scalafix/rules/src/main/scala/fix/LintBqSaveAsTable.scala diff --git a/scalafix/build.sbt b/scalafix/build.sbt index ce633de402..db059b9f5e 100644 --- a/scalafix/build.sbt +++ b/scalafix/build.sbt @@ -47,6 +47,14 @@ def scio11(version: String) = "com.spotify" %% "scio-avro" ).map(_ % version) +def scio12(version: String) = + List( + "com.spotify" %% "scio-core", + "com.spotify" %% "scio-avro", + "com.spotify" %% "scio-google-cloud-platform", + "com.spotify" %% "scio-extra" + ).map(_ % version) + lazy val `input-0_7` = project .settings( libraryDependencies ++= scio(Scio.`0.6`) @@ -84,7 +92,7 @@ lazy val `input-0_12` = project lazy val `output-0_12` = project .settings( - libraryDependencies ++= scio10(Scio.`0.12`) + libraryDependencies ++= scio12(Scio.`0.12`) ) lazy val tests = project diff --git a/scalafix/input-0_12/src/main/scala/fix/FixBqSaveAsTable.scala b/scalafix/input-0_12/src/main/scala/fix/FixBqSaveAsTable.scala index 68b71904bf..55b5bccddf 100644 --- a/scalafix/input-0_12/src/main/scala/fix/FixBqSaveAsTable.scala +++ b/scalafix/input-0_12/src/main/scala/fix/FixBqSaveAsTable.scala @@ -13,10 +13,10 @@ import org.apache.beam.sdk.io.gcp.bigquery.BigQueryIO.Write._ object FixBqSaveAsTable { val tableRef = new TableReference() - val schema: Schema = null - val writeDisposition: WriteDisposition = null - val createDisposition: CreateDisposition = null - val tableDescription: String = null + val s: Schema = null + val wd: WriteDisposition = null + val cd: CreateDisposition = null + val td: String = null def saveAsBigQueryTable(in: SCollection[GenericRecord]): Unit = in.saveAvroAsBigQuery(tableRef) @@ -25,11 +25,20 @@ object FixBqSaveAsTable { in.saveAvroAsBigQuery(table = tableRef) def saveAsBigQueryTableMultiParamsWithoutSchema(in: SCollection[GenericRecord]): Unit = - in.saveAvroAsBigQuery(tableRef, writeDisposition = writeDisposition, createDisposition = createDisposition, tableDescription = tableDescription) + in.saveAvroAsBigQuery(tableRef, writeDisposition = wd, createDisposition = cd, tableDescription = td) def saveAsBigQueryTableMultiParamsWithoutSchemaDiffOrder(in: SCollection[GenericRecord]): Unit = - in.saveAvroAsBigQuery(tableRef, createDisposition = createDisposition, writeDisposition = writeDisposition, tableDescription = tableDescription) + in.saveAvroAsBigQuery(tableRef, createDisposition = cd, writeDisposition = wd, tableDescription = td) def saveAsBigQueryTableMultiParamsAllNamed(in: SCollection[GenericRecord]): Unit = - in.saveAvroAsBigQuery(table = tableRef, writeDisposition = writeDisposition, createDisposition = createDisposition, tableDescription = tableDescription) + in.saveAvroAsBigQuery(table = tableRef, writeDisposition = wd, createDisposition = cd, tableDescription = td) + + def saveAsBigQueryTableMultiParamsWithSchemaUnnamed(in: SCollection[GenericRecord]): Unit = + in.saveAvroAsBigQuery(tableRef, s, wd, cd, td) + + def saveAsBigQueryTableMultiParamsWithSchemaNamed(in: SCollection[GenericRecord]): Unit = + in.saveAvroAsBigQuery(tableRef, avroSchema = s, writeDisposition = wd, createDisposition = cd, tableDescription = td) + + def saveAsBigQueryTableMultiParamsNamedOrderChanged(in: SCollection[GenericRecord]): Unit = + in.saveAvroAsBigQuery(tableRef, writeDisposition = wd, avroSchema = s) } diff --git a/scalafix/input-0_12/src/main/scala/fix/LintBqSaveAsTable.scala b/scalafix/input-0_12/src/main/scala/fix/LintBqSaveAsTable.scala deleted file mode 100644 index 938e73aaf5..0000000000 --- a/scalafix/input-0_12/src/main/scala/fix/LintBqSaveAsTable.scala +++ /dev/null @@ -1,30 +0,0 @@ -/* -rule = LintBqSaveAsTable -*/ -package fix -package v0_12_0 - -import com.google.api.services.bigquery.model.TableReference -import com.spotify.scio.bigquery._ -import com.spotify.scio.extra.bigquery._ -import com.spotify.scio.values.SCollection -import org.apache.avro.generic.GenericRecord -import org.apache.avro.Schema -import org.apache.beam.sdk.io.gcp.bigquery.BigQueryIO.Write._ - -object LintBqSaveAsTable { - val tableRef = new TableReference() - val schema: Schema = null - val writeDisposition: WriteDisposition = null - val createDisposition: CreateDisposition = null - val tableDescription: String = null - - def saveAsBigQueryTableMultiParamsWithSchemaUnnamed(in: SCollection[GenericRecord]): Unit = - in.saveAvroAsBigQuery(tableRef, schema, writeDisposition, createDisposition, tableDescription) // assert: LintBqSaveAsTable - - def saveAsBigQueryTableMultiParamsWithSchemaNamed(in: SCollection[GenericRecord]): Unit = - in.saveAvroAsBigQuery(tableRef, avroSchema = schema, writeDisposition = writeDisposition, createDisposition = createDisposition, tableDescription = tableDescription) // assert: LintBqSaveAsTable - - def saveAsBigQueryTableMultiParamsUnnamed(in: SCollection[GenericRecord]): Unit = - in.saveAvroAsBigQuery(tableRef, schema, writeDisposition, createDisposition, tableDescription) // assert: LintBqSaveAsTable -} diff --git a/scalafix/output-0_12/src/main/scala/fix/FixBqSaveAsTable.scala b/scalafix/output-0_12/src/main/scala/fix/FixBqSaveAsTable.scala index 3b60437087..07ef9e5604 100644 --- a/scalafix/output-0_12/src/main/scala/fix/FixBqSaveAsTable.scala +++ b/scalafix/output-0_12/src/main/scala/fix/FixBqSaveAsTable.scala @@ -8,13 +8,14 @@ import org.apache.avro.generic.GenericRecord import org.apache.avro.Schema import org.apache.beam.sdk.io.gcp.bigquery.BigQueryIO.Write._ import com.spotify.scio.bigquery._ +import com.spotify.scio.extra.bigquery.AvroConverters.toTableSchema object FixBqSaveAsTable { val tableRef = new TableReference() - val schema: Schema = null - val writeDisposition: WriteDisposition = null - val createDisposition: CreateDisposition = null - val tableDescription: String = null + val s: Schema = null + val wd: WriteDisposition = null + val cd: CreateDisposition = null + val td: String = null def saveAsBigQueryTable(in: SCollection[GenericRecord]): Unit = in.saveAsBigQueryTable(Table.Ref(tableRef)) @@ -23,12 +24,21 @@ object FixBqSaveAsTable { in.saveAsBigQueryTable(table = Table.Ref(tableRef)) def saveAsBigQueryTableMultiParamsWithoutSchema(in: SCollection[GenericRecord]): Unit = - in.saveAsBigQueryTable(Table.Ref(tableRef), writeDisposition = writeDisposition, createDisposition = createDisposition, tableDescription = tableDescription) + in.saveAsBigQueryTable(Table.Ref(tableRef), writeDisposition = wd, createDisposition = cd, tableDescription = td) def saveAsBigQueryTableMultiParamsWithoutSchemaDiffOrder(in: SCollection[GenericRecord]): Unit = - in.saveAsBigQueryTable(Table.Ref(tableRef), createDisposition = createDisposition, writeDisposition = writeDisposition, tableDescription = tableDescription) + in.saveAsBigQueryTable(Table.Ref(tableRef), createDisposition = cd, writeDisposition = wd, tableDescription = td) def saveAsBigQueryTableMultiParamsAllNamed(in: SCollection[GenericRecord]): Unit = - in.saveAsBigQueryTable(table = Table.Ref(tableRef), writeDisposition = writeDisposition, createDisposition = createDisposition, tableDescription = tableDescription) + in.saveAsBigQueryTable(table = Table.Ref(tableRef), writeDisposition = wd, createDisposition = cd, tableDescription = td) + + def saveAsBigQueryTableMultiParamsWithSchemaUnnamed(in: SCollection[GenericRecord]): Unit = + in.saveAsBigQueryTable(Table.Ref(tableRef), toTableSchema(s), wd, cd, td) + + def saveAsBigQueryTableMultiParamsWithSchemaNamed(in: SCollection[GenericRecord]): Unit = + in.saveAsBigQueryTable(Table.Ref(tableRef), schema = toTableSchema(s), writeDisposition = wd, createDisposition = cd, tableDescription = td) + + def saveAsBigQueryTableMultiParamsNamedOrderChanged(in: SCollection[GenericRecord]): Unit = + in.saveAsBigQueryTable(Table.Ref(tableRef), writeDisposition = wd, schema = toTableSchema(s)) } diff --git a/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule b/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule index f12511de21..5ce91590c9 100644 --- a/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule +++ b/scalafix/rules/src/main/resources/META-INF/services/scalafix.v1.Rule @@ -11,5 +11,4 @@ fix.v0_8_0.FixBigQueryDeprecations fix.v0_8_0.ConsistenceJoinNames fix.v0_10_0.FixCoderPropagation fix.v0_12_0.FixBqSaveAsTable -fix.v0_12_0.LintBqSaveAsTable fix.v0_12_0.FixPubsubSpecializations \ No newline at end of file diff --git a/scalafix/rules/src/main/scala/fix/FixBqSaveAsTable.scala b/scalafix/rules/src/main/scala/fix/FixBqSaveAsTable.scala index b34c8a7828..b8df0ee43b 100644 --- a/scalafix/rules/src/main/scala/fix/FixBqSaveAsTable.scala +++ b/scalafix/rules/src/main/scala/fix/FixBqSaveAsTable.scala @@ -12,28 +12,48 @@ class FixBqSaveAsTable extends SemanticRule("FixBqSaveAsTable") { override def fix(implicit doc: SemanticDocument): Patch = { val patches = doc.tree.collect { - case a @ Term.Apply(fun, head :: tail) => + case a @ Term.Apply(fun, params) => if (fun.symbol.normalized.toString.contains(methodName)) { fun match { case Term.Select(qual, name) => name match { case Term.Name("saveAvroAsBigQuery") if expectedType(qual, scoll) => - // the rest of args should be named because the parameter order has changed - if ( - tail.exists(!_.toString.contains("=")) || - // `avroSchema` doesn't exist in the list of the new method - tail.exists(_.toString.contains("avroSchema")) - ) { - Patch.empty // not possible to fix, leave it to `LintBqSaveAsTable` - } else { - val headParam = if (head.toString.contains("=")) { - s"table = Table.Ref(${head.toString.split("=").last.trim})" - } else { - s"Table.Ref($head)" - } - val allArgs = (headParam :: tail).mkString(", ") - Patch.replaceTree(a, s"$qual.saveAsBigQueryTable($allArgs)") - } + val paramsUpdated = + params + .zipWithIndex + .map { case (param, index) => + index match { + // table is always the first param and without default value + case 0 => + param match { + case Term.Assign((_, value)) => + q"table = Table.Ref(${value})" + case _ => + q"Table.Ref($param)" + } + case _ => + param match { + case Term.Assign((name, value)) => + // parameter name has changes from `avroSchema` to `schema` + if (name.toString == "avroSchema") { + q"schema = toTableSchema($value)" + } else { + param + } + case _ => + // if not a named param, `avroSchema` param should come second + if (index == 1) { + q"toTableSchema($param)" + } else { + param + } + } + } + } + .mkString(", ") + + Patch.replaceTree(a, s"$qual.saveAsBigQueryTable($paramsUpdated)") + case _ => Patch.empty } @@ -43,20 +63,20 @@ class FixBqSaveAsTable extends SemanticRule("FixBqSaveAsTable") { } else { Patch.empty } - case Source(Pkg(_, imp :: _) :: _) => - Patch.addGlobalImport( - importer"com.spotify.scio.bigquery._" - ) case _ => Patch.empty - } - .filter(_ != Patch.empty) + }.filter(_ != Patch.empty) - // in case the import is the only change, drop the entire patch - if (patches.size == 1) { - List() - } else { - patches - } + // in case the import is the only change, drop the entire patch + if (!patches.isEmpty) { + patches ++ List( + Patch.addGlobalImport(importer"com.spotify.scio.bigquery._"), + Patch.addGlobalImport( + importer"com.spotify.scio.extra.bigquery.AvroConverters.toTableSchema" + ) + ) + } else { + patches + } }.asPatch diff --git a/scalafix/rules/src/main/scala/fix/LintBqSaveAsTable.scala b/scalafix/rules/src/main/scala/fix/LintBqSaveAsTable.scala deleted file mode 100644 index d21e4046c8..0000000000 --- a/scalafix/rules/src/main/scala/fix/LintBqSaveAsTable.scala +++ /dev/null @@ -1,60 +0,0 @@ -package fix -package v0_12_0 - -import scalafix.v1._ -import scala.meta._ - -class LintBqSaveAsTable extends SemanticRule("LintBqSaveAsTable") { - private val scoll = "com/spotify/scio/values/SCollection#" - private val methodName = - "com.spotify.scio.extra.bigquery.syntax.AvroToBigQuerySCollectionOps.saveAvroAsBigQuery" - - case class BigQuerySaveBreakingSignature(fun: scala.meta.Term.Name) extends Diagnostic { - override def position: Position = fun.pos - override def message: String = - s"`com.spotify.scio.bigquery.syntax.SCollectionBeamSchemaOps#saveAsBigQueryTable` should " + - s"be used instead of $fun. Pay attention to the order of the method parameters because " + - s"`avroSchema` is dropped from parameter list in the new method." - } - - override def fix(implicit doc: SemanticDocument): Patch = { - doc.tree.collect { case Term.Apply(fun, _ :: tail) => - if (fun.symbol.normalized.toString.contains(methodName)) { - fun match { - case Term.Select(qual, name) => - name match { - case t @ Term.Name("saveAvroAsBigQuery") if expectedType(qual, scoll) => - // order of the parameters have changed in tail, so only named parameters can be - // reused in the new method11 - if ( - tail.exists(!_.toString.contains("=")) || - // avroSchema param is dropped in the new method - tail.exists(_.toString.contains("avroSchema")) - ) { - Patch.lint(BigQuerySaveBreakingSignature(t)) - } else { - Patch.empty // `FixBqSaveAsTable` captures this - } - case _ => - Patch.empty - } - case _ => Patch.empty - } - } else { - Patch.empty - } - }.asPatch - } - - private def expectedType(qual: Term, typStr: String)(implicit doc: SemanticDocument): Boolean = - qual.symbol.info.get.signature match { - case MethodSignature(_, _, TypeRef(_, typ, _)) => - SymbolMatcher.exact(typStr).matches(typ) - case ValueSignature(AnnotatedType(_, TypeRef(_, typ, _))) => - SymbolMatcher.exact(typStr).matches(typ) - case ValueSignature(TypeRef(_, typ, _)) => - SymbolMatcher.exact(scoll).matches(typ) - case t => - false - } -} diff --git a/site/src/main/paradox/migrations/v0.12.0-Migration-Guide.md b/site/src/main/paradox/migrations/v0.12.0-Migration-Guide.md index 6f2e4a64a8..d348d660a4 100644 --- a/site/src/main/paradox/migrations/v0.12.0-Migration-Guide.md +++ b/site/src/main/paradox/migrations/v0.12.0-Migration-Guide.md @@ -9,10 +9,10 @@ For usages of `saveAvroAsBigQuery`, use `saveAsBigQueryTable` from `com.spotify. + scoll.saveAsBigQueryTable(table) ``` -Note: you can run the following sbt command to run the relevant [scalafix](https://scalacenter.github.io/scalafix/docs/developers/tutorial.html#run-the-rule-from-source-code) rules to update your BQ API usages or get a hint on how to upgrade them: +Note: you can run the following sbt command to run the relevant [scalafix](https://scalacenter.github.io/scalafix/docs/developers/tutorial.html#run-the-rule-from-source-code) rules to update your BQ API usages: ``` -sbt "scalafixEnable; scalafix github:spotify/scio/FixBqSaveAsTable github:spotify/scio/LintBqSaveAsTable" +sbt "scalafixEnable; scalafix github:spotify/scio/FixBqSaveAsTable" ``` ## Removal of `com.spotify.scio.pubsub` specializations