Require parameters by default, and make sure we set default parameter…

…s before validation #224

CHANGELOG.md

@@ -17,11 +17,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 -   Added `ScalaFileType` backed by ScalaMeta
 
+### Fixed
+
 -   Fix: Elm parser failed on files with two multiline comments
-     https://github.com/atomist/rug/issues/268 
+    https://github.com/atomist/rug/issues/268 
 
 -   Raise an `InvalidRugTestScenarioName` when a Rug test scenario is missing a name #71
-
+
+-   Ensure TS parameters are required by default, and ensure defaults are applied
+    before validation: https://github.com/atomist/rug/issues/224
+
 ### Changed
 
 -   Upgrade TS compiler to 2.1.5

src/main/scala/com/atomist/project/edit/ProjectEditorSupport.scala

@@ -26,7 +26,8 @@ trait ProjectEditorSupport
     */
   def failOnNoModification: Boolean = false
 
-  override def modify(as: ArtifactSource, poa: ProjectOperationArguments): ModificationAttempt = {
+  override def modify(as: ArtifactSource, args: ProjectOperationArguments): ModificationAttempt = {
+    val poa = addDefaultParameterValues(args)
     validateParameters(poa)
 
     val r =
@@ -36,7 +37,7 @@ trait ProjectEditorSupport
       } else if (meetsPostcondition(as)) {
         NoModificationNeeded(s"Artifact source meets postcondition already")
       } else {
-        modifyInternal(as, addDefaultParameterValues(poa))
+        modifyInternal(as, poa)
       }
 
     // We may need to make it fail-fast

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

@@ -181,7 +181,13 @@ abstract class JavaScriptInvokingProjectOperation(
     parameter.setDefaultRef(details.get("defaultRef").asInstanceOf[String])
     val disp = details.get("displayable")
     parameter.setDisplayable(if(disp != null) disp.asInstanceOf[Boolean] else true)
-    parameter.setRequired(details.get("required").asInstanceOf[Boolean])
+
+    if(details.hasMember("required")){
+      parameter.setRequired(details.get("required").asInstanceOf[Boolean])
+    }else{
+      parameter.setRequired(true)
+    }
+
 
     parameter.addTags(readTagsFromMetadata(details))
 

src/test/scala/com/atomist/rug/runtime/js/JavaScriptInvokingProjectOperationTest.scala

@@ -1,8 +1,9 @@
 package com.atomist.rug.runtime.js
 
 import com.atomist.project.SimpleProjectOperationArguments
+import com.atomist.project.common.MissingParametersException
 import com.atomist.rug.ts.TypeScriptBuilder
-import com.atomist.rug.{InvalidRugParameterDefaultValue, InvalidRugParameterPatternException, TestUtils}
+import com.atomist.rug.{InvalidRugParameterDefaultValue, InvalidRugParameterPatternException}
 import com.atomist.source.{FileArtifact, SimpleFileBasedArtifactSource, StringFileArtifact}
 import org.scalatest.{FlatSpec, Matchers}
 
@@ -45,6 +46,26 @@ object JavaScriptInvokingProjectOperationTest {
        |export let editor = new SimpleEditor()
     """.stripMargin
 
+  val SimpleEditorWithRequiredParameterButNoDefault: String =
+    s"""
+       |import {Project} from '@atomist/rug/model/Core'
+       |import {ProjectEditor} from '@atomist/rug/operations/ProjectEditor'
+       |import {File} from '@atomist/rug/model/Core'
+       |import {Parameter} from '@atomist/rug/operations/RugOperation'
+       |
+       |class SimpleEditor implements ProjectEditor {
+       |    name: string = "Simple"
+       |    description: string = "A nice little editor"
+       |    parameters: Parameter[] = [{name: "content", description: "Content", pattern: "@url", maxLength: 100}]
+       |    edit(project: Project, {content} : {content: string}) {
+       |       if(content != "http://t.co"){
+       |          throw new Error("Content was not as expected");
+       |       }
+       |    }
+       |  }
+       |export let editor = new SimpleEditor()
+    """.stripMargin
+
   val SimpleReviewerInvokingOtherEditorAndAddingToOurOwnParameters: String =
     s"""
        |import {Project} from '@atomist/rug/model/Core'
@@ -245,7 +266,19 @@ class JavaScriptInvokingProjectOperationTest extends FlatSpec with Matchers {
     jsed.jsVar.get("name") should be ("Simple")
   }
 
-  private  def invokeAndVerifySimpleEditor(tsf: FileArtifact): JavaScriptInvokingProjectEditor = {
+  it should "Should throw an exception if required parameters are not set" in {
+    val tsf = StringFileArtifact(s".atomist/editors/SimpleEditor.ts", SimpleEditorWithRequiredParameterButNoDefault)
+    val as = TypeScriptBuilder.compileWithModel(SimpleFileBasedArtifactSource(tsf))
+    val jsed = JavaScriptOperationFinder.fromJavaScriptArchive(as).head.asInstanceOf[JavaScriptInvokingProjectEditor]
+    assert(jsed.name  == "Simple")
+    val target = SimpleFileBasedArtifactSource(StringFileArtifact("pom.xml", "nasty stuff"))
+
+    assertThrows[MissingParametersException]{
+      jsed.modify(target, SimpleProjectOperationArguments.Empty)
+    }
+  }
+
+  private def invokeAndVerifySimpleEditor(tsf: FileArtifact): JavaScriptInvokingProjectEditor = {
     val as = TypeScriptBuilder.compileWithModel(SimpleFileBasedArtifactSource(tsf))
     val jsed = JavaScriptOperationFinder.fromJavaScriptArchive(as).head.asInstanceOf[JavaScriptInvokingProjectEditor]
     assert(jsed.name === "Simple")

src/test/scala/com/atomist/rug/runtime/js/JavaScriptOperationFinderTest.scala

@@ -38,11 +38,11 @@ class JavaScriptOperationFinderTest  extends FlatSpec with Matchers {
        |    @Parameter({pattern: "^.*$$", description: "foo bar"})
        |    content: string = "Test String";
        |
-       |    @Parameter({pattern: "^\\d+$$", description: "A nice round number"})
+       |    @Parameter({pattern: "^[0-9]+$$", description: "A nice round number"})
        |    amount: number = 10;
        |
-       |    @Parameter({pattern: "^\\d+$$", description: "A nice round number"})
-       |    nope: boolean;
+       |    @Parameter({pattern: "^.*$$", description: "A nice round number", required: false})
+       |    nope: boolean
        |
        |    edit(project: Project) {
        |       if(this.amount != 10) {