Add warning on reserved method names

jvm/src/main/scala/com/nawforce/apexlink/cst/BodyDeclarations.scala

@@ -268,7 +268,7 @@ class ApexMethodDeclaration(
     }
 
     returnTypeName.dependOn(id.location, context)
-    id.validate(context, isMethod = true)
+    id.validateForMethod(context)
     parameters.foreach(_.verify(context))
 
     val blockContext =

jvm/src/main/scala/com/nawforce/apexlink/cst/CST.scala

@@ -15,6 +15,7 @@ package com.nawforce.apexlink.cst
 
 import com.nawforce.apexlink.diagnostics.IssueOps
 import com.nawforce.apexlink.names.XNames.NameUtils
+import com.nawforce.pkgforce.diagnostics.Issue
 import com.nawforce.pkgforce.names.{Name, Names, TypeName}
 import com.nawforce.pkgforce.path.Positionable
 import com.nawforce.runtime.parsers.{CodeParser, Source}
@@ -43,18 +44,36 @@ object CST {
 }
 
 final case class Id(name: Name) extends CST {
-  def validate(context: VerifyContext, isMethod: Boolean = false): Unit = {
-    if (name.nonEmpty) {
-      val illegalError = name.isLegalIdentifier
-      if (illegalError.nonEmpty)
-        context.log(IssueOps.illegalIdentifier(location, name, illegalError.get))
-      else {
-        val isReserved =
-          if (isMethod) name.isReservedMethodIdentifier else name.isReservedIdentifier
-        if (isReserved)
-          context.log(IssueOps.reservedIdentifier(location, name))
+  def validate(context: VerifyContext): Unit = {
+    validateReserved((n: Name) => n.isReservedIdentifier).foreach(context.log)
+  }
+
+  def validateForMethod(context: VerifyContext): Unit = {
+    validateReserved((n: Name) => n.isReservedMethodIdentifier)
+      .orElse {
+        if (name.isReservedIdentifier) {
+          Some(IssueOps.reservedMethodIdentifierWarning(location, name))
+        } else {
+          None
+        }
       }
-    }
+      .foreach(context.log)
+  }
+
+  private def validateReserved(reserved: Name => Boolean): Option[Issue] = {
+    if (name.isEmpty)
+      None
+    else
+      name.isLegalIdentifier
+        .map(error => {
+          IssueOps.illegalIdentifier(location, name, error)
+        })
+        .orElse {
+          if (reserved(name))
+            Some(IssueOps.reservedIdentifier(location, name))
+          else
+            None
+        }
   }
 }
 

jvm/src/main/scala/com/nawforce/apexlink/diagnostics/IssueOps.scala

@@ -39,6 +39,16 @@ object IssueOps {
       Diagnostic(ERROR_CATEGORY, location.location, s"'$name' is a reserved identifier in Apex")
     )
 
+  def reservedMethodIdentifierWarning(location: PathLocation, name: Name): Issue =
+    Issue(
+      location.path,
+      Diagnostic(
+        WARNING_CATEGORY,
+        location.location,
+        s"'$name' is currently a legal method name but is a reserved identifier in Apex so should be avoided"
+      )
+    )
+
   def noTypeDeclaration(location: PathLocation, typeName: TypeName): Issue =
     Issue(
       location.path,

jvm/src/test/scala/com/nawforce/apexlink/cst/IdTest.scala

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2024 Certinia Inc. All rights reserved.
+ */
+
+package com.nawforce.apexlink.cst
+
+import com.nawforce.apexlink.TestHelper
+import org.scalatest.funsuite.AnyFunSuite
+
+class IdTest extends AnyFunSuite with TestHelper {
+
+  test("Local var illegal name") {
+    typeDeclaration("public class Dummy {{ String _i; }}")
+    assert(
+      dummyIssues == "Error: line 1 at 29-31: '_i' is not legal identifier in Apex, identifiers can not start or end with '_'\n"
+    )
+  }
+
+  test("Local var reserved name") {
+    typeDeclaration("public class Dummy {{ String limit; }}")
+    assert(dummyIssues == "Error: line 1 at 29-34: 'limit' is a reserved identifier in Apex\n")
+  }
+
+  test("Field illegal name") {
+    typeDeclaration("public class Dummy {String _i;}")
+    assert(
+      dummyIssues == "Error: line 1 at 27-29: '_i' is not legal identifier in Apex, identifiers can not start or end with '_'\n"
+    )
+  }
+
+  test("Field reserved name") {
+    typeDeclaration("public class Dummy {String limit;}")
+    assert(dummyIssues == "Error: line 1 at 27-32: 'limit' is a reserved identifier in Apex\n")
+  }
+
+  test("For-control var illegal name") {
+    typeDeclaration("public class Dummy {{ for(Integer _i; _i<0; _i++) {} }}")
+    assert(
+      dummyIssues == "Error: line 1 at 34-36: '_i' is not legal identifier in Apex, identifiers can not start or end with '_'\n"
+    )
+  }
+
+  test("For-control var reserved name") {
+    typeDeclaration("public class Dummy {{ for(Integer limit; limit<0; limit++) {} }}")
+    assert(dummyIssues == "Error: line 1 at 34-39: 'limit' is a reserved identifier in Apex\n")
+  }
+
+  test("Method call illegal name") {
+    typeDeclaration("public class Dummy {void _a(){} void f2() {_a();} }")
+    assert(
+      dummyIssues == "Error: line 1 at 25-27: '_a' is not legal identifier in Apex, identifiers can not start or end with '_'\n"
+    )
+  }
+
+  test("Method call reserved name") {
+    typeDeclaration("public class Dummy {void limit(){} void f2() {limit();} }")
+    assert(dummyIssues == "Error: line 1 at 25-30: 'limit' is a reserved identifier in Apex\n")
+  }
+
+  test("Method call reserved name, but legal") {
+    typeDeclaration("public class Dummy {void integer(){} void f2() {integer();} }")
+    assert(
+      dummyIssues == "Warning: line 1 at 25-32: 'integer' is currently a legal method name but is a reserved identifier in Apex so should be avoided\n"
+    )
+  }
+
+  test("Method call illegal param") {
+    typeDeclaration("public class Dummy {void f1(Integer _a){} void f2() {f1(1);} }")
+    assert(
+      dummyIssues == "Error: line 1 at 36-38: '_a' is not legal identifier in Apex, identifiers can not start or end with '_'\n"
+    )
+  }
+
+  test("Method call reserved param") {
+    typeDeclaration("public class Dummy {void f1(Integer limit){} void f2() {f1(1);} }")
+    assert(dummyIssues == "Error: line 1 at 36-41: 'limit' is a reserved identifier in Apex\n")
+  }
+
+}

jvm/src/test/scala/com/nawforce/apexlink/cst/MethodTest.scala

@@ -43,30 +43,6 @@ class MethodTest extends AnyFunSuite with TestHelper {
     assert(dummyIssues.isEmpty)
   }
 
-  test("Method call illegal name") {
-    typeDeclaration("public class Dummy {void _a(){} void f2() {_a();} }")
-    assert(
-      dummyIssues == "Error: line 1 at 25-27: '_a' is not legal identifier in Apex, identifiers can not start or end with '_'\n"
-    )
-  }
-
-  test("Method call reserved name") {
-    typeDeclaration("public class Dummy {void limit(){} void f2() {limit();} }")
-    assert(dummyIssues == "Error: line 1 at 25-30: 'limit' is a reserved identifier in Apex\n")
-  }
-
-  test("Method call illegal param") {
-    typeDeclaration("public class Dummy {void f1(Integer _a){} void f2() {f1(1);} }")
-    assert(
-      dummyIssues == "Error: line 1 at 36-38: '_a' is not legal identifier in Apex, identifiers can not start or end with '_'\n"
-    )
-  }
-
-  test("Method call reserved param") {
-    typeDeclaration("public class Dummy {void f1(Integer limit){} void f2() {f1(1);} }")
-    assert(dummyIssues == "Error: line 1 at 36-41: 'limit' is a reserved identifier in Apex\n")
-  }
-
   test("Method call wrong arguments") {
     typeDeclaration("public class Dummy {static void f1(String a){} void f2() {f1();} }")
     assert(

jvm/src/test/scala/com/nawforce/apexlink/cst/VarTest.scala

@@ -19,11 +19,6 @@ import org.scalatest.funsuite.AnyFunSuite
 
 class VarTest extends AnyFunSuite with TestHelper {
 
-  test("Reserved local var") {
-    typeDeclaration("public class Dummy { void func() {String package;}}")
-    assert(dummyIssues == "Error: line 1 at 41-48: 'package' is a reserved identifier in Apex\n")
-  }
-
   test("Duplicate local var") {
     typeDeclaration("public class Dummy { void func() {String a; String a;}}")
     assert(dummyIssues == "Error: line 1 at 51-52: Duplicate variable 'a'\n")
@@ -39,20 +34,6 @@ class VarTest extends AnyFunSuite with TestHelper {
     assert(dummyIssues == "Error: line 1 at 65-66: Duplicate variable 'a'\n")
   }
 
-  test("Reserved for var") {
-    typeDeclaration(
-      "public class Dummy { void func() {for (Integer package=0; package<0; package++){}}}"
-    )
-    assert(dummyIssues == "Error: line 1 at 47-54: 'package' is a reserved identifier in Apex\n")
-  }
-
-  test("Reserved for-each var") {
-    typeDeclaration(
-      "public class Dummy { void func() {for (Integer package: new List<Integer>{}){}}}"
-    )
-    assert(dummyIssues == "Error: line 1 at 47-54: 'package' is a reserved identifier in Apex\n")
-  }
-
   test("Duplicate for vars") {
     typeDeclaration(
       "public class Dummy { void func() {for (Integer i=0; i<0; i++){} for(Integer i=0; i<0; i++){} }}"