Skip to content

Commit

Permalink
scrooge-generator: Renamed experiment-flag to language-flag
Browse files Browse the repository at this point in the history
Problem

There is not a way to pass arguments to the generator that changes how code is generated.

Solution

Add a new argument called `language-flag` that passes the arguments to the generator.

Result

Renamed the unused `experiment-flag` argument to `language-flag`. Also renamed all the usages
 of experiment flag in the project.

JIRA Issues: CSL-9899

Differential Revision: https://phabricator.twitter.biz/D508950
  • Loading branch information
jay18001 authored and jenkins committed Jul 1, 2020
1 parent 99803e1 commit 8fa0583
Show file tree
Hide file tree
Showing 12 changed files with 28 additions and 26 deletions.
8 changes: 5 additions & 3 deletions CHANGELOG.rst
Expand Up @@ -7,6 +7,9 @@ Note that ``PHAB_ID=#`` and ``RB_ID=#`` correspond to associated messages in com
Unreleased
----------

* scrooge-generator: Removed experiment-flag argument and replaced it with
language-flag. Updated GeneratorFactory object and trait to match. ``PHAB_ID=D508950``

20.6.0
------

Expand All @@ -17,7 +20,7 @@ No Changes

No Changes

* scrooge: Update `sbt-bintray` plugin to 0.5.6 which supports passing environment
* scrooge: Update `sbt-bintray` plugin to 0.5.6 which supports passing environment
variables `BINTRAY_USER` and `BINTRAY_PASS` for username and password credentials
respectively. ``PHAB_ID=D478276``

Expand Down Expand Up @@ -81,7 +84,7 @@ No Changes

* scrooge-generator: Use wrapper class valueOf in apachejavagen's getFieldValue. ``PHAB_ID=D374413``

* scrooge-linter: Warn when function names are reserved words. Add support for reserved
* scrooge-linter: Warn when function names are reserved words. Add support for reserved
words in Javascript and Go. ``PHAB_ID=D379008``

* scrooge-core: Add annotations method to `c.t.scrooge.ThriftEnum` to make the
Expand Down Expand Up @@ -1265,4 +1268,3 @@ Dependencies:
* Correctly resolving enum constants and Const values.
* Title-casing enum value names.
* Added support for namespace renaming from the command line.

2 changes: 1 addition & 1 deletion doc/src/sphinx/CommandLine.rst
Expand Up @@ -74,7 +74,7 @@ A complete command line help menu for scrooge-generator:
-s, --skip-unchanged Don't re-generate if the target is newer than the input
-l, --language <value> name of language to generate code in (currently supported languages: java, lua, scala, cocoa, android)
--java-ser-enum-type Encode a thrift enum as o.a.t.p.TType.ENUM instead of TType.I32
--experiment-flag <flag> [EXPERIMENTAL] DO NOT USE FOR PRODUCTION. This is meant only for enabling/disabling features for benchmarking
--language-flag <flag> Pass arguments to supported language generators
--scala-warn-on-java-ns-fallback Print a warning when the scala generator falls back to the java namespace
--finagle generate finagle classes
--gen-adapt Generate code for adaptive decoding for scala.
Expand Down
Expand Up @@ -10,11 +10,11 @@ class TestGeneratorFactory extends GeneratorFactory {
def apply(
doc: ResolvedDocument,
defaultNamespace: String,
experimentFlags: Seq[String]
languageFlags: Seq[String]
): Generator = new ScalaGenerator(
doc,
defaultNamespace,
experimentFlags,
languageFlags,
new HandlebarLoader("/scalagen/", ".mustache")
)
}
Expand Down
Expand Up @@ -44,7 +44,7 @@ abstract class GoldFileTest extends FunSuite with BeforeAndAfterAll {
"--gen-adapt",
"--dest",
tempDir.getPath) ++
experimentFlags.flatMap(flag => Seq("--experiment-flag", flag)) ++
languageFlags.flatMap(flag => Seq("--language-flag", flag)) ++
inputThrifts

Main.main(args.toArray)
Expand All @@ -67,7 +67,7 @@ abstract class GoldFileTest extends FunSuite with BeforeAndAfterAll {
}
}

protected def experimentFlags: Seq[String] = Seq.empty
protected def languageFlags: Seq[String] = Seq.empty
protected def language: String
protected def deleteTempFiles: Boolean = true

