Bug #12009
OO class level marking calculated from methods
Status:
New
Priority:
Normal
Assignee:
-
Target version:
-
Start date:
Due date:
% Done:
0%
billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
production:
No
env_name:
topics:
History
#1 Updated by Greg Shah 19 days ago
Derive the support level of a built-in OO class from its members¶
Project: Base Language
Related: #11892 (gap marking review, where the rule was set), #9488 (built-in OO classes)
Problem¶
Every built-in OO class incom.goldencode.p2j.oo carries a class-level LegacyResourceSupport annotation, and most of its members carry their own. Both are maintained by hand, and they drift apart. The #11892 review found the drift in both directions:
- Four classes were marked Full at the class level while about 46 of their members still read STUB from the original hand-written skeletons, although those members had real implementations.
- 957 of the 4,412 members carry no marking at all. They silently inherit the class level, so a member is reported at whatever level its class happens to be marked.
- A method that has an implementation, or delegates to one, is Full. A method carrying a TODO is Partial.
- A class takes the lowest conversion level and the lowest runtime level of its annotated methods, and this should be calculated rather than written by hand.
- A
LegacyResourceclass with no markings at all, on the class or on its members, defaults to conversion Partial and runtime Stub, so that a developer has to review it.
The member half of that rule was applied by hand to OpenEdge.Net.URI and OpenEdge.Core.String in branch 11747a. This task implements the class half.
Where class levels are read today¶
pattern/CommonAstSupport.java:getGapMarking/getGapMarkingEx(near lines 600-760) read the class annotation for a class reference. For a member with no marking they fall back to the class annotation. When neither exists, the fallback isCVT_LVL_FULL | RT_LVL_STUB.uast/SupportLevelDocumentationGenerator.java(near lines 298 and 349) reads the class and member annotations to produce the support level documentation.
Work¶
- Write one shared helper that computes the effective level of a built-in class:
- the conversion level is the minimum over all members of the class that carry a marking, and the runtime level is the minimum likewise, with the two sides taken independently;
- a member that carries no marking does not take part in the minimum, which keeps the calculation from being circular;
- a class with no marked members and no class annotation gets
CVT_LVL_PARTIAL | RT_LVL_STUB, replacing theCVT_LVL_FULL | RT_LVL_STUBfallback.
- Decide how inherited members count. The natural answer is that a class's level covers the members it declares plus any it inherits from a built-in superclass. Settle this explicitly, and record the decision in the helper's javadoc.
- Decide what to do with the hand-written class annotation. There are two options:
- remove it wherever it now just repeats the computed value;
- keep it as an explicit override, but have a check report every class whose annotation disagrees with the computed value.
The second option is safer for the first run, because it produces the full list of disagreements for review.
- Generate the gap detail for a class whose computed level is below Full by naming the members that hold it down, for example: "
GetConstructor,GetEventandSetPropertyValueare stubs". A member's own gap detail stays on the member. - Use the helper in both
CommonAstSupportandSupportLevelDocumentationGenerator, so that the gap analysis and the published support levels agree. - Run the check over all of
com.goldencode.p2j.ooand produce the list of classes whose level changes. Review that list with the class owners before the change is committed, because some class rows will reappear in customer gap documents and others will disappear.
Notes¶
- A Partial level on a TODO is a blunt instrument. Some TODOs, such as the one in
Memptr._Debug's exception handler, do not affect behaviour. The review step above is where such cases are caught, and the member annotation, not the class annotation, is where they are corrected. - #9488 will replace or confirm many of these classes against the ADE. The helper means a corrected member annotation is enough, and nobody has to remember to update the class as well.