Merge pull request #127 from abayer/jenkins-42360

[JENKINS-42360] Improve validation on object method calls
jenkinsci · Mar 1, 2017 · 5c215ff · 5c215ff
2 parents faa4b12 + 2750f57
commit 5c215ff
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 4 deletions.
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
@@ -882,7 +882,12 @@ class ModelParser implements Parser {
     protected String parseMethodName(MethodCallExpression exp) {
         def s = matchMethodName(exp)
         if (s==null) {
-            errorCollector.error(ModelASTValue.fromConstant(null, exp), Messages.ModelParser_ExpectedSymbol())
+            if (exp.objectExpression instanceof VariableExpression &&
+                !((VariableExpression)exp.objectExpression).isThisExpression()) {
+                errorCollector.error(ModelASTValue.fromConstant(null, exp), Messages.ModelParser_ObjectMethodCall())
+            } else {
+                errorCollector.error(ModelASTValue.fromConstant(null, exp), Messages.ModelParser_ExpectedSymbol())
+            }
             s = "error";
         }
         return s;

diff --git a/...ion/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/Messages.properties b/...ion/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/Messages.properties
@@ -62,6 +62,7 @@ ModelParser.NoArgForAgent=No argument for agent
 ModelParser.NoArgForAgentKey=No argument for agent key "{0}"
 ModelParser.NoArgForMapMethodKey=No argument for map key "{0}"
 ModelParser.NoArgForTool=No argument for tool "{0}"
+ModelParser.ObjectMethodCall=Method calls on objects not allowed outside "script" blocks.
 ModelParser.OneAgentMax=Only one agent type is allowed per agent section
 ModelParser.PipelineBlockNotAtTop={0} block must be at the top-level, not within another block.
 ModelParser.PipelineStepWithoutBlock=Expected a block with the "{0}" step

diff --git a/...ition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java b/...ition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java
@@ -534,7 +534,7 @@ public void libraryObjectInScript() throws Exception {
 
     @Issue("JENKINS-40657")
     @Test
-    public void libraryObjectOutsideScript() throws Exception {
+    public void libraryObjectDefinedOutsidePipeline() throws Exception {
         otherRepo.init();
         otherRepo.write("src/org/foo/Zot.groovy", "package org.foo;\n" +
                 "\n" +
@@ -553,7 +553,7 @@ public void libraryObjectOutsideScript() throws Exception {
                 new LibraryConfiguration("zot-stuff",
                         new SCMSourceRetriever(new GitSCMSource(null, otherRepo.toString(), "", "*", "", true)))));
 
-        expect("libraryObjectOutsideScript")
+        expect("libraryObjectDefinedOutsidePipeline")
                 .logContains("hello");
     }
 

diff --git a/...efinition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java b/...efinition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java
@@ -410,7 +410,7 @@ public void globalLibraryObjectMethodCall() throws Exception {
         // Test the case of calling a method on an object, i.e., foo.bar(1). This will fail.
         expectError("globalLibraryObjectMethodCall")
                 .logContains("MultipleCompilationErrorsException: startup failed:",
-                        Messages.ModelParser_ExpectedSymbol())
+                        Messages.ModelParser_ObjectMethodCall())
                 .go();
     }
 

diff --git a/...sources/libraryObjectOutsideScript.groovy → ...ibraryObjectDefinedOutsidePipeline.groovy b/...sources/libraryObjectOutsideScript.groovy → ...ibraryObjectDefinedOutsidePipeline.groovy