Project

General

Profile

Feature #9638

resolve OO dependencies during parsing using annotations in already converted classes

Added by Greg Shah over 1 year ago. Updated 7 days ago.

Status:
WIP
Priority:
Normal
Target version:
-
Start date:
Due date:
% Done:

60%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
production:
No
env_name:
topics:

A.cls - Sub-class (575 Bytes) Octavian Adrian Gavril, 01/15/2026 10:35 AM

B.cls - Super-class (loaded from jar) (4.51 KB) Octavian Adrian Gavril, 01/15/2026 10:35 AM

E.cls - Enum (loaded from jar) (257 Bytes) Octavian Adrian Gavril, 01/15/2026 10:35 AM

IScheduledTask.cls - Sub-interface (loaded from jar) (106 Bytes) Octavian Adrian Gavril, 01/15/2026 10:35 AM

ITask.cls - Super-interface (loaded from jar) (68 Bytes) Octavian Adrian Gavril, 01/15/2026 10:35 AM


Related issues

Related to Conversion Tools - Feature #9639: centralized name management (and conversion) OR jar based resolution New
Related to Conversion Tools - Feature #11404: Implement annotation-based resolution for database dependencies. WIP

History

#1 Updated by Greg Shah over 1 year ago

  • Subject changed from eliminate need for skeletons for the built-in 4GL OO classes to resolve OO dependencies during parsing using annotations in already converted classes

