Warn user when forgetting def on a variable

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/LoggingInvoker.java

@@ -30,6 +30,7 @@
 import com.cloudbees.groovy.cps.sandbox.SandboxInvoker;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import groovy.lang.Closure;
 import groovy.lang.GroovyClassLoader;
 import hudson.ExtensionList;
 import java.util.Set;
@@ -137,6 +138,23 @@ private void maybeRecord(Class<?> clazz, Supplier<String> message) {
         Class<?> clazz = classOf(lhs);
         maybeRecord(clazz, () -> clazz.getName() + "." + name);
         delegate.setProperty(lhs, name, value);
+        if (value != null) {
+            var owner = findOwner(lhs);
+            if (owner instanceof CpsScript) {
+                var g = CpsThreadGroup.current();
+                if (g != null) {
+                    var e = g.getExecution();
+                    if (e != null) {
+                        e.getOwner().getListener().getLogger().println(Messages.LoggingInvoker_field_set(findOwner(lhs).getClass().getName(), name, value.getClass().getName()));
+                        assert false : "TODO look for false positives";
+                    }
+                }
+            }
+        }
+    }
+
+    private Object findOwner(Object o) {
+        return o instanceof Closure<?> c ? findOwner(c.getOwner()) : o;
     }
 
     @Override public Object getAttribute(Object lhs, String name) throws Throwable {

plugin/src/main/resources/org/jenkinsci/plugins/workflow/cps/Messages.properties

@@ -6,3 +6,4 @@ SnippetizerLink.OnlineDocsLink.displayName=Online Documentation
 SnippetizerLink.ExamplesLink.displayName=Examples Reference
 SnippetizerLink.GDSLLink.displayName=IntelliJ IDEA GDSL
 Pipeline.Syntax=Pipeline Syntax
+LoggingInvoker.field_set=Apparent field set (did you forget `def`?): {0}.{1}={2}

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsBodyExecutionTest.java

@@ -269,6 +269,7 @@ public static class Execution extends AbstractStepExecutionImpl {
             }
             r.assertBuildStatusSuccess(r.waitForCompletion(b));
             new DepthFirstScanner().allNodes(b.getExecution()).stream().sorted(Comparator.comparing(n -> Integer.valueOf(n.getId()))).forEach(n -> System.out.println(n.getId() + " " + n.getDisplayName()));
+            r.assertLogContains(Messages.LoggingInvoker_field_set("WorkflowScript", "g", CpsClosure2.class.getName()), b);
         });
     }
 

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowDefinition2Test.java

@@ -495,7 +495,7 @@ public void multipleAssignmentOutsideSandbox() throws Exception {
     @Test
     public void multipleAssignmentFunctionCalledOnce() throws Exception {
         WorkflowJob job = jenkins.createProject(WorkflowJob.class);
-        job.setDefinition(new CpsFlowDefinition("alreadyRun = false\n" +
+        job.setDefinition(new CpsFlowDefinition("def alreadyRun = false\n" +
                 "def getAandB() {\n" +
                 "  if (!alreadyRun) {\n" +
                 "    alreadyRun = true\n" +

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorServiceTest.java

@@ -139,7 +139,7 @@ public class CpsVmExecutorServiceTest {
     @Test public void okCatcherMetaClassImpl() throws Exception {
         p.setDefinition(new CpsFlowDefinition(
             "import org.codehaus.groovy.runtime.InvokerHelper \n" + 
-            "c = { println 'doing a thing' } \n" +
+            "def c = { println 'doing a thing' } \n" +
             "InvokerHelper.getMetaClass(c).invokeMethod(c, 'call', null)", false));
         WorkflowRun b = r.buildAndAssertSuccess(p);
         r.assertLogNotContains("MetaClassImpl", b);
@@ -150,7 +150,7 @@ public class CpsVmExecutorServiceTest {
     @Test public void okCatcherExpandoMetaClass() throws Exception {
         p.setDefinition(new CpsFlowDefinition(
             "import org.codehaus.groovy.runtime.InvokerHelper \n" + 
-            "c = { println 'doing a thing' } \n" +
+            "def c = { println 'doing a thing' } \n" +
             "c.getMetaClass().someField = 'r' \n" + 
             "InvokerHelper.getMetaClass(c).invokeMethod(c, 'call', null)", false));
         WorkflowRun b = r.buildAndAssertSuccess(p);

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/DSLTest.java

@@ -351,7 +351,7 @@ public void namedSoleParamForStep() throws Exception {
     * Tests the ability to execute a user defined closure with one arguments
     */
     @Test public void userDefinedClosure1ArgInvocationExecution() throws Exception {
-        p.setDefinition(new CpsFlowDefinition("my_closure = { String message -> \n" +
+        p.setDefinition(new CpsFlowDefinition("def my_closure = { String message -> \n" +
                                               "  echo message \n" +
                                               "}\n" + 
                                               "my_closure(\"my message!\") ", false));
@@ -363,7 +363,7 @@ public void namedSoleParamForStep() throws Exception {
     * Tests the ability to execute a user defined closure with 2 arguments
     */
     @Test public void userDefinedClosure2ArgInvocationExecution() throws Exception {
-        p.setDefinition(new CpsFlowDefinition("my_closure = { String message1, String message2 -> \n" +
+        p.setDefinition(new CpsFlowDefinition("def my_closure = { String message1, String message2 -> \n" +
                                               "  echo \"my message is ${message1} and ${message2}\" \n" +
                                               "}\n" + 
                                               "my_closure(\"string1\", \"string2\") ", false));
@@ -375,7 +375,7 @@ public void namedSoleParamForStep() throws Exception {
     * Tests untyped arguments 
     */
     @Test public void userDefinedClosureUntypedArgInvocationExecution() throws Exception {
-        p.setDefinition(new CpsFlowDefinition("my_closure = { a , b -> \n" +
+        p.setDefinition(new CpsFlowDefinition("def my_closure = { a , b -> \n" +
                                                       "  echo \"my message is ${a} and ${b}\" \n" +
                                                       "}\n" +
                                                       "my_closure(\"string1\" ,2)",false));

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/replay/ReplayPipelineCommandTest.java

@@ -54,6 +54,7 @@ public void rebuildNeedScriptApprovalCLIEdition() throws Exception {
         WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "SECURITY-3362");
         j.jenkins.getWorkspaceFor(p).child("a.groovy").write("echo 'Hello LoadedWorld'", null);
         String script =
+                "def a\n" +
                 "node() {\n" +
                 "    a = load('a.groovy')\n" +
                 "}\n";

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/steps/ParallelStepTest.java

@@ -66,7 +66,7 @@ public void minimumViableParallelRun() throws Exception {
                 p = jenkins().createProject(WorkflowJob.class, "demo");
                 p.setDefinition(new CpsFlowDefinition(join(
                     "node {",
-                    "  x = parallel( a: { echo('echo a'); return 1; }, b: { echo('echo b'); sleep 1; return 2; } )",
+                    "  def x = parallel( a: { echo('echo a'); return 1; }, b: { echo('echo b'); sleep 1; return 2; } )",
                     "  assert x.a==1",
                     "  assert x.b==2",
                     "}"