[v1] Refactor AstNode getChildren() method; change tag to be mutable int

[alancai98]

Relevant Issues

• [v1] Replace sprout-generated AST with hand-written AST #1610
• [V1] Use int primitive for AST tag #1642

Description

• From recent PR comment [v1] Remove PartiQLValue from AST; refactor AST literals #1650 (comment), changing AstNode's children method in a couple ways

1. use List in signature to retain order property
2. rename to getChildren() to allow for Kotlin accessor syntax sugar:

// Previous modeling w/ `children()`
node.children().filter { it is Expr }
// New modeling w/ `getChildren()` allows for slight syntax improvements from Kotlin
node.children.filter { it is Expr }

• Also changes the AstNode tag to be a mutable int

Other Information

• Updated Unreleased Section in CHANGELOG: [NO]

  • No, v1 not yet released.

• Any backward-incompatible changes? [YES]

  • Yes, but v1 not yet released.

• Any new external dependencies? [NO]

• Do your changes comply with the Contributing Guidelines
  and Code Style Guidelines? [YES]

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

CROSS-ENGINE-REPORT ❌

	BASE (LEGACY-V0.14.8)	TARGET (EVAL-0835DB7)	+/-
% Passing	89.67%	94.32%	4.65% ✅
Passing	5287	5561	274 ✅
Failing	609	54	-555 ✅
Ignored	0	281	281 🔶
Total Tests	5896	5896	0 ✅

Testing Details

• Base Commit: v0.14.8
• Base Engine: LEGACY
• Target Commit: 0835db7
• Target Engine: EVAL

Result Details

• ❌ REGRESSION DETECTED. See Now Failing/Ignored Tests. ❌
• Passing in both: 2641
• Failing in both: 17
• Ignored in both: 0
• PASSING in BASE but now FAILING in TARGET: 5
• PASSING in BASE but now IGNORED in TARGET: 108
• FAILING in BASE but now PASSING in TARGET: 180
• IGNORED in BASE but now PASSING in TARGET: 0

Now FAILING Tests ❌

The following 5 test(s) were previously PASSING in BASE but are now FAILING in TARGET:

Click here to see

1. undefinedUnqualifiedVariableWithUndefinedVariableBehaviorMissing, compileOption: PERMISSIVE
2. undefinedUnqualifiedVariableIsNullExprWithUndefinedVariableBehaviorMissing, compileOption: PERMISSIVE
3. undefinedUnqualifiedVariableIsMissingExprWithUndefinedVariableBehaviorMissing, compileOption: PERMISSIVE
4. inPredicateWithTableConstructor, compileOption: PERMISSIVE
5. notInPredicateWithTableConstructor, compileOption: PERMISSIVE

Now IGNORED Tests ❌

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

Now Passing Tests

180 test(s) were previously failing in BASE (LEGACY-V0.14.8) but now pass in TARGET (EVAL-0835DB7). Before merging, confirm they are intended to pass.

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

CROSS-COMMIT-REPORT ✅

	BASE (EVAL-7B818D6)	TARGET (EVAL-0835DB7)	+/-
% Passing	94.32%	94.32%	0.00% ✅
Passing	5561	5561	0 ✅
Failing	54	54	0 ✅
Ignored	281	281	0 ✅
Total Tests	5896	5896	0 ✅

Testing Details

• Base Commit: 7b818d6
• Base Engine: EVAL
• Target Commit: 0835db7
• Target Engine: EVAL

Result Details

• Passing in both: 5561
• Failing in both: 54
• Ignored in both: 281
• PASSING in BASE but now FAILING in TARGET: 0
• PASSING in BASE but now IGNORED in TARGET: 0
• FAILING in BASE but now PASSING in TARGET: 0
• IGNORED in BASE but now PASSING in TARGET: 0

[RCHowell]

Can we also update the tag to mutable int like the plans?

#1642

[alancai98]

Can we also update the tag to mutable int like the plans?

#1642

Let me sync w/ you on a few things for that issue.

[alancai98]

Can we also update the tag to mutable int like the plans?

#1642

Added mutable int tag for the AstNode -- 796f04e.

From offline discussion

• set AstNode tags using default ANTLR parser visitor
• use a private counter to ensure unique tags (since tag could have collisions using Random.nextInt()
• tag should not be used in AstNode equality and hashcode (already solved w/ the lombok annotations)
• TBD if the parsed AST tree should have the highest tag node be the top-level node (for the purposes of AstVisitor pass that adds/modifies nodes)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.03%. Comparing base (d059c81) to head (df60d6c).
Report is 437 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1674      +/-   ##
============================================
+ Coverage     73.16%   80.03%   +6.87%     
+ Complexity     2393       47    -2346     
============================================
  Files           247       19     -228     
  Lines         17627      506   -17121     
  Branches       3178       23    -3155     
============================================
- Hits          12896      405   -12491     
+ Misses         3854       88    -3766     
+ Partials        877       13     -864     

Flag	Coverage Δ
CLI	?
EXAMPLES	80.03% <ø> (-0.04%)	⬇️
LANG	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.