The idea here is that at parse time (and during conversion as needed) we should use annotations in any already converted code (in the customer's application OR in the 4GL OO built-in classes) to resolve any dependencies such as looking up matches to methods or members, or looking up converted names. In other words, we should NOT need access to the original 4GL OO .cls files NOR ANY SKELETONS for these classes. This will require that we augment the annotations to store enough information to satisfy all queries.

In #9488, we are ensuring that FWD has support for all built-in OO classes through the 12.8.4 level. This will eliminate need for skeletons for the built-in 4GL OO classes.

During conversion, customers should be able to provide access to the jar files of already converted "applications" and we should be able to resolve all of our OO lookup requirements from annotations in those classes.

#2 Updated by Greg Shah over 1 year ago

  • Related to Feature #9639: centralized name management (and conversion) OR jar based resolution added

#3 Updated by Greg Shah 8 months ago

  • Assignee set to Octavian Adrian Gavril

#4 Updated by Octavian Adrian Gavril 7 months ago

  • Status changed from New to WIP

#5 Updated by Octavian Adrian Gavril 7 months ago

I think the best place where we can introduce this idea is in SymbolResolver.loadClass(String name, boolean topLevel, UsingType utype). We can include a new UsingType that can mark our converted class. We can add a new class that extends ClassDefinition (e.g. ConvertedClassDefinition) and populate its instances with all the necessary fields using reflection.

I converted a class and added the resulted jar to p2j/build/lib. This way it can be easily found with Class.forName.

This approach should follow what is already done in ClassDefinition(SymbolResolver sym, String qname), but instead of using Aast we should use ConvertedClassDefinition.

#6 Updated by Greg Shah 7 months ago

I think the overall idea of how to build up the class data is correct. My concerns with extending the UsingType are:

  • The concept of the UsingType matches up with specific values of the USING 4GL language statement. This new addition would break that concept.
  • I intend for this new annotations approach to be the way be resolve all different kinds of UsingType lookups. For example, we would use the annotations to populate the OO lookup data for OO built-in classes and OO classes from the converted application. Although we could even use this for DOTNET lookups, in #9637 I intend to replace the .NET skels with data from direct lookups in assemblies.

#7 Updated by Octavian Adrian Gavril 6 months ago

How should we proceed with the filenames for the converted classes?
When we load a new class we search for its filename:

public ClassDefinition(SymbolResolver sym, String qname)
   throws SchemaException
   {
      this.processed = true;
      this.name = qname;
      this.filename = ConversionData.findClassFilename(qname);

So far, I avoided this aspect but now I have errors when the sub-class is scanned and its parent location is needed (which is a converted class in a jar).

#8 Updated by Octavian Adrian Gavril 6 months ago

I'm working on super-class fields references inside the OE sub-class. I managed to load the entire data of the super-class but this isn't enough because FWD references the inherited property/event AST. I did some experiments with minimal manipulation of java names getters/setters but the logic should be more complex. It should cover testcases like bulkGet, bulkSet, setAll, etc.. We need an equivalent logic of what the current processing from annotations/oo_references.rules does.

#9 Updated by Octavian Adrian Gavril 6 months ago

I noticed that this pattern is used for getters/setters. But I think it's used for method calls as well. Should we consider refactoring of this pattern and use ClassDefinition and MemberData instead of AST references? This abstraction would provide a unified implementation that handles inheritance consistently across both scenarios.

#10 Updated by Greg Shah 6 months ago

Octavian Adrian Gavril wrote:

I noticed that this pattern is used for getters/setters. But I think it's used for method calls as well. Should we consider refactoring of this pattern and use ClassDefinition and MemberData instead of AST references? This abstraction would provide a unified implementation that handles inheritance consistently across both scenarios.

Yes, it would be best to shift from direct AST references to ClassDefinition and MemberData.

#11 Updated by Octavian Adrian Gavril 6 months ago

I've committed the latest updates to branch 9638a/16307. This includes a prototype supporting the loading of converted classes, interfaces, and enums. Current test cases handle single-level inheritance and basic interface/enum usage. I've added some temporary fixes to test property references. Moving forward, I'm refactoring the OO reference patterns and will follow up by extending coverage to all remaining object-oriented facilities.

I'm attaching the testcases.

#12 Updated by Greg Shah 6 months ago

Octavian Adrian Gavril wrote:

How should we proceed with the filenames for the converted classes?
When we load a new class we search for its filename:
[...]

So far, I avoided this aspect but now I have errors when the sub-class is scanned and its parent location is needed (which is a converted class in a jar).

I think the lookup must first happen using the qname and the classpath/SourceNameMapper to cover the case of the converted class in a jar. Then if the class isn't already converted, we will have to parse it from the filesystem.

I'm not sure if that properly answers your question.

#13 Updated by Octavian Adrian Gavril 6 months ago

Greg Shah wrote:

I think the lookup must first happen using the qname and the classpath/SourceNameMapper to cover the case of the converted class in a jar. Then if the class isn't already converted, we will have to parse it from the filesystem.

I'm not sure if that properly answers your question.

My concern was about that filename field which points to the .cls that defined this class/interface. My current approach is to use the full name from the jar as filename of a class definition if we handle a converted class. For example, if a class is not converted the filename will be ./abl/cv/B.cls. If it's already converted the filename will be com.goldencode.dataset.cv.B.

#14 Updated by Greg Shah 6 months ago

That seems reasonable.

#15 Updated by Octavian Adrian Gavril 6 months ago

Should we allow having a class in abl folder and it's converted output in an existing jar? If so, should we prioritize the jar source?

#16 Updated by Octavian Adrian Gavril 6 months ago

I've committed the last changes in 9638a/16308. These include:
  • Loaded getters and setters definition as data members in ConvertedJavaClassDefinition initialization. The same for input classes: when we generate ASTs for getters/setters methods, we create related data members for native class definition.
  • Refactored and used MemberData in rules to extract things like: access-modifier, java names, parameters type list for calls and java type to match properly assignments.
  • A new note converted-reference is used to flag that a reference has been successfully resolved. This way we distinguish references to converted java classes from references to other input classes or local references.

#17 Updated by Octavian Adrian Gavril 6 months ago

  • % Done changed from 10 to 20

I used this testcase to test the implementation:

I tested conversion with all dependencies as input and it worked fine.
I continue to expand the testcase and refactor remaining references.

#18 Updated by Greg Shah 6 months ago

Octavian Adrian Gavril wrote:

Should we allow having a class in abl folder and it's converted output in an existing jar? If so, should we prioritize the jar source?

Yes and Yes.

When the OO source file is present and there is also a binary result, we need to check to see if the binary version is up to date. The checksum/hash we store in the conversion database should be stored in class annotations. This will allow us to check if the binary version is out of date.

#19 Updated by Octavian Adrian Gavril 6 months ago

I've added new changes that cover support for the following OO references:
  • class events
  • length-of and resize methods for extent vars
  • parent method calls

I included a better logic that establishes the correct access modifier of properties/events from a converted classes. The default is PUBLIC but any GET, SET, PUBLISH, SUBSCRIBE OR UNSUBSCRIBE method will override the access modifier with its own.
As I needed information about data members like GET/SET/LENGTH-OF/RESIZE, I had to encapsulate these in a map related to that property/var.

All of these changes are in 9638a/16309.

#20 Updated by Octavian Adrian Gavril 6 months ago

About referenced tables/temp-tables in the input class, I think we should load the entire DMO definition as well. This way we can load tables and fields in the shema dictionary and the related tokens would be validated.

#21 Updated by Octavian Adrian Gavril 6 months ago

Octavian Adrian Gavril wrote:

About referenced tables/temp-tables in the input class, I think we should load the entire DMO definition as well. This way we can load tables and fields in the shema dictionary and the related tokens would be validated.

Table names are easy to extract from the converted class but there is no direct access to the table fields.

#22 Updated by Octavian Adrian Gavril 6 months ago

  • % Done changed from 20 to 30
I added new changes in 9638a/16310 and these include the following:
  • Added support for dictionary registration for inherited tables. This way, the access to class defined tables is enabled to the sub-class to be converted.
  • Added javadoc where missing.
  • Fixed non-deterministic class events access modifier. It was override by the PUBLISH, SUBSCRIBE/UNSUBSCRIBE modifier. The event was recognized in a few runs when the SUBSCRIBE/UNSUBSCRIBE modifier was used, not PUBLISH modifier that is always PRIVATE.

#23 Updated by Octavian Adrian Gavril 5 months ago

I used testcases/oo/basic to test the current solution. I need to add member data definitions of getters/setters for builtin classes as well.

#24 Updated by Octavian Adrian Gavril 4 months ago

Rebased branch 9638a with trunk/16495. I had fix a post-rebase error. The new revision is 16500.

#25 Updated by Octavian Adrian Gavril 4 months ago

About builtin classes, how are the legacy class names mapped to the actual builtin implementation? For example: OpenEdge.Core.Collections.Stack -> com.goldencode.p2j.oo.core.collections.LegacyStack?

#26 Updated by Greg Shah 4 months ago

OpenEdge.Core.Collections.Stack becomes com.goldencode.p2j.oo.core.collections.LegacyStack
Progress.Lang.Enum becomes com.goldencode.p2j.oo.lang.LegacyEnum
Progress.Lang.SysError becomes com.goldencode.p2j.oo.lang.SysError

  • OpenEdge or Progress map to com.goldencode.p2j.oo
  • Subsequent packages map directly to intermediate packages in FWD (Lang to com.goldencode.p2j.oo.lang and Core to com.goldencode.p2j.oo.core).
  • Class names map to the equivalent in FWD where possible (e.g. SysError is SysError) but when these conflict with classes in FWD or the JDK, we map them to Legacy[ClassName] (e.g. Enum is LegacyEnum and Stack is LegacyStack).

#27 Updated by Octavian Adrian Gavril 4 months ago

Greg Shah wrote:

OpenEdge.Core.Collections.Stack
Progress.Lang.Enum
Progress.Lang.SysError

  • OpenEdge or Progress map to com.goldencode.p2j.oo
  • Subsequent packages map directly to intermediate packages in FWD (Lang to com.goldencode.p2j.oo.lang and Core to com.goldencode.p2j.oo.core).
  • Class names map to the equivalent in FWD where possible (e.g. SysError is SysError) but when these conflict with classes in FWD or the JDK, we map them to Legacy[ClassName] (e.g. Enum is LegacyEnum and Stack is LegacyStack).

Got it. Thanks for your explanation!

#28 Updated by Octavian Adrian Gavril 3 months ago

I've successfully integrated loading of bultin classes from jars but the current implementation is looking for class members that are marked with LegacySignature instances. So, any extra fields/methods is not loaded into the class definition instance. The current issue is about 4gl class properties/variables. Here is an example from com.goldencode.p2j.oo.core.ByteBucket:


/**
 * Business logic (converted to Java from the 4GL source code
 * in OpenEdge/Core/ByteBucket.cls).
 */
@LegacyResource(resource = "OpenEdge.Core.ByteBucket")
@LegacyResourceSupport(supportLvl = CVT_LVL_FULL | RT_LVL_PARTIAL)
public class ByteBucket extends BaseObject implements ISupportInitialize
{
   [...]

   /** The current position */
   protected int64 position = TypeFactory.int64(0L);

   [...]

   /**
    * Get the current read position.
    * 
    * @return the current read position.
    */
   @LegacySignature(returns = "INT64", type = Type.GETTER, name = "Position")
   @LegacyResourceSupport(supportLvl = CVT_LVL_FULL | RT_LVL_FULL)
   public int64 getPosition()
   {
      return function(this, "Position", int64.class, new Block((Body) () -> {
         returnNormal(position);
      }));
   }

position stores the value for Position 4gl property. But it's not loaded as a data member because it's not annotated with LegacySignature. Only the corresponding getter method carries the annotation. Having these conditions, any usages of Position would require its existence into ClassDefinition.istores.

Maybe we can look for getter method only if we search a builtin class. This would be correct if we know for sure that we have a getter for all public properties.
Another option could be to create a mocked uast.Variable instance when the getter is registered.

#29 Updated by Octavian Adrian Gavril 3 months ago

I've noticed that many builtin classes have incomplete LegacySignature instances. For example com.goldencode.p2j.oo.lang._BaseObject_:


   /**
    * Obtains the next object instance in the linked list of all existing objects.
    *
    * @return   The next object instance. 
    */
   @LegacySignature(type = Type.GETTER, name = "next-sibling")
   @LegacyResourceSupport(supportLvl = CVT_LVL_FULL|RT_LVL_FULL)
   public object<? extends _BaseObject_> getNextSibling();

There is no returns specified and its default value is void. So. I get void return type for a method that returns an object.

#30 Updated by Octavian Adrian Gavril 3 months ago

I've successfully fixed several errors and managed the OO builtin classes to store enough information so that the parsing phase completed successfully for my testcase project.

Now, I have this kind of error:

     [java] java.lang.NullPointerException: Cannot invoke "com.goldencode.artifacts.Artifact.getRelativePath()" because "artifact" is null
     [java]     at com.goldencode.artifacts.ArtifactManager.getPathToProjectFolder(ArtifactManager.java:244)
     [java]     at com.goldencode.p2j.schema.P2OLookup.lambda$new$0(P2OLookup.java:502)
     [java]     at com.goldencode.p2j.uast.ClassDefinition.processParentGraph(ClassDefinition.java:3442)
     [java]     at com.goldencode.p2j.uast.ClassDefinition.processParentGraph(ClassDefinition.java:3421)
     [java]     at com.goldencode.p2j.schema.P2OLookup.<init>(P2OLookup.java:516)
     [java]     at com.goldencode.p2j.schema.P2OLookup.dynamicGet(P2OLookup.java:443)
     [java]     at com.goldencode.p2j.schema.P2OLookup.loadTemporarySchema(P2OLookup.java:592)
     [java]     at com.goldencode.p2j.schema.P2OAccessWorker.visitAst(P2OAccessWorker.java:276)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1554)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1521)
     [java]     at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1079)
     [java]     at com.goldencode.p2j.convert.TransformDriver.processTrees(TransformDriver.java:692)
     [java]     at com.goldencode.p2j.convert.TransformDriver.processTrees(TransformDriver.java:640)
     [java]     at com.goldencode.p2j.convert.TransformDriver.front(TransformDriver.java:334)
     [java]     at com.goldencode.p2j.convert.TransformDriver.executeJob(TransformDriver.java:1152)
     [java]     at com.goldencode.p2j.convert.ConversionDriver.main(ConversionDriver.java:1343)
     [java] 

This occurs due to P2OLookUp parent processing. Do you think we should avoid that kind of processing for converted java classes?
  private P2OLookup(String schema, Artifact artifact, ClassDefinition clsDef)
   throws SchemaException
   {
      [...]

      if (clsDef != null)
      {
         ClassDefinition[] pars = clsDef.getParents();

         if (pars != null)
         {
            ArrayList<P2OLookup> parr = new ArrayList<>();
            SchemaException[]    exc  = new SchemaException[1];

            Consumer<ClassDefinition> consumer = (ClassDefinition cls) ->
            {
               if (exc[0] == null && !cls.isBuiltIn())
               {
                  String parentFile = 
                     ArtifactManager.getPathToProjectFolder(cls.getArtifact(),
                                                            FileExtensions.P2O_POSTFIX);
                  Artifact pArtifact = ArtifactManager.addArtifact(parentFile);
                  try
                  {
                     parr.add(new P2OLookup(schema, pArtifact, cls));
                  }
                  catch (SchemaException e)
                  {
                     exc[0] = e;
                  }
               }
            };

            clsDef.processParentGraph(consumer);

            if (exc[0] != null)
            {
               throw exc[0];
            }
            else
            {
               parents = parr.toArray(new P2OLookup[0]);
            }
         }
      }

      WorkArea wa = locate();
      wa.tempP2OLookup = this;
   }

