Fix TS relative imports by forcing all Rugs var to be exported #156

CHANGELOG.md

@@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 [Unreleased]: https://github.com/atomist/rug/compare/0.9.0...HEAD
 
+### Changed
+
+-   TypeScript/JavaScript should now "export" instances of Rugs inline with CommonJS standard.
+    Hopefully not a breaking change as there is limited support for automatically exporting legacy ones.
+
+### Fixed
+
+-   Comments are removed from JS files as they are 'required', and relative imports now work correctly
+    when 'export' is used from TS or vars are added to the global exports var
+    https://github.com/atomist/rug/issues/156
+
 ## [0.9.0] - 2017-01-09
 
 [0.9.0]: https://github.com/atomist/rug/compare/0.8.0...0.9.0

pom.xml

@@ -51,7 +51,7 @@
         <dependency>
             <groupId>com.atomist</groupId>
             <artifactId>rug-typescript-compiler</artifactId>
-            <version>0.7.2</version>
+            <version>0.8.0</version>
         </dependency>
         <dependency>
             <groupId>com.coveo</groupId>

src/main/scala/com/atomist/project/archive/AtomistConfig.scala

@@ -47,6 +47,8 @@ trait AtomistConfig {
 
   def testExtension: String
 
+  def jsExtension: String
+
   def editorsRoot = s"$atomistRoot/$editorsDirectory"
 
   def templatesRoot = s"$atomistRoot/$templatesDirectory"
@@ -80,14 +82,23 @@ trait AtomistConfig {
     rugArchive.filter(d => d.path.startsWith(atomistRoot), f => f.path.startsWith(atomistRoot))
 
   def isRugSource(f: FileArtifact): Boolean = {
-    f.name.endsWith(rugExtension) && (
+    f.name.endsWith(rugExtension) && isAtomistSource(f)
+
+  }
+
+  def isJsSource(f:FileArtifact): Boolean = {
+    f.name.endsWith(jsExtension) && isAtomistSource(f)
+  }
+
+  def isAtomistSource(f: FileArtifact): Boolean = {
       f.path.startsWith(editorsRoot) ||
-        f.path.startsWith(reviewersRoot) ||
-        f.path.startsWith(executorsRoot) ||
-        f.path.startsWith(editorsDirectory) ||
-        f.path.startsWith(reviewersDirectory) ||
-        f.path.startsWith(executorsDirectory)
-      )
+      f.path.startsWith(reviewersRoot) ||
+      f.path.startsWith(executorsRoot) ||
+      f.path.startsWith(handlersRoot) ||
+      f.path.startsWith(handlersDirectory) ||
+      f.path.startsWith(editorsDirectory) ||
+      f.path.startsWith(reviewersDirectory) ||
+      f.path.startsWith(executorsDirectory)
   }
 
   def isRugTest(f: FileArtifact): Boolean = {
@@ -97,6 +108,11 @@ trait AtomistConfig {
       )
   }
 
+  def isJsHandler(f: FileArtifact): Boolean = {
+    f.name.endsWith(jsExtension) &&  f.path.startsWith(handlersRoot) ||
+      f.path.startsWith(handlersDirectory)
+  }
+
   def templateContentIn(rugAs: ArtifactSource): ArtifactSource =
     rugAs / templatesRoot + rugAs / templatesDirectory
 
@@ -120,5 +136,7 @@ object DefaultAtomistConfig extends AtomistConfig {
 
   override val rugExtension = ".rug"
 
+  override val jsExtension = ".js"
+
   override def testExtension: String = ".rt"
 }

src/main/scala/com/atomist/project/archive/ProjectOperationArchiveReader.scala

@@ -16,6 +16,7 @@ import scala.collection.Seq
 /**
   * Reads an archive and extracts Atomist project operations.
   * These can either be Rug DSL archives or TypeScript or JavaScript files.
+  * Public API!
   */
 class ProjectOperationArchiveReader(
                                      atomistConfig: AtomistConfig = DefaultAtomistConfig,
@@ -25,29 +26,22 @@ class ProjectOperationArchiveReader(
   extends LazyLogging {
 
   val oldInterpreterPipeline = new DefaultRugPipeline(typeRegistry, evaluator, atomistConfig)
-  //val newPipeline = new CompilerChainPipeline(Seq(new RugTranspiler()))
 
   def findImports(startingProject: ArtifactSource): Seq[Import] = {
     oldInterpreterPipeline.parseRugFiles(startingProject).foldLeft(Nil: Seq[Import]) { (acc, rugProgram) => acc ++ rugProgram.imports }
   }
 
-  // We skip any file with declare var atomist as we can't satisfy it here
-  //TODO - remove this!
-  private val hasDeclareVarAtomist: FileArtifact => Boolean = f =>
-    f.name.endsWith(".ts") && "declare[\\s]+var[\\s]+atomist".r.findAllMatchIn(f.content).nonEmpty
-
   def findOperations(startingProject: ArtifactSource,
                      namespace: Option[String],
                      otherOperations: Seq[ProjectOperation],
-                     shouldSuppress: FileArtifact => Boolean = hasDeclareVarAtomist): Operations = {
-    val fromTs = JavaScriptOperationFinder.fromJavaScriptArchive(startingProject)
+                     fileFilter: FileArtifact => Boolean = (x: FileArtifact) => !atomistConfig.isJsHandler(x)): Operations = {
+    val fromTs = JavaScriptOperationFinder.fromJavaScriptArchive(startingProject.filter(_ => true, fileFilter))
     val fromOldPipeline = oldInterpreterPipeline.create(startingProject, namespace, otherOperations ++ fromTs)
 
     val operations = fromOldPipeline ++ fromTs
 
     operations foreach {
       case capo: ContextAwareProjectOperation =>
-        //println(s"Set context on $capo")
         capo.setContext(operations ++ otherOperations)
     }
 

src/main/scala/com/atomist/rug/runtime/js/JavaScriptContext.scala

@@ -1,8 +1,10 @@
 package com.atomist.rug.runtime.js
 
+import java.util.regex.Pattern
 import javax.script.ScriptContext
 
-import com.atomist.source.{ArtifactSource, FileArtifact}
+import com.atomist.project.archive.{AtomistConfig, DefaultAtomistConfig}
+import com.atomist.source.ArtifactSource
 import com.coveo.nashorn_modules.{AbstractFolder, Folder, Require}
 import com.typesafe.scalalogging.LazyLogging
 import jdk.nashorn.api.scripting.{ClassFilter, NashornScriptEngine, NashornScriptEngineFactory, ScriptObjectMirror}
@@ -15,7 +17,7 @@ import scala.collection.JavaConverters._
   * exposing the known vars in a typesafe way so we partly avoid the horrific detyped
   * Nashorn API.
   */
-class JavaScriptContext(rugAs: ArtifactSource, allowedClasses: Set[String] = Set.empty[String]) extends LazyLogging {
+class JavaScriptContext(allowedClasses: Set[String] = Set.empty[String], atomistConfig: AtomistConfig = DefaultAtomistConfig) extends LazyLogging {
 
   private val commonOptions = Array("--optimistic-types", "--language=es6")
 
@@ -25,30 +27,34 @@ class JavaScriptContext(rugAs: ArtifactSource, allowedClasses: Set[String] = Set
     * If you do need to expose some classes to JS, then make sure you configure to use a locked down classloader and security manager
     */
   val engine: NashornScriptEngine =
-      new NashornScriptEngineFactory().getScriptEngine(
-        if(allowedClasses.isEmpty) commonOptions :+ "--no-java" else commonOptions,
-        if(allowedClasses.isEmpty) null else Thread.currentThread().getContextClassLoader,//TODO - do we need our own loader here?
-        new ClassFilter {
-          override def exposeToScripts(s: String): Boolean = {
-            allowedClasses.contains(s)
-          }
+    new NashornScriptEngineFactory().getScriptEngine(
+      if (allowedClasses.isEmpty) commonOptions :+ "--no-java" else commonOptions,
+      if (allowedClasses.isEmpty) null else Thread.currentThread().getContextClassLoader, //TODO - do we need our own loader here?
+      new ClassFilter {
+        override def exposeToScripts(s: String): Boolean = {
+          allowedClasses.contains(s)
         }
-      ).asInstanceOf[NashornScriptEngine]
+      }
+    ).asInstanceOf[NashornScriptEngine]
 
-  configureEngine(engine)
 
-  /**
-    * Evaluate the contents of the file or do nothing if it's not JavaScript
-    * @param f file to evaluate
-    */
-  def eval(f: FileArtifact): Unit = {
-    if (f.name.endsWith(".js"))
-      engine.eval(f.content)
+  def load(rugAs: ArtifactSource) : Unit = {
+
+    configureEngine(engine, rugAs)
+    val filtered = atomistConfig.atomistContent(rugAs)
+      .filter(d => true,
+        f => atomistConfig.isJsSource(f))
+    //require all the atomist stuff
+    for (f <- filtered.allFiles) {
+      val varName = f.path.dropRight(3).replaceAll("/", "_").replaceAll("\\.", "\\$")
+      engine.eval(s"exports.$varName = require('./${f.path.dropRight(3)}');") //because otherwise the loader doesn't know about the paths and can't resolve relative modules
+    }
   }
 
   /**
     * Information about a JavaScript var exposed in the project scripts
-    * @param key name of the var
+    *
+    * @param key                name of the var
     * @param scriptObjectMirror interface for working with Var
     */
   case class Var(key: String, scriptObjectMirror: ScriptObjectMirror) {
@@ -57,52 +63,85 @@ class JavaScriptContext(rugAs: ArtifactSource, allowedClasses: Set[String] = Set
 
   /**
     * Return all the vars known to the engine that expose ScriptObjectMirror objects, with the key
+    *
     * @return ScriptObjectMirror objects for all vars known to the engine
     */
-  def vars: Seq[Var] =
-    engine.getContext.getBindings(ScriptContext.ENGINE_SCOPE).entrySet().asScala.flatMap(e => {
+  def vars: Seq[Var] = {
+    val res = engine.getContext.getBindings(ScriptContext.ENGINE_SCOPE)
+      .get("exports").asInstanceOf[ScriptObjectMirror]
+      .asScala
+      .foldLeft(Seq[Var]())((acc: Seq[Var], kv) => {
+        acc ++ extractVars(kv._2.asInstanceOf[ScriptObjectMirror])
+      })
+    res
+  }
+
+  private def extractVars(obj: ScriptObjectMirror): Seq[Var] = {
+    obj.entrySet().asScala.flatMap(e => {
       (e.getKey, e.getValue) match {
         case (k, som: ScriptObjectMirror) => Some(Var(k, som))
         case _ => None
       }
     }).toSeq
+  }
 
-
-
-  private def configureEngine(scriptEngine: NashornScriptEngine): Unit = {
+  private def configureEngine(scriptEngine: NashornScriptEngine, rugAs: ArtifactSource): Unit = {
     //so we can print stuff out from TS
     val consoleJs =
-    """
-      |console = {
-      |   log: print,
-      |   warn: print,
-      |   error: print
-      |};
-    """.stripMargin
+      """
+        |console = {
+        |   log: print,
+        |   warn: print,
+        |   error: print
+        |};
+      """.stripMargin
     scriptEngine.eval(consoleJs)
-    try
+    try{
       Require.enable(engine, new ArtifactSourceBasedFolder(rugAs))
-    catch {
+  }catch {
       case e: Exception =>
         throw new RuntimeException("Unable to set up ArtifactSource based module loader", e)
     }
   }
 
   private class ArtifactSourceBasedFolder private(var artifacts: ArtifactSource, val parent: Folder, val path: String) extends AbstractFolder(parent, path) {
-    def this(artifacts: ArtifactSource) {
-      this(artifacts.underPath(".atomist"), null, "")
+
+    private val commentPattern: Pattern = Pattern.compile("^//.*$", Pattern.MULTILINE)
+    //put single line vars like var x = new Blah() into exports
+    private val varPattern: Pattern = Pattern.compile("var (\\w+) = new .*\\);\\s+$")
+
+    private val letPattern: Pattern = Pattern.compile("var (\\w+) = \\{.*\\};\\s+$", Pattern.DOTALL)
+
+    def this(artifacts: ArtifactSource, rootPath: String = "") {
+      this(artifacts.underPath(rootPath), null, "")
     }
 
     def getFile(s: String): String = {
       val file = artifacts.findFile(s)
       if (file.isEmpty) return null
-      file.get.content
+      //remove remove these source-map comments because they seem to be breaking nashorn :/
+      val withoutComments = commentPattern.matcher(file.get.content).replaceAll("")
+
+      //add export for those vars without them. TODO should be removed at some point once all have moved over!
+      val js = new StringBuilder(withoutComments)
+      append(varPattern, withoutComments, js)
+      append(letPattern, withoutComments, js)
+      js.toString()
     }
 
     def getFolder(s: String): Folder = {
       val dir = artifacts.findDirectory(s)
       if (dir.isEmpty) return null
       new ArtifactSourceBasedFolder(artifacts.underPath(s), this, getPath + s + "/")
     }
+
+
+    def append(p: Pattern, str: String, sb: StringBuilder): Unit ={
+      val m = p.matcher(str)
+      if(m.find()){
+        val varName = m.group(1)
+        sb.append("\nexports.").append(varName).append(" = ").append(varName).append(";\n")
+      }
+    }
   }
 }

src/main/scala/com/atomist/rug/runtime/js/JavaScriptHandlerFinder.scala

@@ -9,8 +9,6 @@ import com.atomist.source.ArtifactSource
   */
 object JavaScriptHandlerFinder {
 
-  import com.atomist.rug.runtime.js.JavaScriptOperationFinder._
-
   /**
     * Find and handlers operations in the given Rug archive
     *
@@ -22,18 +20,10 @@ object JavaScriptHandlerFinder {
   def registerHandlers(rugAs: ArtifactSource,
                        atomist: AtomistFacade,
                        atomistConfig: AtomistConfig = DefaultAtomistConfig): Unit = {
-    val jsc = new JavaScriptContext(rugAs)
+    val jsc = new JavaScriptContext()
 
     //TODO - remove this when new Handler model put in
     jsc.engine.put("atomist", atomist)
-
-    val filtered = atomistConfig.atomistContent(rugAs)
-      .filter(d => true,
-        f => jsFile(f)
-          && f.path.startsWith(atomistConfig.handlersRoot))
-
-    for (f <- filtered.allFiles) {
-      jsc.eval(f)
-    }
+    jsc.load(rugAs)
   }
 }

src/main/scala/com/atomist/rug/runtime/js/JavaScriptOperationFinder.scala

@@ -28,7 +28,6 @@ object JavaScriptOperationFinder {
     ReviewerType -> JsRugOperationSignature(Set("review")),
     GeneratorType -> JsRugOperationSignature(Set("populate")))
 
-  val jsFile: FileArtifact => Boolean = f => f.name.endsWith(".js")
 
   private def excludedTypeScriptPath(atomistConfig: AtomistConfig) =
     s"${atomistConfig.atomistRoot}/node_modules/"
@@ -40,24 +39,15 @@ object JavaScriptOperationFinder {
     * @return a sequence of instantiated operations backed by JavaScript
     */
   def fromJavaScriptArchive(rugAs: ArtifactSource,
-                            atomistConfig: AtomistConfig = DefaultAtomistConfig,
                             context: JavaScriptContext = null): Seq[ProjectOperation] = {
 
     val jsc: JavaScriptContext =
       if (context == null)
-        new JavaScriptContext(rugAs)
+        new JavaScriptContext()
       else
         context
 
-    val filtered = atomistConfig.atomistContent(rugAs)
-      .filter(d => true,
-        f => jsFile(f)
-          && (f.path.startsWith(atomistConfig.editorsRoot) || f.path.startsWith(atomistConfig.reviewersRoot) || f.path.startsWith(atomistConfig.executorsRoot)))
-
-    for (f <- filtered.allFiles) {
-      jsc.eval(f)
-    }
-
+    jsc.load(rugAs)
     val operations = operationsFromVars(rugAs, jsc)
     operations
   }

src/test/scala/com/atomist/project/archive/ProjectOperationArchiveReaderTest.scala

@@ -5,7 +5,7 @@ import com.atomist.rug.{Import, TestUtils}
 import com.atomist.rug.exec.FakeServiceSource
 import com.atomist.rug.runtime.js.TypeScriptRugEditorTest
 import com.atomist.rug.runtime.lang.js.NashornConstructorTest
-import com.atomist.source.{SimpleFileBasedArtifactSource, StringFileArtifact}
+import com.atomist.source.{SimpleDirectoryArtifact, SimpleFileBasedArtifactSource, StringFileArtifact}
 import org.scalatest.{FlatSpec, Matchers}
 
 class ProjectOperationArchiveReaderTest extends FlatSpec with Matchers {
@@ -165,12 +165,16 @@ class ProjectOperationArchiveReaderTest extends FlatSpec with Matchers {
     val apc = new ProjectOperationArchiveReader(atomistConfig)
     val f1 = StringFileArtifact("package.json", "{}")
     val f2 = StringFileArtifact("app/Thing.js", "var Thing = {};")
-    val rugAs = SimpleFileBasedArtifactSource(
+
+
+    val rugAs = TestUtils.addUserModel(SimpleFileBasedArtifactSource(
       StringFileArtifact(".atomist/editors/SimpleEditor.js",
         NashornConstructorTest.SimpleJavascriptEditor),
       f1,
       f2
-    ) + TestUtils.user_model
+    ))
+
+
 
     val ops = apc.findOperations(rugAs, None, Nil)
     ops.editors.size should be(1)