fix: runtime type check can ignore type package name

[Peefy]

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

• N
• Y

fix #1536

fix: runtime type check can ignore type package name

[zong-zhe]

LGTM

[coveralls]

Pull Request Test Coverage Report for Build 10242340617

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 77 of 125 (61.6%) changed or added relevant lines in 10 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.006%) to 70.441%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
kclvm/sema/src/resolver/calculation.rs	0	2	0.0%
kclvm/runtime/src/value/val_type.rs	20	29	68.97%
kclvm/evaluator/src/ty.rs	6	16	37.5%
kclvm/sema/src/ty/mod.rs	6	19	31.58%
kclvm/runtime/src/value/val_plan.rs	34	48	70.83%

Files with Coverage Reduction	New Missed Lines	%
kclvm/runtime/src/value/val_schema.rs	1	85.36%

Totals
Change from base Build 10213739947:	0.006%
Covered Lines:	50302
Relevant Lines:	71410

---

💛 - Coveralls

[bozaro]

A very unexpected fix...

I tried to figure out this problem on my own and got to the following:

• the as operator receives the object type name and, if the type is declared in the same package, its short name as input;
• this short name is not a string from the code, but is generated somewhere during the AST tree processing stage (I made this conclusion because when using alias, there will be the type name, not the alias name);
• inside the AST tree, there is type information as SchemaType.

I was unable to find the place where AST SchemaType became to the short type name for the execution tree.

I expected that in the execution tree, the full type name would be input for the as operator...

[Peefy]

Hello @bozaro

Sorry, I didn't understand what you meant. Are you saying that I made the wrong fix? Can you provide some specific examples?

[bozaro]

If I understand correctly, the problem in this case is that check_type can receive a short type name.

kcl/kclvm/runtime/src/value/val_type.rs

Line 390 in 5417342

pub fn check_type(value: &ValueRef, pkgpath: &str, tpe: &str, strict: bool) -> bool {

However, if I'm not mistaken, this method is only used for runtime checking and never receives a type name from a string literal in the code.

I would simply always pass the full type name to check_type (and this should reduce the complexity of the code). I have very little knowledge of the KCL code and adding a package name to check_type surprised me a lot.

[Peefy]

I see.

The correct approach would be to handle type aliases or type cast during compilation instead of adding a pkgpath for the check_type function, including adding package names for the type alias, such as this pass: https://github.com/kcl-lang/kcl/blob/main/kclvm/sema/src/resolver/ty_alias.rs, but there are now some other restrictions and it is not related to this issue. I will open a new issue or refactor the code to track it in the future.

[bozaro]

Oh wow... Type alias is resolved by a separate mechanism...

Due to the processing of type alias, I seem to have false expectations about generating an execution tree (I expected SchemaType from AST to turn into a string when creating an execution tree)...

[Peefy]

Yes, SchemaType from AST is grammar type and is not a semantic type. Due to the existence of user-defined types and type aliases, we need to go through the semantic analysis stage to obtain the true schema type.