Skip to content

Commit

Permalink
scrooge: Add setting to configure root dir importer
Browse files Browse the repository at this point in the history
Problem
As described in twitter/scrooge/#341 a lot of warnings are generated
when using scrooge in projects with sbt sub modules as a root dir
importer is always added.

Solution
Add a new setting scroogeThriftIncludeRoot that makes this behaviour
configurable. The default behaviour is as it is now.

Result
Users won't see a ton of warnings.

Closes #345

Signed-off-by: Joy Bestourous <jbestourous@twitter.com>

Differential Revision: https://phabricator.twitter.biz/D633782
  • Loading branch information
muuki88 authored and jenkins committed Mar 15, 2021
1 parent 72f5a0a commit fe679c0
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,15 @@ package com.twitter.scrooge

import com.twitter.scrooge.ast.Document
import com.twitter.scrooge.backend.{GeneratorFactory, ScalaGenerator}
import com.twitter.scrooge.frontend.{FileParseException, TypeResolver, ThriftParser, Importer}
import com.twitter.scrooge.frontend.{
FileParseException,
Importer,
NullImporter,
ThriftParser,
TypeResolver
}
import com.twitter.scrooge.java_generator.ApacheJavaGenerator

import java.io.{File, FileWriter}
import scala.collection.concurrent.TrieMap

Expand All @@ -45,7 +52,12 @@ class Compiler(val config: ScroogeConfig) {
new FileWriter(file)
}

val importer = Importer(new File(".")) +: Importer(config.includePaths.toSeq)
val importer = {
val rootImporter =
if (config.addRootDirImporter) Importer(new File("."))
else NullImporter
rootImporter +: Importer(config.includePaths.toSeq)
}

val isJava = config.language.equals("java")
val documentCache = new TrieMap[String, Document]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ case class ScroogeConfig(
language: String = CompilerDefaults.language,
defaultNamespace: String = CompilerDefaults.defaultNamespace,
scalaWarnOnJavaNSFallback: Boolean = false,
javaSerEnumType: Boolean = false)
javaSerEnumType: Boolean = false,
addRootDirImporter: Boolean = true)

object ScroogeOptionParser {

Expand Down
16 changes: 13 additions & 3 deletions scrooge-sbt-plugin/src/main/scala/com/twitter/ScroogeSBT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ object ScroogeSBT extends AutoPlugin {
flags: Set[ServiceOption],
disableStrict: Boolean,
scalaWarnOnJavaNSFallback: Boolean,
defaultNamespace: String
defaultNamespace: String,
addRootDirImporter: Boolean
): Unit = {

val originalLoader: Option[ClassLoader] =
Expand All @@ -43,7 +44,8 @@ object ScroogeSBT extends AutoPlugin {
strict = !disableStrict,
scalaWarnOnJavaNSFallback = scalaWarnOnJavaNSFallback,
defaultNamespace = defaultNamespace,
language = language.toLowerCase
language = language.toLowerCase,
addRootDirImporter = addRootDirImporter
)

try {
Expand Down Expand Up @@ -105,6 +107,11 @@ object ScroogeSBT extends AutoPlugin {
"complete list of folders to search for thrift 'include' directives"
)

val scroogeThriftIncludeRoot = SettingKey[Boolean](
"scrooge-thrift-include-root",
"If true scrooge will always search the project root for script files"
)

val scroogeThriftNamespaceMap = SettingKey[Map[String, String]](
"scrooge-thrift-namespace-map",
"namespace rewriting, to support generation of java/finagle/scrooge into the same jar"
Expand Down Expand Up @@ -187,6 +194,7 @@ object ScroogeSBT extends AutoPlugin {
scroogeThriftSources := (scroogeThriftSourceFolder.value ** "*.thrift").get,
// complete list of include directories
scroogeThriftIncludes := scroogeThriftIncludeFolders.value ++ scroogeUnpackDeps.value,
scroogeThriftIncludeRoot := true,
// unpack thrift files from all dependencies in the `thrift` configuration
//
// returns Seq[File] - directories that include thrift files
Expand Down Expand Up @@ -259,6 +267,7 @@ object ScroogeSBT extends AutoPlugin {
val disableStrict = scroogeDisableStrict.value
val warnOnFallBack = scroogeScalaWarnOnJavaNSFallback.value
val javaNamespace = scroogeDefaultJavaNamespace.value
val addRootDirImporter = scroogeThriftIncludeRoot.value
// for some reason, sbt sometimes calls us multiple times, often with no source files.
if (scroogeIsDirty.value && thriftSources.nonEmpty) {
streamValue.log.info(s"Generating scrooge thrift for ${thriftSources.mkString(", ")} ...")
Expand All @@ -273,7 +282,8 @@ object ScroogeSBT extends AutoPlugin {
buildOptions.toSet,
disableStrict,
warnOnFallBack,
javaNamespace
javaNamespace,
addRootDirImporter
)
}
}
Expand Down

0 comments on commit fe679c0

Please sign in to comment.