diff --git a/CHANGELOG.md b/CHANGELOG.md index 80ecde4bd..4f7c0f407 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,6 @@ 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 @@ -29,6 +28,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). editors declare the same parameter. The first one is chosen. https://github.com/atomist/rug/issues/258 +- 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 diff --git a/src/main/scala/com/atomist/project/edit/ProjectEditorSupport.scala b/src/main/scala/com/atomist/project/edit/ProjectEditorSupport.scala index c7ffa711f..5cb9982a1 100644 --- a/src/main/scala/com/atomist/project/edit/ProjectEditorSupport.scala +++ b/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 diff --git a/src/main/scala/com/atomist/rug/runtime/js/JavaScriptInvokingProjectOperation.scala b/src/main/scala/com/atomist/rug/runtime/js/JavaScriptInvokingProjectOperation.scala index 017561d3e..e5c9ab01f 100644 --- a/src/main/scala/com/atomist/rug/runtime/js/JavaScriptInvokingProjectOperation.scala +++ b/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)) diff --git a/src/test/scala/com/atomist/rug/runtime/js/JavaScriptInvokingProjectOperationTest.scala b/src/test/scala/com/atomist/rug/runtime/js/JavaScriptInvokingProjectOperationTest.scala index 460b73145..5c5e147d7 100644 --- a/src/test/scala/com/atomist/rug/runtime/js/JavaScriptInvokingProjectOperationTest.scala +++ b/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") diff --git a/src/test/scala/com/atomist/rug/runtime/js/JavaScriptOperationFinderTest.scala b/src/test/scala/com/atomist/rug/runtime/js/JavaScriptOperationFinderTest.scala index 819c349af..18c4b3e42 100644 --- a/src/test/scala/com/atomist/rug/runtime/js/JavaScriptOperationFinderTest.scala +++ b/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) {