#31 Updated by Greg Shah 3 months ago

Do you think we should avoid that kind of processing for converted java classes?

No, I think this is needed for OO child classes to access the temp-tables defined in parent classes.

#32 Updated by Octavian Adrian Gavril 3 months ago

Greg Shah wrote:

Do you think we should avoid that kind of processing for converted java classes?

No, I think this is needed for OO child classes to access the temp-tables defined in parent classes.

OK. I will find a way to create P2OLookup instances for these classes as well.

#33 Updated by Octavian Adrian Gavril 3 months ago

Another scenario involves a method call that is not annotated properly because the used parameter is a TABLE-HANDLE but my signature has HANDLE. It's about this method:

  @LegacySignature(type = Type.METHOD, name = "ToTable", parameters = 
   {
      @LegacyParameter(name = "tt", type = "HANDLE", mode = "OUTPUT")
   })
   @LegacyResourceSupport(supportLvl = CVT_LVL_FULL | RT_LVL_FULL)
   public void toTable(final OutputTableHandle _tt)
   {

I'm not sure where the translation is handled. I found something in InternalEntry but util.Parameter is used, not ParameterKey:

   private String buildSignature()
   {
      [...]

         for (Parameter p : parameters)
         {
            [...]
            else if ("HANDLE".equalsIgnoreCase(p.getType()) && p.getAttribute("TABLE") != null)
            {
               String name = p.getAttribute("pname");

               sb.append("TABLE-HANDLE").append(" ");
               sb.append(name);
            }

#34 Updated by Octavian Adrian Gavril 3 months ago

Greg Shah wrote:

Do you think we should avoid that kind of processing for converted java classes?

No, I think this is needed for OO child classes to access the temp-tables defined in parent classes.

I’ve looked into this further. My conclusions are that P2OLookup is fully based on artifacts and ASTs. So, there is no way to bound such instance to a converted class definition. Note that temp-tables and permanent tables are loaded in SymbolResolver during converted class definition constructor. I haven't encountered any issues about that so far.

#35 Updated by Octavian Adrian Gavril 3 months ago

Octavian Adrian Gavril wrote:

Another scenario involves a method call that is not annotated properly because the used parameter is a TABLE-HANDLE but my signature has HANDLE. It's about this method:
[...]

I'm not sure where the translation is handled. I found something in InternalEntry but util.Parameter is used, not ParameterKey:
[...]

I think the problem here is the signature from OO builtin classes. I have identified over a thousand annotateMethodCall FAILURE exceptions resulting from type mismatches, such as HANDLE being used instead of TABLE-HANDLE. Furthermore, several OO built-in classes lack necessary signatures. For instance, the Equals(expected as Progress.Lang.Object, actual as Progress.Lang.Object) method is defined in skeleton/oo4gl/OpenEdge/Core/Assert.cls but is absent from com.goldencode.p2j.oo.core.Assert.

So, I need to refactor multiple classes from the oo package.

#36 Updated by Octavian Adrian Gavril 3 months ago

Octavian Adrian Gavril wrote:

Octavian Adrian Gavril wrote:

Another scenario involves a method call that is not annotated properly because the used parameter is a TABLE-HANDLE but my signature has HANDLE. It's about this method:
[...]

I'm not sure where the translation is handled. I found something in InternalEntry but util.Parameter is used, not ParameterKey:
[...]

Furthermore, several OO built-in classes lack necessary signatures. For instance, the Equals(expected as Progress.Lang.Object, actual as Progress.Lang.Object) method is defined in skeleton/oo4gl/OpenEdge/Core/Assert.cls but is absent from com.goldencode.p2j.oo.core.Assert.

This part is caused by broken inheritance of builtin class methods actually. So, it's an issue caused by my implementation.

#37 Updated by Greg Shah 3 months ago

Octavian Adrian Gavril wrote:

Greg Shah wrote:

Do you think we should avoid that kind of processing for converted java classes?

No, I think this is needed for OO child classes to access the temp-tables defined in parent classes.

I’ve looked into this further. My conclusions are that P2OLookup is fully based on artifacts and ASTs. So, there is no way to bound such instance to a converted class definition. Note that temp-tables and permanent tables are loaded in SymbolResolver during converted class definition constructor. I haven't encountered any issues about that so far.

The problem here is that when the classes come from a jar, the artifacts and ASTs are not available. We need to extend this annotatoins based solution to the database features too. This was going to be needed for app-by-app anyway.

We can do that in another task but it can't wait for long.

#38 Updated by Octavian Adrian Gavril 3 months ago

  • Related to Feature #11404: Implement annotation-based resolution for database dependencies. added

#39 Updated by Octavian Adrian Gavril 3 months ago

Octavian Adrian Gavril wrote:

I think the problem here is the signature from OO builtin classes. I have identified over a thousand annotateMethodCall FAILURE exceptions resulting from type mismatches, such as HANDLE being used instead of TABLE-HANDLE. Furthermore, several OO built-in classes lack necessary signatures. For instance, the Equals(expected as Progress.Lang.Object, actual as Progress.Lang.Object) method is defined in skeleton/oo4gl/OpenEdge/Core/Assert.cls but is absent from com.goldencode.p2j.oo.core.Assert.

So, I need to refactor multiple classes from the oo package.

I’ve resolved all annotateMethodCall FAILURE warnings by updating the OO built-in classes annotations with the correct method names where required.

Next, I’m addressing the following warning:

[java] WARNING: annotateOptions cannot annotate Class Ref (class==null), ast=IteratedArray [VAR_CLASS]

This indicates that legacy variable types aren't loading correctly, which is interfering with annotateOptions processing. I've investigated the issue and found that this can be fixed by ensuring all required variables are assigned their proper legacy types.

#40 Updated by Octavian Adrian Gavril 3 months ago

Octavian Adrian Gavril wrote:

Next, I’m addressing the following warning:
[...]

This indicates that legacy variable types aren't loading correctly, which is interfering with annotateOptions processing. I've investigated the issue and found that this can be fixed by ensuring all required variables are assigned their proper legacy types.

I have resolved these warnings as well. I can now shift my focus to the errors occurring during the Code Conversion Annotations phase.

#41 Updated by Octavian Adrian Gavril 3 months ago

The testcase project converted successfully without errors. However, I’m now seeing compilation issues in the main build, likely the root cause is from some constructor-related annotations I implemented earlier:

--- oo_testcase_project/src/com/goldencode/dataset/oo/basic/UpperMiddle.java    2026-04-28 11:14:54.799053127 +0300
+++ oo_testcase_project_orig/src/com/goldencode/dataset/oo/basic/UpperMiddle.java    2026-04-28 11:16:58.548796024 +0300
@@ -18,7 +18,7 @@
    {
       internalProcedure(UpperMiddle.class, this, "UpperMiddle", new Block((Body) () -> 
       {
-         top();
+         __oo_basic_top_constructor__();
       }));
    }
 }


#42 Updated by Octavian Adrian Gavril 3 months ago

Found a fix for the default constructor names, but I'm still encountering errors with the java names for other methods:

--- oo_testcase_project/src/com/goldencode/dataset/oo/basic/ProtectedMethod.java    2026-04-28 14:17:28.572710337 +0300
+++ oo_testcase_project_orig/src/com/goldencode/dataset/oo/basic/ProtectedMethod.java    2026-04-28 13:18:35.917906838 +0300
@@ -150,7 +150,7 @@
       {
          frame0.openScope();

-         if (_isEqual(oo_references.rules_FIXME6(ProtectedMethod.this.getDesc1(new integer(1))), true))
+         if (_isEqual(stringIsEmpty(ProtectedMethod.this.getDesc1(new integer(1))), true))
          {
             FrameElement[] elementList0 = new FrameElement[]
             {
@@ -175,7 +175,7 @@
       {
          frame1.openScope();

-         if (_isEqual(oo_references.rules_FIXME6(ProtectedMethod.this.getDesc2()), true))
+         if (_isEqual(stringIsEmpty(ProtectedMethod.this.getDesc2()), true))
          {
             FrameElement[] elementList1 = new FrameElement[]
             {

I continue investigating this.

#43 Updated by Octavian Adrian Gavril 3 months ago

I’ve noticed several other differences in the conversion output. These include incorrect method selection, missing object wrappers, and the use of legacy variable names instead of java names. I'm currently addressing these issues.

#44 Updated by Octavian Adrian Gavril 3 months ago

Octavian Adrian Gavril wrote:

I’ve noticed several other differences in the conversion output. These include incorrect method selection, missing object wrappers, and the use of legacy variable names instead of java names. I'm currently addressing these issues.

Additionally, I encountered several inconsistencies caused by mismatched casing in method signatures. Specifically, these related to parameter type casing (e.g., KW_INPUT character vs. KW_INPUT CHARACTER). This occurs because the built-in OO classes use mixed casing for the dataType attribute.

I’ve fixed this by applying toLowerCase() to the relevant usages, but I’m curious if there is an established convention we should be following instead.

#45 Updated by Octavian Adrian Gavril 3 months ago

Another thing. I have such differences:

+++ oo_testcase_project_orig/src/com/goldencode/dataset/oo/discovery/openedge/core/assertion/AssertArray.java    2026-04-29 15:51:17.172686136 +0300
@@ -95,20 +95,20 @@
          com.goldencode.p2j.oo.core.assertion.AssertArray.legacyEquals(oaObject[0], oaObject[0]);
          com.goldencode.p2j.oo.core.assertion.AssertArray.legacyEquals(vaRecid[0], vaRecid[0]);
          com.goldencode.p2j.oo.core.assertion.AssertArray.legacyEquals(vaRowid[0], vaRowid[0]);
-         com.goldencode.p2j.oo.core.AbstractAssert.hasDeterminateExtent(vaCharacter[0], vCharacter);
-         com.goldencode.p2j.oo.core.AbstractAssert.hasDeterminateExtent(vaCharacter[0]);
-         com.goldencode.p2j.oo.core.AbstractAssert.isIndeterminateArray(vaCharacter[0], vCharacter);
-         com.goldencode.p2j.oo.core.AbstractAssert.isIndeterminateArray(vaCharacter[0]);
-         com.goldencode.p2j.oo.core.AbstractAssert.isIndeterminateArray(oaObject[0], vCharacter);
-         com.goldencode.p2j.oo.core.AbstractAssert.hasDeterminateExtent(oaObject[0], vCharacter);
-         com.goldencode.p2j.oo.core.AbstractAssert.hasDeterminateExtent(vaInteger[0], vCharacter);
-         com.goldencode.p2j.oo.core.AbstractAssert.hasDeterminateExtent(vaInteger[0]);
-         com.goldencode.p2j.oo.core.AbstractAssert.isIndeterminateArray(vaInteger[0], vCharacter);
-         com.goldencode.p2j.oo.core.AbstractAssert.isIndeterminateArray(vaInteger[0]);
-         com.goldencode.p2j.oo.core.AbstractAssert.hasDeterminateExtent(vaInt64[0], vCharacter);
-         com.goldencode.p2j.oo.core.AbstractAssert.hasDeterminateExtent(vaInt64[0]);
-         com.goldencode.p2j.oo.core.AbstractAssert.isIndeterminateArray(vaInt64[0], vCharacter);
-         com.goldencode.p2j.oo.core.AbstractAssert.isIndeterminateArray(vaInt64[0]);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.hasDeterminateExtent(vaCharacter[0], vCharacter);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.hasDeterminateExtent(vaCharacter[0]);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.isIndeterminateArray(vaCharacter[0], vCharacter);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.isIndeterminateArray(vaCharacter[0]);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.isIndeterminateArray(oaObject[0], vCharacter);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.hasDeterminateExtent(oaObject[0], vCharacter);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.hasDeterminateExtent(vaInteger[0], vCharacter);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.hasDeterminateExtent(vaInteger[0]);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.isIndeterminateArray(vaInteger[0], vCharacter);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.isIndeterminateArray(vaInteger[0]);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.hasDeterminateExtent(vaInt64[0], vCharacter);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.hasDeterminateExtent(vaInt64[0]);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.isIndeterminateArray(vaInt64[0], vCharacter);
+         com.goldencode.p2j.oo.core.assertion.AssertArray.isIndeterminateArray(vaInt64[0]);
       }));
       /* variables definition */ 
    }

Now that we no longer consider skeleton definitions, the OO built-in implementations are more reliable. With trunk, hasDeterminateExtent is found in AssertArray.cls, but the OO built-in classes place those methods in AbstractArray to avoid code duplication. So, static calls are made from AbstractArray instead of AssertArray. Do you agree leaving it this way, or should I load it into AssertArray as well?

#46 Updated by Octavian Adrian Gavril 3 months ago

Aside from the documented differences in #9638-45, I have resolved all other differences. I'm currently focusing on proper documentation and code cleanup, after which I will commit all changes.

Greg, how should we proceed with the remaining items from #9638-45?

#47 Updated by Greg Shah 3 months ago

Greg, how should we proceed with the remaining items from #9638-45?

Constantin: What do you think?

#48 Updated by Octavian Adrian Gavril 3 months ago

Octavian Adrian Gavril wrote:

Aside from the documented differences in #9638-45, I have resolved all other differences. I'm currently focusing on proper documentation and code cleanup, after which I will commit all changes.

The changes are complete and have been committed in rev16501. I’ve populated the built-in classes with the correct information and updated SymbolResolver and ClassDefinition to align with the new implementation and class hierarchy. Additionally, I reworked ConvertedJavaClassDefinition to better integrate with the current processing logic.

I’m going to run the current version against a different project to see how the built-in classes are being loaded and utilized.

#49 Updated by Octavian Adrian Gavril 3 months ago

  • % Done changed from 30 to 60

Is there a convention for the casing of types like DATASET-HANDLE and TABLE-HANDLE? I noticed that in ExpressionConversionWorker.simpleExpressionType, we return uppercase for some types and lowercase for others:

public static String simpleExpressionType(int type)
   {
      String jcls = null;

      switch (type)
      {
         [...]

         case VAR_HANDLE:
         case FIELD_HANDLE:
         case FUNC_HANDLE:
         case ATTR_HANDLE:
         case METH_HANDLE:
         case OO_METH_HANDLE:
         case SYS_HANDLE:
         case KW_HANDLE:
         case KW_STRM_HND:
         case KW_WID_HAND:
            jcls  = "handle";
            break;
         case KW_TAB_HAND:
            jcls  = "TABLE-HANDLE";
            break;
         case KW_DSET_HND:
            jcls  = "DATASET-HANDLE";
            break;

         [...]

         case STREAM:
            jcls = "Stream";
            break;

         case TABLE:
         case BUFFER:
         case WORK_TABLE:
         case TEMP_TABLE:
         case KW_BUFFER:
         case KW_TABLE:
            jcls = "Buffer";
            break;

         case QUERY:
            jcls = "P2JQuery";
            break;

         case KW_DATASET:
            jcls = "DATASET";
            break;

         default:
            // check for known attributes
            Integer attrType = SignatureHelper.getAttributeType(type);
            if (attrType != null)
            {
               // recursive call to match ATTR_XYZ
               jcls = simpleExpressionType(attrType); 
            }
            break;
      }

      return jcls;
   }

I fixed some casing mismatch issues by adding @toLowerCase() where needed, but I want to have a clean logic for this. Is there a rule I should follow?

#50 Updated by Octavian Adrian Gavril 3 months ago

I've updated the casing of the data types in the loaded signatures to match the method call signatures. These changes are in rev16502.

While testing with another project, I discovered other incomplete built-in classes. I'm going to populate them with the required information.

#51 Updated by Greg Shah 3 months ago

Is there a rule I should follow?

There was no rule. I agree with making it consistent.

#52 Updated by Octavian Adrian Gavril 3 months ago

I've noticed some special processing for the setParameter method in Progress.Lang.ParameterList. Our built-in version has two signatures for this method, while the skeleton version has only one. This means that during a signature lookup, two methods are loaded for the 9638a branch, but only one for trunk.

This wouldn't normally be an issue, but there is a first flag used when searching for method candidates. It is calculated via the isSetParam method, which identifies whether we are looking for setParameter. The issue is that this flag determines whether or not to consider the number of parameters. Specifically, it selects the first signature from imethods/smethods without performing the usual parameter check. Because the methods are not loaded in a specific order, the results are non-deterministic: for a 4 parameter method call, we sometimes find a match and sometimes throw an exception due to a signature mismatch.

I need to know which signature is expected to be 'first' so I can order the candidates properly.

#53 Updated by Greg Shah 3 months ago

What are the signatures of setParameter() in the built-in version and what is the signature in the skeleton version?

#54 Updated by Octavian Adrian Gavril 3 months ago

Greg Shah wrote:

What are the signatures of setParameter() in the built-in version and what is the signature in the skeleton version?

This is in ParameterList.java:

@LegacySignature(type = Type.METHOD, name = "SetParameter", returns = "LOGICAL", parameters = 
   {
      @LegacyParameter(name = "pos", type = "INTEGER", mode = "INPUT"),
      @LegacyParameter(name = "val", type = "BDT", mode = "INPUT")
   })
   @LegacyResourceSupport(supportLvl = CVT_LVL_FULL|RT_LVL_STUB)
   public logical setParameter(final integer pos, final Object val)
   {
      int idx = pos.isUnknown() ? 0 : pos.intValue();
      CallParameter param = null;

      if (idx > 0 && idx <= this.getNumParameters().intValue())
      {
         param = parameters[idx - 1];

         if (param == null) 
         {
            String msg = "The SetParameter overload with 2 parameters is not allowed unless the ParameterList has been initialized previously for parameter " + pos.intValue() + ".";
            ErrorManager.recordOrThrowError(15294, msg, false);
            return logical.of(false);
         }    
      }

      return logical.of(setParameter(idx, param != null ? param.dataType : null, param != null ? param.mode : null, val));
   }

   /**
    * Implementation of the SET-PARAMETER method.
    * 
    * @param    pos
    *           The 1-based parameter index.
    * @param    type
    *           The parameter's data type.
    * @param    mode
    *           The parameter's mode (INPUT, OUTPUT, INPUT-OUTPUT, with -APPEND or -BY-REFERENCE
    *           suffix).
    * @param    val
    *           The argument's value.   Can be only a variable/field-ref or extent, if mode is
    *           OUTPUT or INPUT-OUTPUT.
    *           
    * @return   &lt;code&gt;true&lt;/code&gt; if the parameter was set successfully. 
    */
   @LegacySignature(returns = "LOGICAL", type = Type.METHOD, name = "SetParameter", parameters =
   {
      @LegacyParameter(name = "pos",  type = "INTEGER",   mode = "INPUT"),
      @LegacyParameter(name = "type", type = "CHARACTER", mode = "INPUT"),
      @LegacyParameter(name = "mode", type = "CHARACTER", mode = "INPUT"),
      @LegacyParameter(name = "val",  type = "BDT",       mode = "INPUT"),
   })
   @LegacyResourceSupport(supportLvl = CVT_LVL_FULL|RT_LVL_BASIC)
   public logical setParameter(integer pos, character type, character mode, Object val)
   {
      // space is used as separator instead of '-' for call, trailing spaces are allowed
      CallMode cmode = CallMode.buildMode(TextOps.replaceAll(TextOps.rightTrim(mode), " ", "-"));

      return logical.of(setParameter(pos.isUnknown() ? 0 : pos.intValue(), type.getValue(), cmode, val));
   }

And this is in ParameterList.cls:

class Progress.Lang.ParameterList
inherits Progress.Lang.Object:

   define property NumParameters as integer
      get.
      set.

   constructor public ParameterList(input num-parms as integer):
   end constructor.

   method public logical Clear():
   end method.

   /* the actual parm-val data type is variable but we are encoding it as */
   /* Object until we have a better solution */
   method public logical SetParameter(input idx as integer,
                                      input dtype as character,
                                      input pmode as character,
                                      input parm-val as _polymorphic_):
   end method.

end class.

The method call uses 4-parameters version but sometimes the 2-parameter signature ends up as the candidate.

#55 Updated by Greg Shah 3 months ago

From the OE docs:

This method supports two overloaded versions. The first version is used to initialize and populate elements in the ParameterList object. It’s calling sequence is similar to the Call object’s SET-PARAMETER method.

SetParameter ( INPUT position AS INTEGER, INPUT data-type AS CHARACTER, INPUT iomode AS CHARACTER, INPUT parameter-value )

The second overloaded version may ONLY be called if SetParameter has been called before on this parameter, so that data-type and iomode are already initialized.

SetParameter ( INPUT position AS INTEGER, INPUT parameter-value )

This and the fact that the 4 parameter version is in the skeleton suggests that the 4 parameter version should be matched first.

Constantin?

#56 Updated by Octavian Adrian Gavril 2 months ago

I've resolved additional issues in the parsing phase for the other OO application. The parsing phase now passes without errors. Changes have been committed to rev16503.

I'm waiting for the entire conversion to finish so I can check the differences.

#57 Updated by Octavian Adrian Gavril 2 months ago

Unfortunately, during the Code Conversion Annotation phase, I ran into some errors related to inherited class names. I'm continuing to investigate this.

#58 Updated by Octavian Adrian Gavril 2 months ago

I resolved the errors from the Code Conversion Annotation phase. However, new errors related to OBJECT_INVOCATION ASTs appeared during the Code Core Conversion phase. I fixed all of those, and the conversion for the OO application is now complete. I'm currently investigating some differences with the java constructor names.

#59 Updated by Octavian Adrian Gavril 2 months ago

Octavian Adrian Gavril wrote:

I resolved the errors from the Code Conversion Annotation phase. However, new errors related to OBJECT_INVOCATION ASTs appeared during the Code Core Conversion phase. I fixed all of those, and the conversion for the OO application is now complete. I'm currently investigating some differences with the java constructor names.

I fixed java constructor names. The remaining differneces are:
  • two mismatched method names.
  • occurences where invoke appears instead of invokeStandalone
  • an extra wrapped character parameter in certain signatures calls.

#60 Updated by Octavian Adrian Gavril 2 months ago

Octavian Adrian Gavril wrote:

I fixed java constructor names. The remaining differneces are:
  • two mismatched method names.
  • occurences where invoke appears instead of invokeStandalone
  • an extra wrapped character parameter in certain signatures calls.

I fixed all these differences and retested the conversion for the OO application. The conversion completed successfully, and the differences are gone.

There is just one remaining difference that I missed regarding the declaration order:

--- /home/aog/gcd/oo_app/File.java    2026-05-12 15:59:23.674375526 +0300
+++ /home/aog/gcd/oo_app_orig/File.java
@@ -47,10 +47,10 @@
    @LegacySignature(type = Type.VARIABLE, name = "cDir", dataType = "CHARACTER")
    character cDir = TypeFactory.character();

-   final AdaptiveFind query18 = new AdaptiveFind();
-
    final AdaptiveFind query19 = new AdaptiveFind();

+   final AdaptiveFind query18 = new AdaptiveFind();
+
    /**
     * External procedure (converted to Java from the 4GL source code
     * in oo_app/File.p).
@@ -60,8 +60,8 @@
    {
       externalProcedure(File.this, new Block((Init) () -> 
       {
-         query18.initialize(ttline, null, "ttline.iline asc", "WHOLE-INDEX,main");
-         query19.initialize(ttline, null, "ttline.iline desc", true, "WHOLE-INDEX,main");
+         query19.initialize(ttline, null, "ttline.iline asc", "WHOLE-INDEX,main");
+         query18.initialize(ttline, null, "ttline.iline desc", true, "WHOLE-INDEX,main");
       }, 
       (Body) () -> 
       {
@@ -1112,12 +1112,12 @@
                                  leave("loopLabel9");
                               }

-                              query18.silentNext();
+                              query19.silentNext();
                            }));

                            if (ttline._available())
                            {
-                              query19.silentNext();
+                              query18.silentNext();
                            }
                         }
                      }

I need to investigate the root cause.

#61 Updated by Octavian Adrian Gavril 2 months ago

I've successfully reproduced the issue documented in #9638-60 and will continue investigating it.

#62 Updated by Octavian Adrian Gavril 2 months ago

Octavian Adrian Gavril wrote:

Octavian Adrian Gavril wrote:

I fixed java constructor names. The remaining differneces are:
  • two mismatched method names.
  • occurences where invoke appears instead of invokeStandalone
  • an extra wrapped character parameter in certain signatures calls.

I fixed all these differences and retested the conversion for the OO application. The conversion completed successfully, and the differences are gone.

I've committed the changes related to these conversion differences in rev16504. I'm planning a rebase to see if a revision mismatch caused the last reported differences.

#63 Updated by Octavian Adrian Gavril 2 months ago

Rebased branch 9638a with trunk rev16574. The new revision is 16583.

#64 Updated by Octavian Adrian Gavril 2 months ago

I'm using this testcase to investigate the last difference:

define temp-table tt2 no-undo
    field m1 as character
    field m2 as character
    index main m1 m2.

define temp-table tt3 no-undo
    field l1 as integer
    field l2 as character
    index main is primary l1.

procedure p1:
    define input parameter cCls as character no-undo.

    for each tt2 no-lock where tt2.m1 = cCls break by tt2.m2:
        if not first-of(tt2.m2) then next.

        find first tt3 no-lock 
             where trim(tt3.l2) begins 'GenChild' 
               and index(tt3.l2, tt2.m2) > 0 no-error.

        if not available tt3 then do:
            find first tt3 no-lock where not trim(tt3.l2) begins 'genobj1' and index(tt3.l2, 'genobj1') > 0 no-error.
            if not available tt3 then 
                find first tt3 no-lock where not trim(tt3.l2) begins 'genobj2' and index(tt3.l2, 'genobj2') > 0 no-error.

            if available tt3 then do:
                repeat:
                    if not available tt3 or trim(tt3.l2) begins 'end_m' then leave.
                    find next tt3 no-error.
                end.
                if available tt3 then find prev tt3 no-error.
            end.
        end.

        if available tt3 then 
            tt3.l2 = tt3.l2 + '~n' 
                   + replace(tt3.l2, trim(tt3.l2), '') 
                   + tt2.m2 + ':PropSet = ~' + '~':U.'.
    end.
end procedure.

I noticed these differences while running the incremental conversion (with trunk) on that testcase:

--- /home/aog/gcd/dataset/Custom.java    2026-05-21 11:58:58.355267812 +0300
+++ /home/aog/gcd/dataset_orig/Custom.java    2026-05-21 13:10:44.751984768 +0300
@@ -33,8 +33,8 @@
    {
       externalProcedure(Custom.this, new Block((Init) () -> 
       {
-         query1.initialize(tt3, null, "tt3.l1 asc", "WHOLE-INDEX,main");
-         query2.initialize(tt3, null, "tt3.l1 desc", true, "WHOLE-INDEX,main");
+         query1.initialize(tt3, null, "tt3.recid asc");
+         query2.initialize(tt3, null, "tt3.recid desc", true);
       }, 
       (Body) () -> 
       {
@@ -56,7 +56,7 @@
          FieldReference byExpr0 = new FieldReference(tt2, "m2");
          forEach(query0, "loopLabel0", new Block((Init) () -> 
          {
-            query0.initialize(tt2, "upper(tt2.m1) = ?", null, "tt2.m1 asc, tt2.m2 asc", "main", new Object[]
+            query0.initialize(tt2, "upper(tt2.m1) = ?", null, "tt2.recid asc", new Object[]
             {
                toUpperCase(cCls)
             }, LockType.NONE);
@@ -72,7 +72,7 @@
             }

-            new FindQuery(tt3, "trimws(upper(tt3.l2)) like 'GENCHILD%' and indexOf(upper(tt3.l2), ?) > 0", null, "tt3.l1 asc", "WHOLE-INDEX,main", new Object[]
+            new FindQuery(tt3, "trimws(upper(tt3.l2)) like 'GENCHILD%' and indexOf(upper(tt3.l2), ?) > 0", null, "tt3.recid asc", new Object[]
             {
                (P2JQuery.Parameter) () -> toUpperCase(tt2.getM2())
             }, LockType.NONE).silentFirst();
@@ -80,11 +80,11 @@

             if (_not(tt3.available()))
             {
-               new FindQuery(tt3, "not (trimws(upper(tt3.l2)) like 'GENOBJ1%') and indexOf(upper(tt3.l2), 'GENOBJ1') > 0", null, "tt3.l1 asc", "WHOLE-INDEX,main", LockType.NONE).silentFirst();
+               new FindQuery(tt3, "not (trimws(upper(tt3.l2)) like 'GENOBJ1%') and indexOf(upper(tt3.l2), 'GENOBJ1') > 0", null, "tt3.recid asc", LockType.NONE).silentFirst();

                if (_not(tt3.available()))
                {
-                  new FindQuery(tt3, "not (trimws(upper(tt3.l2)) like 'GENOBJ2%') and indexOf(upper(tt3.l2), 'GENOBJ2') > 0", null, "tt3.l1 asc", "WHOLE-INDEX,main", LockType.NONE).silentFirst();
+                  new FindQuery(tt3, "not (trimws(upper(tt3.l2)) like 'GENOBJ2%') and indexOf(upper(tt3.l2), 'GENOBJ2') > 0", null, "tt3.recid asc", LockType.NONE).silentFirst();
                }


The full conversion output had zero differences prior to this, so I suspect this is a bug.
I think the root cause is that the index map from P2OLookup is not populated properly. This way, there is no explicit index and the default primary key name is used: recid.

#65 Updated by Octavian Adrian Gavril 2 months ago

Has anyone run a full conversion for the OO application recently using trunk/16574 or the latest revision? It's taking much longer than usual, and I'm not sure why. It's been almost two hours and Code Conversion Annotations phase hasn't finished yet.

#66 Updated by Octavian Adrian Gavril 2 months ago

Octavian Adrian Gavril wrote:

Has anyone run a full conversion for the OO application recently using trunk/16574 or the latest revision? It's taking much longer than usual, and I'm not sure why. It's been almost two hours and Code Conversion Annotations phase hasn't finished yet.

False alarm. The tracing rule tool was enabled, that's why it was so slow...

#67 Updated by Octavian Adrian Gavril 2 months ago

Octavian Adrian Gavril wrote:

There is just one remaining difference that I missed regarding the declaration order:
[...]

I need to investigate the root cause.

I managed to fix this conversion difference. The problem is that the current trunk output seems incorrect, resulting in a bad generation order for the query names (query19 is generated before query18).

My fix enforces explicit sorting on the key set of the AdaptiveFind map based on the AST IDs, which guarantees the correct query naming order. I'm still not sure why the trunk version reverses these names, but applying this change aligns the outputs perfectly.

These changes are committed in rev16584.

#68 Updated by Octavian Adrian Gavril about 2 months ago

I've found an issue with classes that don't have a package base in the abl folder. For example, if the class path is ./abl/A.cls, there is no package base, which triggers an IndexOutOfBounds exception due to this edge case.

Besides that, I added the LegacyResource annotation for all generated classes, using the changes from 9488a documented in #9488-40. The need for this annotation is illustrated by the following scenario: if I need a converted class A that inherits from a parent B (which is also a converted class), the superclass name resolves to something like com.goldencode.dataset.B during a recursive definition load.

To SymbolResolver.loadClass, this looks like a standard java class rather than a converted class from a jar. By using the LegacyResource annotation, we intentionally fail the java class definition test, allowing us to compute the proper name inside loadConvertedJavaClass. This ensures our converted class is resolved correctly.

These changes are committed in rev16585.

#69 Updated by Octavian Adrian Gavril about 2 months ago

Another observation:
While testing the current solution with a jar copied into p2j/build/lib, I noticed some issues with schema loading.

This is the code responsible for parsing the p2j.cfg.xml file:

package com.goldencode.p2j.cfg;

[...]

public final class Configuration

[...]

         // attempt to load from jar first
         ClassLoader cl = MultiClassLoader.getClassLoader();
         cfgStream = cl.getResourceAsStream(CFG_DIR + "/" + DEF_CFG_FILE);

         if (cfgStream == null)
         {
            // if the configuration file was not found in jar, try locating it as normal file
            File cfgFile = getConfigFile(getConfigDirectory(), DEF_CFG_FILE);
            if (!cfgFile.exists())
            {
               return config;
            }

            cfgStream = Files.newInputStream(cfgFile.toPath());
         }

         config.saxParserFactory = SAXParserFactory.newInstance();
         config.saxParser = config.saxParserFactory.newSAXParser();
         config.saxHandler = new GenericSAXHandler();

         // Prepare the generic handler.
         setupHandler(config.saxHandler);

         // Parse the file.
         config.loadXml(cfgStream);

It seems the original intent was to load the file from the jar. If so, we will end up retrieving the p2j.cfg.xml file from the imported jar, meaning we are essentially using the configurations of the converted module rather than just the converted classes. I don't think this is the expected behavior for this task.

My primary error was related to missing schema namespaces, but this issue could potentially surface in other areas as well.

#70 Updated by Octavian Adrian Gavril about 2 months ago

I have three additional aspects that I want to solve for now:
  1. The incremental conversion is currently broken. This needs to be fixed.
  2. I need to integrate a versioning system for the generated class files. This is necessary for the incremental conversion to correctly decide if a file needs to be reconverted when a dependency class changes inside the jar.
  3. Decide how to handle duplicate class names across the jar and the abl folder. We could either log a warning and prioritize the one from the abl folder, or throw an error and stop the process, given that in the classic conversion, filesystem structure wouldn't allow this anyway.

#71 Updated by Greg Shah about 2 months ago

Octavian Adrian Gavril wrote:

Another observation:
While testing the current solution with a jar copied into p2j/build/lib, I noticed some issues with schema loading.

This is the code responsible for parsing the p2j.cfg.xml file:
[...]

It seems the original intent was to load the file from the jar. If so, we will end up retrieving the p2j.cfg.xml file from the imported jar, meaning we are essentially using the configurations of the converted module rather than just the converted classes. I don't think this is the expected behavior for this task.

My primary error was related to missing schema namespaces, but this issue could potentially surface in other areas as well.

Anything that now can be found from converted artifacts should be read that way instead of through the jar-based 2j.cfg.xml. There may still be things we need to read from there, so it probably still needs to load.

#72 Updated by Octavian Adrian Gavril about 2 months ago

Greg Shah wrote:

Anything that now can be found from converted artifacts should be read that way instead of through the jar-based 2j.cfg.xml. There may still be things we need to read from there, so it probably still needs to load.

Just to clarify, the issue isn't about migration or choosing between the artifact data and the configuration file.

The problem is a classpath conflict. Right now, because we added the jar to p2j/build/lib, the ClassLoader is accidentally picking up the p2j.cfg.xml located inside the imported jar instead of our own local p2j.cfg.xml from the current project.

So, the conversion is running with the converted module's configurations instead of its own. I need to fix the loading logic so it strictly targets our project's configuration file, but why was it designed to be loaded from jars in the first place?

#73 Updated by Greg Shah about 2 months ago

So, the conversion is running with the converted module's configurations instead of its own. I need to fix the loading logic so it strictly targets our project's configuration file, but why was it designed to be loaded from jars in the first place?

This is needed in the runtime case (for runtime conversion of dynamic queries).

Normal conversion (both full and incremental) should not use the p2j.cfg.xml from any jar file.

#74 Updated by Octavian Adrian Gavril about 2 months ago

I've committed new changes that fix the incremental conversion process. Converted classes are now loaded properly during incremental conversion.

The previous implementation didn't consider converted classes because it checked for a .ast file, which is absent in this case. I kept the initial logic but moved the converted classes logic to a place that is accessible during incremental conversion as well.

Additionally, we needed to register property bindings for the source class definition. This was originally handled by naming.rules, but classes that were not processed during incremental conversion ended up lacking property methods like GET or SET.

I also addressed the issue documented in #9638-69. The p2j.cfg.xml file is now searched inside a jar file during runtime conversion only.

These changes are committed in rev16586.

#75 Updated by Octavian Adrian Gavril about 2 months ago

Octavian Adrian Gavril wrote:

I have three additional aspects that I want to solve for now:
  1. The incremental conversion is currently broken. This needs to be fixed.
  2. I need to integrate a versioning system for the generated class files. This is necessary for the incremental conversion to correctly decide if a file needs to be reconverted when a dependency class changes inside the jar.
  3. Decide how to handle duplicate class names across the jar and the abl folder. We could either log a warning and prioritize the one from the abl folder, or throw an error and stop the process, given that in the classic conversion, filesystem structure wouldn't allow this anyway.

Regarding point 2, I suggest storing external converted classes in file_signature as well. We could use a pseudo-artifact format like java:[MD5-JAVAFILE]. At the same time, we could add a new file_dependencies table to store dependency pairs (e.g., "Class/file A depends on Class B").

With this information established during a full conversion, we can identify files affected by external changes by checking the java: pseudo-artifacts. If an MD5 hash differs, we can use the file_dependencies table to retrieve all files that depend on the modified class.

What do you think?

#76 Updated by Octavian Adrian Gavril about 2 months ago

Octavian Adrian Gavril wrote:

I have three additional aspects that I want to solve for now:
  1. The incremental conversion is currently broken. This needs to be fixed.
  2. I need to integrate a versioning system for the generated class files. This is necessary for the incremental conversion to correctly decide if a file needs to be reconverted when a dependency class changes inside the jar.
  3. Decide how to handle duplicate class names across the jar and the abl folder. We could either log a warning and prioritize the one from the abl folder, or throw an error and stop the process, given that in the classic conversion, filesystem structure wouldn't allow this anyway.

Regarding point 3, I re-checked the scenario, and I was wrong during today's meeting. The error occurs during the Code Conversion Annotation Prep phase. If we have A.cls in both the abl folder and a jar, the file from the jar is registered first in the class_definitions table. However, since the abl/A.cls file is also converted but not registered in class_definitions, it throws an error stating that no class can be associated with abl/A.cls. This is the core problem. Sorry for the confusion.

I need to investigate why abl/A.cls isn't being added. It seems like the class_definitions table is keyed by filename, so I don't see why it would be failing.

#77 Updated by Greg Shah about 2 months ago

I need to integrate a versioning system for the generated class files. This is necessary for the incremental conversion to correctly decide if a file needs to be reconverted when a dependency class changes inside the jar.

...

Regarding point 2, I suggest storing external converted classes in file_signature as well. We could use a pseudo-artifact format like java:[MD5-JAVAFILE]. At the same time, we could add a new file_dependencies table to store dependency pairs (e.g., "Class/file A depends on Class B").

With this information established during a full conversion, we can identify files affected by external changes by checking the java: pseudo-artifacts. If an MD5 hash differs, we can use the file_dependencies table to retrieve all files that depend on the modified class.

In the meeting today, Octavian clarified that this info would be stored in the cvt database and is for OO dependencies that are outside of the current code set being converted. I'm good with the approach.

Constantin: Any concerns?

#78 Updated by Octavian Adrian Gavril about 2 months ago

Octavian Adrian Gavril wrote:

Regarding point 3, I re-checked the scenario, and I was wrong during today's meeting. The error occurs during the Code Conversion Annotation Prep phase. If we have A.cls in both the abl folder and a jar, the file from the jar is registered first in the class_definitions table. However, since the abl/A.cls file is also converted but not registered in class_definitions, it throws an error stating that no class can be associated with abl/A.cls. This is the core problem. Sorry for the confusion.

I need to investigate why abl/A.cls isn't being added. It seems like the class_definitions table is keyed by filename, so I don't see why it would be failing.

The issue is that ./abl/A.cls is using the class definition loaded from the jar as well. That’s why there is no class definition associated with ./abl/A.cls. This happens because they share the same qualified name.

This scenario is similar to how we decide which class to reference by looking at the propath. In the current version of 9638a, external jars are prioritized. Should we keep it this way, or should we prioritize the abl folder?

#79 Updated by Greg Shah about 2 months ago

This scenario is similar to how we decide which class to reference by looking at the propath. In the current version of 9638a, external jars are prioritized. Should we keep it this way, or should we prioritize the abl folder?

Yes, the external jars should be prioritized.

#80 Updated by Octavian Adrian Gavril about 2 months ago

Greg Shah wrote:

In the meeting today, Octavian clarified that this info would be stored in the cvt database and is for OO dependencies that are outside of the current code set being converted. I'm good with the approach.

Constantin: Any concerns?

I've committed new changes in rev16587, which introduce the initial version of our dependency tracking system. We now register dependencies between input files and external JARs, and these records are used by the ConversionData.mustConvertArtifact method during incremental conversion. Note that this implementation has currently only been tested against my local test case.

I also addressed an issue regarding the default super constructor name (noticed during incremental conversion). The root cause was that some previous changes used getConvertedMethodName to generate the constructor name, while incremental conversion requires the final Java name (e.g., __a_constructor_). I have implemented a temporary workaround, but I'm still looking for a more permanent solution.

#81 Updated by Octavian Adrian Gavril about 2 months ago

I refactored the constructor generation in rev16588. These changes include updating the constructor name in legacyToJavaMethod to use the _constructor__ format. This provides two advantages. First, we no longer need to recalculate the default constructor name for each INHERITS AST node. Second, legacyToJavaMethod is now consistent across both normal and incremental conversions.

In addition, I noticed that in trunk we treat constructors as being overridden between a superclass and a subclass (Please see ClassDefinition.registerMethod). Since this is not a true override, I disallowed it.

As a consequence, this changed how my test case project converted. The project has a BuildRegistry class that inherits from OpenEdge.Core.Util.BuilderRegistry. The superclass has two constructors: BuilderRegistry(poRegistry as OpenEdge.Core.Util.BuilderRegistry) and BuilderRegistry(poValueType as Progress.Lang.Class). The generated superclass gets two methods, and the one with Progress.Lang.Class correctly gets a _1 suffix. However, my subclass BuildRegistry only has the BuilderRegistry(poValueType as Progress.Lang.Class) constructor. In trunk, we were incorrectly associating these signatures, which caused the subclass constructor to end up with a useless _1 suffix. With 9638a/16588, this no longer happens, resulting in cleaner conversion output. Please let me know if you have any objections to this approach.

There are no new conversion differences in oo test case project (except for the known ones). I'm currently testing against the OO app to ensure we haven't introduced any new regressions.

#82 Updated by Octavian Adrian Gavril about 2 months ago

Unfortunately, I'm seeing a few new differences, so I might have regressed something regarding constructor names in the OO app. Some classes are showing unnecessary _1 suffixes in their constructor names. I'm currently investigating this.

#83 Updated by Octavian Adrian Gavril about 2 months ago

Okay, it seems that calculateUniqueJavaMethodName is searching for parent constructors as well. Since I disallowed treating a parent constructor name as an overridden method, the system is now seeing this as a collision. I need to rethink this strategy.

#84 Updated by Octavian Adrian Gavril about 2 months ago

I've fixed the regression documented above by skipping collision checks for parent constructor names.

While testing incremental conversion with JARs using the new solution, I encountered a ${superjavaname} placeholder for the default constructor call. Initially, I thought this was a new regression, but it is actually an existing bug from trunk that produces a different output in 9638a.

Here is the test case used:

// A.cls
CLASS A INHERITS B: 

   DEFINE PROPERTY propInt        AS INTEGER     NO-UNDO GET. SET.

   CONSTRUCTOR A():
       propInt = 15.
   END.

END CLASS.

// B.cls
CLASS B: 

   CONSTRUCTOR B(INPUT s as Progress.Lang.Object):
      MESSAGE "CONSTRUCTOR".
   END.

END CLASS.

B.cls does not define a default constructor, yet the converted A.cls ends up attempting to call one:

/**
 * Business logic (converted to Java from the 4GL source code
 * in A.cls).
 */
public class A
extends com.goldencode.dataset.B
{
   [...]

   @LegacySignature(type = Type.CONSTRUCTOR, name = "A")
   public void __a_constructor__()
   {
      internalProcedure(A.class, this, "A", new Block((Body) () -> 
      {
         __b_constructor__();
         setPropInt(new integer(15));
      }));
   }
}
public class B
extends BaseObject
{
   @LegacySignature(type = Type.CONSTRUCTOR, name = "B", parameters = 
   {
      @LegacyParameter(name = "s", type = "OBJECT", qualified = "progress.lang.object", mode = "INPUT")
   })
   public void __b_constructor__(final object<? extends com.goldencode.p2j.oo.lang._BaseObject_> _s)
   {
      object<? extends com.goldencode.p2j.oo.lang._BaseObject_> s = TypeFactory.initInput(_s);

      internalProcedure(B.class, this, "B", new Block((Body) () -> 
      {
         __lang_BaseObject_constructor__();
         message("CONSTRUCTOR");
      }));
   }
}

As a result, compilation ultimately fails with the following error:

error: method __test_b_constructor__ in class B cannot be applied to given types;
    [javac]          __b_constructor__();
    [javac]          ^
    [javac]   required: object<? extends _BaseObject_>
    [javac]   found:    no arguments
    [javac]   reason: actual and formal argument lists differ in length
    [javac] 1 error

#85 Updated by Octavian Adrian Gavril about 2 months ago

I found a fix for the issue described in my previous note.

In annotations/method_defs.rules, we decide whether to define an implicit constructor with a super call based on a flag called hasInstCtor. The current implementation defines an implicit constructor if there is no other instance constructor defined (regardless of parameters). That's the root cause of the issue. Since hasInstCtor is only used for implicit constructors, we should set it to true only if we find an instance constructor with no parameters:

         <rule>not hasInstCtor
            <!-- create an implicit ctor, with a super call -->
            <action>
               tpl.graftAt("implicit_legacy_ctor_with_super", null, copy, 0, 
                           "name", legacyname,
                           "javaname", defctor,
                           "signature", sprintf("%s()", legacyname.toLowerCase()),
                           "rettype", sprintf("%s", prog.oo_meth_void),
                           "accessmode", sprintf("%s", prog.kw_public),
                           "superjavaname", sqname)
            </action>
         </rule>

This is the fix:

=== modified file 'rules/annotations/method_defs.rules'
--- old/rules/annotations/method_defs.rules    2025-12-11 09:13:53 +0000
+++ new/rules/annotations/method_defs.rules    2026-06-05 06:47:08 +0000
@@ -147,6 +147,7 @@
    <variable name="ctorOverload" type="java.lang.String" />
    <variable name="classname"    type="java.lang.String" />
    <variable name="defctor"      type="java.lang.String" />
+   <variable name="signature"    type="java.lang.String" />
    <variable name="extent"       type="java.lang.Long" />
    <variable name="access"       type="java.lang.Long" />
    <variable name="refid"        type="java.lang.Long" />
@@ -583,7 +585,10 @@
                <rule>not isNote("javaname")
                   <action>copy.putAnnotation("javaname", cname)</action>
                </rule>
-               <action>hasInstCtor = true</action>
+               <action>signature = #(java.lang.String) copy.getAnnotation("signature")</action>
+               <rule>signature.endsWith("()")
+                  <action>hasInstCtor = true</action>
+               </rule>
             </rule>
          </rule>

I will include this patch in the next revision of 9638a.

#86 Updated by Octavian Adrian Gavril about 2 months ago

Octavian Adrian Gavril wrote:

This is the fix:
[...]

I will include this patch in the next revision of 9638a.

This fix requires adding a default constructor to a few built-in classes.

Constantin, what do you think about this approach?

#87 Updated by Octavian Adrian Gavril 28 days ago

Constantin, have you had a chance to look at the approach mentioned above? Let me know what you think.

#88 Updated by Octavian Adrian Gavril 20 days ago

It looks like the test case above isn't compiling in OE and requires a SUPER() call to be correct. Consequently, I reverted the patch documented in #9638-85 and committed the latest changes regarding constructor name collisions. These can be found in 9638a/16589.

I continue retesting the last revision with the OO app to see if there are any left differences to be solved.

During this implementation, I added extra information to the built-in classes to support loading from JARs. However, we need to ensure this is supported for all future converted code as well. Therefore, I will revert the changes in the com.goldencode.p2j.oo package and start working on converting the skeleton folder into a format that includes all the necessary information to support loading converted classes. Let me know if you have any objections to this approach.

#89 Updated by Octavian Adrian Gavril 19 days ago

During today's meeting, we decided to keep the changes in the oo package since the skeleton project is difficult to convert and lacks the necessary implementation.

Instead, I will check if there is any information I added manually that is missing from the output code generated by our current implementation. After that, I will implement rules to include this information.

#90 Updated by Octavian Adrian Gavril 11 days ago

I created this testcase based on the changes made to oo package so far:

The only thing I've noticed about the current implementation is that we convert both HANDLE and TABLE-HANDLE data types to HANDLE. Since this had to be bypassed in the built-in classes to make things work, I will work on implementing a permanent fix.

#91 Updated by Octavian Adrian Gavril 8 days ago

I've noticed that the current implementation translates the HANDLE type to the TABLE-HANDLE type based on ASTs only. Since these are not available for converted classes, I chose to adapt the ParameterKey constructor based on the java parameter type. This way, no other changes to the generated builtin classes are required and the HANDLE data type information will work just fine even if we are actually using a TABLE-HANDLE.

I've reverted the changes from builtin classes that involved TABLE-HANDLE and committed the new changes in rev16590.

#92 Updated by Octavian Adrian Gavril 8 days ago

I've noticed some warnings about null class names for objects referenced by class variables. I've committed a fix in rev16591, so my OO testcase project conversion log is clean now. However, before continuing with the OO application testing, I plan to rebase to get more accurate results compared to trunk.

#93 Updated by Octavian Adrian Gavril 8 days ago

Rebased branch 9638a with trunk rev16648. The new revision is 16665.

#94 Updated by Octavian Adrian Gavril 7 days ago

I tested the last revision with the OO application and encountered an issue regarding built-in classes that are not implemented in FWD, which prevents them from being loaded as converted classes.

The root cause was that I had previously removed the loadBuiltinDefinition method from ClassDefinition, having noticed that we no longer needed it since we load them as converted java classes. However, this is not the case for classes that are not implemented. It appears the OO application is using such classes, which triggered the error.

To fix this, I've restored the method and applied it only when handling a non-converted class definition. The fix is in rev16666. I've retested the OO application, and the conversion completed successfully with no new, unexpected differences.

My next step is to test the incremental conversion (checking the dependency mechanism) and perform full conversion testing for a large GUI application.

Also available in: Atom PDF