[JENKINS-37781] Split ModelASTScriptBlock and ModelASTWhen

Into having one common ancestor instead of when extending script
jenkinsci · Nov 3, 2016 · efe93be · efe93be
1 parent b8463d5
commit efe93be
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 43 deletions.
diff --git a/...in/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/AbstractModelASTCodeBlock.java b/...in/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/AbstractModelASTCodeBlock.java
@@ -0,0 +1,62 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2016, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ */
+
+package org.jenkinsci.plugins.pipeline.modeldefinition.ast;
+
+/**
+ * Represents the special step which are executed without validation against the declarative subset.
+ * @see ModelASTScriptBlock
+ * @see ModelASTWhen
+ */
+public abstract class AbstractModelASTCodeBlock extends ModelASTStep {
+
+    protected AbstractModelASTCodeBlock(Object sourceLocation, String name) {
+        super(sourceLocation);
+        this.setName(name);
+    }
+
+    @Override
+    public String toGroovy() {
+        StringBuilder result = new StringBuilder(getName()).append(" {\n");
+        if (getArgs() != null
+                && getArgs() instanceof ModelASTSingleArgument
+                && ((ModelASTSingleArgument) getArgs()).getValue()!=null
+                && ((ModelASTSingleArgument) getArgs()).getValue().isLiteral()) {
+            result.append(((ModelASTSingleArgument) getArgs()).getValue().getValue());
+        } else if (getArgs() != null) {
+            result.append(getArgs().toGroovy());
+        }
+        result.append("\n}\n");
+        return result.toString();
+    }
+
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + "{" +
+                "name='" + getName() + '\'' +
+                ", args=" + getArgs() +
+                "}";
+    }
+}
diff --git a/...src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTScriptBlock.java b/...src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTScriptBlock.java
@@ -5,37 +5,8 @@
  *
  * @author Andrew Bayer
  */
-public class ModelASTScriptBlock extends ModelASTStep {
+public class ModelASTScriptBlock extends AbstractModelASTCodeBlock {
     public ModelASTScriptBlock(Object sourceLocation) {
-        this(sourceLocation, "script");
+        super(sourceLocation, "script");
     }
-
-    protected ModelASTScriptBlock(Object sourceLocation, String name) {
-        super(sourceLocation);
-        this.setName(name);
-    }
-
-    @Override
-    public String toGroovy() {
-        StringBuilder result = new StringBuilder(getName()).append(" {\n");
-        if (getArgs() != null
-                && getArgs() instanceof ModelASTSingleArgument
-                && ((ModelASTSingleArgument) getArgs()).getValue()!=null
-                && ((ModelASTSingleArgument) getArgs()).getValue().isLiteral()) {
-            result.append(((ModelASTSingleArgument) getArgs()).getValue().getValue());
-        } else if (getArgs() != null) {
-            result.append(getArgs().toGroovy());
-        }
-        result.append("\n}\n");
-        return result.toString();
-    }
-
-    @Override
-    public String toString() {
-        return "ModelASTScriptBlock{" +
-                "name='" + getName() + '\'' +
-                ", args=" + getArgs() +
-                "}";
-    }
-
 }
diff --git a/...el-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWhen.java b/...el-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWhen.java
@@ -31,19 +31,11 @@
 /**
  * Represents a block for when/if a {@link ModelASTStage} will be executed or not.
  */
-public class ModelASTWhen extends ModelASTScriptBlock { //Model wise we don't really extend it but there are plenty of reuse
+public class ModelASTWhen extends AbstractModelASTCodeBlock {
     public ModelASTWhen(Object sourceLocation) {
         super(sourceLocation, "when");
     }
 
-    @Override
-    public String toString() {
-        return "ModelASTWhen{" +
-                "name='" + getName() + '\'' +
-                ", args=" + getArgs() +
-                "}";
-    }
-
     @Override
     public JSONObject toJSON() {
         JSONObject o = new JSONObject();

diff --git a/.../src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy b/.../src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy
@@ -34,6 +34,7 @@ import org.codehaus.groovy.ast.stmt.ExpressionStatement
 import org.codehaus.groovy.ast.stmt.Statement
 import org.codehaus.groovy.control.SourceUnit
 import org.codehaus.groovy.syntax.Types
+import org.jenkinsci.plugins.pipeline.modeldefinition.ast.AbstractModelASTCodeBlock
 import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTPostStage
 import org.jenkinsci.plugins.pipeline.modeldefinition.ModelStepLoader
 import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTArgumentList
@@ -641,17 +642,17 @@ class ModelParser {
     }
 
     public ModelASTWhen parseWhen(Statement st) {
-        return parseScriptBlockInternal(st, new ModelASTWhen(st), "When")
+        return parseCodeBlockInternal(st, new ModelASTWhen(st), "When")
     }
 
     /**
      * Parses a statement into a {@link ModelASTScriptBlock}
      */
     public ModelASTScriptBlock parseScriptBlock(Statement st) {
-        return parseScriptBlockInternal(st, new ModelASTScriptBlock(st), "Script")
+        return parseCodeBlockInternal(st, new ModelASTScriptBlock(st), "Script")
     }
 
-    private <T extends ModelASTScriptBlock> T parseScriptBlockInternal(Statement st, T scriptBlock, String pronoun) {
+    private <T extends AbstractModelASTCodeBlock> T parseCodeBlockInternal(Statement st, T scriptBlock, String pronoun) {
         // TODO: Probably error out for cases with parameters?
         def bs = matchBlockStatement(st);
         if (bs != null) {