Expand Down
Expand Up @@ -40,7 +40,7 @@ class Compiler {
var strict = true
var genAdapt = false
var skipUnchanged = false
var experimentFlags = new mutable.ListBuffer[String]
var languageFlags = new mutable.ListBuffer[String]
var fileMapPath: scala.Option[String] = None
var fileMapWriter: scala.Option[FileWriter] = None
var dryRun: Boolean = false
Expand Down Expand Up @@ -83,7 +83,7 @@ class Compiler {
if (verbose) println("+ Compiling %s".format(inputFile))
val resolvedDoc = TypeResolver()(doc)
val generator =
GeneratorFactory(language, resolvedDoc, defaultNamespace, experimentFlags.toSeq)
GeneratorFactory(language, resolvedDoc, defaultNamespace, languageFlags.toSeq)

generator match {
case g: ScalaGenerator => g.warnOnJavaNamespaceFallback = scalaWarnOnJavaNSFallback
Expand Down
Expand Up @@ -168,15 +168,15 @@ object Main {
}
.text("Encode a thrift enum as o.a.t.p.TType.ENUM instead of TType.I32")

opt[String]("experiment-flag")
opt[String]("language-flag")
.valueName("<flag>")
.unbounded()
.action { (flag, c) =>
c.experimentFlags += flag
c.languageFlags += flag
c
}
.text(
"[EXPERIMENTAL] DO NOT USE FOR PRODUCTION. This is meant only for enabling/disabling features for benchmarking"
"Pass arguments to supported language generators"
)

opt[Unit]("scala-warn-on-java-ns-fallback")
Expand Down
Expand Up @@ -22,7 +22,7 @@ object AndroidGeneratorFactory extends GeneratorFactory {
def apply(
doc: ResolvedDocument,
defaultNamespace: String,
experimentFlags: Seq[String]
languageFlags: Seq[String]
): Generator = new AndroidGenerator(doc, defaultNamespace, templateCache)
}

Expand Down
Expand Up @@ -15,7 +15,7 @@ object CocoaGeneratorFactory extends GeneratorFactory {
def apply(
doc: ResolvedDocument,
defaultNamespace: String,
experimentFlags: Seq[String]
languageFlags: Seq[String]
): Generator = new CocoaGenerator(
doc,
defaultNamespace,
Expand All @@ -36,7 +36,7 @@ class CocoaGenerator(
val fileExtension = ".m"
val headerExtension = ".h"
val templateDirName = "/cocoagen/"
val experimentFlags: Seq[String] = Seq.empty[String]
val languageFlags: Seq[String] = Seq.empty[String]

// Namespace for the current thrift file is not avaialbe when we construct the code generator.
// It will only be available when we call the apply method.
Expand Down
Expand Up @@ -106,9 +106,9 @@ object GeneratorFactory {
lan: String,
doc: ResolvedDocument,
defaultNamespace: String,
experimentFlags: Seq[String]
languageFlags: Seq[String]
): Generator = factories.get(lan) match {
case Some(factory) => factory(doc, defaultNamespace, experimentFlags)
case Some(factory) => factory(doc, defaultNamespace, languageFlags)
case None => throw new Exception("Generator for language \"%s\" not found".format(lan))
}
}
Expand All @@ -122,7 +122,7 @@ trait GeneratorFactory {
def apply(
doc: ResolvedDocument,
defaultNamespace: String,
experimentFlags: Seq[String]
languageFlags: Seq[String]
): Generator
}

Expand Down Expand Up @@ -200,7 +200,7 @@ abstract class TemplateGenerator(val resolvedDoc: ResolvedDocument)
* Map from included file names to the namespaces defined in those files.
*/
val defaultNamespace: String
val experimentFlags: Seq[String]
val languageFlags: Seq[String]

/******************** helper functions ************************/
protected def namespacedFolder(destFolder: File, namespace: String, dryRun: Boolean): File =
Expand Down
Expand Up @@ -28,14 +28,14 @@ object ScalaGeneratorFactory extends GeneratorFactory {
def apply(
doc: ResolvedDocument,
defaultNamespace: String,
experimentFlags: Seq[String]
): Generator = new ScalaGenerator(doc, defaultNamespace, experimentFlags, handlebarLoader)
languageFlags: Seq[String]
): Generator = new ScalaGenerator(doc, defaultNamespace, languageFlags, handlebarLoader)
}

class ScalaGenerator(
override val resolvedDoc: ResolvedDocument,
val defaultNamespace: String,
val experimentFlags: Seq[String],
val languageFlags: Seq[String],
val templatesLoader: HandlebarLoader)
extends TemplateGenerator(resolvedDoc) {
def templates: HandlebarLoader = templatesLoader
Expand Down
Expand Up @@ -27,7 +27,7 @@ object LuaGeneratorFactory extends GeneratorFactory {
def apply(
doc: ResolvedDocument,
defaultNamespace: String,
experimentFlags: Seq[String]
languageFlags: Seq[String]
): Generator = new LuaGenerator(
doc,
defaultNamespace,
Expand All @@ -45,7 +45,7 @@ class LuaGenerator(

val namespaceLanguage = "lua"
val fileExtension = ".lua"
val experimentFlags: Seq[String] = Seq.empty[String]
val languageFlags: Seq[String] = Seq.empty[String]

def templates: HandlebarLoader = templateLoader

Expand Down
Expand Up @@ -15,7 +15,7 @@ object ApacheJavaGeneratorFactory extends GeneratorFactory {
def apply(
doc: ResolvedDocument,
defaultNamespace: String,
experimentFlags: Seq[String]
languageFlags: Seq[String]
): Generator = new ApacheJavaGenerator(doc, defaultNamespace, templateCache)
}

Expand Down

0 comments on commit 8fa0583

Please sign in to comment.