Add nodes to handle types #49785
Add nodes to handle types #49785
Conversation
jdconrad
added
:Core/Infra/Scripting
|
Pinging @elastic/es-core-infra (:Core/Infra/Scripting) |
|
|
||
| import java.util.Objects; | ||
|
|
||
| public class DResolvedType extends DType { |
| public class DResolvedType extends DType { | ||
|
|
||
| protected final Class<?> type; | ||
| protected final boolean check; |
There was a problem hiding this comment.
check means check against the whitelist, correct? Please add a comment.
There was a problem hiding this comment.
Done. Also renamed to checkInLookup.
| } | ||
|
|
||
| return new DResolvedType(location, type, false); | ||
| } else { |
There was a problem hiding this comment.
Can you do if (check == false) { return this; } to reduce the indention level?
| @@ -37,13 +37,13 @@ | |||
| */ | |||
| public final class SDeclaration extends AStatement { | |||
|
|
|||
| private final String type; | |||
| private DType type; | |||
There was a problem hiding this comment.
Can you please add a comment here explaining why this is mutable and what are plans are moving forward here?
There was a problem hiding this comment.
Changed this to final instead. Will add mutable DResolvedType nodes where necessary as the conversions take place. These should be removed with the creation of a second tree.
|
@rjernst @stu-elastic Thanks for the reviews. I believe I've addressed the feedback, and I will commit as soon as CI passes. |
This PR adds 3 nodes to handle types defined by a front-end creating a Painless AST. These types are decided with data immutability in mind - hence the reason for more than a single node.
This PR adds 3 nodes to handle types defined by a front-end creating a Painless AST. These types are decided with data immutability in mind - hence the reason for more than a single node.
This PR adds 3 nodes to handle types defined by a front-end creating a Painless AST. These types are decided with data immutability in mind - hence the reason for more than a single node.
DType - base class to allow for a getType call
DUnresolvedType - stores a String to be resolved to a type
DResolvedType - stores a Class<?> representative of a type
Initially this accomplishes consistency in how types are resolved in Painless instead of having loose types across multiple nodes. This PR replaces the type in SDeclaration as an example of usage, and follow up PRs will replace them in all nodes that have defined types.
Secondarily, this will be used allow future Painless improvements including making the AST more generic with the removal of custom try/catch handling as an example because now if we know a type internally we can just use a DResolvedType node instead of being forced to define a String type and resolve it multiple times.
Finally, this will be helpful if/when we decide to create a separate IR for valid semantic trees.
Edit: Adding @stu-elastic for visibility.